Skip to content

Commit 4336a5b

Browse files
wind57wind57
authored
Fabric leader clean up 6 (#1647)
1 parent 279ee4d commit 4336a5b

File tree

9 files changed

+226
-286
lines changed

9 files changed

+226
-286
lines changed

‎spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/leader/LeadershipController.java

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
import java.util.Objects;
2222
import java.util.Optional;
2323

24-
import org.slf4j.Logger;
25-
import org.slf4j.LoggerFactory;
26-
24+
import org.springframework.core.log.LogAccessor;
2725
import org.springframework.integration.leader.Candidate;
2826
import org.springframework.integration.leader.Context;
2927
import org.springframework.integration.leader.event.LeaderEventPublisher;
@@ -34,7 +32,7 @@
3432
*/
3533
public abstract class LeadershipController {
3634

37-
private static final Logger LOGGER = LoggerFactory.getLogger(LeadershipController.class);
35+
private static final LogAccessor LOGGER = new LogAccessor(LeadershipController.class);
3836

3937
protected static final String PROVIDER_KEY = "provider";
4038

@@ -62,15 +60,15 @@ public LeadershipController(Candidate candidate, LeaderProperties leaderProperti
6260
}
6361

6462
public Optional<Leader> getLocalLeader() {
65-
return Optional.ofNullable(this.localLeader);
63+
return Optional.ofNullable(localLeader);
6664
}
6765

6866
public abstract void update();
6967

7068
public abstract void revoke();
7169

7270
protected String getLeaderKey() {
73-
return this.leaderProperties.getLeaderIdPrefix() + this.candidate.getRole();
71+
return leaderProperties.getLeaderIdPrefix() + candidate.getRole();
7472
}
7573

7674
protected Map<String, String> getLeaderData(Candidate candidate) {
@@ -89,68 +87,68 @@ protected Leader extractLeader(Map<String, String> data) {
8987
return null;
9088
}
9189

92-
return new Leader(this.candidate.getRole(), leaderId);
90+
return new Leader(candidate.getRole(), leaderId);
9391
}
9492

9593
protected void handleLeaderChange(Leader newLeader) {
96-
if (Objects.equals(this.localLeader, newLeader)) {
97-
LOGGER.debug("Leader is still '{}'", this.localLeader);
94+
if (Objects.equals(localLeader, newLeader)) {
95+
LOGGER.debug(() -> "Leader is still : " + localLeader);
9896
return;
9997
}
10098

101-
Leader oldLeader = this.localLeader;
102-
this.localLeader = newLeader;
99+
Leader oldLeader = localLeader;
100+
localLeader = newLeader;
103101

104-
if (oldLeader != null && oldLeader.isCandidate(this.candidate)) {
102+
if (oldLeader != null && oldLeader.isCandidate(candidate)) {
105103
notifyOnRevoked();
106104
}
107-
else if (newLeader != null && newLeader.isCandidate(this.candidate)) {
105+
else if (newLeader != null && newLeader.isCandidate(candidate)) {
108106
notifyOnGranted();
109107
}
110108

111109
restartLeaderReadinessWatcher();
112110

113-
LOGGER.debug("New leader is '{}'", this.localLeader);
111+
LOGGER.debug(() -> "New leader is " + localLeader);
114112
}
115113

116114
protected void notifyOnGranted() {
117-
LOGGER.debug("Leadership has been granted for '{}'", this.candidate);
115+
LOGGER.debug(() -> "Leadership has been granted for :" + candidate);
118116

119-
Context context = new LeaderContext(this.candidate, this);
120-
this.leaderEventPublisher.publishOnGranted(this, context, this.candidate.getRole());
117+
Context context = new LeaderContext(candidate, this);
118+
leaderEventPublisher.publishOnGranted(this, context, candidate.getRole());
121119
try {
122-
this.candidate.onGranted(context);
120+
candidate.onGranted(context);
123121
}
124122
catch (InterruptedException e) {
125-
LOGGER.warn(e.getMessage());
123+
LOGGER.warn(e::getMessage);
126124
Thread.currentThread().interrupt();
127125
}
128126
}
129127

130128
protected void notifyOnRevoked() {
131-
LOGGER.debug("Leadership has been revoked for '{}'", this.candidate);
129+
LOGGER.debug(() -> "Leadership has been revoked for :" + candidate);
132130

133-
Context context = new LeaderContext(this.candidate, this);
134-
this.leaderEventPublisher.publishOnRevoked(this, context, this.candidate.getRole());
135-
this.candidate.onRevoked(context);
131+
Context context = new LeaderContext(candidate, this);
132+
leaderEventPublisher.publishOnRevoked(this, context, candidate.getRole());
133+
candidate.onRevoked(context);
136134
}
137135

138136
protected void notifyOnFailedToAcquire() {
139-
if (this.leaderProperties.isPublishFailedEvents()) {
140-
Context context = new LeaderContext(this.candidate, this);
141-
this.leaderEventPublisher.publishOnFailedToAcquire(this, context, this.candidate.getRole());
137+
if (leaderProperties.isPublishFailedEvents()) {
138+
Context context = new LeaderContext(candidate, this);
139+
leaderEventPublisher.publishOnFailedToAcquire(this, context, candidate.getRole());
142140
}
143141
}
144142

145143
protected void restartLeaderReadinessWatcher() {
146-
if (this.leaderReadinessWatcher != null) {
147-
this.leaderReadinessWatcher.stop();
148-
this.leaderReadinessWatcher = null;
144+
if (leaderReadinessWatcher != null) {
145+
leaderReadinessWatcher.stop();
146+
leaderReadinessWatcher = null;
149147
}
150148

151-
if (this.localLeader != null && !this.localLeader.isCandidate(this.candidate)) {
152-
this.leaderReadinessWatcher = createPodReadinessWatcher(this.localLeader.getId());
153-
this.leaderReadinessWatcher.start();
149+
if (localLeader != null && !localLeader.isCandidate(candidate)) {
150+
leaderReadinessWatcher = createPodReadinessWatcher(localLeader.getId());
151+
leaderReadinessWatcher.start();
154152
}
155153
}
156154

‎spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcher.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@
2525
import org.slf4j.LoggerFactory;
2626

2727
import org.springframework.cloud.kubernetes.commons.leader.LeaderProperties;
28+
import org.springframework.cloud.kubernetes.commons.leader.LeaderRecordWatcher;
2829

2930
/**
3031
* @author Gytis Trikleris
3132
*/
32-
public class Fabric8LeaderRecordWatcher
33-
implements org.springframework.cloud.kubernetes.commons.leader.LeaderRecordWatcher, Watcher<ConfigMap> {
33+
public class Fabric8LeaderRecordWatcher implements LeaderRecordWatcher, Watcher<ConfigMap> {
3434

3535
private static final Logger LOGGER = LoggerFactory.getLogger(Fabric8LeaderRecordWatcher.class);
3636

‎spring-cloud-kubernetes-fabric8-leader/src/main/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8PodReadinessWatcher.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void start() {
6464
guarded(lock, () -> {
6565
if (watch == null) {
6666
LOGGER.debug(() -> "Starting pod readiness watcher for :" + podName);
67-
PodResource podResource = kubernetesClient.pods().withName(this.podName);
67+
PodResource podResource = kubernetesClient.pods().withName(podName);
6868
previousState = podResource.isReady();
6969
watch = podResource.watch(this);
7070
}

‎spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderAutoConfigurationTests.java

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2019 the original author or authors.
2+
* Copyright 2013-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,8 +17,6 @@
1717
package org.springframework.cloud.kubernetes.fabric8.leader;
1818

1919
import org.junit.jupiter.api.Test;
20-
import org.junit.jupiter.api.extension.ExtendWith;
21-
import org.mockito.junit.jupiter.MockitoExtension;
2220

2321
import org.springframework.beans.factory.annotation.Autowired;
2422
import org.springframework.boot.SpringBootConfiguration;
@@ -30,12 +28,11 @@
3028

3129
import static org.hamcrest.Matchers.containsString;
3230

33-
@ExtendWith(MockitoExtension.class)
3431
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
3532
properties = { "spring.main.cloud-platform=KUBERNETES", "spring.cloud.kubernetes.leader.autoStartup=false",
3633
"management.endpoints.web.exposure.include=info", "management.endpoint.info.show-details=always",
3734
"management.info.kubernetes.enabled=true" })
38-
public class Fabric8LeaderAutoConfigurationTests {
35+
class Fabric8LeaderAutoConfigurationTests {
3936

4037
@LocalManagementPort
4138
private int port;
@@ -44,13 +41,13 @@ public class Fabric8LeaderAutoConfigurationTests {
4441
private WebTestClient webClient;
4542

4643
@Test
47-
public void contextLoads() {
44+
void contextLoads() {
4845
}
4946

5047
@Test
51-
public void infoEndpointShouldContainLeaderElection() {
52-
this.webClient.get().uri("http://localhost:{port}/actuator/info", this.port).accept(MediaType.APPLICATION_JSON)
53-
.exchange().expectStatus().isOk().expectBody(String.class).value(containsString("kubernetes"));
48+
void infoEndpointShouldContainLeaderElection() {
49+
webClient.get().uri("http://localhost:{port}/actuator/info", port).accept(MediaType.APPLICATION_JSON).exchange()
50+
.expectStatus().isOk().expectBody(String.class).value(containsString("kubernetes"));
5451
}
5552

5653
@SpringBootConfiguration

‎spring-cloud-kubernetes-fabric8-leader/src/test/java/org/springframework/cloud/kubernetes/fabric8/leader/Fabric8LeaderRecordWatcherTest.java

Lines changed: 48 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2019 the original author or authors.
2+
* Copyright 2013-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -27,112 +27,99 @@
2727
import io.fabric8.kubernetes.client.dsl.Resource;
2828
import org.junit.jupiter.api.BeforeEach;
2929
import org.junit.jupiter.api.Test;
30-
import org.junit.jupiter.api.extension.ExtendWith;
31-
import org.mockito.Mock;
32-
import org.mockito.junit.jupiter.MockitoExtension;
30+
import org.mockito.Mockito;
3331

3432
import org.springframework.cloud.kubernetes.commons.leader.LeaderProperties;
3533

36-
import static org.mockito.BDDMockito.given;
3734
import static org.mockito.Mockito.times;
3835
import static org.mockito.Mockito.verify;
3936

4037
/**
4138
* @author Gytis Trikleris
4239
*/
43-
@ExtendWith(MockitoExtension.class)
44-
public class Fabric8LeaderRecordWatcherTest {
40+
class Fabric8LeaderRecordWatcherTest {
4541

46-
@Mock
47-
private LeaderProperties mockLeaderProperties;
42+
private final LeaderProperties mockLeaderProperties = Mockito.mock(LeaderProperties.class);
4843

49-
@Mock
50-
private Fabric8LeadershipController mockFabric8LeadershipController;
44+
private final Fabric8LeadershipController mockFabric8LeadershipController = Mockito
45+
.mock(Fabric8LeadershipController.class);
5146

52-
@Mock
53-
private KubernetesClient mockKubernetesClient;
47+
private final KubernetesClient mockKubernetesClient = Mockito.mock(KubernetesClient.class);
5448

55-
@Mock
56-
private MixedOperation<ConfigMap, ConfigMapList, Resource<ConfigMap>> mockConfigMapsOperation;
49+
@SuppressWarnings("unchecked")
50+
private final MixedOperation<ConfigMap, ConfigMapList, Resource<ConfigMap>> mockConfigMapsOperation = Mockito
51+
.mock(MixedOperation.class);
5752

58-
@Mock
59-
private NonNamespaceOperation<ConfigMap, ConfigMapList, Resource<ConfigMap>> mockInNamespaceOperation;
53+
@SuppressWarnings("unchecked")
54+
private final NonNamespaceOperation<ConfigMap, ConfigMapList, Resource<ConfigMap>> mockInNamespaceOperation = Mockito
55+
.mock(NonNamespaceOperation.class);
6056

61-
@Mock
62-
private Resource<ConfigMap> mockWithNameResource;
57+
@SuppressWarnings("unchecked")
58+
private final Resource<ConfigMap> mockWithNameResource = Mockito.mock(Resource.class);
6359

64-
@Mock
65-
private Watch mockWatch;
60+
private final Watch mockWatch = Mockito.mock(Watch.class);
6661

67-
@Mock
68-
private ConfigMap mockConfigMap;
62+
private final ConfigMap mockConfigMap = Mockito.mock(ConfigMap.class);
6963

70-
@Mock
71-
private WatcherException mockKubernetesClientException;
64+
private final WatcherException mockKubernetesClientException = Mockito.mock(WatcherException.class);
7265

7366
private Fabric8LeaderRecordWatcher watcher;
7467

7568
@BeforeEach
76-
public void before() {
77-
this.watcher = new Fabric8LeaderRecordWatcher(this.mockLeaderProperties, this.mockFabric8LeadershipController,
78-
this.mockKubernetesClient);
69+
void beforeEach() {
70+
watcher = new Fabric8LeaderRecordWatcher(mockLeaderProperties, mockFabric8LeadershipController,
71+
mockKubernetesClient);
7972
}
8073

8174
@Test
82-
public void shouldStartOnce() {
75+
void shouldStartOnce() {
8376
initStubs();
84-
this.watcher.start();
85-
this.watcher.start();
86-
87-
verify(this.mockWithNameResource).watch(this.watcher);
77+
watcher.start();
78+
watcher.start();
79+
verify(mockWithNameResource).watch(watcher);
8880
}
8981

9082
@Test
91-
public void shouldStopOnce() {
83+
void shouldStopOnce() {
9284
initStubs();
93-
this.watcher.start();
94-
this.watcher.stop();
95-
this.watcher.stop();
96-
97-
verify(this.mockWatch).close();
85+
watcher.start();
86+
watcher.stop();
87+
watcher.stop();
88+
verify(mockWatch).close();
9889
}
9990

10091
@Test
101-
public void shouldHandleEvent() {
102-
this.watcher.eventReceived(Watcher.Action.ADDED, this.mockConfigMap);
103-
this.watcher.eventReceived(Watcher.Action.DELETED, this.mockConfigMap);
104-
this.watcher.eventReceived(Watcher.Action.MODIFIED, this.mockConfigMap);
105-
106-
verify(this.mockFabric8LeadershipController, times(3)).update();
92+
void shouldHandleEvent() {
93+
watcher.eventReceived(Watcher.Action.ADDED, mockConfigMap);
94+
watcher.eventReceived(Watcher.Action.DELETED, mockConfigMap);
95+
watcher.eventReceived(Watcher.Action.MODIFIED, mockConfigMap);
96+
verify(mockFabric8LeadershipController, times(3)).update();
10797
}
10898

10999
@Test
110-
public void shouldIgnoreErrorEvent() {
111-
this.watcher.eventReceived(Watcher.Action.ERROR, this.mockConfigMap);
112-
113-
verify(this.mockFabric8LeadershipController, times(0)).update();
100+
void shouldIgnoreErrorEvent() {
101+
watcher.eventReceived(Watcher.Action.ERROR, mockConfigMap);
102+
verify(mockFabric8LeadershipController, times(0)).update();
114103
}
115104

116105
@Test
117-
public void shouldHandleClose() {
106+
void shouldHandleClose() {
118107
initStubs();
119-
this.watcher.onClose(this.mockKubernetesClientException);
120-
121-
verify(this.mockWithNameResource).watch(this.watcher);
108+
watcher.onClose(mockKubernetesClientException);
109+
verify(mockWithNameResource).watch(watcher);
122110
}
123111

124112
@Test
125-
public void shouldIgnoreCloseWithoutCause() {
126-
this.watcher.onClose(null);
127-
128-
verify(this.mockWithNameResource, times(0)).watch(this.watcher);
113+
void shouldIgnoreCloseWithoutCause() {
114+
watcher.onClose(null);
115+
verify(mockWithNameResource, times(0)).watch(watcher);
129116
}
130117

131118
private void initStubs() {
132-
given(this.mockKubernetesClient.configMaps()).willReturn(this.mockConfigMapsOperation);
133-
given(this.mockConfigMapsOperation.inNamespace(null)).willReturn(this.mockInNamespaceOperation);
134-
given(this.mockInNamespaceOperation.withName(null)).willReturn(this.mockWithNameResource);
135-
given(this.mockWithNameResource.watch(this.watcher)).willReturn(this.mockWatch);
119+
Mockito.when(mockKubernetesClient.configMaps()).thenReturn(mockConfigMapsOperation);
120+
Mockito.when(mockConfigMapsOperation.inNamespace(null)).thenReturn(mockInNamespaceOperation);
121+
Mockito.when(mockInNamespaceOperation.withName(null)).thenReturn(mockWithNameResource);
122+
Mockito.when(mockWithNameResource.watch(watcher)).thenReturn(mockWatch);
136123
}
137124

138125
}

0 commit comments

Comments
 (0)