WIP Allow updating the cluster one instance group at a time

[hakman]

/cc @justinsb @rifelpet

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from hakman. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[justinsb]

Nit: might be nice to split this into a function?

[justinsb]

Remind me - I think you said there's a gotcha with this approach @hakman ?

[hakman]

Remind me - I think you said there's a gotcha with this approach @hakman ?

@justinsb If you try to apply the config to a single IG, the KubeletConfig is removed.

[justinsb]

Looked at this for a while, and I think I know what is happening!

So the issue is:

// upgradeSpecs ensures that fields are fully populated / defaulted
func (c *ApplyClusterCmd) upgradeSpecs(ctx context.Context, assetBuilder *assets.AssetBuilder) error {

We store the instanceGroup as the user specified it. But there are many computed fields - the so called "full" instanceGroup or cluster spec. upgradeSpecs converts the instanceGroups to populate these fields.

In this PR, we aren't calling upgradeSpecs or the population logic. So these instance groups don't have many fields populated, including (I would guess) the kubelet config.

I suggest that the easy thing to do is to convert FilteredInstanceGroups into a predicate (i.e. func() bool), and then basically filter these out at the last minute. Then we don't need to duplicate the defaulting logic.

[justinsb]

Applying a predicate does seem to work ... https://github.com/justinsb/kops/tree/update-cluster-per-ig

The problem is that I don't think we can just do ASGs. Even on AWS, we need to deal with NodeTerminationHandler object. But I think this problem also exists on other clouds, so we need to solve it there also IIUC

[justinsb]

I opened another PR #16936 with the filter. I also had to do something similar to what you did - all tasks have access to InstanceGroups (the filtered instance groups) and AllInstanceGroups (unfiltered). The "trick" is that most tasks work with "InstanceGroups" (the filtered), there are just a handful that need to be repointed to work with AllInstanceGroups (e.g. things that need a list of roles)

Likely we'll need a few tweaks for other clouds, but they shouldn't be too big!

[hakman]

I opened another PR #16936 with the filter. I also had to do something similar to what you did - all tasks have access to InstanceGroups (the filtered instance groups) and AllInstanceGroups (unfiltered). The "trick" is that most tasks work with "InstanceGroups" (the filtered), there are just a handful that need to be repointed to work with AllInstanceGroups (e.g. things that need a list of roles)

Likely we'll need a few tweaks for other clouds, but they shouldn't be too big!

Awesome! 🎉