add check of lease length to allow 60s of kube-apiserver communication disruption by deads2k · Pull Request #26215 · openshift/origin

deads2k · 2021-06-09T16:07:00Z

We cannot find them perfectly, but we can find those that are obviously broken.

There will need to be a library-go PR.

romfreiman · 2021-06-09T21:02:08Z

test/extended/operators/leases.go

+			if err := json.Unmarshal([]byte(leaderElection), leaderElectionRecord); err != nil {
+				o.Expect(err).NotTo(o.HaveOccurred())
+			}
+			if leaderElectionRecord.LeaseDurationSeconds < 90 {


Use 90 as const.
Didnt we say 60?

romfreiman · 2021-06-09T21:03:07Z

test/extended/operators/leases.go

@@ -0,0 +1,77 @@
+package operators
+
+import (


do we want to start from exclude list so the test will be green?

do we want to start from exclude list so the test will be green?

given how far off we are, let's close a few and then add the list. Many ought to be easy.

romfreiman · 2021-06-09T21:03:52Z

test/extended/operators/leases.go

+				o.Expect(err).NotTo(o.HaveOccurred())
+			}
+			if leaderElectionRecord.LeaseDurationSeconds < 90 {
+				shortLeases = append(shortLeases, fmt.Sprintf("configmap/%s used by %q, has too short a lease to span 60s kube-apiserver disruption.  Try 99s leaseDuration with 13s retryPeriod and a 17s renewDeadline.  Be sure you have the graceful release properly wired.", endpoint.Name, leaderElectionRecord.HolderIdentity))


endpoints, not configmap

romfreiman · 2021-06-09T21:04:35Z

test/extended/operators/leases.go

+		endpoints, err := kubeClient.CoreV1().Endpoints("").List(ctx, metav1.ListOptions{})
+		o.Expect(err).NotTo(o.HaveOccurred())
+		for _, endpoint := range endpoints.Items {
+			leaderElection, ok := endpoint.Annotations[resourcelock.LeaderElectionRecordAnnotationKey]


maybe to have this as an function and call it both for cm and endpoints?

romfreiman · 2021-06-09T21:05:09Z

test/extended/operators/leases.go

+		o.Expect(err).NotTo(o.HaveOccurred())
+		for _, lease := range leases.Items {
+			if lease.Spec.LeaseDurationSeconds != nil && *lease.Spec.LeaseDurationSeconds < 90 {
+				shortLeases = append(shortLeases, fmt.Sprintf("configmap/%s used by %q, has too short a lease to span 60s kube-apiserver disruption.  Try 99s leaseDuration with 13s retryPeriod and a 17s renewDeadline.  Be sure you have the graceful release properly wired.", lease.Name, lease.Spec.HolderIdentity))


leases, not configmaps

openshift-ci · 2021-06-10T15:34:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/extended/operators/OWNERS~~ [deads2k]
~~test/extended/util/annotate/OWNERS~~ [deads2k]

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

…n disruption

openshift-ci · 2021-06-25T17:14:05Z

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

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp-disruptive	`5b0e26a`	link	`/test e2e-gcp-disruptive`
ci/prow/e2e-gcp	`5b0e26a`	link	`/test e2e-gcp`
ci/prow/e2e-metal-ipi-ovn-ipv6	`5b0e26a`	link	`/test e2e-metal-ipi-ovn-ipv6`
ci/prow/e2e-gcp-upgrade	`5b0e26a`	link	`/test e2e-gcp-upgrade`
ci/prow/e2e-aws-fips	`5b0e26a`	link	`/test e2e-aws-fips`
ci/prow/e2e-aws-serial	`5b0e26a`	link	`/test e2e-aws-serial`
ci/prow/e2e-aws-disruptive	`5b0e26a`	link	`/test e2e-aws-disruptive`
ci/prow/e2e-aws-jenkins	`5b0e26a`	link	`/test e2e-aws-jenkins`

Full PR test history. Your PR dashboard.

Details

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.

p0lyn0mial · 2021-07-13T10:57:06Z

test/extended/operators/leases.go

+
+		shortLeases := []string{}
+
+		configMaps, err := kubeClient.CoreV1().ConfigMaps("").List(ctx, metav1.ListOptions{})


how about using a paginated list instead?

p0lyn0mial · 2021-07-13T10:57:58Z

test/extended/operators/leases.go

+			}
+		}
+
+		leases, err := kubeClient.CoordinationV1().Leases("").List(ctx, metav1.ListOptions{})


it looks like kube-apiservers use the lease, we could filter them out

romfreiman · 2021-07-18T13:09:20Z

@deads2k is this test will run as part of the fips suite?

romfreiman · 2021-07-20T19:41:34Z

@deads2k I assume you have to update the values according to the enhancement?
https://github.com/openshift/enhancements/pull/832/files#diff-2e28754e69aa417e5b6d89e99e42f05bfb6330800fa823753383db1d170fbc2fR183

openshift-bot · 2021-10-18T21:10:38Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

romfreiman · 2021-10-31T09:18:41Z

@deads2k dont we want to merge this test?

romfreiman · 2021-10-31T09:18:57Z

/remove-lifecycle stale

openshift-bot · 2022-01-29T10:47:37Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2022-02-28T11:07:11Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-bot · 2022-03-30T11:32:54Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2022-03-30T11:33:30Z

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/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.

openshift-ci bot requested review from adambkaplan and smarterclayton June 9, 2021 16:07

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 9, 2021

deads2k force-pushed the leases branch from 6fcd477 to 1399e8e Compare June 9, 2021 16:07

deads2k mentioned this pull request Jun 9, 2021

update leader election defaults so it handles 60s of kube-apiserver communication disruption openshift/library-go#1104

Merged

romfreiman reviewed Jun 9, 2021

View reviewed changes

deads2k force-pushed the leases branch from 1399e8e to 03616be Compare June 10, 2021 15:34

deads2k force-pushed the leases branch from 03616be to db75752 Compare June 10, 2021 15:38

add check of lease length to allow 60s of kube-apiserver communicatio…

5b0e26a

…n disruption

deads2k force-pushed the leases branch from db75752 to 5b0e26a Compare June 10, 2021 20:49

p0lyn0mial reviewed Jul 13, 2021

View reviewed changes

romfreiman mentioned this pull request Jul 13, 2021

handling kube-apiserver disruption openshift/enhancements#832

Merged

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 18, 2021

openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 31, 2021

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 29, 2022

openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 28, 2022

openshift-ci bot added the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Feb 28, 2022

openshift-ci bot closed this Mar 30, 2022


		shortLeases := []string{}

		configMaps, err := kubeClient.CoreV1().ConfigMaps("").List(ctx, metav1.ListOptions{})

Comments

Conversation

deads2k commented Jun 9, 2021

Uh oh!

romfreiman Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romfreiman Jun 9, 2021

Choose a reason for hiding this comment

Uh oh!

deads2k Jun 10, 2021

Choose a reason for hiding this comment

Uh oh!

romfreiman Jun 9, 2021

Choose a reason for hiding this comment

Uh oh!

romfreiman Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

romfreiman Jun 9, 2021

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jun 10, 2021

Uh oh!

openshift-ci bot commented Jun 25, 2021

Uh oh!

p0lyn0mial Jul 13, 2021

Choose a reason for hiding this comment

Uh oh!

p0lyn0mial Jul 13, 2021

Choose a reason for hiding this comment

Uh oh!

romfreiman commented Jul 18, 2021

Uh oh!

romfreiman commented Jul 20, 2021

Uh oh!

openshift-bot commented Oct 18, 2021

Uh oh!

romfreiman commented Oct 31, 2021

Uh oh!

romfreiman commented Oct 31, 2021

Uh oh!

openshift-bot commented Jan 29, 2022

Uh oh!

openshift-bot commented Feb 28, 2022

Uh oh!

openshift-bot commented Mar 30, 2022

Uh oh!

openshift-ci bot commented Mar 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

romfreiman Jun 9, 2021 •

edited

Loading

romfreiman Jun 9, 2021 •

edited

Loading