From af02c5fd1676bb3a102409e6dbf01efad00ed8fd Mon Sep 17 00:00:00 2001 From: erabii Date: Thu, 18 Apr 2024 20:30:42 +0300 Subject: [PATCH 1/9] Fabric leader clean up 3 (#1643) --- .../kubernetes/commons/leader/Leader.java | 2 +- .../commons/leader/LeaderInfoContributor.java | 19 ++-- .../leader/LeaderInfoContributorTests.java | 93 ++++++++++++++++ .../leader/LeadershipControllerStub.java | 61 +++++++++++ .../Fabric8LeaderAutoConfiguration.java | 27 +++-- .../leader/LeaderInfoContributorTest.java | 100 ------------------ .../kubernetes/fabric8/leader/LeaderTest.java | 37 +++---- 7 files changed, 195 insertions(+), 144 deletions(-) create mode 100644 spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInfoContributorTests.java create mode 100644 spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipControllerStub.java delete mode 100644 spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderInfoContributorTest.java diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/Leader.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/Leader.java index fa179fe402..7076c9187e 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/Leader.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/Leader.java @@ -47,7 +47,7 @@ public boolean isCandidate(Candidate candidate) { return false; } - return Objects.equals(this.role, candidate.getRole()) && Objects.equals(this.id, candidate.getId()); + return Objects.equals(role, candidate.getRole()) && Objects.equals(id, candidate.getId()); } @Override diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInfoContributor.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInfoContributor.java index d49f48ed41..d218955656 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInfoContributor.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInfoContributor.java @@ -1,5 +1,5 @@ /* - * Copyright 2019-2019 the original author or authors. + * Copyright 2019-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,7 +18,6 @@ import java.util.HashMap; import java.util.Map; -import java.util.Optional; import org.springframework.boot.actuate.info.Info.Builder; import org.springframework.boot.actuate.info.InfoContributor; @@ -38,16 +37,12 @@ public LeaderInfoContributor(LeadershipController leadershipController, Candidat @Override public void contribute(Builder builder) { Map details = new HashMap<>(); - Optional leader = leadershipController.getLocalLeader(); - if (leader.isPresent()) { - Leader l = leader.get(); - details.put("leaderId", l.getId()); - details.put("role", l.getRole()); - details.put("isLeader", l.isCandidate(candidate)); - } - else { - details.put("leaderId", "Unknown"); - } + leadershipController.getLocalLeader().ifPresentOrElse(leader -> { + details.put("leaderId", leader.getId()); + details.put("role", leader.getRole()); + details.put("isLeader", leader.isCandidate(candidate)); + }, () -> details.put("leaderId", "Unknown")); + builder.withDetail("leaderElection", details); } diff --git a/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInfoContributorTests.java b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInfoContributorTests.java new file mode 100644 index 0000000000..941d37752d --- /dev/null +++ b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInfoContributorTests.java @@ -0,0 +1,93 @@ +/* + * Copyright 2013-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.kubernetes.commons.leader; + +import java.util.Map; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import org.springframework.boot.actuate.info.Info; +import org.springframework.integration.leader.Candidate; +import org.springframework.integration.leader.DefaultCandidate; +import org.springframework.integration.leader.event.LeaderEventPublisher; + +/** + * @author wind57 + */ +class LeaderInfoContributorTests { + + @Test + void testLeaderMissing() { + + Candidate candidate = new DefaultCandidate("id", "role"); + LeaderProperties leaderProperties = new LeaderProperties(); + LeaderEventPublisher leaderEventPublisher = Mockito.mock(LeaderEventPublisher.class); + LeadershipController leadershipController = new LeadershipControllerStub(candidate, leaderProperties, + leaderEventPublisher); + + LeaderInfoContributor leaderInfoContributor = new LeaderInfoContributor(leadershipController, candidate); + Info.Builder builder = new Info.Builder(); + leaderInfoContributor.contribute(builder); + + Assertions.assertEquals(builder.build().getDetails().get("leaderElection"), Map.of("leaderId", "Unknown")); + } + + @Test + void testLeaderPresentIsLeader() { + + Candidate candidate = new DefaultCandidate("leaderId", "leaderRole"); + LeaderProperties leaderProperties = new LeaderProperties(); + LeaderEventPublisher leaderEventPublisher = Mockito.mock(LeaderEventPublisher.class); + LeadershipController leadershipController = new LeadershipControllerStub(candidate, leaderProperties, + leaderEventPublisher); + + Leader leader = new Leader("leaderRole", "leaderId"); + + leadershipController.handleLeaderChange(leader); + + LeaderInfoContributor leaderInfoContributor = new LeaderInfoContributor(leadershipController, candidate); + Info.Builder builder = new Info.Builder(); + leaderInfoContributor.contribute(builder); + + Assertions.assertEquals(builder.build().getDetails().get("leaderElection"), + Map.of("role", "leaderRole", "isLeader", true, "leaderId", "leaderId")); + } + + @Test + void testLeaderPresentIsNotLeader() { + + Candidate candidate = new DefaultCandidate("leaderId", "notLeaderRole"); + LeaderProperties leaderProperties = new LeaderProperties(); + LeaderEventPublisher leaderEventPublisher = Mockito.mock(LeaderEventPublisher.class); + LeadershipController leadershipController = new LeadershipControllerStub(candidate, leaderProperties, + leaderEventPublisher); + + Leader leader = new Leader("leaderRole", "leaderId"); + + leadershipController.handleLeaderChange(leader); + + LeaderInfoContributor leaderInfoContributor = new LeaderInfoContributor(leadershipController, candidate); + Info.Builder builder = new Info.Builder(); + leaderInfoContributor.contribute(builder); + + Assertions.assertEquals(builder.build().getDetails().get("leaderElection"), + Map.of("role", "leaderRole", "isLeader", false, "leaderId", "leaderId")); + } + +} diff --git a/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipControllerStub.java b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipControllerStub.java new file mode 100644 index 0000000000..2d5a5aba84 --- /dev/null +++ b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipControllerStub.java @@ -0,0 +1,61 @@ +/* + * Copyright 2013-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.kubernetes.commons.leader; + +import java.util.Optional; + +import org.springframework.integration.leader.Candidate; +import org.springframework.integration.leader.event.LeaderEventPublisher; + +/** + * @author wind57 + */ +final class LeadershipControllerStub extends LeadershipController { + + private Leader leader; + + LeadershipControllerStub(Candidate candidate, LeaderProperties leaderProperties, + LeaderEventPublisher leaderEventPublisher) { + super(candidate, leaderProperties, leaderEventPublisher); + } + + @Override + public void update() { + + } + + @Override + public void revoke() { + + } + + @Override + protected PodReadinessWatcher createPodReadinessWatcher(String localLeaderId) { + return null; + } + + @Override + protected void handleLeaderChange(Leader leader) { + this.leader = leader; + } + + @Override + public Optional getLocalLeader() { + return Optional.ofNullable(leader); + } + +} diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfiguration.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfiguration.java index 7e58819167..a8fd814701 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfiguration.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfiguration.java @@ -47,20 +47,38 @@ @ConditionalOnProperty(value = "spring.cloud.kubernetes.leader.enabled", matchIfMissing = true) public class Fabric8LeaderAutoConfiguration { + /* + * Used for publishing application events that happen: granted, revoked or failed to + * acquire mutex. + */ @Bean @ConditionalOnMissingBean(LeaderEventPublisher.class) public LeaderEventPublisher defaultLeaderEventPublisher(ApplicationEventPublisher applicationEventPublisher) { return new DefaultLeaderEventPublisher(applicationEventPublisher); } + /* + * This can be thought as "self" or the pod that participates in leader election + * process. The implementation that we return simply logs events that happen during + * that process. + */ @Bean public Candidate candidate(LeaderProperties leaderProperties) throws UnknownHostException { String id = LeaderUtils.hostName(); String role = leaderProperties.getRole(); - return new DefaultCandidate(id, role); } + /* + * Add an info contributor with leader information. + */ + @Bean + @ConditionalOnClass(InfoContributor.class) + public LeaderInfoContributor leaderInfoContributor(Fabric8LeadershipController fabric8LeadershipController, + Candidate candidate) { + return new LeaderInfoContributor(fabric8LeadershipController, candidate); + } + @Bean public Fabric8LeadershipController leadershipController(Candidate candidate, LeaderProperties leaderProperties, LeaderEventPublisher leaderEventPublisher, KubernetesClient kubernetesClient) { @@ -87,11 +105,4 @@ public LeaderInitiator leaderInitiator(LeaderProperties leaderProperties, hostPodWatcher); } - @Bean - @ConditionalOnClass(InfoContributor.class) - public LeaderInfoContributor leaderInfoContributor(Fabric8LeadershipController fabric8LeadershipController, - Candidate candidate) { - return new LeaderInfoContributor(fabric8LeadershipController, candidate); - } - } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderInfoContributorTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderInfoContributorTest.java deleted file mode 100644 index 40931ca23f..0000000000 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderInfoContributorTest.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright 2019-2019 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cloud.kubernetes.fabric8.leader; - -import java.util.Map; -import java.util.Optional; - -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; - -import org.springframework.boot.actuate.info.Info; -import org.springframework.cloud.kubernetes.commons.leader.Leader; -import org.springframework.cloud.kubernetes.commons.leader.LeaderInfoContributor; -import org.springframework.integration.leader.Candidate; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.BDDMockito.given; - -@ExtendWith(MockitoExtension.class) -public class LeaderInfoContributorTest { - - @Mock - private Candidate mockCandidate; - - @Mock - private Fabric8LeadershipController mockFabric8LeadershipController; - - @Mock - private Leader mockLeader; - - private LeaderInfoContributor leaderInfoContributor; - - @BeforeEach - public void before() { - this.leaderInfoContributor = new LeaderInfoContributor(this.mockFabric8LeadershipController, - this.mockCandidate); - } - - @SuppressWarnings("unchecked") - @Test - public void infoWithoutLeader() { - Info.Builder builder = new Info.Builder(); - - leaderInfoContributor.contribute(builder); - - Map details = (Map) builder.build().get("leaderElection"); - assertThat(details).containsEntry("leaderId", "Unknown"); - } - - @SuppressWarnings("unchecked") - @Test - public void infoWhenLeader() { - given(this.mockFabric8LeadershipController.getLocalLeader()).willReturn(Optional.of(this.mockLeader)); - given(this.mockLeader.isCandidate(this.mockCandidate)).willReturn(true); - given(this.mockLeader.getRole()).willReturn("testRole"); - given(this.mockLeader.getId()).willReturn("id"); - Info.Builder builder = new Info.Builder(); - - leaderInfoContributor.contribute(builder); - - Map details = (Map) builder.build().get("leaderElection"); - assertThat(details).containsEntry("isLeader", true); - assertThat(details).containsEntry("leaderId", "id"); - assertThat(details).containsEntry("role", "testRole"); - } - - @SuppressWarnings("unchecked") - @Test - public void infoWhenAnotherIsLeader() { - given(this.mockFabric8LeadershipController.getLocalLeader()).willReturn(Optional.of(this.mockLeader)); - given(this.mockLeader.getRole()).willReturn("testRole"); - given(this.mockLeader.getId()).willReturn("id"); - Info.Builder builder = new Info.Builder(); - - leaderInfoContributor.contribute(builder); - - Map details = (Map) builder.build().get("leaderElection"); - assertThat(details).containsEntry("isLeader", false); - assertThat(details).containsEntry("leaderId", "id"); - assertThat(details).containsEntry("role", "testRole"); - } - -} diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderTest.java index e5d6619f8c..7ac9b62f95 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderTest.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,57 +18,48 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.cloud.kubernetes.commons.leader.Leader; import org.springframework.integration.leader.Candidate; +import org.springframework.integration.leader.DefaultCandidate; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.BDDMockito.given; /** * @author Gytis Trikleris */ -@ExtendWith(MockitoExtension.class) -public class LeaderTest { +class LeaderTest { private static final String ROLE = "test-role"; private static final String ID = "test-id"; - @Mock - private Candidate mockCandidate; - private Leader leader; @BeforeEach - public void before() { - this.leader = new Leader(ROLE, ID); + void before() { + leader = new Leader(ROLE, ID); } @Test - public void shouldGetRole() { - assertThat(this.leader.getRole()).isEqualTo(ROLE); + void shouldGetRole() { + assertThat(leader.getRole()).isEqualTo(ROLE); } @Test - public void shouldGetId() { - assertThat(this.leader.getId()).isEqualTo(ID); + void shouldGetId() { + assertThat(leader.getId()).isEqualTo(ID); } @Test - public void shouldCheckWithNullCandidate() { - assertThat(this.leader.isCandidate(null)).isEqualTo(false); + void shouldCheckWithNullCandidate() { + assertThat(leader.isCandidate(null)).isEqualTo(false); } @Test - public void shouldCheckCandidate() { - given(this.mockCandidate.getId()).willReturn(ID); - given(this.mockCandidate.getRole()).willReturn(ROLE); - - assertThat(this.leader.isCandidate(this.mockCandidate)).isTrue(); + void shouldCheckCandidate() { + Candidate candidate = new DefaultCandidate(ID, ROLE); + assertThat(leader.isCandidate(candidate)).isTrue(); } } From 1c819d00b16c81613277c5a682565a8922d2b993 Mon Sep 17 00:00:00 2001 From: erabii Date: Wed, 24 Apr 2024 21:52:01 +0300 Subject: [PATCH 2/9] Fabric leader clean up 4 (#1644) --- .../commons/leader/LeaderUtils.java | 12 ++++ .../Fabric8LeaderAutoConfiguration.java | 16 +++-- .../leader/Fabric8PodReadinessWatcher.java | 72 ++++++++++--------- 3 files changed, 60 insertions(+), 40 deletions(-) diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderUtils.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderUtils.java index dda9752899..015de23280 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderUtils.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderUtils.java @@ -18,6 +18,7 @@ import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.concurrent.locks.ReentrantLock; import org.springframework.cloud.kubernetes.commons.EnvReader; import org.springframework.util.StringUtils; @@ -44,4 +45,15 @@ public static String hostName() throws UnknownHostException { } } + public static void guarded(ReentrantLock lock, Runnable runnable) { + try { + lock.lock(); + runnable.run(); + } + finally { + lock.unlock(); + } + + } + } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfiguration.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfiguration.java index a8fd814701..a76730f125 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfiguration.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfiguration.java @@ -79,6 +79,16 @@ public LeaderInfoContributor leaderInfoContributor(Fabric8LeadershipController f return new LeaderInfoContributor(fabric8LeadershipController, candidate); } + /** + * watches the readiness of the pod. In case of a readiness change, it has to go + * through leader process again. + */ + @Bean + public Fabric8PodReadinessWatcher hostPodWatcher(Candidate candidate, KubernetesClient kubernetesClient, + Fabric8LeadershipController fabric8LeadershipController) { + return new Fabric8PodReadinessWatcher(candidate.getId(), kubernetesClient, fabric8LeadershipController); + } + @Bean public Fabric8LeadershipController leadershipController(Candidate candidate, LeaderProperties leaderProperties, LeaderEventPublisher leaderEventPublisher, KubernetesClient kubernetesClient) { @@ -91,12 +101,6 @@ public Fabric8LeaderRecordWatcher leaderRecordWatcher(LeaderProperties leaderPro return new Fabric8LeaderRecordWatcher(leaderProperties, fabric8LeadershipController, kubernetesClient); } - @Bean - public Fabric8PodReadinessWatcher hostPodWatcher(Candidate candidate, KubernetesClient kubernetesClient, - Fabric8LeadershipController fabric8LeadershipController) { - return new Fabric8PodReadinessWatcher(candidate.getId(), kubernetesClient, fabric8LeadershipController); - } - @Bean(destroyMethod = "stop") public LeaderInitiator leaderInitiator(LeaderProperties leaderProperties, Fabric8LeadershipController fabric8LeadershipController, diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java index 43cb1d8743..86eaa9131e 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ package org.springframework.cloud.kubernetes.fabric8.leader; +import java.util.concurrent.locks.ReentrantLock; + import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.Watch; @@ -23,19 +25,21 @@ import io.fabric8.kubernetes.client.WatcherException; import io.fabric8.kubernetes.client.dsl.PodResource; import io.fabric8.kubernetes.client.readiness.Readiness; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.apache.commons.logging.LogFactory; import org.springframework.cloud.kubernetes.commons.leader.PodReadinessWatcher; +import org.springframework.core.log.LogAccessor; + +import static org.springframework.cloud.kubernetes.commons.leader.LeaderUtils.guarded; /** * @author Gytis Trikleris */ public class Fabric8PodReadinessWatcher implements PodReadinessWatcher, Watcher { - private static final Logger LOGGER = LoggerFactory.getLogger(Fabric8PodReadinessWatcher.class); + private static final LogAccessor LOGGER = new LogAccessor(LogFactory.getLog(Fabric8PodReadinessWatcher.class)); - private final Object lock = new Object(); + private final ReentrantLock lock = new ReentrantLock(); private final String podName; @@ -43,9 +47,9 @@ public class Fabric8PodReadinessWatcher implements PodReadinessWatcher, Watcher< private final Fabric8LeadershipController fabric8LeadershipController; - private boolean previousState; + private volatile boolean previousState; - private Watch watch; + private volatile Watch watch; public Fabric8PodReadinessWatcher(String podName, KubernetesClient kubernetesClient, Fabric8LeadershipController fabric8LeadershipController) { @@ -56,54 +60,54 @@ public Fabric8PodReadinessWatcher(String podName, KubernetesClient kubernetesCli @Override public void start() { - if (this.watch == null) { - synchronized (this.lock) { - if (this.watch == null) { - LOGGER.debug("Starting pod readiness watcher for '{}'", this.podName); - PodResource podResource = this.kubernetesClient.pods().withName(this.podName); - this.previousState = podResource.isReady(); - this.watch = podResource.watch(this); + if (watch == null) { + guarded(lock, () -> { + if (watch == null) { + LOGGER.debug(() -> "Starting pod readiness watcher for :" + podName); + PodResource podResource = kubernetesClient.pods().withName(this.podName); + previousState = podResource.isReady(); + watch = podResource.watch(this); } - } + }); } } @Override public void stop() { - if (this.watch != null) { - synchronized (this.lock) { - if (this.watch != null) { - LOGGER.debug("Stopping pod readiness watcher for '{}'", this.podName); - this.watch.close(); - this.watch = null; + if (watch != null) { + guarded(lock, () -> { + if (watch != null) { + LOGGER.debug(() -> "Stopping pod readiness watcher for :" + podName); + watch.close(); + watch = null; } - } + }); } } @Override public void eventReceived(Action action, Pod pod) { boolean currentState = Readiness.isPodReady(pod); - if (this.previousState != currentState) { - synchronized (this.lock) { - if (this.previousState != currentState) { - LOGGER.debug("'{}' readiness status changed to '{}', triggering leadership update", this.podName, - currentState); - this.previousState = currentState; - this.fabric8LeadershipController.update(); + if (previousState != currentState) { + guarded(lock, () -> { + if (previousState != currentState) { + LOGGER.debug(() -> "readiness status changed for pod : " + podName + " to state: " + currentState + + ", triggering leadership update"); + previousState = currentState; + fabric8LeadershipController.update(); } - } + }); } } @Override public void onClose(WatcherException cause) { if (cause != null) { - synchronized (this.lock) { - LOGGER.warn("Watcher stopped unexpectedly, will restart", cause); - this.watch = null; + guarded(lock, () -> { + LOGGER.warn(() -> "Watcher stopped unexpectedly, will restart" + cause.getMessage()); + watch = null; start(); - } + }); } } From 279ee4d5ad20b89375bddcf2901a3954674e6283 Mon Sep 17 00:00:00 2001 From: erabii Date: Mon, 29 Apr 2024 16:00:03 +0300 Subject: [PATCH 3/9] Fabric leader clean up 5 (#1646) --- .../kubernetes/commons/leader/Leader.java | 12 +++--- .../commons/leader/LeaderContext.java | 11 +++-- .../commons/leader/LeaderInitiator.java | 42 +++++++++---------- .../commons/leader/LeaderUtils.java | 1 - 4 files changed, 34 insertions(+), 32 deletions(-) diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/Leader.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/Leader.java index 7076c9187e..53e8536aa7 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/Leader.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/Leader.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,11 +35,11 @@ public Leader(String role, String id) { } public String getRole() { - return this.role; + return role; } public String getId() { - return this.id; + return id; } public boolean isCandidate(Candidate candidate) { @@ -62,17 +62,17 @@ public boolean equals(Object o) { Leader leader = (Leader) o; - return Objects.equals(this.role, leader.role) && Objects.equals(this.id, leader.id); + return Objects.equals(role, leader.role) && Objects.equals(id, leader.id); } @Override public int hashCode() { - return Objects.hash(this.role, this.id); + return Objects.hash(role, id); } @Override public String toString() { - return String.format("Leader{role='%s', id='%s'}", this.role, this.id); + return String.format("Leader{role='%s', id='%s'}", role, id); } } diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderContext.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderContext.java index ef91a2f718..4dc5d8ccd5 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderContext.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,12 +35,17 @@ public LeaderContext(Candidate candidate, LeadershipController leadershipControl @Override public boolean isLeader() { - return this.leadershipController.getLocalLeader().filter(l -> l.isCandidate(this.candidate)).isPresent(); + return leadershipController.getLocalLeader().filter(l -> l.isCandidate(candidate)).isPresent(); } @Override public void yield() { - this.leadershipController.revoke(); + leadershipController.revoke(); + } + + @Override + public String getRole() { + return candidate.getRole(); } } diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInitiator.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInitiator.java index 3e6c682b88..1b51562ef3 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInitiator.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInitiator.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,17 +20,15 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.springframework.context.SmartLifecycle; +import org.springframework.core.log.LogAccessor; /** * @author Gytis Trikleris */ public class LeaderInitiator implements SmartLifecycle { - private static final Logger LOGGER = LoggerFactory.getLogger(LeaderInitiator.class); + private static final LogAccessor LOGGER = new LogAccessor(LeaderInitiator.class); private final LeaderProperties leaderProperties; @@ -54,33 +52,33 @@ public LeaderInitiator(LeaderProperties leaderProperties, LeadershipController l @Override public boolean isAutoStartup() { - return this.leaderProperties.isAutoStartup(); + return leaderProperties.isAutoStartup(); } @Override public void start() { if (!isRunning()) { - LOGGER.debug("Leader initiator starting"); - this.leaderRecordWatcher.start(); - this.hostPodWatcher.start(); - this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(); - this.scheduledExecutorService.scheduleAtFixedRate(this.leadershipController::update, - this.leaderProperties.getUpdatePeriod().toMillis(), - this.leaderProperties.getUpdatePeriod().toMillis(), TimeUnit.MILLISECONDS); - this.isRunning = true; + LOGGER.debug(() -> "Leader initiator starting"); + leaderRecordWatcher.start(); + hostPodWatcher.start(); + scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(); + scheduledExecutorService.scheduleAtFixedRate(leadershipController::update, + leaderProperties.getUpdatePeriod().toMillis(), + leaderProperties.getUpdatePeriod().toMillis(), TimeUnit.MILLISECONDS); + isRunning = true; } } @Override public void stop() { if (isRunning()) { - LOGGER.debug("Leader initiator stopping"); - this.scheduledExecutorService.shutdown(); - this.scheduledExecutorService = null; - this.hostPodWatcher.stop(); - this.leaderRecordWatcher.stop(); - this.leadershipController.revoke(); - this.isRunning = false; + LOGGER.debug(() -> "Leader initiator stopping"); + scheduledExecutorService.shutdown(); + scheduledExecutorService = null; + hostPodWatcher.stop(); + leaderRecordWatcher.stop(); + leadershipController.revoke(); + isRunning = false; } } @@ -92,7 +90,7 @@ public void stop(Runnable callback) { @Override public boolean isRunning() { - return this.isRunning; + return isRunning; } @Override diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderUtils.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderUtils.java index 015de23280..94771916d3 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderUtils.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderUtils.java @@ -53,7 +53,6 @@ public static void guarded(ReentrantLock lock, Runnable runnable) { finally { lock.unlock(); } - } } From 4336a5b59c11c4c8605f0eb5b0832120090cb36d Mon Sep 17 00:00:00 2001 From: erabii Date: Mon, 29 Apr 2024 21:20:48 +0300 Subject: [PATCH 4/9] Fabric leader clean up 6 (#1647) --- .../commons/leader/LeadershipController.java | 62 +++++---- .../leader/Fabric8LeaderRecordWatcher.java | 4 +- .../leader/Fabric8PodReadinessWatcher.java | 2 +- .../Fabric8LeaderAutoConfigurationTests.java | 15 +-- .../Fabric8LeaderRecordWatcherTest.java | 109 +++++++-------- .../Fabric8LeadershipControllerTest.java | 40 +++--- .../Fabric8PodReadinessWatcherTest.java | 103 ++++++--------- .../fabric8/leader/LeaderContextTest.java | 53 +++----- .../fabric8/leader/LeaderInitiatorTest.java | 124 ++++++++---------- 9 files changed, 226 insertions(+), 286 deletions(-) diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java index d616c55a48..b7f30b6aea 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java @@ -21,9 +21,7 @@ import java.util.Objects; import java.util.Optional; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - +import org.springframework.core.log.LogAccessor; import org.springframework.integration.leader.Candidate; import org.springframework.integration.leader.Context; import org.springframework.integration.leader.event.LeaderEventPublisher; @@ -34,7 +32,7 @@ */ public abstract class LeadershipController { - private static final Logger LOGGER = LoggerFactory.getLogger(LeadershipController.class); + private static final LogAccessor LOGGER = new LogAccessor(LeadershipController.class); protected static final String PROVIDER_KEY = "provider"; @@ -62,7 +60,7 @@ public LeadershipController(Candidate candidate, LeaderProperties leaderProperti } public Optional getLocalLeader() { - return Optional.ofNullable(this.localLeader); + return Optional.ofNullable(localLeader); } public abstract void update(); @@ -70,7 +68,7 @@ public Optional getLocalLeader() { public abstract void revoke(); protected String getLeaderKey() { - return this.leaderProperties.getLeaderIdPrefix() + this.candidate.getRole(); + return leaderProperties.getLeaderIdPrefix() + candidate.getRole(); } protected Map getLeaderData(Candidate candidate) { @@ -89,68 +87,68 @@ protected Leader extractLeader(Map data) { return null; } - return new Leader(this.candidate.getRole(), leaderId); + return new Leader(candidate.getRole(), leaderId); } protected void handleLeaderChange(Leader newLeader) { - if (Objects.equals(this.localLeader, newLeader)) { - LOGGER.debug("Leader is still '{}'", this.localLeader); + if (Objects.equals(localLeader, newLeader)) { + LOGGER.debug(() -> "Leader is still : " + localLeader); return; } - Leader oldLeader = this.localLeader; - this.localLeader = newLeader; + Leader oldLeader = localLeader; + localLeader = newLeader; - if (oldLeader != null && oldLeader.isCandidate(this.candidate)) { + if (oldLeader != null && oldLeader.isCandidate(candidate)) { notifyOnRevoked(); } - else if (newLeader != null && newLeader.isCandidate(this.candidate)) { + else if (newLeader != null && newLeader.isCandidate(candidate)) { notifyOnGranted(); } restartLeaderReadinessWatcher(); - LOGGER.debug("New leader is '{}'", this.localLeader); + LOGGER.debug(() -> "New leader is " + localLeader); } protected void notifyOnGranted() { - LOGGER.debug("Leadership has been granted for '{}'", this.candidate); + LOGGER.debug(() -> "Leadership has been granted for :" + candidate); - Context context = new LeaderContext(this.candidate, this); - this.leaderEventPublisher.publishOnGranted(this, context, this.candidate.getRole()); + Context context = new LeaderContext(candidate, this); + leaderEventPublisher.publishOnGranted(this, context, candidate.getRole()); try { - this.candidate.onGranted(context); + candidate.onGranted(context); } catch (InterruptedException e) { - LOGGER.warn(e.getMessage()); + LOGGER.warn(e::getMessage); Thread.currentThread().interrupt(); } } protected void notifyOnRevoked() { - LOGGER.debug("Leadership has been revoked for '{}'", this.candidate); + LOGGER.debug(() -> "Leadership has been revoked for :" + candidate); - Context context = new LeaderContext(this.candidate, this); - this.leaderEventPublisher.publishOnRevoked(this, context, this.candidate.getRole()); - this.candidate.onRevoked(context); + Context context = new LeaderContext(candidate, this); + leaderEventPublisher.publishOnRevoked(this, context, candidate.getRole()); + candidate.onRevoked(context); } protected void notifyOnFailedToAcquire() { - if (this.leaderProperties.isPublishFailedEvents()) { - Context context = new LeaderContext(this.candidate, this); - this.leaderEventPublisher.publishOnFailedToAcquire(this, context, this.candidate.getRole()); + if (leaderProperties.isPublishFailedEvents()) { + Context context = new LeaderContext(candidate, this); + leaderEventPublisher.publishOnFailedToAcquire(this, context, candidate.getRole()); } } protected void restartLeaderReadinessWatcher() { - if (this.leaderReadinessWatcher != null) { - this.leaderReadinessWatcher.stop(); - this.leaderReadinessWatcher = null; + if (leaderReadinessWatcher != null) { + leaderReadinessWatcher.stop(); + leaderReadinessWatcher = null; } - if (this.localLeader != null && !this.localLeader.isCandidate(this.candidate)) { - this.leaderReadinessWatcher = createPodReadinessWatcher(this.localLeader.getId()); - this.leaderReadinessWatcher.start(); + if (localLeader != null && !localLeader.isCandidate(candidate)) { + leaderReadinessWatcher = createPodReadinessWatcher(localLeader.getId()); + leaderReadinessWatcher.start(); } } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java index 0296b20715..f573698f45 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java @@ -25,12 +25,12 @@ import org.slf4j.LoggerFactory; import org.springframework.cloud.kubernetes.commons.leader.LeaderProperties; +import org.springframework.cloud.kubernetes.commons.leader.LeaderRecordWatcher; /** * @author Gytis Trikleris */ -public class Fabric8LeaderRecordWatcher - implements org.springframework.cloud.kubernetes.commons.leader.LeaderRecordWatcher, Watcher { +public class Fabric8LeaderRecordWatcher implements LeaderRecordWatcher, Watcher { private static final Logger LOGGER = LoggerFactory.getLogger(Fabric8LeaderRecordWatcher.class); diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java index 86eaa9131e..11576c5298 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java @@ -64,7 +64,7 @@ public void start() { guarded(lock, () -> { if (watch == null) { LOGGER.debug(() -> "Starting pod readiness watcher for :" + podName); - PodResource podResource = kubernetesClient.pods().withName(this.podName); + PodResource podResource = kubernetesClient.pods().withName(podName); previousState = podResource.isReady(); watch = podResource.watch(this); } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfigurationTests.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfigurationTests.java index 319653b511..6ee975b483 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfigurationTests.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,8 +17,6 @@ package org.springframework.cloud.kubernetes.fabric8.leader; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringBootConfiguration; @@ -30,12 +28,11 @@ import static org.hamcrest.Matchers.containsString; -@ExtendWith(MockitoExtension.class) @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, properties = { "spring.main.cloud-platform=KUBERNETES", "spring.cloud.kubernetes.leader.autoStartup=false", "management.endpoints.web.exposure.include=info", "management.endpoint.info.show-details=always", "management.info.kubernetes.enabled=true" }) -public class Fabric8LeaderAutoConfigurationTests { +class Fabric8LeaderAutoConfigurationTests { @LocalManagementPort private int port; @@ -44,13 +41,13 @@ public class Fabric8LeaderAutoConfigurationTests { private WebTestClient webClient; @Test - public void contextLoads() { + void contextLoads() { } @Test - public void infoEndpointShouldContainLeaderElection() { - this.webClient.get().uri("http://localhost:{port}/actuator/info", this.port).accept(MediaType.APPLICATION_JSON) - .exchange().expectStatus().isOk().expectBody(String.class).value(containsString("kubernetes")); + void infoEndpointShouldContainLeaderElection() { + webClient.get().uri("http://localhost:{port}/actuator/info", port).accept(MediaType.APPLICATION_JSON).exchange() + .expectStatus().isOk().expectBody(String.class).value(containsString("kubernetes")); } @SpringBootConfiguration diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcherTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcherTest.java index de161be0e6..704361817c 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcherTest.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcherTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,112 +27,99 @@ import io.fabric8.kubernetes.client.dsl.Resource; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.Mockito; import org.springframework.cloud.kubernetes.commons.leader.LeaderProperties; -import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; /** * @author Gytis Trikleris */ -@ExtendWith(MockitoExtension.class) -public class Fabric8LeaderRecordWatcherTest { +class Fabric8LeaderRecordWatcherTest { - @Mock - private LeaderProperties mockLeaderProperties; + private final LeaderProperties mockLeaderProperties = Mockito.mock(LeaderProperties.class); - @Mock - private Fabric8LeadershipController mockFabric8LeadershipController; + private final Fabric8LeadershipController mockFabric8LeadershipController = Mockito + .mock(Fabric8LeadershipController.class); - @Mock - private KubernetesClient mockKubernetesClient; + private final KubernetesClient mockKubernetesClient = Mockito.mock(KubernetesClient.class); - @Mock - private MixedOperation> mockConfigMapsOperation; + @SuppressWarnings("unchecked") + private final MixedOperation> mockConfigMapsOperation = Mockito + .mock(MixedOperation.class); - @Mock - private NonNamespaceOperation> mockInNamespaceOperation; + @SuppressWarnings("unchecked") + private final NonNamespaceOperation> mockInNamespaceOperation = Mockito + .mock(NonNamespaceOperation.class); - @Mock - private Resource mockWithNameResource; + @SuppressWarnings("unchecked") + private final Resource mockWithNameResource = Mockito.mock(Resource.class); - @Mock - private Watch mockWatch; + private final Watch mockWatch = Mockito.mock(Watch.class); - @Mock - private ConfigMap mockConfigMap; + private final ConfigMap mockConfigMap = Mockito.mock(ConfigMap.class); - @Mock - private WatcherException mockKubernetesClientException; + private final WatcherException mockKubernetesClientException = Mockito.mock(WatcherException.class); private Fabric8LeaderRecordWatcher watcher; @BeforeEach - public void before() { - this.watcher = new Fabric8LeaderRecordWatcher(this.mockLeaderProperties, this.mockFabric8LeadershipController, - this.mockKubernetesClient); + void beforeEach() { + watcher = new Fabric8LeaderRecordWatcher(mockLeaderProperties, mockFabric8LeadershipController, + mockKubernetesClient); } @Test - public void shouldStartOnce() { + void shouldStartOnce() { initStubs(); - this.watcher.start(); - this.watcher.start(); - - verify(this.mockWithNameResource).watch(this.watcher); + watcher.start(); + watcher.start(); + verify(mockWithNameResource).watch(watcher); } @Test - public void shouldStopOnce() { + void shouldStopOnce() { initStubs(); - this.watcher.start(); - this.watcher.stop(); - this.watcher.stop(); - - verify(this.mockWatch).close(); + watcher.start(); + watcher.stop(); + watcher.stop(); + verify(mockWatch).close(); } @Test - public void shouldHandleEvent() { - this.watcher.eventReceived(Watcher.Action.ADDED, this.mockConfigMap); - this.watcher.eventReceived(Watcher.Action.DELETED, this.mockConfigMap); - this.watcher.eventReceived(Watcher.Action.MODIFIED, this.mockConfigMap); - - verify(this.mockFabric8LeadershipController, times(3)).update(); + void shouldHandleEvent() { + watcher.eventReceived(Watcher.Action.ADDED, mockConfigMap); + watcher.eventReceived(Watcher.Action.DELETED, mockConfigMap); + watcher.eventReceived(Watcher.Action.MODIFIED, mockConfigMap); + verify(mockFabric8LeadershipController, times(3)).update(); } @Test - public void shouldIgnoreErrorEvent() { - this.watcher.eventReceived(Watcher.Action.ERROR, this.mockConfigMap); - - verify(this.mockFabric8LeadershipController, times(0)).update(); + void shouldIgnoreErrorEvent() { + watcher.eventReceived(Watcher.Action.ERROR, mockConfigMap); + verify(mockFabric8LeadershipController, times(0)).update(); } @Test - public void shouldHandleClose() { + void shouldHandleClose() { initStubs(); - this.watcher.onClose(this.mockKubernetesClientException); - - verify(this.mockWithNameResource).watch(this.watcher); + watcher.onClose(mockKubernetesClientException); + verify(mockWithNameResource).watch(watcher); } @Test - public void shouldIgnoreCloseWithoutCause() { - this.watcher.onClose(null); - - verify(this.mockWithNameResource, times(0)).watch(this.watcher); + void shouldIgnoreCloseWithoutCause() { + watcher.onClose(null); + verify(mockWithNameResource, times(0)).watch(watcher); } private void initStubs() { - given(this.mockKubernetesClient.configMaps()).willReturn(this.mockConfigMapsOperation); - given(this.mockConfigMapsOperation.inNamespace(null)).willReturn(this.mockInNamespaceOperation); - given(this.mockInNamespaceOperation.withName(null)).willReturn(this.mockWithNameResource); - given(this.mockWithNameResource.watch(this.watcher)).willReturn(this.mockWatch); + Mockito.when(mockKubernetesClient.configMaps()).thenReturn(mockConfigMapsOperation); + Mockito.when(mockConfigMapsOperation.inNamespace(null)).thenReturn(mockInNamespaceOperation); + Mockito.when(mockInNamespaceOperation.withName(null)).thenReturn(mockWithNameResource); + Mockito.when(mockWithNameResource.watch(watcher)).thenReturn(mockWatch); } } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java index 652c3f7634..adee10a8c7 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,16 +16,15 @@ package org.springframework.cloud.kubernetes.fabric8.leader; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapList; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; import io.fabric8.kubernetes.client.dsl.Resource; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Answers; -import org.mockito.Mock; import org.mockito.Mockito; -import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.boot.test.system.CapturedOutput; import org.springframework.boot.test.system.OutputCaptureExtension; @@ -43,42 +42,41 @@ /** * @author Gytis Trikleris */ -@ExtendWith(MockitoExtension.class) public class Fabric8LeadershipControllerTest { - @Mock - private Candidate mockCandidate; + private final Candidate mockCandidate = Mockito.mock(Candidate.class); - @Mock - private LeaderProperties mockLeaderProperties; + private final LeaderProperties mockLeaderProperties = Mockito.mock(LeaderProperties.class); - @Mock - private LeaderEventPublisher mockLeaderEventPublisher; + private final LeaderEventPublisher mockLeaderEventPublisher = Mockito.mock(LeaderEventPublisher.class); - @Mock(answer = Answers.RETURNS_DEEP_STUBS) - private KubernetesClient mockKubernetesClient; + private final KubernetesClient mockKubernetesClient = Mockito.mock(KubernetesClient.class, + Mockito.RETURNS_DEEP_STUBS); private Fabric8LeadershipController fabric8LeadershipController; @BeforeEach - public void before() { - this.fabric8LeadershipController = new Fabric8LeadershipController(this.mockCandidate, - this.mockLeaderProperties, this.mockLeaderEventPublisher, this.mockKubernetesClient); + void beforeEach() { + fabric8LeadershipController = new Fabric8LeadershipController(mockCandidate, mockLeaderProperties, + mockLeaderEventPublisher, mockKubernetesClient); } @Test - public void shouldGetEmptyLocalLeader() { - assertThat(this.fabric8LeadershipController.getLocalLeader().isPresent()).isFalse(); + void shouldGetEmptyLocalLeader() { + assertThat(fabric8LeadershipController.getLocalLeader().isPresent()).isFalse(); } - @ExtendWith(OutputCaptureExtension.class) @Test + @ExtendWith(OutputCaptureExtension.class) void whenNonExistentConfigmapAndCreationNotAllowedStopLeadershipAcquire(CapturedOutput output) { // given String testNamespace = "test-namespace"; String testConfigmap = "test-configmap"; - Resource mockResource = Mockito.mock(Resource.class); - NonNamespaceOperation mockNonNamespaceOperation = Mockito.mock(NonNamespaceOperation.class); + @SuppressWarnings("unchecked") + Resource mockResource = Mockito.mock(Resource.class); + @SuppressWarnings("unchecked") + NonNamespaceOperation> mockNonNamespaceOperation = Mockito + .mock(NonNamespaceOperation.class); Fabric8LeadershipController fabric8LeadershipController = new Fabric8LeadershipController(mockCandidate, mockLeaderProperties, mockLeaderEventPublisher, mockKubernetesClient); diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcherTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcherTest.java index c26d76ad7c..bd58f353b0 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcherTest.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcherTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,115 +27,98 @@ import io.fabric8.kubernetes.client.dsl.PodResource; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.Mockito; -import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; /** * @author Gytis Trikleris */ -@ExtendWith(MockitoExtension.class) public class Fabric8PodReadinessWatcherTest { private static final String POD_NAME = "test-pod"; - @Mock - private Fabric8LeadershipController mockFabric8LeadershipController; + private final Fabric8LeadershipController mockFabric8LeadershipController = Mockito + .mock(Fabric8LeadershipController.class); - @Mock - private KubernetesClient mockKubernetesClient; + private final KubernetesClient mockKubernetesClient = Mockito.mock(KubernetesClient.class); - @Mock - private MixedOperation mockPodsOperation; + @SuppressWarnings("unchecked") + private final MixedOperation mockPodsOperation = Mockito.mock(MixedOperation.class); - @Mock - private PodResource mockPodResource; + private final PodResource mockPodResource = Mockito.mock(PodResource.class); - @Mock - private Pod mockPod; + private final Pod mockPod = Mockito.mock(Pod.class); - @Mock - private PodStatus mockPodStatus; + private final PodStatus mockPodStatus = Mockito.mock(PodStatus.class); - @Mock - private Watch mockWatch; + private final Watch mockWatch = Mockito.mock(Watch.class); - @Mock - private WatcherException mockKubernetesClientException; + private final WatcherException mockKubernetesClientException = Mockito.mock(WatcherException.class); private Fabric8PodReadinessWatcher watcher; @BeforeEach - public void before() { - this.watcher = new Fabric8PodReadinessWatcher(POD_NAME, this.mockKubernetesClient, - this.mockFabric8LeadershipController); + void beforeEach() { + watcher = new Fabric8PodReadinessWatcher(POD_NAME, mockKubernetesClient, mockFabric8LeadershipController); } @Test - public void shouldStartOnce() { + void shouldStartOnce() { initStubs(); - this.watcher.start(); - this.watcher.start(); - - verify(this.mockPodResource).watch(this.watcher); + watcher.start(); + watcher.start(); + verify(mockPodResource).watch(watcher); } @Test - public void shouldStopOnce() { + void shouldStopOnce() { initStubs(); - this.watcher.start(); - this.watcher.stop(); - this.watcher.stop(); - - verify(this.mockWatch).close(); + watcher.start(); + watcher.stop(); + watcher.stop(); + verify(mockWatch).close(); } @Test - public void shouldHandleEventWithStateChange() { + void shouldHandleEventWithStateChange() { initStubs(); - given(this.mockPodResource.isReady()).willReturn(true); - given(this.mockPod.getStatus()).willReturn(this.mockPodStatus); - - this.watcher.start(); - this.watcher.eventReceived(Watcher.Action.ADDED, this.mockPod); + Mockito.when(mockPodResource.isReady()).thenReturn(true); + Mockito.when(mockPod.getStatus()).thenReturn(mockPodStatus); - verify(this.mockFabric8LeadershipController).update(); + watcher.start(); + watcher.eventReceived(Watcher.Action.ADDED, mockPod); + verify(mockFabric8LeadershipController).update(); } @Test - public void shouldIgnoreEventIfStateDoesNotChange() { + void shouldIgnoreEventIfStateDoesNotChange() { initStubs(); - given(this.mockPod.getStatus()).willReturn(this.mockPodStatus); + Mockito.when(mockPod.getStatus()).thenReturn(mockPodStatus); - this.watcher.start(); - this.watcher.eventReceived(Watcher.Action.ADDED, this.mockPod); - - verify(this.mockFabric8LeadershipController, times(0)).update(); + watcher.start(); + watcher.eventReceived(Watcher.Action.ADDED, mockPod); + verify(mockFabric8LeadershipController, times(0)).update(); } @Test - public void shouldHandleClose() { + void shouldHandleClose() { initStubs(); - this.watcher.onClose(this.mockKubernetesClientException); - - verify(this.mockPodResource).watch(this.watcher); + watcher.onClose(mockKubernetesClientException); + verify(mockPodResource).watch(watcher); } @Test - public void shouldIgnoreCloseWithoutCause() { - this.watcher.onClose(null); - - verify(this.mockPodResource, times(0)).watch(this.watcher); + void shouldIgnoreCloseWithoutCause() { + watcher.onClose(null); + verify(mockPodResource, times(0)).watch(watcher); } private void initStubs() { - given(this.mockKubernetesClient.pods()).willReturn(this.mockPodsOperation); - given(this.mockPodsOperation.withName(POD_NAME)).willReturn(this.mockPodResource); - given(this.mockPodResource.watch(this.watcher)).willReturn(this.mockWatch); + Mockito.when(mockKubernetesClient.pods()).thenReturn(mockPodsOperation); + Mockito.when(mockPodsOperation.withName(POD_NAME)).thenReturn(mockPodResource); + Mockito.when(mockPodResource.watch(watcher)).thenReturn(mockWatch); } } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderContextTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderContextTest.java index 41055988ef..f0804c1f5a 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderContextTest.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderContextTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,73 +20,60 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.Mockito; import org.springframework.cloud.kubernetes.commons.leader.Leader; import org.springframework.cloud.kubernetes.commons.leader.LeaderContext; import org.springframework.integration.leader.Candidate; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.verify; /** * @author Gytis Trikleris */ -@ExtendWith(MockitoExtension.class) public class LeaderContextTest { - @Mock - private Candidate mockCandidate; + private final Candidate mockCandidate = Mockito.mock(Candidate.class); - @Mock - private Fabric8LeadershipController mockFabric8LeadershipController; + private final Fabric8LeadershipController mockFabric8LeadershipController = Mockito + .mock(Fabric8LeadershipController.class); - @Mock - private Leader mockLeader; + private final Leader mockLeader = Mockito.mock(Leader.class); private LeaderContext leaderContext; @BeforeEach - public void before() { - this.leaderContext = new LeaderContext(this.mockCandidate, this.mockFabric8LeadershipController); + void beforeEach() { + leaderContext = new LeaderContext(mockCandidate, mockFabric8LeadershipController); } @Test - public void testIsLeaderWithoutLeader() { - given(this.mockFabric8LeadershipController.getLocalLeader()).willReturn(Optional.empty()); - - boolean result = this.leaderContext.isLeader(); - + void testIsLeaderWithoutLeader() { + Mockito.when(mockFabric8LeadershipController.getLocalLeader()).thenReturn(Optional.empty()); + boolean result = leaderContext.isLeader(); assertThat(result).isFalse(); } @Test - public void testIsLeaderWithAnotherLeader() { - given(this.mockFabric8LeadershipController.getLocalLeader()).willReturn(Optional.of(this.mockLeader)); - - boolean result = this.leaderContext.isLeader(); - + void testIsLeaderWithAnotherLeader() { + Mockito.when(mockFabric8LeadershipController.getLocalLeader()).thenReturn(Optional.of(mockLeader)); + boolean result = leaderContext.isLeader(); assertThat(result).isFalse(); } @Test - public void testIsLeaderWhenLeader() { - given(this.mockFabric8LeadershipController.getLocalLeader()).willReturn(Optional.of(this.mockLeader)); - given(this.mockLeader.isCandidate(this.mockCandidate)).willReturn(true); - + void testIsLeaderWhenLeader() { + Mockito.when(mockFabric8LeadershipController.getLocalLeader()).thenReturn(Optional.of(mockLeader)); + Mockito.when(mockLeader.isCandidate(mockCandidate)).thenReturn(true); boolean result = this.leaderContext.isLeader(); - assertThat(result).isTrue(); } @Test - public void shouldYieldLeadership() { - this.leaderContext.yield(); - - verify(this.mockFabric8LeadershipController).revoke(); + void shouldYieldLeadership() { + leaderContext.yield(); + verify(mockFabric8LeadershipController).revoke(); } } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderInitiatorTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderInitiatorTest.java index 8388de2565..7bca401472 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderInitiatorTest.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/LeaderInitiatorTest.java @@ -18,123 +18,113 @@ import java.time.Duration; +import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.Mockito; import org.springframework.cloud.kubernetes.commons.leader.LeaderInitiator; import org.springframework.cloud.kubernetes.commons.leader.LeaderProperties; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.verify; import static org.mockito.internal.verification.VerificationModeFactory.atLeastOnce; /** * @author Gytis Trikleris */ -@ExtendWith(MockitoExtension.class) public class LeaderInitiatorTest { - @Mock - private LeaderProperties mockLeaderProperties; + private final LeaderProperties leaderProperties = new LeaderProperties(); - @Mock - private Fabric8LeadershipController mockFabric8LeadershipController; + private final Fabric8LeadershipController mockFabric8LeadershipController = Mockito + .mock(Fabric8LeadershipController.class); - @Mock - private Fabric8LeaderRecordWatcher mockFabric8LeaderRecordWatcher; + private final Fabric8LeaderRecordWatcher mockFabric8LeaderRecordWatcher = Mockito + .mock(Fabric8LeaderRecordWatcher.class); - @Mock - private Fabric8PodReadinessWatcher mockFabric8PodReadinessWatcher; + private final Fabric8PodReadinessWatcher mockFabric8PodReadinessWatcher = Mockito + .mock(Fabric8PodReadinessWatcher.class); - @Mock - private Runnable mockRunnable; + private final Runnable runnable = Mockito.mock(Runnable.class); private LeaderInitiator leaderInitiator; @BeforeEach - public void before() { - this.leaderInitiator = new LeaderInitiator(this.mockLeaderProperties, this.mockFabric8LeadershipController, - this.mockFabric8LeaderRecordWatcher, this.mockFabric8PodReadinessWatcher); + void beforeEach() { + leaderInitiator = new LeaderInitiator(leaderProperties, mockFabric8LeadershipController, + mockFabric8LeaderRecordWatcher, mockFabric8PodReadinessWatcher); } @AfterEach - public void after() { - this.leaderInitiator.stop(); + void afterEach() { + leaderInitiator.stop(); } @Test - public void testIsAutoStartup() { - given(this.mockLeaderProperties.isAutoStartup()).willReturn(true); - - assertThat(this.leaderInitiator.isAutoStartup()).isTrue(); + void testIsAutoStartup() { + assertThat(leaderInitiator.isAutoStartup()).isTrue(); } @Test - public void shouldStart() throws InterruptedException { - given(this.mockLeaderProperties.getUpdatePeriod()).willReturn(Duration.ofMillis(1L)); + void shouldStart() { + leaderProperties.setUpdatePeriod(Duration.ofMillis(1L)); + + leaderInitiator.start(); - this.leaderInitiator.start(); + assertThat(leaderInitiator.isRunning()).isTrue(); + verify(mockFabric8LeaderRecordWatcher).start(); + verify(mockFabric8PodReadinessWatcher).start(); + boolean[] updateCalled = new boolean[1]; + Mockito.doAnswer(x -> { + updateCalled[0] = true; + return null; + }).when(mockFabric8LeadershipController).update(); - assertThat(this.leaderInitiator.isRunning()).isTrue(); - verify(this.mockFabric8LeaderRecordWatcher).start(); - verify(this.mockFabric8PodReadinessWatcher).start(); + Awaitility.await().atMost(Duration.ofSeconds(3)).until(() -> updateCalled[0]); - // TODO this tests needs to be reviewed not to use sleep - Thread.sleep(1000); - verify(this.mockFabric8LeadershipController, atLeastOnce()).update(); + verify(mockFabric8LeadershipController, atLeastOnce()).update(); } @Test - public void shouldStartOnlyOnce() { - given(this.mockLeaderProperties.getUpdatePeriod()).willReturn(Duration.ofMillis(10000L)); + void shouldStartOnlyOnce() { + leaderInitiator.start(); + leaderInitiator.start(); - this.leaderInitiator.start(); - this.leaderInitiator.start(); - - verify(this.mockFabric8LeaderRecordWatcher).start(); + verify(mockFabric8LeaderRecordWatcher).start(); } @Test - public void shouldStop() { - given(this.mockLeaderProperties.getUpdatePeriod()).willReturn(Duration.ofMillis(10000L)); - - this.leaderInitiator.start(); - this.leaderInitiator.stop(); - - assertThat(this.leaderInitiator.isRunning()).isFalse(); - verify(this.mockFabric8LeaderRecordWatcher).stop(); - verify(this.mockFabric8PodReadinessWatcher).start(); - verify(this.mockFabric8LeadershipController).revoke(); + void shouldStop() { + leaderInitiator.start(); + leaderInitiator.stop(); + + assertThat(leaderInitiator.isRunning()).isFalse(); + verify(mockFabric8LeaderRecordWatcher).stop(); + verify(mockFabric8PodReadinessWatcher).start(); + verify(mockFabric8LeadershipController).revoke(); } @Test - public void shouldStopOnlyOnce() { - given(this.mockLeaderProperties.getUpdatePeriod()).willReturn(Duration.ofMillis(10000L)); - - this.leaderInitiator.start(); - this.leaderInitiator.stop(); - this.leaderInitiator.stop(); + void shouldStopOnlyOnce() { + leaderInitiator.start(); + leaderInitiator.stop(); + leaderInitiator.stop(); - verify(this.mockFabric8LeaderRecordWatcher).stop(); + verify(mockFabric8LeaderRecordWatcher).stop(); } @Test - public void shouldStopAndExecuteCallback() { - given(this.mockLeaderProperties.getUpdatePeriod()).willReturn(Duration.ofMillis(10000L)); - - this.leaderInitiator.start(); - this.leaderInitiator.stop(this.mockRunnable); - - assertThat(this.leaderInitiator.isRunning()).isFalse(); - verify(this.mockFabric8LeaderRecordWatcher).stop(); - verify(this.mockFabric8PodReadinessWatcher).start(); - verify(this.mockFabric8LeadershipController).revoke(); - verify(this.mockRunnable).run(); + void shouldStopAndExecuteCallback() { + leaderInitiator.start(); + leaderInitiator.stop(runnable); + + assertThat(leaderInitiator.isRunning()).isFalse(); + verify(mockFabric8LeaderRecordWatcher).stop(); + verify(mockFabric8PodReadinessWatcher).start(); + verify(mockFabric8LeadershipController).revoke(); + verify(runnable).run(); } } From 95e49167a647cc01b64fc4af7a1fab5456a29fa2 Mon Sep 17 00:00:00 2001 From: erabii Date: Tue, 30 Apr 2024 14:18:02 +0300 Subject: [PATCH 5/9] Load images (#1649) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .../commons/leader/LeaderInitiator.java | 4 ++-- .../fabric8/client/istio/Fabric8IstioIT.java | 4 +++- .../integration/tests/commons/Images.java | 24 +++++++++++++++---- .../tests/commons/fabric8_client/Util.java | 2 +- .../src/main/resources/current-images.txt | 2 ++ 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInitiator.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInitiator.java index 1b51562ef3..ebf0e33252 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInitiator.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderInitiator.java @@ -63,8 +63,8 @@ public void start() { hostPodWatcher.start(); scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(); scheduledExecutorService.scheduleAtFixedRate(leadershipController::update, - leaderProperties.getUpdatePeriod().toMillis(), - leaderProperties.getUpdatePeriod().toMillis(), TimeUnit.MILLISECONDS); + leaderProperties.getUpdatePeriod().toMillis(), leaderProperties.getUpdatePeriod().toMillis(), + TimeUnit.MILLISECONDS); isRunning = true; } } diff --git a/spring-cloud-kubernetes-integration-tests/spring-cloud-kubernetes-fabric8-client-istio/src/test/java/org/springframework/cloud/kubernetes/fabric8/client/istio/Fabric8IstioIT.java b/spring-cloud-kubernetes-integration-tests/spring-cloud-kubernetes-fabric8-client-istio/src/test/java/org/springframework/cloud/kubernetes/fabric8/client/istio/Fabric8IstioIT.java index 5cd75c50cb..efdb91bdb2 100644 --- a/spring-cloud-kubernetes-integration-tests/spring-cloud-kubernetes-fabric8-client-istio/src/test/java/org/springframework/cloud/kubernetes/fabric8/client/istio/Fabric8IstioIT.java +++ b/spring-cloud-kubernetes-integration-tests/spring-cloud-kubernetes-fabric8-client-istio/src/test/java/org/springframework/cloud/kubernetes/fabric8/client/istio/Fabric8IstioIT.java @@ -65,7 +65,9 @@ static void beforeAll() throws Exception { Commons.validateImage(IMAGE_NAME, K3S); Commons.loadSpringCloudKubernetesImage(IMAGE_NAME, K3S); - Images.loadIstioctl(K3S); + Images.loadIstioCtl(K3S); + Images.loadIstioProxyV2(K3S); + Images.loadIstioPilot(K3S); processExecResult(K3S.execInContainer("sh", "-c", "kubectl create namespace istio-test")); processExecResult( diff --git a/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java b/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java index 06a82370e4..5c672334e1 100644 --- a/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java +++ b/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java @@ -36,7 +36,15 @@ public final class Images { private static final String ISTIOCTL = "istio/istioctl"; - private static final String ISTIOCTL_TAR = ISTIOCTL.replace('/', '-') + ":" + istioctlVersion(); + private static final String ISTIOCTL_TAR = ISTIOCTL.replace('/', '-') + ":" + istioVersion(); + + private static final String ISTIO_PROXY_V2 = "istio/proxyv2"; + + private static final String ISTIO_PROXY_V2_TAR = ISTIO_PROXY_V2.replace('/', '-') + ":" + istioVersion(); + + private static final String ISTIO_PILOT = "istio/istioctl"; + + private static final String ISTIO_PILOT_TAR = ISTIO_PILOT.replace('/', '-') + ":" + istioVersion(); private static final String KAFKA = "confluentinc/cp-kafka"; @@ -54,7 +62,7 @@ public static String busyboxVersion() { return imageVersion(BUSYBOX); } - public static String istioctlVersion() { + public static String istioVersion() { return imageVersion(ISTIOCTL); } @@ -78,8 +86,16 @@ public static void loadWiremock(K3sContainer container) { Commons.load(container, WIREMOCK_TAR, WIREMOCK, wiremockVersion()); } - public static void loadIstioctl(K3sContainer container) { - Commons.load(container, ISTIOCTL_TAR, ISTIOCTL, istioctlVersion()); + public static void loadIstioCtl(K3sContainer container) { + Commons.load(container, ISTIOCTL_TAR, ISTIOCTL, istioVersion()); + } + + public static void loadIstioProxyV2(K3sContainer container) { + Commons.load(container, ISTIO_PROXY_V2_TAR, ISTIO_PROXY_V2, istioVersion()); + } + + public static void loadIstioPilot(K3sContainer container) { + Commons.load(container, ISTIO_PILOT_TAR, ISTIO_PILOT, istioVersion()); } public static void loadKafka(K3sContainer container) { diff --git a/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/fabric8_client/Util.java b/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/fabric8_client/Util.java index fe01d5312d..9e7b9917fc 100644 --- a/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/fabric8_client/Util.java +++ b/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/fabric8_client/Util.java @@ -267,7 +267,7 @@ public void setUpIstioctl(String namespace, Phase phase) { String imageWithoutVersion = istioctlDeployment.getSpec().getTemplate().getSpec().getContainers().get(0) .getImage(); - String imageWithVersion = imageWithoutVersion + ":" + Images.istioctlVersion(); + String imageWithVersion = imageWithoutVersion + ":" + Images.istioVersion(); istioctlDeployment.getSpec().getTemplate().getSpec().getContainers().get(0).setImage(imageWithVersion); if (phase.equals(Phase.CREATE)) { diff --git a/spring-cloud-kubernetes-test-support/src/main/resources/current-images.txt b/spring-cloud-kubernetes-test-support/src/main/resources/current-images.txt index de0626cdf0..34d4371080 100644 --- a/spring-cloud-kubernetes-test-support/src/main/resources/current-images.txt +++ b/spring-cloud-kubernetes-test-support/src/main/resources/current-images.txt @@ -1,5 +1,7 @@ busybox:1.36.1 istio/istioctl:1.20.5 +istio/proxyv2:1.20.5 +istio/pilot:1.20.5 confluentinc/cp-kafka:7.2.1 rabbitmq:3-management wiremock/wiremock:3.4.2 From 8c03d0cf05f2c7cb3ad5a2b2bbeb19cd6cac109f Mon Sep 17 00:00:00 2001 From: Ryan Baxter <524254+ryanjbaxter@users.noreply.github.com> Date: Tue, 30 Apr 2024 09:37:19 -0400 Subject: [PATCH 6/9] Fixing istio pilot image name --- .../cloud/kubernetes/integration/tests/commons/Images.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java b/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java index 5c672334e1..9aff9473a8 100644 --- a/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java +++ b/spring-cloud-kubernetes-test-support/src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java @@ -42,7 +42,7 @@ public final class Images { private static final String ISTIO_PROXY_V2_TAR = ISTIO_PROXY_V2.replace('/', '-') + ":" + istioVersion(); - private static final String ISTIO_PILOT = "istio/istioctl"; + private static final String ISTIO_PILOT = "istio/pilot"; private static final String ISTIO_PILOT_TAR = ISTIO_PILOT.replace('/', '-') + ":" + istioVersion(); From 53fa5555c8a3268875cd6cf8fd2243c2cfe9eb96 Mon Sep 17 00:00:00 2001 From: erabii Date: Tue, 30 Apr 2024 16:39:09 +0300 Subject: [PATCH 7/9] Fabric leader clean up 7 (#1648) --- .../commons/leader/LeaderProperties.java | 3 +- .../leader/Fabric8LeaderRecordWatcher.java | 57 ++++++++++--------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderProperties.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderProperties.java index c5a5bf073d..fe54bc1084 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderProperties.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeaderProperties.java @@ -19,6 +19,7 @@ import java.time.Duration; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.util.StringUtils; /** * @author Gytis Trikleris @@ -105,7 +106,7 @@ public void setNamespace(String namespace) { } public String getNamespace(String defaultValue) { - if (namespace == null || namespace.isEmpty()) { + if (!StringUtils.hasText(defaultValue)) { return defaultValue; } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java index f573698f45..0900c6253e 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,25 +16,28 @@ package org.springframework.cloud.kubernetes.fabric8.leader; +import java.util.concurrent.locks.ReentrantLock; + import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.Watch; import io.fabric8.kubernetes.client.Watcher; import io.fabric8.kubernetes.client.WatcherException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.cloud.kubernetes.commons.leader.LeaderProperties; import org.springframework.cloud.kubernetes.commons.leader.LeaderRecordWatcher; +import org.springframework.core.log.LogAccessor; + +import static org.springframework.cloud.kubernetes.commons.leader.LeaderUtils.guarded; /** * @author Gytis Trikleris */ public class Fabric8LeaderRecordWatcher implements LeaderRecordWatcher, Watcher { - private static final Logger LOGGER = LoggerFactory.getLogger(Fabric8LeaderRecordWatcher.class); + private static final LogAccessor LOGGER = new LogAccessor(Fabric8LeaderRecordWatcher.class); - private final Object lock = new Object(); + private final ReentrantLock lock = new ReentrantLock(); private final Fabric8LeadershipController fabric8LeadershipController; @@ -42,7 +45,7 @@ public class Fabric8LeaderRecordWatcher implements LeaderRecordWatcher, Watcher< private final KubernetesClient kubernetesClient; - private Watch watch; + private volatile Watch configMapWatch; public Fabric8LeaderRecordWatcher(LeaderProperties leaderProperties, Fabric8LeadershipController fabric8LeadershipController, KubernetesClient kubernetesClient) { @@ -52,47 +55,47 @@ public Fabric8LeaderRecordWatcher(LeaderProperties leaderProperties, } public void start() { - if (this.watch == null) { - synchronized (this.lock) { - if (this.watch == null) { - LOGGER.debug("Starting leader record watcher"); - this.watch = this.kubernetesClient.configMaps() - .inNamespace(this.leaderProperties.getNamespace(this.kubernetesClient.getNamespace())) - .withName(this.leaderProperties.getConfigMapName()).watch(this); + if (configMapWatch == null) { + guarded(lock, () -> { + if (configMapWatch == null) { + LOGGER.debug(() -> "Starting leader record watcher"); + configMapWatch = kubernetesClient.configMaps() + .inNamespace(leaderProperties.getNamespace(kubernetesClient.getNamespace())) + .withName(leaderProperties.getConfigMapName()).watch(this); } - } + }); } } public void stop() { - if (this.watch != null) { - synchronized (this.lock) { - if (this.watch != null) { - LOGGER.debug("Stopping leader record watcher"); - this.watch.close(); - this.watch = null; + if (configMapWatch != null) { + guarded(lock, () -> { + if (this.configMapWatch != null) { + LOGGER.debug(() -> "Stopping leader record watcher"); + configMapWatch.close(); + configMapWatch = null; } - } + }); } } @Override public void eventReceived(Action action, ConfigMap configMap) { - LOGGER.debug("'{}' event received, triggering leadership update", action); + LOGGER.debug(() -> action + " event received, triggering leadership update"); if (!Action.ERROR.equals(action)) { - this.fabric8LeadershipController.update(); + fabric8LeadershipController.update(); } } @Override public void onClose(WatcherException cause) { if (cause != null) { - synchronized (this.lock) { - LOGGER.warn("Watcher stopped unexpectedly, will restart", cause); - this.watch = null; + guarded(lock, () -> { + LOGGER.warn(() -> "Watcher stopped unexpectedly, will restart because of : " + cause); + configMapWatch = null; start(); - } + }); } } From 65f5109864f3fd8d6ee9f424f1e6249a9ff6872e Mon Sep 17 00:00:00 2001 From: erabii Date: Tue, 30 Apr 2024 20:44:50 +0300 Subject: [PATCH 8/9] Fabric leader clean up 8 (#1650) --- .../leader/Fabric8LeadershipController.java | 116 +++++++++--------- .../Fabric8LeadershipControllerTest.java | 3 +- 2 files changed, 60 insertions(+), 59 deletions(-) diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipController.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipController.java index cbec9e68ae..8ae555218f 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipController.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipController.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,30 +17,34 @@ package org.springframework.cloud.kubernetes.fabric8.leader; import java.util.Map; +import java.util.concurrent.locks.ReentrantLock; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.cloud.kubernetes.commons.leader.Leader; import org.springframework.cloud.kubernetes.commons.leader.LeaderProperties; import org.springframework.cloud.kubernetes.commons.leader.LeadershipController; import org.springframework.cloud.kubernetes.commons.leader.PodReadinessWatcher; +import org.springframework.core.log.LogAccessor; import org.springframework.integration.leader.Candidate; import org.springframework.integration.leader.event.LeaderEventPublisher; +import static org.springframework.cloud.kubernetes.commons.leader.LeaderUtils.guarded; + /** * @author Gytis Trikleris */ public class Fabric8LeadershipController extends LeadershipController { - private static final Logger LOGGER = LoggerFactory.getLogger(Fabric8LeadershipController.class); + private static final LogAccessor LOGGER = new LogAccessor(Fabric8LeadershipController.class); private final KubernetesClient kubernetesClient; + private final ReentrantLock lock = new ReentrantLock(); + public Fabric8LeadershipController(Candidate candidate, LeaderProperties leaderProperties, LeaderEventPublisher leaderEventPublisher, KubernetesClient kubernetesClient) { super(candidate, leaderProperties, leaderEventPublisher); @@ -48,41 +52,47 @@ public Fabric8LeadershipController(Candidate candidate, LeaderProperties leaderP } @Override - public synchronized void update() { - LOGGER.debug("Checking leader state"); - ConfigMap configMap = getConfigMap(); - if (configMap == null && !leaderProperties.isCreateConfigMap()) { - LOGGER.warn("ConfigMap '{}' does not exist and leaderProperties.isCreateConfigMap() " - + "is false, cannot acquire leadership", leaderProperties.getConfigMapName()); - notifyOnFailedToAcquire(); - return; - } - Leader leader = extractLeader(configMap); + public void update() { + guarded(lock, () -> { + LOGGER.debug(() -> "Checking leader state"); + ConfigMap configMap = getConfigMap(); + if (configMap == null && !leaderProperties.isCreateConfigMap()) { + LOGGER.warn("ConfigMap '" + leaderProperties.getConfigMapName() + + "' does not exist and leaderProperties.isCreateConfigMap() " + + "is false, cannot acquire leadership"); + notifyOnFailedToAcquire(); + return; + } + Leader leader = extractLeader(configMap); - if (leader != null && isPodReady(leader.getId())) { - handleLeaderChange(leader); - return; - } + if (leader != null && isPodReady(leader.getId())) { + handleLeaderChange(leader); + return; + } + + if (leader != null && leader.isCandidate(candidate)) { + revoke(configMap); + } + else { + acquire(configMap); + } + }); - if (leader != null && leader.isCandidate(this.candidate)) { - revoke(configMap); - } - else { - acquire(configMap); - } } - public synchronized void revoke() { - ConfigMap configMap = getConfigMap(); - Leader leader = extractLeader(configMap); + public void revoke() { + guarded(lock, () -> { + ConfigMap configMap = getConfigMap(); + Leader leader = extractLeader(configMap); - if (leader != null && leader.isCandidate(this.candidate)) { - revoke(configMap); - } + if (leader != null && leader.isCandidate(candidate)) { + revoke(configMap); + } + }); } private void revoke(ConfigMap configMap) { - LOGGER.debug("Trying to revoke leadership for '{}'", this.candidate); + LOGGER.debug(() -> "Trying to revoke leadership for :" + candidate); try { String leaderKey = getLeaderKey(); @@ -90,20 +100,20 @@ private void revoke(ConfigMap configMap) { handleLeaderChange(null); } catch (KubernetesClientException e) { - LOGGER.warn("Failure when revoking leadership for '{}': {}", this.candidate, e.getMessage()); + LOGGER.warn("Failure when revoking leadership for : " + candidate + "because : " + e.getMessage()); } } private void acquire(ConfigMap configMap) { - LOGGER.debug("Trying to acquire leadership for '{}'", this.candidate); + LOGGER.debug(() -> "Trying to acquire leadership for :" + this.candidate); - if (!isPodReady(this.candidate.getId())) { - LOGGER.debug("Pod of '{}' is not ready at the moment, cannot acquire leadership", this.candidate); + if (!isPodReady(candidate.getId())) { + LOGGER.debug("Pod : " + candidate + "is not ready at the moment, cannot acquire leadership"); return; } try { - Map data = getLeaderData(this.candidate); + Map data = getLeaderData(candidate); if (configMap == null) { createConfigMap(data); @@ -112,11 +122,11 @@ private void acquire(ConfigMap configMap) { updateConfigMapEntry(configMap, data); } - Leader newLeader = new Leader(this.candidate.getRole(), this.candidate.getId()); + Leader newLeader = new Leader(candidate.getRole(), candidate.getId()); handleLeaderChange(newLeader); } catch (KubernetesClientException e) { - LOGGER.warn("Failure when acquiring leadership for '{}': {}", this.candidate, e.getMessage()); + LOGGER.warn(() -> "Failure when acquiring leadership for : " + candidate + " because : " + e.getMessage()); notifyOnFailedToAcquire(); } } @@ -135,47 +145,39 @@ private Leader extractLeader(ConfigMap configMap) { } private boolean isPodReady(String name) { - return this.kubernetesClient.pods().withName(name).isReady(); + return kubernetesClient.pods().withName(name).isReady(); } private ConfigMap getConfigMap() { - return this.kubernetesClient.configMaps() - .inNamespace(this.leaderProperties.getNamespace(this.kubernetesClient.getNamespace())) - .withName(this.leaderProperties.getConfigMapName()).get(); + return kubernetesClient.configMaps().inNamespace(leaderProperties.getNamespace(kubernetesClient.getNamespace())) + .withName(leaderProperties.getConfigMapName()).get(); } private void createConfigMap(Map data) { - LOGGER.debug("Creating new config map with data: {}", data); + LOGGER.debug(() -> "Creating new config map with data: " + data); - ConfigMap newConfigMap = new ConfigMapBuilder().withNewMetadata() - .withName(this.leaderProperties.getConfigMapName()).addToLabels(PROVIDER_KEY, PROVIDER) - .addToLabels(KIND_KEY, KIND).endMetadata().addToData(data).build(); + ConfigMap newConfigMap = new ConfigMapBuilder().withNewMetadata().withName(leaderProperties.getConfigMapName()) + .addToLabels(PROVIDER_KEY, PROVIDER).addToLabels(KIND_KEY, KIND).endMetadata().addToData(data).build(); - this.kubernetesClient.configMaps() - .inNamespace(this.leaderProperties.getNamespace(this.kubernetesClient.getNamespace())) + kubernetesClient.configMaps().inNamespace(leaderProperties.getNamespace(kubernetesClient.getNamespace())) .resource(newConfigMap).create(); } private void updateConfigMapEntry(ConfigMap configMap, Map newData) { - LOGGER.debug("Adding new data to config map: {}", newData); - + LOGGER.debug(() -> "Adding new data to config map: " + newData); ConfigMap newConfigMap = new ConfigMapBuilder(configMap).addToData(newData).build(); - updateConfigMap(configMap, newConfigMap); } private void removeConfigMapEntry(ConfigMap configMap, String key) { - LOGGER.debug("Removing config map entry '{}'", key); - + LOGGER.debug(() -> "Removing config map entry: " + key); ConfigMap newConfigMap = new ConfigMapBuilder(configMap).removeFromData(key).build(); - updateConfigMap(configMap, newConfigMap); } private void updateConfigMap(ConfigMap oldConfigMap, ConfigMap newConfigMap) { - this.kubernetesClient.configMaps() - .inNamespace(this.leaderProperties.getNamespace(this.kubernetesClient.getNamespace())) - .resource(newConfigMap).lockResourceVersion(oldConfigMap.getMetadata().getResourceVersion()).replace(); + kubernetesClient.configMaps().inNamespace(leaderProperties.getNamespace(kubernetesClient.getNamespace())) + .resource(newConfigMap).lockResourceVersion(oldConfigMap.getMetadata().getResourceVersion()).update(); } } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java index adee10a8c7..5347f71eec 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java @@ -94,8 +94,7 @@ void whenNonExistentConfigmapAndCreationNotAllowedStopLeadershipAcquire(Captured fabric8LeadershipController.update(); // then - assertThat(output).contains("ConfigMap '" + testConfigmap + "' does not exist " - + "and leaderProperties.isCreateConfigMap() is false, cannot acquire leadership"); + assertThat(output).contains("ConfigMap 'test-configmap' does not exist and leaderProperties.isCreateConfigMap() is false, cannot acquire leadership"); verify(mockLeaderEventPublisher).publishOnFailedToAcquire(any(), any(), any()); verify(mockKubernetesClient, never()).pods(); From 353dfe7eb43bfacfd39c415cf3607f5c68e12b93 Mon Sep 17 00:00:00 2001 From: erabii Date: Wed, 1 May 2024 15:36:46 +0300 Subject: [PATCH 9/9] Fabric leader clean up 9 (#1651) --- .../kubernetes/commons/leader/LeadershipController.java | 5 +++-- .../fabric8/leader/Fabric8LeaderRecordWatcher.java | 2 +- .../fabric8/leader/Fabric8LeadershipController.java | 9 +++++---- .../fabric8/leader/Fabric8LeadershipControllerTest.java | 3 ++- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java index b7f30b6aea..ced9e3ad2c 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java @@ -83,6 +83,7 @@ protected Leader extractLeader(Map data) { String leaderKey = getLeaderKey(); String leaderId = data.get(leaderKey); + LOGGER.debug(() -> "retrieved leaderId: " + leaderId + " from leaderKey : " + leaderId); if (!StringUtils.hasText(leaderId)) { return null; } @@ -112,7 +113,7 @@ else if (newLeader != null && newLeader.isCandidate(candidate)) { } protected void notifyOnGranted() { - LOGGER.debug(() -> "Leadership has been granted for :" + candidate); + LOGGER.debug(() -> "Leadership has been granted to : " + candidate); Context context = new LeaderContext(candidate, this); leaderEventPublisher.publishOnGranted(this, context, candidate.getRole()); @@ -126,7 +127,7 @@ protected void notifyOnGranted() { } protected void notifyOnRevoked() { - LOGGER.debug(() -> "Leadership has been revoked for :" + candidate); + LOGGER.debug(() -> "Leadership has been revoked from :" + candidate); Context context = new LeaderContext(candidate, this); leaderEventPublisher.publishOnRevoked(this, context, candidate.getRole()); diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java index 0900c6253e..c65fea0608 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java @@ -70,7 +70,7 @@ public void start() { public void stop() { if (configMapWatch != null) { guarded(lock, () -> { - if (this.configMapWatch != null) { + if (configMapWatch != null) { LOGGER.debug(() -> "Stopping leader record watcher"); configMapWatch.close(); configMapWatch = null; diff --git a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipController.java b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipController.java index 8ae555218f..f6aadc4706 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipController.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipController.java @@ -92,7 +92,7 @@ public void revoke() { } private void revoke(ConfigMap configMap) { - LOGGER.debug(() -> "Trying to revoke leadership for :" + candidate); + LOGGER.debug(() -> "Trying to revoke leadership from :" + candidate); try { String leaderKey = getLeaderKey(); @@ -105,7 +105,7 @@ private void revoke(ConfigMap configMap) { } private void acquire(ConfigMap configMap) { - LOGGER.debug(() -> "Trying to acquire leadership for :" + this.candidate); + LOGGER.debug(() -> "Trying to acquire leadership for :" + candidate); if (!isPodReady(candidate.getId())) { LOGGER.debug("Pod : " + candidate + "is not ready at the moment, cannot acquire leadership"); @@ -133,7 +133,7 @@ private void acquire(ConfigMap configMap) { @Override protected PodReadinessWatcher createPodReadinessWatcher(String localLeaderId) { - return new Fabric8PodReadinessWatcher(localLeaderId, this.kubernetesClient, this); + return new Fabric8PodReadinessWatcher(localLeaderId, kubernetesClient, this); } private Leader extractLeader(ConfigMap configMap) { @@ -176,8 +176,9 @@ private void removeConfigMapEntry(ConfigMap configMap, String key) { } private void updateConfigMap(ConfigMap oldConfigMap, ConfigMap newConfigMap) { + String oldResourceVersion = oldConfigMap.getMetadata().getResourceVersion(); kubernetesClient.configMaps().inNamespace(leaderProperties.getNamespace(kubernetesClient.getNamespace())) - .resource(newConfigMap).lockResourceVersion(oldConfigMap.getMetadata().getResourceVersion()).update(); + .resource(newConfigMap).lockResourceVersion(oldResourceVersion).update(); } } diff --git a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java index 5347f71eec..424e789a12 100644 --- a/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java +++ b/spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeadershipControllerTest.java @@ -94,7 +94,8 @@ void whenNonExistentConfigmapAndCreationNotAllowedStopLeadershipAcquire(Captured fabric8LeadershipController.update(); // then - assertThat(output).contains("ConfigMap 'test-configmap' does not exist and leaderProperties.isCreateConfigMap() is false, cannot acquire leadership"); + assertThat(output).contains( + "ConfigMap 'test-configmap' does not exist and leaderProperties.isCreateConfigMap() is false, cannot acquire leadership"); verify(mockLeaderEventPublisher).publishOnFailedToAcquire(any(), any(), any()); verify(mockKubernetesClient, never()).pods();