Skip to content
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.

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

fix #4547: preventing a cancelled leader elector #4548

Merged
merged 1 commit into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### 6.3-SNAPSHOT

#### Bugs
Fix #4547: preventing timing issues with leader election cancel

#### Improvements

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class LeaderElector {
private final AtomicReference<LeaderElectionRecord> observedRecord = new AtomicReference<>();
private final AtomicReference<LocalDateTime> observedTime = new AtomicReference<>();
private final Executor executor;
private boolean stopped;

public LeaderElector(KubernetesClient kubernetesClient, LeaderElectionConfig leaderElectionConfig, Executor executor) {
this.kubernetesClient = kubernetesClient;
Expand Down Expand Up @@ -110,7 +111,8 @@ public CompletableFuture<?> start() {
return result;
}

private void stopLeading() {
private synchronized void stopLeading() {
stopped = true;
LeaderElectionRecord current = observedRecord.get();
if (current == null || !isLeader(current)) {
return; // not leading
Expand Down Expand Up @@ -182,7 +184,10 @@ private CompletableFuture<Void> renewWithTimeout() {
}, () -> leaderElectionConfig.getRetryPeriod().toMillis(), executor);
}

boolean tryAcquireOrRenew() throws LockException {
synchronized boolean tryAcquireOrRenew() throws LockException {
if (stopped) {
return false;
}
final Lock lock = leaderElectionConfig.getLock();
final ZonedDateTime now = now();
final LeaderElectionRecord oldLeaderElectionRecord = lock.get(kubernetesClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ void shouldReleaseWhenCanceled() throws Exception {
Awaitility.await().atMost(10, TimeUnit.SECONDS).until(() -> Utils.isNullOrEmpty(activeLer.get().getHolderIdentity()));
assertEquals(0, activeLer.get().getLeaderTransitions());

// create a new elector, they are no good after a single use
leaderElector = new LeaderElector(mock(NamespacedKubernetesClient.class), lec, CommonThreadPool.get());
// the leader is now empty/null, we should be able to re-acquire
assertTrue(leaderElector.tryAcquireOrRenew());

Expand Down