Support rolling upgrade on openstack

[olemarkus]

This is a rather brutish approach running close to full update cluster between rolls.
In order to do a smaller parts of update cluster, I think we first need to split ApplyClusterCmd.Run() into smaller parts, perhaps with cloud-specific functions instead of all the switch statements.

There are lots of good suggestions in #9635 on how to make the update process smaller, such as writing bootstrapscript to VFS. But there are too many other interdependencies between tasks (such as instance -> port -> subnet -> network) that needs splitting first. Constructing tasks for resources we know exists when inside a rolling-pudate should be easy once ApplyClusterCmd has been split out in the various "phases".

Fixes #9635

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[olemarkus]

@zetaab maybe worth having a look at the changes here together with the detach instance work?

[mitch000001]

actually there is no need to embed the context within this struct. We can propagate it at every step.

[olemarkus]

I got tired of passing on this one all the time :p what is the benefit of keep propagating it?

[mitch000001]

I mean, the doc states not to embed a context.Context within a struct. However, I found golang/go#22602 which points out that if it is a parameter struct, i.e. to not get a parameter explosion within the function then it would be fine. In our case it is not really only a parameter struct so I'd prefer to keep the context as a first parameter.

[olemarkus]

It is not strictly a parameter struct since we define functions on it, but none of the functions mutate the struct, so for all intents and purposes it is. The concerns in that issue is not applicable to this struct either as far as I can tell.

[olemarkus]

/cc @zetaab

[zetaab]

I cannot test this because of #10003 and #10004

[zetaab]

% ./kops rolling-update cluster rolling2.k8s.local --yes
NAME		STATUS		NEEDUPDATE	READY	MIN	TARGET	MAX	NODES
bastions	NeedsUpdate	1		0	1	1	1	0
master-zone-1	NeedsUpdate	1		0	1	1	1	1
master-zone-2	NeedsUpdate	1		0	1	1	1	1
master-zone-3	NeedsUpdate	1		0	1	1	1	1
nodes-zone-1	NeedsUpdate	1		0	1	1	1	1
nodes-zone-2	NeedsUpdate	1		0	1	1	1	1
nodes-zone-3	Ready		0		0	0	0	0	0
I1001 17:26:58.490257   19777 instancegroups.go:519] Stopping instance "386bf040-b0a7-4734-a71f-66aefcff220d", in group "rolling2.k8s.local-bastions" (this may take a while).
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x40 pc=0x3b42ce0]

goroutine 73 [running]:
k8s.io/kops/upup/pkg/fi/cloudup.(*ApplyClusterCmd).Run(0xc0004dbad0, 0x524a840, 0xc00019c010, 0x0, 0x0)
	/Users/jessehaka/go/src/k8s.io/kops/upup/pkg/fi/cloudup/apply_cluster.go:140 +0x7dc0
k8s.io/kops/pkg/instancegroups.(*RollingUpdateCluster).reconcileInstanceGroup(0xc000fd00c0, 0xc0000ec2a0, 0x0)
	/Users/jessehaka/go/src/k8s.io/kops/pkg/instancegroups/instancegroups.go:407 +0x17d
k8s.io/kops/pkg/instancegroups.(*RollingUpdateCluster).drainTerminateAndWait(0xc000fd00c0, 0xc0000ec2a0, 0x37e11d600, 0xc000a96720, 0x0)
	/Users/jessehaka/go/src/k8s.io/kops/pkg/instancegroups/instancegroups.go:377 +0x238
k8s.io/kops/pkg/instancegroups.(*RollingUpdateCluster).rollingUpdateInstanceGroup.func1(0xc000d288a0, 0xc000fd00c0, 0x37e11d600, 0xc0000ec2a0)
	/Users/jessehaka/go/src/k8s.io/kops/pkg/instancegroups/instancegroups.go:168 +0x3f
created by k8s.io/kops/pkg/instancegroups.(*RollingUpdateCluster).rollingUpdateInstanceGroup
	/Users/jessehaka/go/src/k8s.io/kops/pkg/instancegroups/instancegroups.go:167 +0x5d8

[zetaab]

like mentioned in last comment: this will panic. The problem is that c.Clientset is NIL in reconcileInstanceGroup function

You need pass clientset to variables in cmd/kops/rollingupdatecluster.go row 330

[olemarkus]

Looks like a bit went missing when I factored out some of the BuildCloud stuff. Could you try again now, @zetaab ?

[zetaab]

Okay this is not perfect, but it is already much better than before this PR.

Two issues:

1. if something happens between single instance delete & apply cluster run command. You need run kops update cluster once to get machines up and running.
2. running whole kops cluster update is not the perfect way. We should have possibility to run only some parts of it

However, this is good start!

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: olemarkus, zetaab

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [zetaab]

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

[kciredor]

Will this make it into 1.18.2?

[olemarkus]

No this will be not be backported. The change is pretty big.