Bug 2093462: status: Watch clusteroperators by Miciah · Pull Request #714 · openshift/cluster-ingress-operator

Miciah · 2022-03-03T20:26:06Z

Add a watch on clusteroperators in the status controller so that it updates or recreates the ingress clusteroperator as necessary should something else update or delete it.

manifests/00-cluster-role.yaml: Allow the operator to list and watch clusteroperators.
pkg/operator/controller/status/controller.go (New): Watch clusteroperators with a map function and predicate to reconcile the default ingresscontroller if clusteroperators.config.openshift.io/ingress is updated.
test/e2e/all_test.go (TestAll): Add the new TestOperatorRecreatesItsClusterOperator test to the serial tests.
test/e2e/operator_test.go (TestOperatorRecreatesItsClusterOperator): New test. Verify that the operator recreates the "ingress" clusteroperator if the clusteroperator is deleted.

This PR is the cluster-ingress-operator equivalent of openshift/cluster-dns-operator#261.

This PR supersedes #702. Cc: @mfojtik

openshift-bot · 2022-06-02T00:33:10Z

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

gcs278 · 2022-06-03T19:06:45Z

The new E2E test will need to be added to all_tests.go due to E2E updates

gcs278 · 2022-06-03T19:18:32Z

/retitle Bug 2093462: status: Watch clusteroperators

openshift-ci · 2022-06-03T19:18:37Z

@Miciah: This pull request references Bugzilla bug 2093462, which is invalid:

expected the bug to target the "4.11.0" release, but it targets "---" 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.

Details

In response to this:

Bug 2093462: status: Watch clusteroperators

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.

gcs278 · 2022-06-03T19:18:47Z

/bugzilla refresh

openshift-ci · 2022-06-03T19:18:56Z

@gcs278: This pull request references Bugzilla bug 2093462, 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.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.11.0) matches configured target release for branch (4.11.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @lihongan

Details

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.

openshift-ci · 2022-06-03T20:44:58Z

@Miciah: This pull request references Bugzilla bug 2093462, which is invalid:

expected the bug to target the "4.11.0" release, but it targets "---" 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.

Details

In response to this:

Bug 2093462: status: Watch clusteroperators

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.

Miciah · 2022-06-03T20:45:49Z

Updated for #756.

Miciah · 2022-06-03T20:52:08Z

/bugzilla refresh

openshift-ci · 2022-06-03T20:52:18Z

@Miciah: This pull request references Bugzilla bug 2093462, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.11.0) matches configured target release for branch (4.11.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @lihongan

Details

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.

Miciah · 2022-06-03T21:53:27Z

         --- FAIL: TestAll/parallel/TestHAProxyTimeouts (35.86s)
        --- FAIL: TestAll/parallel/TestHAProxyTimeoutsRejection (38.09s)

/test e2e-aws-operator

Miciah · 2022-06-06T04:37:54Z

Must-gather.
/test e2e-aws-operator

Failing invariants:

[bz-monitoring][invariant] alert/Watchdog must have no gaps or changes

/test e2e-aws-single-node

candita · 2022-06-07T18:01:06Z

I think the bindata.go update was missing from openshift/cluster-dns-operator#261

candita · 2022-06-07T17:44:49Z

pkg/operator/controller/status/controller.go

+		&source.Kind{Type: &configv1.ClusterOperator{}},
+		// The status controller doesn't care which ingresscontroller it
+		// is reconciling, so just enqueue a request to reconcile the
+		// default ingresscontroller.


Why isn't it necessary to reconcile all the ingresscontrollers? Because there is only one cluster operator for all ingresscontrollers?

Right, the status controller just needs some ingresscontroller for its reconciliation request, which will cause it to update the "ingress" clusteroperator, and there should always be a "default" ingresscontroller.

If we accept #778, there might not be a default ingresscontroller, but I think more work probably needs to be done on that.

candita · 2022-06-07T17:53:03Z

test/e2e/operator_test.go

+	}
+	if err := kclient.Delete(context.TODO(), co); err != nil {
+		t.Fatalf("failed to delete clusteroperator %q: %v", coName.Name, err)
+	}


Is there a way to check/wait until it's deleted before verifying that it is recreated automatically?

I could add a check to verify that the UID changed.

Miciah · 2022-06-08T16:27:12Z

I think the bindata.go update was missing from openshift/cluster-dns-operator#261

The ingress operator only includes manifests/ in its bindata.go because the ingress-operator executable includes a subcommand that emits the manifests, which the installer uses to provide manifests that the user can edit to customize them at install-time. See c77d6aa.

Miciah · 2022-06-08T16:33:25Z

Last push updates TestOperatorRecreatesItsClusterOperator to check the clusteroperator's deletion timestamp and uid to verify that it really has been recreated.

candita · 2022-06-08T20:02:45Z

/test e2e-upgrade

