✨ Bump controller-runtime to v0.6.0 #2961

detiber · 2020-04-24T18:13:31Z

What this PR does / why we need it:

Bumps the version of controller-runtime to v0.6.0

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 #2954

k8s-ci-robot · 2020-04-24T18:13:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: detiber

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

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

detiber · 2020-04-24T20:45:03Z

Still working through test failures, but at least a subset of them are related to kubernetes-sigs/controller-runtime#926

detiber · 2020-04-27T16:59:45Z

controlplane/kubeadm/internal/workload_cluster_coredns.go

+	if err != nil {
+		return errors.Wrap(err, "unable to create jsonpatch for CoreDNS config map with backup Corefile")
+	}
+	if err := w.Client.Patch(ctx, cm, ctrlclient.RawPatch(types.JSONPatchType, patchData)); err != nil {


Switched this up, since we were hitting an issue with the fake client not accepting Update without ResourceVersion being set for ConfigMaps (kubernetes-sigs/controller-runtime#926). This should result in the same net effect that we want.

detiber · 2020-04-27T17:02:56Z

controlplane/kubeadm/internal/workload_cluster_coredns_test.go

+		fakeClient := fake.NewFakeClientWithScheme(scheme.Scheme)
+		g.Expect(fakeClient.Create(context.TODO(), depl)).To(Succeed())
+		g.Expect(fakeClient.Create(context.TODO(), cm)).To(Succeed())


This might not be needed, this was introduced when I was testing to make sure the resourceVersioning behavior wasn't what was causing issues

detiber · 2020-04-27T17:03:50Z

third_party/kubernetes-drain/README.md

@@ -1,2 +0,0 @@
-The code in this directory has been copied from:
-github.com/kubernetes/kubectl/pkg/drain@a17d91f9f5b34c73bed0bfc75b70bd762b725231


Good bye third_party/kubernetes-drain, you served us well 👋

detiber · 2020-04-27T17:05:42Z

util/patch/patch_test.go

@@ -51,8 +51,7 @@ func TestHelperUnstructuredPatch(t *testing.T) {
 			},
 		},
 	}
-	fakeClient := fake.NewFakeClientWithScheme(scheme.Scheme)
-	g.Expect(fakeClient.Create(ctx, obj)).To(Succeed())
+	fakeClient := fake.NewFakeClientWithScheme(scheme.Scheme, obj)


With controller-runtime v0.6.0 ResourceVersion behavior is different if resources are populated with NewFakeClientWithScheme (will not set a ResourceVersion if empty) compared to with Create (will set a ResourceVersion)

detiber · 2020-04-27T17:09:18Z

controllers/machinehealthcheck_controller_test.go

+	defer func() {
+		g.Expect(r.Client.Delete(context.Background(), otherNamespace))
+	}()
+	otherNamespaceName := otherNamespace.Name


This is needed because previously envtest was only allowing this due to disabling of admission plugins (namespace not existing already).

detiber · 2020-04-27T17:10:21Z

bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml

-                              description: Required. The taint value corresponding
-                                to the taint key.
+                              description: The taint value corresponding to the taint
+                                key.


This and other CRD yaml updates are related to the referenced api type description updates in k8s v1.18

detiber · 2020-04-27T17:11:04Z

controllers/machine_controller.go

@@ -356,7 +357,7 @@ func (r *MachineReconciler) drainNode(ctx context.Context, cluster *clusterv1.Cl
 		return errors.Errorf("unable to get node %q: %v", nodeName, err)
 	}

