Fix so setup-files don't recreate/invalidate certificates that already exist #23550
Conversation
|
Labelling this PR as size/M |
k8s-github-robot
added
the
size/M
| # Create known tokens for service accounts | ||
| echo "$(create_token),admin,admin" >> /data/known_tokens.csv | ||
| echo "$(create_token),kubelet,kubelet" >> /data/known_tokens.csv | ||
| echo "$(create_token),kube_proxy,kube_proxy" >> /data/known_tokens.csv7 |
There was a problem hiding this comment.
looks like a 7 got added here.
|
other than the 7 looks good to me. My other comment is more speculative and a nice to have. |
luxas
force-pushed
the
fix_hyperkube_certs
branch
from
84fbbea
to
dcbc95e
Compare
|
Yep, fixed the typo |
|
@vishh Does this look good to you? |
|
GCE e2e build/test passed for commit 84fbbea48d6ee6ecf9b4e38e529d13932f4c312a. |
|
GCE e2e build/test passed for commit dcbc95ea953bc2f3d92407d1f42a7d9e7941fad2. |
|
The author of this PR is not in the whitelist for merge, can one of the admins add the 'ok-to-merge' label? |
|
LGTM |
|
How do one retest travis? |
|
@fgrzadkowski Could we please just merge it? 😄 But this will make it a much better for |
|
+1 for merging. Side note: I do think I've seen the script die while generating the tokens, but not since the alphas, but I can't remember clearly, so it's probably not worth pursuing. Plus I can add a comment to trouble shooting section of the docs so people can blow away any kube state. |
luxas
force-pushed
the
fix_hyperkube_certs
branch
from
dcbc95e
to
858b953
Compare
|
@fgrzadkowski Thought about it a bit more and updated to separate checks. |
fgrzadkowski
added
the
lgtm
|
LGTM. Thanks for the change! |
|
Then I think the |
|
AFAIU once we set @david-mcmahon Is my understand correct? |
fgrzadkowski
added
release-note
|
Yes, I think so. |
|
That is correct right now. We are considering future alternatives/changes. |
|
Yes, apologies. We've been debating in an updated version of a PR updating the release-note guidance. I hope to get this in soon. |
|
GCE e2e build/test passed for commit 858b953. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 858b953. |
|
Automatic merge from submit-queue |
|
I assume we still need the |
|
@zmerlynn Yes, please add the |
|
@zmerlynn you do need that label. I'm sure @bgrant0607 will come through soonish. appears there are 4 PRs like this that need review. |
bgrant0607
added
the
cherry-pick-approved
…upstream-release-1.2 Automated cherry pick of #23550 upstream release 1.2
|
Commit found in the "release-1.2" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
…of-#23550-upstream-release-1.2 Automated cherry pick of kubernetes#23550 upstream release 1.2
…of-#23550-upstream-release-1.2 Automated cherry pick of kubernetes#23550 upstream release 1.2
Fixes: #23197 and a lot of other DNS and dashboard issues
This is quite critical for
docker-based users and should be considered as a cherrypick-candidate as it makes a lot of people wonder why Dashboard and/or DNS doesn't work. Example: kubernetes/dashboard#374Earlier when you shut your
docker.mdcluster down and started it again, all ServiceAccounts became invalidated bysetup-filesthat happily ran once again and replaced all files. That madeapiserverandcontroller-managerpick up the new certs (or there was a race condition, they could have picked up the old certs too, but that's unlikely) and the old certs were put into/var/run/secretsbecause the ServiceAccount's Secrets were stored in etcd, whichsetup-filesdidn't touch.@fgrzadkowski @huggsboson @thockin @mikedanese @vishh @pwittrock @eparis @bgrant0607