enable leader election for control plane operator to prevent race conditions on upgrades

[relyt0925]

There is a race condition without leader election for the control plane operator when two instances are active at a given moment of time. It can lead to them simultaneous modifying deployments and resulting them in unexpected states (sometimes a merge of the data between the two versions of the operator). This pr enables leader election for the component by default to eliminate this reace condition

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes # Race condition that can result in unexpected cluster states

Checklist

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

[enxebre]

Thanks! I'm fine with this change though I'd like some discussion aside before proceeding:

@relyt0925 How is it that you have 2 instances of CPO running simultaneously? is it because an upgrade? Is it ha topology?

For now, we do not want to turn on leader election because it produces aggregate load on the kube-apiserver that can lead to cluster performance degregation at scale.

Shouldn't we be enforcing leader election for the cpo from the hc controller? @ironcladlou @csrwng @alvaroaleman @sjenning

@relyt0925 Are there evidences of this happening atm you can share? If not, I'd prefer to use leader election consistently (so we ensure no simultaneous controllers are ever active regardless of the deployment replica number) and not to try to solve a problem we don't have yet.

[alvaroaleman]

Yeah, we definitely should be using leader election, I thought we already did.

[ironcladlou]

I agree with @enxebre's assessment:

@relyt0925 Are there evidences of this happening atm you can share? If not, I'd prefer to use leader election consistently (so we ensure no simultaneous controllers are ever active regardless of the deployment replica number) and not to try to solve a problem we don't have yet.

It seems like elections would solve it generically.

[relyt0925]

I posted evidence in this thread:
https://coreos.slack.com/archives/C01C8502FMM/p1643409582906399?thread_ts=1643404538.076709&channel=C01C8502FMM&message_ts=1643409582.906399

but yes it does occur

[ironcladlou]

Another thought I had was that given these components aren't servicing client connections, the scope of disruption using recreate seems to be temporary pause in reconciliation. So I don't know there's any specific advantage to using rolling updates in this case, and they definitely introduce their own complexities.

[netlify]

✔️ Deploy Preview for hypershift-docs ready!

🔨 Explore the source changes: 2ff65b1

🔍 Inspect the deploy log: https://app.netlify.com/sites/hypershift-docs/deploys/61f802b091bfc3000728c9c6

😎 Browse the preview: https://deploy-preview-935--hypershift-docs.netlify.app

[alvaroaleman]

/rettitle Enable leader election for control plane operator

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, relyt0925

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

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

[enxebre]

Assuming we are using the version of controller runtime which uses leases by default, you will need to update reconcileControlPlaneOperatorRole to manage them, e.g

		{
			APIGroups: []string{"coordination.k8s.io"},
			Resources: []string{
				"leases",
			},
			Verbs: []string{"*"},
		},

[enxebre]

I think controller runtime uses configmapsleases by default. Can we please make sure we set LeaderElectionResourceLock: "leases", in the CPO?
/hold

[relyt0925]

good catch updating

[relyt0925]

/unhold

[sjenning]

/lgtm

[openshift-ci]

@relyt0925: 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/test-infra repository. I understand the commands that are listed here.