kubeadm: Make the creation of the RBAC rules phase idempotent #47081
Conversation
luxas
added
kind/bug
k8s-ci-robot
added
the
cncf-cla: yes
k8s-github-robot
added
approved
| return fmt.Errorf("unable to create a new kube-proxy configmap: %v", err) | ||
| } | ||
|
|
||
| if _, err := client.CoreV1().ConfigMaps(metav1.NamespaceSystem).Update(kubeproxyConfigMap); err != nil { |
There was a problem hiding this comment.
Isn't this really an overwrite?
Could we stash somewhere, the previous one as a backup in case someone has modified the originals.
Same applies for *.
There was a problem hiding this comment.
What's the reason to upgrade system-internal manifests if we don't overwrite?
Yes, it's overwriting, but on the other hand it will be super hard to detect a change without building in earlier manifests into kubeadm.
Also, how do we know what the user wants? Do they want to keep old, modified manifests or do they really want to upgrade.
If you have a preferred location for stashing things, I'm all ears though...
There was a problem hiding this comment.
If I treat package management as an analogous example, it would never explicitly overwrite without user specification so they know they are going to blast them.
As of today, there is no ObjectMeta record on who authored/signed a resource update, if we knew that we could have a check.
There was a problem hiding this comment.
Yeah, that's the problem. If that was in ObjectMeta (maybe shouldn't be, but anyway), we could easily check that.
I think this should just be a caveat in the instructions. If you have modified things kubeadm installed in the kube-system name, you must redo those mods after upgrading.
Agree so we can proceed with this?
There was a problem hiding this comment.
So long as we spell it out in bold in the docs I'm ok.
There was a problem hiding this comment.
Also update a release note on this one to denote, and lgtm from me.
luxas
force-pushed
the
kubeadm_postinst_idempotency
branch
from
ea07f3a to
1b93a6a
Compare
|
@timothysc Updated the release note. Please approve/lgtm then |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: luxas, timothysc Associated issue: 288 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
timothysc
added
the
lgtm
|
@k8s-bot pull-kubernetes-e2e-gce-etcd3 test this |
luxas
added
release-note
|
@k8s-bot pull-kubernetes-e2e-gce-etcd3 test this |
|
Automatic merge from submit-queue (batch tested with PRs 47024, 47050, 47086, 47081, 47013) |
What this PR does / why we need it:
Bugfix: Currently kubeadm fails with a non-zero code if resources it's trying to create already exist. This PR fixes that by making kubeadm try to Update resources that already exist.
After this PR, #46879 and a beta.1 release, kubeadm will be fully upgradeable from v1.6 to v1.7 using only kubeadm init.
Last piece of kubernetes/kubeadm#288
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Fixes: kubernetes/kubeadm#288
Special notes for your reviewer:
Release note:
@pipejakob @mikedanese @timothysc