-	drainer := &kubedrain.Helper{
+	drainer := &drain.Helper{


Now that kubectl is out of tree we no longer need to maintain a copied version of the drain Helper.

detiber · 2020-04-27T17:28:57Z

The failing capd e2e tests seems like it's a bit racy anyway:

cluster-api/test/infrastructure/docker/e2e/docker_test.go

Lines 212 to 231 in fff092e

    
           Expect(mgmtClient.Delete(ctx, &machineList.Items[0])).To(Succeed()) 
        
           Eventually(func() (int, error) { 
        
           	machineList := &clusterv1.MachineList{} 
        
           	if err := mgmtClient.List(ctx, machineList, inClustersNamespaceListOption, matchClusterListOption); err != nil { 
        
           		fmt.Println(err) 
        
           		return 0, err 
        
           	} 
        
           	return len(machineList.Items), nil 
        
           }, "15m", "5s").Should(Equal(int(*controlPlane.Spec.Replicas) - 1)) 
        
           By("ensuring a replacement machine is created") 
        
           Eventually(func() (int, error) { 
        
           	machineList := &clusterv1.MachineList{} 
        
           	if err := mgmtClient.List(ctx, machineList, inClustersNamespaceListOption, matchClusterListOption); err != nil { 
        
           		fmt.Println(err) 
        
           		return 0, err 
        
           	} 
        
           	return len(machineList.Items), nil 
        
           }, "15m", "30s").Should(Equal(int(*controlPlane.Spec.Replicas)))

It's trying to catch the period where the MachineList count is replicas-2 after manually deleting a Machine outside of the control of KCP, but is polling every 5s, so if the replacement is created before the next polling cycle the test will fail.

k8s-ci-robot · 2020-04-28T15:43:10Z

@detiber: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-cluster-api-apidiff	`c6e4115`	link	`/test pull-cluster-api-apidiff`

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.

detiber · 2020-04-30T15:14:58Z

@vincepri this should be ready for review again.

vincepri · 2020-05-01T18:09:49Z

Following yesterday's feedback in Slack, seems we should cut a new v0.4.0 release to include these breaking changes.

/milestone v0.4.0

k8s-ci-robot · 2020-05-07T18:52:38Z

@detiber: 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.

vincepri · 2020-05-29T18:41:33Z

@detiber Should we close this for now?

detiber · 2020-05-29T19:58:08Z

/close

k8s-ci-robot · 2020-05-29T19:58:14Z

@detiber: Closed this PR.

In response to this:

/close

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.

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 24, 2020

k8s-ci-robot requested review from benmoss and justinsb April 24, 2020 18:13

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

detiber commented Apr 27, 2020

View reviewed changes

detiber added 2 commits April 28, 2020 11:39

Bump controller-runtime to v0.6.0

f9a18e1

Use jsonpatch for coredns update instead of Update

c6e4115

detiber force-pushed the c-r-0.6.0 branch from 083ee4a to c6e4115 Compare April 28, 2020 15:40

detiber changed the title ~~[WIP] ✨ Bump controller-runtime to v0.6.0~~ ✨ Bump controller-runtime to v0.6.0 Apr 28, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 28, 2020

k8s-ci-robot added this to the v0.4.0 milestone May 1, 2020

vincepri mentioned this pull request May 3, 2020

Bump kind to v0.8.0 #2992

Closed

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 7, 2020

vincepri marked this pull request as draft May 7, 2020 19:12

k8s-ci-robot closed this May 29, 2020

vincepri mentioned this pull request Jul 15, 2020

Apply k8s 1.18.5 #3345

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Bump controller-runtime to v0.6.0 #2961

✨ Bump controller-runtime to v0.6.0 #2961

detiber commented Apr 24, 2020

k8s-ci-robot commented Apr 24, 2020

detiber commented Apr 24, 2020

detiber Apr 27, 2020

detiber Apr 27, 2020

detiber Apr 27, 2020

detiber Apr 27, 2020

detiber Apr 27, 2020

detiber Apr 27, 2020

detiber Apr 27, 2020

detiber commented Apr 27, 2020

k8s-ci-robot commented Apr 28, 2020 •

edited

Loading

detiber commented Apr 30, 2020

vincepri commented May 1, 2020

k8s-ci-robot commented May 7, 2020

vincepri commented May 29, 2020

detiber commented May 29, 2020

k8s-ci-robot commented May 29, 2020

		@@ -1,2 +0,0 @@
		The code in this directory has been copied from:
		github.com/kubernetes/kubectl/pkg/drain@a17d91f9f5b34c73bed0bfc75b70bd762b725231

✨ Bump controller-runtime to v0.6.0 #2961

✨ Bump controller-runtime to v0.6.0 #2961

Conversation

detiber commented Apr 24, 2020

k8s-ci-robot commented Apr 24, 2020

detiber commented Apr 24, 2020

detiber Apr 27, 2020

Choose a reason for hiding this comment

detiber Apr 27, 2020

Choose a reason for hiding this comment

detiber Apr 27, 2020

Choose a reason for hiding this comment

detiber Apr 27, 2020

Choose a reason for hiding this comment

detiber Apr 27, 2020

Choose a reason for hiding this comment

detiber Apr 27, 2020

Choose a reason for hiding this comment

detiber Apr 27, 2020

Choose a reason for hiding this comment

detiber commented Apr 27, 2020

k8s-ci-robot commented Apr 28, 2020 • edited Loading

detiber commented Apr 30, 2020

vincepri commented May 1, 2020

k8s-ci-robot commented May 7, 2020

vincepri commented May 29, 2020

detiber commented May 29, 2020

k8s-ci-robot commented May 29, 2020

k8s-ci-robot commented Apr 28, 2020 •

edited

Loading