Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

add tolerations to aad pod identity #3947

Merged
merged 1 commit into from
Nov 23, 2022
Merged

add tolerations to aad pod identity #3947

merged 1 commit into from
Nov 23, 2022

Conversation

sonasingh46
Copy link
Contributor

@sonasingh46 sonasingh46 commented Nov 18, 2022

Signed-off-by: Ashutosh Kumar sonasingh46@gmail.com

What this PR does / why we need it

Add tolerations to aad-pod-identity component YAML.

CAPZ pods has control plane tolerations and this means that capz pod can get scheduled to control plane nodes. The aad-pod-identity does not have tolerations which can cause authentication issue if the capz pod gets scheduled to a ctonrol plane node.
This PR adds control plane tolerations to aad pod identity so that the their pods can be scheduled to control plane nodes too.

Which issue(s) this PR fixes

Fixes #3946

Describe testing done for PR

Release note

None

Additional information

Special notes for your reviewer

Tested this PR by building the cli binary and creating a bootstrap cluster. Observed that the capz-nmi DaemonSet has the required tolerations.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3947/20221118105657/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #3947 (10c1cc0) into main (dc70681) will decrease coverage by 0.94%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3947      +/-   ##
==========================================
- Coverage   47.72%   46.78%   -0.95%     
==========================================
  Files         420      445      +25     
  Lines       41985    43574    +1589     
==========================================
+ Hits        20037    20384     +347     
- Misses      20037    21260    +1223     
- Partials     1911     1930      +19     
Impacted Files Coverage Δ
...ons/controllers/packageinstallstatus_controller.go 77.99% <0.00%> (-2.32%) ⬇️
cmd/cli/plugin/cluster/kubeconfig_get.go 8.82% <0.00%> (ø)
cmd/cli/plugin/cluster/set_node_pool.go 14.63% <0.00%> (ø)
...in/cluster/set_machinehealthcheck_control_plane.go 21.21% <0.00%> (ø)
cmd/cli/plugin/cluster/list.go 11.36% <0.00%> (ø)
cmd/cli/plugin/cluster/delete.go 12.50% <0.00%> (ø)
cmd/cli/plugin/cluster/upgrade.go 58.50% <0.00%> (ø)
cmd/cli/plugin/cluster/get_machinehealthcheck.go 11.42% <0.00%> (ø)
...i/plugin/cluster/delete_machinehealthcheck_node.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/main.go 0.00% <0.00%> (ø)
... and 16 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rajathagasthya rajathagasthya added the ok-to-merge PRs should be labelled with this before merging label Nov 18, 2022
Copy link
Member

@rajathagasthya rajathagasthya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix lint errors and make sure CI is green. You might want to rebase on latest where CAPD tests are disabled.

@sonasingh46 sonasingh46 force-pushed the aad-pod-identity-bump branch from 88b15a6 to 7b7e42a Compare November 18, 2022 15:33
@sonasingh46
Copy link
Contributor Author

@rajathagasthya -- I believe the linter is not failing because of my PR. Let me know if you think otherwise. Anyways, I have rebased and pushed.

@sonasingh46
Copy link
Contributor Author

Will see if the lint fails again and figure out the problem.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3947/20221118154134/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@sonasingh46
Copy link
Contributor Author

I see the yamllint problem -- may be I missed seeing that. My bad.
Let me fix those.

Signed-off-by: Ashutosh Kumar <sonasingh46@gmail.com>
@sonasingh46 sonasingh46 force-pushed the aad-pod-identity-bump branch from 7b7e42a to 10c1cc0 Compare November 19, 2022 09:26
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3947/20221119093520/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@wjun wjun merged commit 4fd3ef5 into main Nov 23, 2022
@wjun wjun deleted the aad-pod-identity-bump branch November 23, 2022 06:30
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing control plane toleration for aad-pod-identity
4 participants