Skip to content

Bug 2048563: feat added leader election conventions#228

Merged
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
eggfoobar:leader_election_config
Feb 23, 2022
Merged

Bug 2048563: feat added leader election conventions#228
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
eggfoobar:leader_election_config

Conversation

@eggfoobar
Copy link
Copy Markdown
Contributor

This should take advantage of leader election changes for SNO clusters proposed in this library-go PR.

Changes:

  • updated leader election to follow convention and use different leader election for SNO topology

Signed-off-by: ehila ehila@redhat.com

@kevinrizza
Copy link
Copy Markdown
Member

@eggfoobar this PR seems reasonable to me, but I do want to call out that it only resolves the problem for the downstream only controller you updated, and none of the controllers that we're pulling in from the upstream operator-framework project in the staging directory.

@eggfoobar
Copy link
Copy Markdown
Contributor Author

Thanks for the heads up @kevinrizza , I think I might have misunderstood then, from what I could see from the holderIdentity it seemed like just the downstream operator was doing leader election, the upstream project seemed to not have that setup just yet. Is this the right spot to look for that code ?

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2021
@eggfoobar eggfoobar force-pushed the leader_election_config branch from 7995341 to 7d8495d Compare December 16, 2021 05:43
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2021
@eggfoobar eggfoobar force-pushed the leader_election_config branch from 7d8495d to 6947a74 Compare December 16, 2021 15:11
@eggfoobar
Copy link
Copy Markdown
Contributor Author

/retest-required

7 similar comments
@ggiguash
Copy link
Copy Markdown

/retest-required

@ggiguash
Copy link
Copy Markdown

/retest-required

@ggiguash
Copy link
Copy Markdown

/retest-required

@qJkee
Copy link
Copy Markdown

qJkee commented Jan 1, 2022

/retest-required

@ggiguash
Copy link
Copy Markdown

/retest-required

@qJkee
Copy link
Copy Markdown

qJkee commented Jan 13, 2022

/retest-required

@ggiguash
Copy link
Copy Markdown

/retest-required

@ggiguash
Copy link
Copy Markdown

/test e2e-upgrade

@eggfoobar eggfoobar force-pushed the leader_election_config branch from 6947a74 to d5844fb Compare January 20, 2022 14:35
@eggfoobar
Copy link
Copy Markdown
Contributor Author

/retest-required

@kevinrizza
Copy link
Copy Markdown
Member

/retest
/approve

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 21, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eggfoobar, kevinrizza

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 openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 21, 2022
Comment thread pkg/leaderelection/leaderelection.go Outdated
Comment thread base.Dockerfile Outdated
@eggfoobar eggfoobar force-pushed the leader_election_config branch 2 times, most recently from a45395d to 9037438 Compare January 24, 2022 20:05
@eggfoobar
Copy link
Copy Markdown
Contributor Author

/retest-required

@eggfoobar eggfoobar changed the title feat: added leader election conventons Bug 2034484: feat added leader election conventons Jan 25, 2022
@openshift-ci openshift-ci Bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Jan 25, 2022
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

12 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@dinhxuanvu
Copy link
Copy Markdown
Contributor

/hold

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 7, 2022
@eggfoobar
Copy link
Copy Markdown
Contributor Author

/test e2e-upgrade

@eggfoobar
Copy link
Copy Markdown
Contributor Author

Hey @dinhxuanvu seems the latest e2e-upgrade passed, should we remove the hold?

@dinhxuanvu
Copy link
Copy Markdown
Contributor

/hold cancel

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 22, 2022
@eggfoobar
Copy link
Copy Markdown
Contributor Author

/retest-required

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@eggfoobar
Copy link
Copy Markdown
Contributor Author

/retest-required

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Feb 23, 2022

@eggfoobar: 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-merge-robot openshift-merge-robot merged commit 6858269 into openshift:master Feb 23, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Feb 23, 2022

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

Bugzilla bug 2048563 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2048563: feat added leader election conventions

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.

@eggfoobar
Copy link
Copy Markdown
Contributor Author

/cherrypick release-4.10

@openshift-cherrypick-robot
Copy link
Copy Markdown

@eggfoobar: #228 failed to apply on top of branch "release-4.10":

Applying: feat: added leader election conventons
Applying: upkeep: ran make verify
Using index info to reconstruct a base tree...
M	go.mod
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 upkeep: ran make verify
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick release-4.10

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.