Add a watch on clusteroperators in the status controller so that it updates or recreates the ingress clusteroperator as necessary should something else update or delete it. This commit fixes bug 2093462. https://bugzilla.redhat.com/show_bug.cgi?id=2093462 * manifests/00-cluster-role.yaml: Allow the operator to list and watch clusteroperators. * pkg/manifests/bindata.go: Regenerate. * pkg/operator/controller/status/controller.go (New): Watch clusteroperators with a map function and predicate to reconcile the default ingresscontroller if clusteroperators.config.openshift.io/ingress is updated. * test/e2e/all_test.go (TestAll): Add the new TestOperatorRecreatesItsClusterOperator test to the serial tests. * test/e2e/operator_test.go (TestOperatorRecreatesItsClusterOperator): New test. Verify that the operator recreates the "ingress" clusteroperator if the clusteroperator is deleted.

Miciah · 2022-06-23T15:37:54Z

Rebased.

candita · 2022-06-23T19:18:47Z

/lgtm

candita · 2022-06-23T19:19:21Z

/test e2e-aws-operator

openshift-ci · 2022-06-23T19:19:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: candita, Miciah

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:

~~OWNERS~~ [Miciah,candita]

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

Miciah · 2022-06-23T21:35:36Z

/retest

openshift-ci-robot · 2022-06-23T23:42:49Z

/retest-required

Remaining retests: 2 against base HEAD 84d7e67 and 8 for PR HEAD a21bde2 in total

openshift-ci-robot · 2022-06-24T01:43:15Z

/retest-required

Remaining retests: 1 against base HEAD 84d7e67 and 7 for PR HEAD a21bde2 in total

openshift-ci-robot · 2022-06-24T03:42:46Z

/retest-required

Remaining retests: 0 against base HEAD 84d7e67 and 6 for PR HEAD a21bde2 in total

openshift-ci · 2022-06-24T07:51:53Z

@Miciah: all tests passed!

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.

openshift-ci · 2022-06-24T07:54:12Z

@Miciah: All pull requests linked via external trackers have merged:

openshift/cluster-ingress-operator#714

Bugzilla bug 2093462 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2093462: status: Watch clusteroperators

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.

Miciah mentioned this pull request Mar 3, 2022

Properly reconcile and react to changes in cluster operator resource #702

Closed

openshift-ci bot requested review from knobunc and miheer March 3, 2022 20:27

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2022

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

openshift-ci bot changed the title ~~status: Watch clusteroperators~~ Bug 2093462: status: Watch clusteroperators Jun 3, 2022

openshift-ci bot added the bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. label Jun 3, 2022

openshift-ci bot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jun 3, 2022

openshift-ci bot 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 Jun 3, 2022

openshift-ci bot requested a review from lihongan June 3, 2022 19:18

Miciah force-pushed the status-watch-clusteroperators branch 2 times, most recently from 9c35ad7 to 869481c Compare June 3, 2022 20:44

openshift-ci bot 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 Jun 3, 2022

candita reviewed Jun 7, 2022

View reviewed changes

Miciah force-pushed the status-watch-clusteroperators branch from 869481c to 1d65abf Compare June 8, 2022 16:32

Miciah force-pushed the status-watch-clusteroperators branch from 1d65abf to a21bde2 Compare June 23, 2022 15:37

openshift-ci bot assigned candita Jun 23, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 23, 2022

openshift-ci bot merged commit de9a92f into openshift:master Jun 24, 2022

Conversation

Miciah commented Mar 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Jun 2, 2022

Uh oh!

gcs278 commented Jun 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gcs278 commented Jun 3, 2022

Uh oh!

openshift-ci bot commented Jun 3, 2022

Uh oh!

gcs278 commented Jun 3, 2022

Uh oh!

openshift-ci bot commented Jun 3, 2022

Uh oh!

openshift-ci bot commented Jun 3, 2022

Uh oh!

Miciah commented Jun 3, 2022

Uh oh!

Miciah commented Jun 3, 2022

Uh oh!

openshift-ci bot commented Jun 3, 2022

Uh oh!

Miciah commented Jun 3, 2022

Uh oh!

Miciah commented Jun 6, 2022

Uh oh!

candita commented Jun 7, 2022

Uh oh!

candita Jun 7, 2022

Choose a reason for hiding this comment

Uh oh!

Miciah Jun 8, 2022

Choose a reason for hiding this comment

Uh oh!

candita Jun 23, 2022

Choose a reason for hiding this comment

Uh oh!

candita Jun 7, 2022

Choose a reason for hiding this comment

Uh oh!

Miciah Jun 8, 2022

Choose a reason for hiding this comment

Uh oh!

Miciah commented Jun 8, 2022

Uh oh!

Miciah commented Jun 8, 2022

Uh oh!

candita commented Jun 8, 2022

Uh oh!

Miciah commented Jun 23, 2022

Uh oh!

candita commented Jun 23, 2022

Uh oh!

candita commented Jun 23, 2022

Uh oh!

openshift-ci bot commented Jun 23, 2022

Uh oh!

Miciah commented Jun 23, 2022

Uh oh!

openshift-ci-robot commented Jun 23, 2022

Uh oh!

openshift-ci-robot commented Jun 24, 2022

Uh oh!

openshift-ci-robot commented Jun 24, 2022

Uh oh!

openshift-ci bot commented Jun 24, 2022

Uh oh!

openshift-ci bot commented Jun 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Miciah commented Mar 3, 2022 •

edited

Loading

gcs278 commented Jun 3, 2022 •

edited

Loading