Skip to content

Implement leader election for manager #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Jul 14, 2020
Merged

Implement leader election for manager #55

openshift-merge-robot merged 1 commit into from
Jul 14, 2020

Conversation

@Danil-Grigorev
Copy link

@Danil-Grigorev Danil-Grigorev commented Jul 13, 2020

Implemented leader election for ovirt manager, needed to compatibility with latest machine-api

Couple of new cli arguments for configuring leader election:

  -leader-elect
    	Start a leader election client and gain leadership before executing the main loop. Enable this when running replicated components for high availability. (default true)
  -leader-elect-lease-duration int
    	The duration that non-leader candidates will wait after observing a leadership renewal until attempting to acquire leadership of a led but unrenewed leader slot. This is effectively the maximum duration that a leader can be stopped before it is replaced by another candidate. This is only applicable if leader election is enabled. (default 15s)
  -leader-elect-resource-namespace string
    	The namespace of resource object that is used for locking during leader election. If unspecified, the controller watches for machine-api objects across all namespaces.

Allows integration with openshift/machine-api-operator#571

@Danil-Grigorev Danil-Grigorev mentioned this pull request Jul 13, 2020
Merged
Danil-Grigorev pushed a commit to Danil-Grigorev/machine-api-operator that referenced this pull request Jul 13, 2020
Set leader-election for all controllers enabled
f0929b9 
Using leader election by default will add stronger guarantees than we have today that only one controller is running at a time to protect against edge cases where the deployment replica could be increased or upgrades with permissive maxSurge.

Relevant provider PRs:

- openshift/cluster-api-provider-gcp#85
- openshift/cluster-api-provider-aws#315
- openshift/cluster-api-provider-azure#122
- openshift/cluster-api-provider-openstack#108
- openshift/cluster-api-provider-baremetal#81
- openshift/cluster-api-provider-ovirt#55
- openshift#571
@rgolangh
Copy link

rgolangh commented Jul 13, 2020

/test e2e-ovirt

rgolangh
rgolangh reviewed Jul 13, 2020
View reviewed changes
cmd/manager/main.go

leaderElect := flag.Bool(
"leader-elect",
false,
Copy link

@rgolangh rgolangh Jul 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we want this to be true?

Copy link
Author

@Danil-Grigorev Danil-Grigorev Jul 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be set on true in in-cluster environment (openshift/machine-api-operator@f0929b9), but for local development this feature has no value, and having this set on false is just simpler to use.

@rgolangh
Copy link

rgolangh commented Jul 14, 2020

/test e2e-ovirt

@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 14, 2020

@Danil-Grigorev: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-ovirt b076381 link /test e2e-ovirt

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@rgolangh
Copy link

rgolangh commented Jul 14, 2020

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 14, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Danil-Grigorev, rgolangh

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2020
@openshift-merge-robot openshift-merge-robot merged commit 48b4746 into openshift:master Jul 14, 2020
elmiko pushed a commit to elmiko/machine-api-operator that referenced this pull request Aug 3, 2020
@elmiko
Set leader-election for all controllers enabled
303f931 
Using leader election by default will add stronger guarantees than we have today that only one controller is running at a time to protect against edge cases where the deployment replica could be increased or upgrades with permissive maxSurge.

Relevant provider PRs:

- openshift/cluster-api-provider-gcp#85
- openshift/cluster-api-provider-aws#315
- openshift/cluster-api-provider-azure#122
- openshift/cluster-api-provider-openstack#108
- openshift/cluster-api-provider-baremetal#81
- openshift/cluster-api-provider-ovirt#55
- openshift#571
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rgolangh rgolangh rgolangh left review comments

Assignees

@rgolangh rgolangh

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Danil-Grigorev @rgolangh @openshift-ci-robot @openshift-merge-robot