Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove cluster init token check #273

Merged
merged 1 commit into from
May 23, 2022

Conversation

shaneutt
Copy link
Contributor

Back when we first made KTF we added a check to wait for the service account that would be used for testing until it's Secret was generated. Modern versions of Kubernetes since v1.24.0 have a different implementation for ServiceAccounts which doesn't make this secret available like it used to be. As such this patch removes the check as it is irrelevant going forward and is breaking recent deployments.

@shaneutt shaneutt added bug Something isn't working priority/high labels May 23, 2022
@shaneutt shaneutt self-assigned this May 23, 2022
@shaneutt shaneutt temporarily deployed to integration-tests May 23, 2022 16:47 Inactive
@codecov-commenter
Copy link

Codecov Report

Merging #273 (8fdff88) into main (31a6656) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #273   +/-   ##
=======================================
  Coverage   11.75%   11.75%           
=======================================
  Files           5        5           
  Lines         604      604           
=======================================
  Hits           71       71           
  Misses        528      528           
  Partials        5        5           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df000e6...8fdff88. Read the comment docs.

@shaneutt shaneutt marked this pull request as ready for review May 23, 2022 17:03
@shaneutt shaneutt requested a review from a team as a code owner May 23, 2022 17:03
@shaneutt shaneutt enabled auto-merge (squash) May 23, 2022 17:04
@mflendrich
Copy link
Contributor

So the wait is not necessary anymore?

@shaneutt
Copy link
Contributor Author

So the wait is not necessary anymore?

The wait was probably never really necessary in the first place, but it's definitely not necessary now. In fact, it simply won't work because the Secret is no longer made.

@shaneutt shaneutt merged commit edc3b66 into main May 23, 2022
@shaneutt shaneutt deleted the shaneutt/remove-sa-token-check-in-cluster-init branch May 23, 2022 17:18
@mflendrich
Copy link
Contributor

Did some research:

This KEP says

The NodeAuthorizer will allow the kubelet to use its credentials to request a service account token on behalf of pods running on that node.

Which means that getting SA credentials for a pod is now a synchronous operation performed by kubelet.

@shaneutt
Copy link
Contributor Author

https://github.com/kubernetes/enhancements/tree/master/keps/sig-auth/2799-reduction-of-secret-based-service-account-token is the KEP for posterity, if anyone is seeing and wanting to better understand this PR in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority/high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants