Bug 1744984: Add e2e test for kube controller manager lease configmap deletion #311

damemi · 2019-11-12T21:42:06Z

This is a work in progress, trying to create a simple e2e that deletes the KCM configmap then attempts to create a replicaset to see if KCM recovered from the deletion, as described here: https://bugzilla.redhat.com/show_bug.cgi?id=1744984

openshift-ci-robot · 2019-11-12T21:42:20Z

@damemi: This pull request references Bugzilla bug 1744984, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1744984: Add e2e test for kube controller manager lease configmap deletion

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.

damemi · 2019-11-13T14:19:43Z

/retest

soltysh · 2019-11-13T16:00:50Z

test/e2e/operator_test.go

+	_, err = kubeClient.AppsV1().ReplicaSets(operatorclient.OperatorNamespace).Create(replicaSet)
+	if err != nil {
+		t.Fatal(err)
+	}


You're missing a bit where you wait and verify if that replica set actually run and KCM recovered.

Updated to add a wait loop that looks for pods created with the replicaset labels, that would indicate the KCM has recovered right?

deads2k · 2019-11-14T18:52:50Z

test/e2e/operator_test.go

+		}
+	}
+
+	// Now try to create a replicaset and see that it is successfully created by the controller manager


wait for the original pods to be gone before creating this.

deads2k · 2019-11-26T14:48:29Z

test/e2e/operator_test.go

+	}
+
+	// Get all the currently running KCM pods, and then delete them
+	pods, err := kubeClient.CoreV1().Pods("openshift-kube-controller-manager").List(metav1.ListOptions{})


this is pointless

deads2k · 2019-11-26T14:48:44Z

test/e2e/operator_test.go

+			},
+		},
+	}
+	_, err = kubeClient.AppsV1().ReplicaSets(operatorclient.OperatorNamespace).Create(replicaSet)


because the pods aren't gone, this doesn't matter

test/e2e/operator_test.go

deads2k · 2019-11-27T13:58:09Z

test/e2e/operator_test.go

+	}
+
+	// Check to see that the configmap then gets recreated
+	wait.Poll(time.Second*5, time.Second*300, func() (bool, error) {


you forgot to check this error.

deads2k · 2019-11-27T13:58:37Z

test/e2e/operator_test.go

+	// Check to see that the configmap then gets recreated
+	wait.Poll(time.Second*5, time.Second*300, func() (bool, error) {
+		_, err := kubeClient.CoreV1().ConfigMaps("kube-system").Get("kube-controller-manager", metav1.GetOptions{})
+		if err != nil {


if errors.IsNotFound(err){ return false, nil } if err != nil{ return false, err } return true, nil

…overy

deads2k · 2019-12-02T20:20:51Z

/lgtm

openshift-bot · 2019-12-02T21:06:01Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-02T21:32:04Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-02T22:24:23Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-02T22:37:23Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-02T23:17:50Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2019-12-05T21:57:30Z

@damemi: This pull request references Bugzilla bug 1744984, which is invalid:

expected the bug to target the "4.4.0" release, but it targets "4.3.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Ok, switching to 2 namespaced Roles (as opposed to 1 ClusterRole) seems to fix the problem, and all tests pass. Squashing now and this is ready for final review
cc @soltysh @deads2k
/bugzilla refresh

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.

damemi · 2019-12-06T13:21:19Z

/retest

deads2k · 2019-12-06T16:04:10Z

....1.0/kube-controller-manager/leader-election-cluster-policy-controller-role-kube-system.yaml

+kind: Role
+metadata:
+  namespace: kube-system
+  name: system:openshift:leader-election-lock-cluster-policy-controller


this name doesn't match what you're binding to. This is a general leader election role

deads2k · 2019-12-06T16:04:30Z

....1.0/kube-controller-manager/leader-election-cluster-policy-controller-role-kube-system.yaml

+  - get
+  - list
+  - watch
+  - create


only create needs to be unconstrained by names. The rest can use the stock role, right?

…stem namespace

damemi · 2019-12-06T18:11:02Z

@deads2k updated with your feedback, thanks

damemi · 2019-12-07T16:04:14Z

/bugzilla refresh

openshift-ci-robot · 2019-12-07T16:04:18Z

@damemi: This pull request references Bugzilla bug 1744984, which is valid.

In response to this:

/bugzilla refresh

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.

soltysh

/lgtm
/hold cancel

soltysh · 2019-12-09T12:16:07Z

/cherry-pick release-4.3

openshift-cherrypick-robot · 2019-12-09T12:16:08Z

@soltysh: once the present PR merges, I will cherry-pick it on top of release-4.3 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.3

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.

openshift-ci-robot · 2019-12-09T12:16:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damemi, deads2k, soltysh

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~~ [deads2k,soltysh]

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

openshift-bot · 2019-12-09T13:53:27Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2019-12-09T14:05:55Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2019-12-09T15:25:58Z

@damemi: All pull requests linked via external trackers have merged. Bugzilla bug 1744984 has been moved to the MODIFIED state.

In response to this:

Bug 1744984: Add e2e test for kube controller manager lease configmap deletion

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.

openshift-cherrypick-robot · 2019-12-09T15:26:11Z

@soltysh: new pull request created: #321

In response to this:

/cherry-pick release-4.3

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.

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 12, 2019

openshift-ci-robot requested review from deads2k and soltysh November 12, 2019 21:42

soltysh requested changes Nov 13, 2019

View reviewed changes

damemi force-pushed the kcm-recovery-test branch 2 times, most recently from 9cd6ba9 to 1016466 Compare November 13, 2019 18:37

deads2k reviewed Nov 14, 2019

View reviewed changes

deads2k reviewed Nov 26, 2019

View reviewed changes

test/e2e/operator_test.go Outdated Show resolved Hide resolved

deads2k reviewed Nov 26, 2019

View reviewed changes

test/e2e/operator_test.go Show resolved Hide resolved

damemi force-pushed the kcm-recovery-test branch 3 times, most recently from ef569c6 to 64c65ca Compare November 26, 2019 20:14

deads2k reviewed Nov 27, 2019

View reviewed changes

Add e2e test for kube controller manager lease configmap deletion rec…

87dfc9b

…overy

damemi force-pushed the kcm-recovery-test branch from 64c65ca to 87dfc9b Compare December 2, 2019 20:10

openshift-ci-robot assigned deads2k Dec 2, 2019

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 2, 2019

openshift-ci-robot added bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. and removed bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Dec 5, 2019

deads2k reviewed Dec 6, 2019

View reviewed changes

damemi force-pushed the kcm-recovery-test branch from b4023d6 to 98ea815 Compare December 6, 2019 16:16

Update RBAC roles and bindings to allow configmap creation in kube-sy…

263dd58

…stem namespace

damemi force-pushed the kcm-recovery-test branch from 98ea815 to 263dd58 Compare December 6, 2019 16:17

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Dec 7, 2019

soltysh approved these changes Dec 9, 2019

View reviewed changes

openshift-ci-robot assigned soltysh Dec 9, 2019

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Dec 9, 2019

openshift-merge-robot merged commit 096960a into openshift:master Dec 9, 2019

openshift-cherrypick-robot mentioned this pull request Dec 9, 2019

[release-4.3] Bug 1780843: Add e2e test for kube controller manager lease configmap deletion #321

Merged

damemi deleted the kcm-recovery-test branch December 9, 2019 15:40

This was referenced Dec 11, 2019

Bug 1718061: Fix KCM leader election configmap deletion recovery #323

Merged

Bug 1781240: Fix leader election configmap deletion recovery #324

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1744984: Add e2e test for kube controller manager lease configmap deletion #311

Bug 1744984: Add e2e test for kube controller manager lease configmap deletion #311

damemi commented Nov 12, 2019

openshift-ci-robot commented Nov 12, 2019

damemi commented Nov 13, 2019

soltysh Nov 13, 2019

damemi Nov 13, 2019

deads2k Nov 14, 2019

deads2k Nov 26, 2019

deads2k Nov 26, 2019

deads2k Nov 27, 2019

deads2k Nov 27, 2019

deads2k commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-ci-robot commented Dec 5, 2019

damemi commented Dec 6, 2019

deads2k Dec 6, 2019

deads2k Dec 6, 2019

damemi commented Dec 6, 2019

damemi commented Dec 7, 2019

openshift-ci-robot commented Dec 7, 2019

soltysh left a comment

soltysh commented Dec 9, 2019

openshift-cherrypick-robot commented Dec 9, 2019

openshift-ci-robot commented Dec 9, 2019

openshift-bot commented Dec 9, 2019

openshift-bot commented Dec 9, 2019

openshift-ci-robot commented Dec 9, 2019

openshift-cherrypick-robot commented Dec 9, 2019

Bug 1744984: Add e2e test for kube controller manager lease configmap deletion #311

Bug 1744984: Add e2e test for kube controller manager lease configmap deletion #311

Conversation

damemi commented Nov 12, 2019

openshift-ci-robot commented Nov 12, 2019

damemi commented Nov 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-bot commented Dec 2, 2019

openshift-ci-robot commented Dec 5, 2019

damemi commented Dec 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damemi commented Dec 6, 2019

damemi commented Dec 7, 2019

openshift-ci-robot commented Dec 7, 2019

soltysh left a comment

Choose a reason for hiding this comment

soltysh commented Dec 9, 2019

openshift-cherrypick-robot commented Dec 9, 2019

openshift-ci-robot commented Dec 9, 2019

openshift-bot commented Dec 9, 2019

openshift-bot commented Dec 9, 2019

openshift-ci-robot commented Dec 9, 2019

openshift-cherrypick-robot commented Dec 9, 2019