✨ Adds kubeadm control plane scale up/down #2335

dlipovetsky · 2020-02-14T06:43:20Z

What this PR does / why we need it:
Kubeadm control plane scale up and down, as described in #2241

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2241, #2016

I'm replacing the scale up unit tests because with the health checks introduced for scale up, they would become integration tests. For the new scale up/down unit tests, I'm going to fake the management cluster.

I'm also replacing the delete unit tests, because the existing ones rely on scaling up the cluster. I'll replace them with a test that works against a static set of machines.

/assign @chuckha @detiber @randomvariable

chuckha

The changes to go.mod and go.sum should be reverted. We want client-go 0.17.2 and the framework should not be importing CAPD

controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go

controlplane/kubeadm/controllers/kubeadm_control_plane_controller_test.go

detiber · 2020-02-14T14:42:41Z

controlplane/kubeadm/controllers/kubeadm_control_plane_controller_test.go

+
+		for i := 0; i < 3; i++ {
+			m, _ := createMachineNodePair(fmt.Sprintf("test-%d", i), cluster, kcp, true)
+			g.Expect(fakeClient.Create(context.Background(), m)).To(Succeed())


It might be a bit cleaner if we create these resources prior to creating the fake client and feed them into the function for creating the fake client.

controlplane/kubeadm/controllers/kubeadm_control_plane_controller_test.go

controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go

chuckha

The only other change I had to make was in the test, increasing a timeout:

-                               framework.WaitForKubeadmControlPlaneMachinesToExist(ctx, assertKubeadmControlPlaneNodesExistInput)
+                               framework.WaitForKubeadmControlPlaneMachinesToExist(ctx, assertKubeadmControlPlaneNodesExistInput, "5m", "10s")

capd-e2es passed!

controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go

dlipovetsky · 2020-02-14T17:44:43Z

The changes to go.mod and go.sum should be reverted. We want client-go 0.17.2 and the framework should not be importing CAPD

I'll revert that. What do I do about it locally, though? That's the diff after running make modules (go 1.13.8) 😕

$ git status --untracked-files=no
On branch controlplane-scale
nothing to commit (use -u to show untracked files)

$ make modules --debug
GNU Make 4.2.1
Built for x86_64-redhat-linux-gnu
Copyright (C) 1988-2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Reading makefiles...
Updating makefiles....
Updating goal targets....
 File 'modules' does not exist.
Must remake target 'modules'.
go mod tidy
cd hack/tools; go mod tidy
cd test/framework; go mod tidy
go: finding sigs.k8s.io/cluster-api/test/infrastructure/docker latest
cd test/infrastructure/docker; make modules
GNU Make 4.2.1
Built for x86_64-redhat-linux-gnu
Copyright (C) 1988-2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Reading makefiles...
make[1]: Entering directory '/home/dlipovetsky/projects/cluster-api/test/infrastructure/docker'
Updating makefiles....
Updating goal targets....
 File 'modules' does not exist.
Must remake target 'modules'.
go mod tidy
cd hack/tools; go mod tidy
Successfully remade target file 'modules'.
make[1]: Leaving directory '/home/dlipovetsky/projects/cluster-api/test/infrastructure/docker'
Successfully remade target file 'modules'.

$ git status --untracked-files=no
On branch controlplane-scale
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   test/framework/go.mod
        modified:   test/framework/go.sum
        modified:   test/infrastructure/docker/go.mod
        modified:   test/infrastructure/docker/go.sum

no changes added to commit (use "git add" and/or "git commit -a")

$ go version
go version go1.13.8 linux/amd64

vincepri · 2020-02-14T17:45:20Z

Ping when you're ready for another set of 👀

chuckha · 2020-02-14T17:49:44Z

The changes to go.mod and go.sum should be reverted. We want client-go 0.17.2 and the framework should not be importing CAPD

I'll revert that. What do I do about it locally, though? That's the diff after running make modules (go 1.13.8) 😕

That's very weird. On my system, that is not the case. Once I revert those changes and run make modules, go.sum cleans up and my changes persist 🤔

Let's see if prow complains after you revert them. It will complain about outdated files if go modules is not up to date.

chuckha · 2020-02-14T22:09:47Z

/approve

/assign @vincepri @detiber
for lgtm

k8s-ci-robot · 2020-02-14T22:09:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chuckha, dlipovetsky

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~~ [chuckha]

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

vincepri · 2020-02-14T22:16:29Z

Reviewing

controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go

vincepri · 2020-02-14T22:29:40Z

util/util.go

+func (o MachinesByCreationTimestamp) Less(i, j int) bool {
+	if o[i].CreationTimestamp.Equal(&o[j].CreationTimestamp) {
+		return o[i].Name < o[j].Name
+	}


I'd use sort.Slice instead of creating yet a new type here

Signed-off-by: Daniel Lipovetsky <dlipovetsky@d2iq.com>

vincepri

/lgtm

k8s-ci-robot assigned chuckha, detiber and randomvariable Feb 14, 2020

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 14, 2020

k8s-ci-robot requested review from chuckha and ncdc February 14, 2020 06:43

chuckha reviewed Feb 14, 2020

View reviewed changes

detiber reviewed Feb 14, 2020

View reviewed changes

chuckha reviewed Feb 14, 2020

View reviewed changes

controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go Outdated Show resolved Hide resolved

dlipovetsky force-pushed the controlplane-scale branch from 25381d1 to 57a9876 Compare February 14, 2020 19:40

chuckha added this to the v0.3.0-rc.0 milestone Feb 14, 2020

dlipovetsky force-pushed the controlplane-scale branch from 2750001 to c7648e1 Compare February 14, 2020 20:12

k8s-ci-robot assigned vincepri Feb 14, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2020

dlipovetsky force-pushed the controlplane-scale branch from e8fd512 to 6c6af06 Compare February 14, 2020 22:16

vincepri reviewed Feb 14, 2020

View reviewed changes

✨ Adds kubeadm control plane scale up/down

386d3bf

Signed-off-by: Daniel Lipovetsky <dlipovetsky@d2iq.com>

dlipovetsky force-pushed the controlplane-scale branch from 6c6af06 to 386d3bf Compare February 14, 2020 22:51

vincepri approved these changes Feb 14, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2020

k8s-ci-robot merged commit d09f59e into kubernetes-sigs:master Feb 14, 2020

ncdc mentioned this pull request Mar 2, 2020

Control plane should scale up serially, not in parallel #2016

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Adds kubeadm control plane scale up/down #2335

✨ Adds kubeadm control plane scale up/down #2335

dlipovetsky commented Feb 14, 2020

chuckha left a comment

detiber Feb 14, 2020

chuckha left a comment

dlipovetsky commented Feb 14, 2020

vincepri commented Feb 14, 2020

chuckha commented Feb 14, 2020

chuckha commented Feb 14, 2020

k8s-ci-robot commented Feb 14, 2020

vincepri commented Feb 14, 2020

vincepri Feb 14, 2020

vincepri left a comment

✨ Adds kubeadm control plane scale up/down #2335

✨ Adds kubeadm control plane scale up/down #2335

Conversation

dlipovetsky commented Feb 14, 2020

chuckha left a comment

Choose a reason for hiding this comment

detiber Feb 14, 2020

Choose a reason for hiding this comment

chuckha left a comment

Choose a reason for hiding this comment

dlipovetsky commented Feb 14, 2020

vincepri commented Feb 14, 2020

chuckha commented Feb 14, 2020

chuckha commented Feb 14, 2020

k8s-ci-robot commented Feb 14, 2020

vincepri commented Feb 14, 2020

vincepri Feb 14, 2020

Choose a reason for hiding this comment

vincepri left a comment

Choose a reason for hiding this comment