WIP: Introduce cleanup phase #16291
WIP: Introduce cleanup phase #16291
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1a87cbb to
cd8c898
Compare
cd8c898 to
57a852d
Compare
fbac091 to
236532a
Compare
17d360a to
4ae3c03
Compare
|
This is getting relatively minimal I think, we do need #16293 to go in to remove a lot of the noise, but it's not obvious to me how to split what's left into any more standalone parts. (I do need to clean up the commits though, they are a mess on this branch!) |
4ae3c03 to
ec8d1d6
Compare
This lets us safely make changes to otherwise immutable fields, in particular for adding security groups to NLBs created without them. We detect the older versions, and create deletion tasks to remove them. These tasks can be deferred, and we expect them to be deferred to a "prune" phase that runs after cluster apply.
ec8d1d6 to
182dd96
Compare
|
@justinsb: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
This was done in #16356 (and other earlier PRs), closing /close |
|
@justinsb: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
A large "integration branch" PR to show the big picture; I will whittle this down with smaller PRs but want to show & agree the direction.
Because of the challenges of adding security groups to an existing NLB - see issue #16276 - the best way around this seems to be to create a second NLB during
kops update, do akops rolling-update(the old nodes use the old NLB, the new nodes use the new NLB), then do a cleanup phase where we delete the original (now unused) NLB and associated resources.Even with this, as @zetaab pointed out the kubeconfig DNS name will change if we're using the ELB, though this "just" means that users will need to distribute a new kubeconfig. Users using a real domain will not need to distribute a new kubeconfig.
Generally, the trick here is to introduce the idea of "revisions" of a Task or Cloud Resource. Two NLBs / TargetGroups / etc might have the same Name tag on AWS, but would have a different revision tag (
kops.k8s.io/revision).Another trick we use is to stash more state in private fields in the Task; we already have a "linking" phase such that if you refer to a Task with its "ID" (as defined by CompareWithID) that gets replaced with the canonical Task object.