OCPBUGS-34077: Always Disable Default Rolebindings Controller

[adambkaplan]

With OCPBUILD-7, default namespace rolebindings for the Image Registry, DeploymentConfig, and Builds are handled by separate controllers. The initial refactor in OCPBUILD-8 kept the default rolebindings controller in place to allow upgrade from 4.15 -> 4.16. However, the corresponding operator change in OCPBUILD-9 forgot to disable the default rolebindings controller. This leads to the following side effects:

• On clusters with no capabilities, the default role bindings are created and reconciled.
• On a standard installation, there is potential for race conditions and server-side conflicts as two or more controllers fight to reconcile a resource. At scale, this can lead to excessive load on the apiserver.

This change ensures that the default rolebindings controller is always disabled. As a follow-up, the code for the default rolebindings controller can be safely removed in OCP 4.17.

[openshift-ci-robot]

@adambkaplan: This pull request references Jira Issue OCPBUGS-34077, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.17.0) matches configured target version for branch (4.17.0)
• bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jitendar-singh

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

With OCPBUILD-7, default namespace rolebindings for the Image Registry, DeploymentConfig, and Builds are handled by separate controllers. The initial refactor in OCPBUILD-8 kept the default rolebindings controller in place to allow upgrade from 4.15 -> 4.16. However, the corresponding operator change in OCPBUILD-9 forgot to disable the default rolebindings controller. This leads to the following side effects:

• On clusters with no capabilities, the default role bindings are created and reconciled.
• On a standard installation, there is potential for race conditions and server-side conflicts as two or more controllers fight to reconcile a resource. At scale, this can lead to excessive load on the apiserver.

This change ensures that the default rolebindings controller is always disabled. As a follow-up, the code for the default rolebindings controller can be safely removed in OCP 4.17.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@adambkaplan: This pull request references Jira Issue OCPBUGS-34077, which is valid.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @jitendar-singh

In response to this:

With OCPBUILD-7, default namespace rolebindings for the Image Registry, DeploymentConfig, and Builds are handled by separate controllers. The initial refactor in OCPBUILD-8 kept the default rolebindings controller in place to allow upgrade from 4.15 -> 4.16. However, the corresponding operator change in OCPBUILD-9 forgot to disable the default rolebindings controller. This leads to the following side effects:

• On clusters with no capabilities, the default role bindings are created and reconciled.
• On a standard installation, there is potential for race conditions and server-side conflicts as two or more controllers fight to reconcile a resource. At scale, this can lead to excessive load on the apiserver.

This change ensures that the default rolebindings controller is always disabled. As a follow-up, the code for the default rolebindings controller can be safely removed in OCP 4.17.

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 openshift-eng/jira-lifecycle-plugin repository.

[adambkaplan]

/assign @apoorvajagtap

/cc @ingvagabund

[apoorvajagtap]

/retest

[apoorvajagtap]

@adambkaplan thanks for taking care of this!
I tried testing it via a CI cluster, but the default-rolebindings controller was still not disabled. It seems operator.disableControllers just "syncs" the controller states.

I believe, we need to append the controller details in the controller.disabledControllers as well.

func ObserveControllers(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
...
        // compile list of controllers to disable
	var disabledControllers []openshiftcontrolplanev1.OpenShiftControllerName

>>	// openshift.io/default-rolebindings controller shall be always disabled:
>>	disabledControllers = append(disabledControllers, openshiftcontrolplanev1.OpenShiftDefaultRoleBindingsController)
...

[adambkaplan]

@ingvagabund now I am wondering why I am duplicating logic. Should this code be removed in favor of what is in observe_controllers.go?

[adambkaplan]

Discussed in slack - for now we are going to leave this logic in place. I may craft a follow-up PR to address this concern.

[adambkaplan]

/label qe-approved

@apoorvajagtap tested this on a no-capabilities cluster, then enabled Build, Image Registry, and DeploymentConfig one by one.

[openshift-ci-robot]

@adambkaplan: This pull request references Jira Issue OCPBUGS-34077, which is valid.

3 validation(s) were run on this bug

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

No GitHub users were found matching the public email listed for the QA contact in Jira (olemefer@redhat.com), skipping review request.

In response to this:

With OCPBUILD-7, default namespace rolebindings for the Image Registry, DeploymentConfig, and Builds are handled by separate controllers. The initial refactor in OCPBUILD-8 kept the default rolebindings controller in place to allow upgrade from 4.15 -> 4.16. However, the corresponding operator change in OCPBUILD-9 forgot to disable the default rolebindings controller. This leads to the following side effects:

• On clusters with no capabilities, the default role bindings are created and reconciled.
• On a standard installation, there is potential for race conditions and server-side conflicts as two or more controllers fight to reconcile a resource. At scale, this can lead to excessive load on the apiserver.

This change ensures that the default rolebindings controller is always disabled. As a follow-up, the code for the default rolebindings controller can be safely removed in OCP 4.17.

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 openshift-eng/jira-lifecycle-plugin repository.

[adambkaplan]

/label docs-approved

Documentation via release note in JIRA.

[adambkaplan]

/label px-approved

No change to product experience - admin workflow to enable a capability is not changing.

[apoorvajagtap]

/label qe-approved

@apoorvajagtap tested this on a no-capabilities cluster, then enabled Build, Image Registry, and DeploymentConfig one by one.

The default-rolebindings controller was disabled by default for a no-caps cluster. Adding the config cm for reference:

"controllers":["-openshift.io/build","-openshift.io/build-config-change","-openshift.io/builder-rolebindings",
"-openshift.io/builder-serviceaccount","-openshift.io/default-rolebindings","-openshift.io/deployer",
"-openshift.io/deployer-rolebindings","-openshift.io/deployer-serviceaccount",
"-openshift.io/deploymentconfig","openshift.io/image-import","-openshift.io/image-puller-rolebindings",
"openshift.io/image-signature-import","openshift.io/image-trigger","openshift.io/ingress-ip",
"openshift.io/ingress-to-route","openshift.io/origin-namespace","openshift.io/serviceaccount",
"-openshift.io/serviceaccount-pull-secrets","openshift.io/templateinstance",
"openshift.io/templateinstancefinalizer","openshift.io/unidling"]

[apoorvajagtap]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, apoorvajagtap

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 [adambkaplan]

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

[openshift-ci]

@adambkaplan: all tests passed!

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

[openshift-ci-robot]

@adambkaplan: Jira Issue OCPBUGS-34077: All pull requests linked via external trackers have merged:

• openshift/cluster-openshift-controller-manager-operator#346

Jira Issue OCPBUGS-34077 has been moved to the MODIFIED state.

In response to this:

With OCPBUILD-7, default namespace rolebindings for the Image Registry, DeploymentConfig, and Builds are handled by separate controllers. The initial refactor in OCPBUILD-8 kept the default rolebindings controller in place to allow upgrade from 4.15 -> 4.16. However, the corresponding operator change in OCPBUILD-9 forgot to disable the default rolebindings controller. This leads to the following side effects:

• On clusters with no capabilities, the default role bindings are created and reconciled.
• On a standard installation, there is potential for race conditions and server-side conflicts as two or more controllers fight to reconcile a resource. At scale, this can lead to excessive load on the apiserver.

This change ensures that the default rolebindings controller is always disabled. As a follow-up, the code for the default rolebindings controller can be safely removed in OCP 4.17.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-bot]

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-openshift-controller-manager-operator-container-v4.17.0-202405240410.p0.g9e5ec64.assembly.stream.el9 for distgit ose-cluster-openshift-controller-manager-operator.
All builds following this will include this PR.