From f3749fab75fba14d5a5cb0067b2fdeea3465eca3 Mon Sep 17 00:00:00 2001 From: Gytis Trikleris Date: Mon, 4 Jun 2018 19:51:51 +0200 Subject: [PATCH] LeadershipController#revoke --- .../leader/LeadershipController.java | 47 ++++++++--- .../leader/LeadershipControllerTest.java | 78 +++++++++++++++++-- 2 files changed, 109 insertions(+), 16 deletions(-) diff --git a/spring-cloud-kubernetes-leader/src/main/java/org/springframework/cloud/kubernetes/leader/LeadershipController.java b/spring-cloud-kubernetes-leader/src/main/java/org/springframework/cloud/kubernetes/leader/LeadershipController.java index 13bf56e2..01552eda 100644 --- a/spring-cloud-kubernetes-leader/src/main/java/org/springframework/cloud/kubernetes/leader/LeadershipController.java +++ b/spring-cloud-kubernetes-leader/src/main/java/org/springframework/cloud/kubernetes/leader/LeadershipController.java @@ -51,14 +51,14 @@ public class LeadershipController { try { ConfigMap configMap = kubernetesHelper.getConfigMap(); if (configMap == null) { - createLeaderConfigMap(candidate); + createConfigMapWithLeader(candidate); handleOnGranted(candidate); return true; } Leader leader = getLeader(candidate.getRole(), configMap); if (leader == null || !leader.isValid()) { - updateLeaderConfigMap(candidate, configMap); + updateLeaderInConfigMap(candidate, configMap); handleOnGranted(candidate); return true; } @@ -76,11 +76,27 @@ public class LeadershipController { } public boolean revoke(Candidate candidate) { - // Get config map - // Check if candidate is a leader - if not, return false - // Try to revoke leadership - return true - // Return false - // In case of an exception - pass it forward + try { + ConfigMap configMap = kubernetesHelper.getConfigMap(); + if (configMap == null) { + return false; + } + + Leader leader = getLeader(candidate.getRole(), configMap); + if (leader == null) { + return false; + } + + if (candidate.getId().equals(leader.getId())) { + removeLeaderFromConfigMap(candidate, configMap); + handleOnRevoked(candidate); + return true; + } + } catch (KubernetesClientException e) { + LOGGER.warn("Failed to revoke leadership with role='{}' for candidate='{}': {}", candidate.getRole(), + candidate.getId(), e.getMessage()); + } + return false; } @@ -113,14 +129,19 @@ public class LeadershipController { return new Leader(role, leaderId, kubernetesHelper); } - private void createLeaderConfigMap(Candidate candidate) { + private void createConfigMapWithLeader(Candidate candidate) { Map data = getLeaderData(candidate); kubernetesHelper.createConfigMap(data); } - private void updateLeaderConfigMap(Candidate candidate, ConfigMap configMap) { + private void updateLeaderInConfigMap(Candidate candidate, ConfigMap configMap) { Map data = getLeaderData(candidate); - kubernetesHelper.updateConfigMap(configMap, data); + kubernetesHelper.updateConfigMapEntry(configMap, data); + } + + private void removeLeaderFromConfigMap(Candidate candidate, ConfigMap configMap) { + String leaderKey = leaderProperties.getLeaderIdPrefix() + candidate.getRole(); + kubernetesHelper.removeConfigMapEntry(configMap, leaderKey); } private void handleOnGranted(Candidate candidate) { @@ -134,6 +155,12 @@ public class LeadershipController { } } + private void handleOnRevoked(Candidate candidate) { + Context context = new LeaderContext(candidate, this); + leaderEventPublisher.publishOnRevoked(this, context, candidate.getRole()); + candidate.onRevoked(context); + } + private void handleOnFailed(Candidate candidate) { Context context = new LeaderContext(candidate, this); leaderEventPublisher.publishOnFailedToAcquire(this, context, candidate.getRole()); diff --git a/spring-cloud-kubernetes-leader/src/test/java/org/springframework/cloud/kubernetes/leader/LeadershipControllerTest.java b/spring-cloud-kubernetes-leader/src/test/java/org/springframework/cloud/kubernetes/leader/LeadershipControllerTest.java index ecf841ce..101df4e7 100644 --- a/spring-cloud-kubernetes-leader/src/test/java/org/springframework/cloud/kubernetes/leader/LeadershipControllerTest.java +++ b/spring-cloud-kubernetes-leader/src/test/java/org/springframework/cloud/kubernetes/leader/LeadershipControllerTest.java @@ -79,7 +79,7 @@ public class LeadershipControllerTest { boolean result = leadershipController.acquire(mockCandidate); assertThat(result).isTrue(); - verify(mockKubernetesHelper).updateConfigMap(mockConfigMap, leaderData); + verify(mockKubernetesHelper).updateConfigMapEntry(mockConfigMap, leaderData); verifyPublishOnGranted(); } @@ -93,10 +93,8 @@ public class LeadershipControllerTest { assertThat(result).isTrue(); verify(mockKubernetesHelper, times(0)).createConfigMap(any()); - verify(mockKubernetesHelper, times(0)).updateConfigMap(any(), any()); + verify(mockKubernetesHelper, times(0)).updateConfigMapEntry(any(), any()); verify(mockLeaderEventPublisher, times(0)).publishOnGranted(any(), any(), any()); - verify(mockLeaderEventPublisher, times(0)).publishOnRevoked(any(), any(), any()); - verify(mockLeaderEventPublisher, times(0)).publishOnFailedToAcquire(any(), any(), any()); } @Test @@ -110,7 +108,8 @@ public class LeadershipControllerTest { boolean result = leadershipController.acquire(mockCandidate); assertThat(result).isTrue(); - verify(mockKubernetesHelper).updateConfigMap(mockConfigMap, Collections.singletonMap(PREFIX + ROLE, anotherId)); + verify(mockKubernetesHelper).updateConfigMapEntry(mockConfigMap, + Collections.singletonMap(PREFIX + ROLE, anotherId)); verifyPublishOnGranted(); } @@ -125,7 +124,7 @@ public class LeadershipControllerTest { assertThat(result).isFalse(); verify(mockKubernetesHelper, times(0)).createConfigMap(any()); - verify(mockKubernetesHelper, times(0)).updateConfigMap(any(), any()); + verify(mockKubernetesHelper, times(0)).updateConfigMapEntry(any(), any()); verifyPublishOnFailedToAcquire(); } @@ -139,6 +138,65 @@ public class LeadershipControllerTest { verifyPublishOnFailedToAcquire(); } + @Test + public void shouldRevokeLeadership() { + given(mockKubernetesHelper.getConfigMap()).willReturn(mockConfigMap); + given(mockConfigMap.getData()).willReturn(leaderData); + + boolean result = leadershipController.revoke(mockCandidate); + + assertThat(result).isTrue(); + verify(mockKubernetesHelper).removeConfigMapEntry(mockConfigMap, PREFIX + ROLE); + verifyPublishOnRevoked(); + } + + @Test + public void shouldNotRevokeLeadershipIfThereIsNoConfigMap() { + boolean result = leadershipController.revoke(mockCandidate); + + assertThat(result).isFalse(); + verify(mockKubernetesHelper, times(0)).removeConfigMapEntry(any(), any()); + verify(mockLeaderEventPublisher, times(0)).publishOnRevoked(any(), any(), any()); + } + + @Test + public void shouldNotRevokeLeadershipIfThereIsNoLeader() { + given(mockKubernetesHelper.getConfigMap()).willReturn(mockConfigMap); + + boolean result = leadershipController.revoke(mockCandidate); + + assertThat(result).isFalse(); + verify(mockKubernetesHelper, times(0)).removeConfigMapEntry(any(), any()); + verify(mockLeaderEventPublisher, times(0)).publishOnRevoked(any(), any(), any()); + } + + @Test + public void shouldNotRevokeLeadershipIfThereIsAnotherLeader() { + given(mockKubernetesHelper.getConfigMap()).willReturn(mockConfigMap); + given(mockConfigMap.getData()).willReturn(leaderData); + given(mockCandidate.getId()).willReturn("another-test-id"); + + boolean result = leadershipController.revoke(mockCandidate); + + assertThat(result).isFalse(); + verify(mockKubernetesHelper, times(0)).removeConfigMapEntry(any(), any()); + verify(mockLeaderEventPublisher, times(0)).publishOnRevoked(any(), any(), any()); + } + + @Test + public void shouldFailToRevokeLeadership() { + given(mockKubernetesHelper.getConfigMap()).willReturn(mockConfigMap); + given(mockConfigMap.getData()).willReturn(leaderData); + doThrow(new KubernetesClientException("Test exception")).when(mockKubernetesHelper) + .removeConfigMapEntry(any(), any()); + + boolean result = leadershipController.revoke(mockCandidate); + + assertThat(result).isFalse(); + verify(mockKubernetesHelper).removeConfigMapEntry(mockConfigMap, PREFIX + ROLE); + verify(mockLeaderEventPublisher, times(0)).publishOnRevoked(any(), any(), any()); + } + @Test public void shouldGetLeader() { given(mockKubernetesHelper.getConfigMap()).willReturn(mockConfigMap); @@ -192,6 +250,14 @@ public class LeadershipControllerTest { assertThat(leaderContextCaptor.getValue()).isEqualToComparingFieldByField(expectedLeaderContext); } + private void verifyPublishOnRevoked() { + ArgumentCaptor leaderContextCaptor = ArgumentCaptor.forClass(LeaderContext.class); + verify(mockLeaderEventPublisher).publishOnRevoked(eq(leadershipController), + leaderContextCaptor.capture(), eq(ROLE)); + LeaderContext expectedLeaderContext = new LeaderContext(mockCandidate, leadershipController); + assertThat(leaderContextCaptor.getValue()).isEqualToComparingFieldByField(expectedLeaderContext); + } + public void verifyPublishOnFailedToAcquire() { ArgumentCaptor leaderContextCaptor = ArgumentCaptor.forClass(LeaderContext.class); verify(mockLeaderEventPublisher).publishOnFailedToAcquire(eq(leadershipController),