From 92ebb3af5b0f7b2a7a11454215b82e322448e07a Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Fri, 25 Apr 2025 11:52:38 +0900 Subject: [PATCH 01/21] Update pauser to handle exception and run unpause as long as possible --- .../com/scalar/admin/kubernetes/Pauser.java | 50 ++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 0cda1a3..78ed7ae 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -1,5 +1,6 @@ package com.scalar.admin.kubernetes; +import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.Uninterruptibles; import com.scalar.admin.RequestCoordinator; import io.kubernetes.client.openapi.Configuration; @@ -34,7 +35,7 @@ @NotThreadSafe public class Pauser { - private static final int MAX_UNPAUSE_RETRY_COUNT = 3; + @VisibleForTesting static final int MAX_UNPAUSE_RETRY_COUNT = 3; private final Logger logger = LoggerFactory.getLogger(Pauser.class); private final TargetSelector targetSelector; @@ -84,17 +85,30 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) throw new PauserException("Failed to find the target pods to pause.", e); } - RequestCoordinator coordinator = getRequestCoordinator(target); + RequestCoordinator coordinator; + try { + coordinator = getRequestCoordinator(target); + } catch (Exception e) { + throw new PauserException("Failed to initialize the coordinator.", e); + } + + Instant startTime; + Instant endTime; + try { + coordinator.pause(true, maxPauseWaitTime); - coordinator.pause(true, maxPauseWaitTime); + startTime = Instant.now(); - Instant startTime = Instant.now(); + Uninterruptibles.sleepUninterruptibly(pauseDuration, TimeUnit.MILLISECONDS); - Uninterruptibles.sleepUninterruptibly(pauseDuration, TimeUnit.MILLISECONDS); + endTime = Instant.now(); - Instant endTime = Instant.now(); + unpauseWithRetry(coordinator, MAX_UNPAUSE_RETRY_COUNT, target); - unpauseWithRetry(coordinator, MAX_UNPAUSE_RETRY_COUNT, target); + } catch (Exception e) { + unpauseWithRetry(coordinator, MAX_UNPAUSE_RETRY_COUNT, target); + throw e; + } TargetSnapshot targetAfterPause; try { @@ -113,8 +127,9 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) return new PausedDuration(startTime, endTime); } - private void unpauseWithRetry( - RequestCoordinator coordinator, int maxRetryCount, TargetSnapshot target) { + @VisibleForTesting + void unpauseWithRetry(RequestCoordinator coordinator, int maxRetryCount, TargetSnapshot target) + throws PauserException { int retryCounter = 0; while (true) { @@ -123,12 +138,23 @@ private void unpauseWithRetry( return; } catch (Exception e) { if (++retryCounter >= maxRetryCount) { - logger.warn( + // If someone uses this library directly instead of using our CLI, users should handle the + // exception properly. However, this case is a very critical issue. Therefore, we output + // the error message here despite whether the exception is handled or not on the caller + // side. + logger.error( "Failed to unpause Scalar product. They are still in paused. You must restart related" + " pods by using the `kubectl rollout restart deployment {}`" - + " command to unpase all pods.", + + " command to unpause all pods.", target.getDeployment().getMetadata().getName()); - return; + // In our CLI, we catch this exception and output the message as an error on the CLI side. + throw new PauserException( + String.format( + "Failed to unpause Scalar product. They are still in paused. You must restart" + + " related pods by using the `kubectl rollout restart deployment %s` command" + + " to unpause all pods.", + target.getDeployment().getMetadata().getName()), + e); } } } From 07d832509267f2d5590bcd475fed52ca3e2b2dc8 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Fri, 25 Apr 2025 19:12:40 +0900 Subject: [PATCH 02/21] [WIP] Add several unit tests --- lib/build.gradle | 1 + .../com/scalar/admin/kubernetes/Pauser.java | 15 +- .../scalar/admin/kubernetes/PauserTest.java | 232 ++++++++++++++++++ 3 files changed, 242 insertions(+), 6 deletions(-) create mode 100644 lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java diff --git a/lib/build.gradle b/lib/build.gradle index f97c5a7..2b76355 100644 --- a/lib/build.gradle +++ b/lib/build.gradle @@ -8,6 +8,7 @@ dependencies { testImplementation(platform("org.junit:junit-bom:${junitVersion}")) testImplementation 'org.junit.jupiter:junit-jupiter' testImplementation "org.mockito:mockito-core:${mockitoVersion}" + testImplementation "org.mockito:mockito-inline:${mockitoVersion}" } test { diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 78ed7ae..ff9d591 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -78,12 +78,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) "pauseDuration is required to be greater than 0 millisecond."); } - TargetSnapshot target; - try { - target = targetSelector.select(); - } catch (Exception e) { - throw new PauserException("Failed to find the target pods to pause.", e); - } + TargetSnapshot target = getTarget(); RequestCoordinator coordinator; try { @@ -160,6 +155,14 @@ void unpauseWithRetry(RequestCoordinator coordinator, int maxRetryCount, TargetS } } + TargetSnapshot getTarget() throws PauserException { + try { + return targetSelector.select(); + } catch (Exception e) { + throw new PauserException("Failed to find the target pods to pause.", e); + } + } + RequestCoordinator getRequestCoordinator(TargetSnapshot target) { return new RequestCoordinator( target.getPods().stream() diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java new file mode 100644 index 0000000..42a0da6 --- /dev/null +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -0,0 +1,232 @@ +package com.scalar.admin.kubernetes; + +import static com.scalar.admin.kubernetes.Pauser.MAX_UNPAUSE_RETRY_COUNT; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import com.scalar.admin.RequestCoordinator; +import io.kubernetes.client.openapi.models.V1Deployment; +import io.kubernetes.client.openapi.models.V1ObjectMeta; +import io.kubernetes.client.util.Config; +import java.io.IOException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +class PauserTest { + + private MockedStatic mockedConfig; + private RequestCoordinator requestCoordinator; + private TargetSnapshot targetSnapshot; + private TargetSelector targetSelector; + private V1Deployment deployment; + private V1ObjectMeta objectMeta; + private final String dummyObjectName = "dummyObjectName"; + + @BeforeEach + void beforeEach() throws PauserException { + mockedConfig = mockStatic(Config.class); + mockedConfig.when(() -> Config.defaultClient()).thenReturn(null); + requestCoordinator = mock(RequestCoordinator.class); + targetSnapshot = mock(TargetSnapshot.class); + targetSelector = mock(TargetSelector.class); + deployment = mock(V1Deployment.class); + objectMeta = mock(V1ObjectMeta.class); + + when(targetSnapshot.getDeployment()).thenReturn(deployment); + when(deployment.getMetadata()).thenReturn(objectMeta); + when(objectMeta.getName()).thenReturn(dummyObjectName); + when(targetSelector.select()).thenReturn(targetSnapshot); + } + + @AfterEach + void afterEach() { + mockedConfig.close(); + } + + @Nested + class ConstructorPauser { + @Test + void WithCorrectArgsReturnPauser() throws PauserException, IOException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + + // Act & Assert + assertDoesNotThrow(() -> new Pauser(namespace, helmReleaseName)); + } + + @Test + void WithNullNamespaceShouldThrowException() { + // Arrange + String namespace = null; + String helmReleaseName = "dummyRelease"; + + // Act & Assert + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, () -> new Pauser(namespace, helmReleaseName)); + assertEquals("namespace is required", thrown.getMessage()); + } + + @Test + void WithNullHelmReleaseNameShouldThrowException() { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = null; + + // Act & Assert + IllegalArgumentException thrown = + assertThrows( + IllegalArgumentException.class, () -> new Pauser(namespace, helmReleaseName)); + assertEquals("helmReleaseName is required", thrown.getMessage()); + } + + @Test + void WhenSetDefaultApiClientThrowIOExceptionShouldThrowException() { + // Arrange + mockedConfig.when(() -> Config.defaultClient()).thenThrow(IOException.class); + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> new Pauser(namespace, helmReleaseName)); + assertEquals("Failed to set default Kubernetes client.", thrown.getMessage()); + } + } + + @Nested + class MethodPause { + @Test + void LessThanOnePauseDurationShouldThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 0; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + // Act & Assert + IllegalArgumentException thrown = + assertThrows(IllegalArgumentException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "pauseDuration is required to be greater than 0 millisecond.", thrown.getMessage()); + } + + @Test + void WhenFirstTargetSelectorThrowExceptionShouldThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = new Pauser(namespace, helmReleaseName); + when(targetSelector.select()).thenThrow(RuntimeException.class); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals("Failed to find the target pods to pause.", thrown.getMessage()); + } + + @Test + void WhenGetRequestCoordinatorThrowExceptionShouldThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetSnapshot).when(pauser).getTarget(); + doThrow(RuntimeException.class).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).unpause(); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals("Failed to initialize the coordinator.", thrown.getMessage()); + } + + @Test + void WhenCoordinatorPauseThrowExceptionShouldRunUnpause() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetSnapshot).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doThrow(RuntimeException.class).when(requestCoordinator).pause(true, null); + doNothing().when(requestCoordinator).unpause(); + + // Act & Assert + RuntimeException thrown = + assertThrows(RuntimeException.class, () -> pauser.pause(pauseDuration, null)); + verify(pauser, times(1)) + .unpauseWithRetry( + any(RequestCoordinator.class), + eq(MAX_UNPAUSE_RETRY_COUNT), + any(TargetSnapshot.class)); + } + + @Test + void WhenInstantNowThrowExceptionShouldRunUnpause() {} + + @Test + void WhenSleepThrowExceptionShouldRunUnpause() {} + + @Test + void WhenUnpauseThrowExceptionShouldRunUnpauseAgain() {} + + @Test + void WhenSecondTargetSelectorThrowExceptionShouldThrowException() {} + + @Test + void WhenTargetPodStatusChangedShouldThrowException() {} + + @Test + void WhenPauseSucceededReturnPausedDuration() {} + } + + @Nested + class MethodUnpauseWithRetry { + @Test + void WhenUnpauseSucceededReturnWithoutException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + doNothing().when(requestCoordinator).unpause(); + + // Act & Assert + assertDoesNotThrow( + () -> + pauser.unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot)); + } + + @Test + void WhenExceptionOccurShouldRetryThreeTimes() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + doThrow(RuntimeException.class).when(requestCoordinator).unpause(); + + // Act & Assert + PauserException thrown = + assertThrows( + PauserException.class, + () -> + pauser.unpauseWithRetry( + requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot)); + assertEquals( + "Failed to unpause Scalar product. They are still in paused. You must restart related" + + " pods by using the `kubectl rollout restart deployment " + + dummyObjectName + + "` command to" + + " unpause all pods.", + thrown.getMessage()); + verify(requestCoordinator, times(MAX_UNPAUSE_RETRY_COUNT)).unpause(); + } + } +} From 08312b2009b5ed35485e8c3a0fc2c4eb824ffcb7 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Mon, 28 Apr 2025 15:41:21 +0900 Subject: [PATCH 03/21] [WIP] Add several unit tests --- .../com/scalar/admin/kubernetes/Pauser.java | 31 +- .../scalar/admin/kubernetes/PauserTest.java | 264 +++++++++++++++++- 2 files changed, 277 insertions(+), 18 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index ff9d591..6a62906 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -78,7 +78,12 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) "pauseDuration is required to be greater than 0 millisecond."); } - TargetSnapshot target = getTarget(); + TargetSnapshot target; + try { + target = getTarget(); + } catch (Exception e) { + throw new PauserException("Failed to find the target pods to pause.", e); + } RequestCoordinator coordinator; try { @@ -101,13 +106,24 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) unpauseWithRetry(coordinator, MAX_UNPAUSE_RETRY_COUNT, target); } catch (Exception e) { - unpauseWithRetry(coordinator, MAX_UNPAUSE_RETRY_COUNT, target); - throw e; + try { + unpauseWithRetry(coordinator, MAX_UNPAUSE_RETRY_COUNT, target); + } catch (PauserException ex) { + throw new PauserException("unpauseWithRetry() method failed twice.", e); + } catch (Exception ex) { + throw new PauserException( + "unpauseWithRetry() method failed twice due to unexpected exception.", e); + } + throw new PauserException( + "The pause operation failed for some reason. However, the unpause operation succeeded" + + " afterward. Currently, the scalar products are running with the unpause status." + + " You should retry the pause operation to ensure proper backup.", + e); } TargetSnapshot targetAfterPause; try { - targetAfterPause = targetSelector.select(); + targetAfterPause = getTarget(); } catch (Exception e) { throw new PauserException( "Failed to find the target pods to examine if the targets pods were updated during" @@ -155,12 +171,9 @@ void unpauseWithRetry(RequestCoordinator coordinator, int maxRetryCount, TargetS } } + @VisibleForTesting TargetSnapshot getTarget() throws PauserException { - try { - return targetSelector.select(); - } catch (Exception e) { - throw new PauserException("Failed to find the target pods to pause.", e); - } + return targetSelector.select(); } RequestCoordinator getRequestCoordinator(TargetSnapshot target) { diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index 42a0da6..9ed9c99 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -1,14 +1,20 @@ package com.scalar.admin.kubernetes; import static com.scalar.admin.kubernetes.Pauser.MAX_UNPAUSE_RETRY_COUNT; +import static java.time.temporal.ChronoUnit.SECONDS; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; +import com.google.common.util.concurrent.Uninterruptibles; import com.scalar.admin.RequestCoordinator; import io.kubernetes.client.openapi.models.V1Deployment; import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.util.Config; import java.io.IOException; +import java.time.Instant; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; @@ -139,7 +145,6 @@ void WhenGetRequestCoordinatorThrowExceptionShouldThrowException() throws Pauser Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); doReturn(targetSnapshot).when(pauser).getTarget(); doThrow(RuntimeException.class).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).unpause(); // Act & Assert PauserException thrown = @@ -160,8 +165,13 @@ void WhenCoordinatorPauseThrowExceptionShouldRunUnpause() throws PauserException doNothing().when(requestCoordinator).unpause(); // Act & Assert - RuntimeException thrown = - assertThrows(RuntimeException.class, () -> pauser.pause(pauseDuration, null)); + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "The pause operation failed for some reason. However, the unpause operation succeeded" + + " afterward. Currently, the scalar products are running with the unpause status." + + " You should retry the pause operation to ensure proper backup.", + thrown.getMessage()); verify(pauser, times(1)) .unpauseWithRetry( any(RequestCoordinator.class), @@ -170,22 +180,258 @@ void WhenCoordinatorPauseThrowExceptionShouldRunUnpause() throws PauserException } @Test - void WhenInstantNowThrowExceptionShouldRunUnpause() {} + void WhenInstantNowThrowExceptionShouldRunUnpause() throws PauserException { + // Arrange + MockedStatic mockedTime = mockStatic(Instant.class); + mockedTime.when(() -> Instant.now()).thenThrow(RuntimeException.class); + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetSnapshot).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).pause(true, null); + doNothing().when(requestCoordinator).unpause(); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "The pause operation failed for some reason. However, the unpause operation succeeded" + + " afterward. Currently, the scalar products are running with the unpause status." + + " You should retry the pause operation to ensure proper backup.", + thrown.getMessage()); + verify(pauser, times(1)) + .unpauseWithRetry( + any(RequestCoordinator.class), + eq(MAX_UNPAUSE_RETRY_COUNT), + any(TargetSnapshot.class)); + + mockedTime.close(); + } @Test - void WhenSleepThrowExceptionShouldRunUnpause() {} + void WhenSleepThrowExceptionShouldRunUnpause() throws PauserException { + // Arrange + int pauseDuration = 1; + MockedStatic mockedSleep = mockStatic(Uninterruptibles.class); + mockedSleep + .when(() -> Uninterruptibles.sleepUninterruptibly(pauseDuration, TimeUnit.MILLISECONDS)) + .thenThrow(RuntimeException.class); + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetSnapshot).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).pause(true, null); + doNothing().when(requestCoordinator).unpause(); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "The pause operation failed for some reason. However, the unpause operation succeeded" + + " afterward. Currently, the scalar products are running with the unpause status." + + " You should retry the pause operation to ensure proper backup.", + thrown.getMessage()); + verify(pauser, times(1)) + .unpauseWithRetry( + any(RequestCoordinator.class), + eq(MAX_UNPAUSE_RETRY_COUNT), + any(TargetSnapshot.class)); + + mockedSleep.close(); + } @Test - void WhenUnpauseThrowExceptionShouldRunUnpauseAgain() {} + void WhenUnpauseThrowExceptionShouldRunUnpauseAgain() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetSnapshot).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).pause(true, null); + doThrow(RuntimeException.class) + .doNothing() + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "The pause operation failed for some reason. However, the unpause operation succeeded" + + " afterward. Currently, the scalar products are running with the unpause status." + + " You should retry the pause operation to ensure proper backup.", + thrown.getMessage()); + verify(pauser, times(2)) + .unpauseWithRetry( + any(RequestCoordinator.class), + eq(MAX_UNPAUSE_RETRY_COUNT), + any(TargetSnapshot.class)); + } @Test - void WhenSecondTargetSelectorThrowExceptionShouldThrowException() {} + void WhenUnpauseThrowPauserExceptionTwiceShouldRunUnpauseAgain() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetSnapshot).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).pause(true, null); + doThrow(PauserException.class) + .doThrow(PauserException.class) + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals("unpauseWithRetry() method failed twice.", thrown.getMessage()); + verify(pauser, times(2)) + .unpauseWithRetry( + any(RequestCoordinator.class), + eq(MAX_UNPAUSE_RETRY_COUNT), + any(TargetSnapshot.class)); + } + + @Test + void WhenUnpauseThrowUnexpectedExceptionTwiceShouldRunUnpauseAgain() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetSnapshot).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).pause(true, null); + doThrow(RuntimeException.class) + .doThrow(RuntimeException.class) + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "unpauseWithRetry() method failed twice due to unexpected exception.", + thrown.getMessage()); + verify(pauser, times(2)) + .unpauseWithRetry( + any(RequestCoordinator.class), + eq(MAX_UNPAUSE_RETRY_COUNT), + any(TargetSnapshot.class)); + } + + @Test + void WhenSecondTargetSelectorThrowExceptionShouldThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetSnapshot).doThrow(RuntimeException.class).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).pause(true, null); + doNothing().when(requestCoordinator).unpause(); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "Failed to find the target pods to examine if the targets pods were updated during" + + " paused.", + thrown.getMessage()); + } @Test - void WhenTargetPodStatusChangedShouldThrowException() {} + void WhenTargetPodStatusChangedShouldThrowException() throws PauserException { + // Arrange + TargetSnapshot targetBeforePause = mock(TargetSnapshot.class); + TargetSnapshot targetAfterPause = mock(TargetSnapshot.class); + Map podRestartCounts = + new HashMap() { + { + put("dummyKey", 1); + } + }; + Map podResourceVersions = + new HashMap() { + { + put("dummyKey", "dummyValue"); + } + }; + TargetStatus beforeTargetStatus = + new TargetStatus(podRestartCounts, podResourceVersions, "beforeDifferentValue"); + TargetStatus afterTargetStatus = + new TargetStatus(podRestartCounts, podResourceVersions, "afterDifferentValue"); + doReturn(beforeTargetStatus).when(targetBeforePause).getStatus(); + doReturn(afterTargetStatus).when(targetAfterPause).getStatus(); + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).pause(true, null); + doNothing().when(requestCoordinator).unpause(); + + // Act & Assert + PauserException thrown = + assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "The target pods were updated during paused. Please retry.", thrown.getMessage()); + } @Test - void WhenPauseSucceededReturnPausedDuration() {} + void WhenPauseSucceededReturnPausedDuration() throws PauserException { + TargetSnapshot targetBeforePause = mock(TargetSnapshot.class); + TargetSnapshot targetAfterPause = mock(TargetSnapshot.class); + Map podRestartCounts = + new HashMap() { + { + put("dummyKey", 1); + } + }; + Map podResourceVersions = + new HashMap() { + { + put("dummyKey", "dummyValue"); + } + }; + TargetStatus beforeTargetStatus = + new TargetStatus(podRestartCounts, podResourceVersions, "sameValue"); + TargetStatus afterTargetStatus = + new TargetStatus(podRestartCounts, podResourceVersions, "sameValue"); + doReturn(beforeTargetStatus).when(targetBeforePause).getStatus(); + doReturn(afterTargetStatus).when(targetAfterPause).getStatus(); + + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + MockedStatic mockedTime = mockStatic(Instant.class); + mockedTime.when(() -> Instant.now()).thenReturn(startTime).thenReturn(endTime); + + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doNothing().when(requestCoordinator).pause(true, null); + doNothing().when(requestCoordinator).unpause(); + + // Act & Assert + PausedDuration actual = assertDoesNotThrow(() -> pauser.pause(pauseDuration, 3000L)); + PausedDuration expected = new PausedDuration(startTime, endTime); + assertEquals(actual.getStartTime(), expected.getStartTime()); + assertEquals(actual.getEndTime(), expected.getEndTime()); + + mockedTime.close(); + } } @Nested From 34a03fc24df43367f4eb3206f46e9e4cc41ea58d Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Mon, 28 Apr 2025 17:49:39 +0900 Subject: [PATCH 04/21] Fix minor things --- .../main/java/com/scalar/admin/kubernetes/Pauser.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 6a62906..7cd2daf 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -149,10 +149,10 @@ void unpauseWithRetry(RequestCoordinator coordinator, int maxRetryCount, TargetS return; } catch (Exception e) { if (++retryCounter >= maxRetryCount) { - // If someone uses this library directly instead of using our CLI, users should handle the - // exception properly. However, this case is a very critical issue. Therefore, we output - // the error message here despite whether the exception is handled or not on the caller - // side. + // Users who directly utilize this library, bypassing our CLI, are responsible for proper + // exception handling. However, this scenario represents a critical issue. Consequently, + // we output the error message here regardless of whether the calling code handles the + // exception. logger.error( "Failed to unpause Scalar product. They are still in paused. You must restart related" + " pods by using the `kubectl rollout restart deployment {}`" @@ -176,6 +176,7 @@ TargetSnapshot getTarget() throws PauserException { return targetSelector.select(); } + @VisibleForTesting RequestCoordinator getRequestCoordinator(TargetSnapshot target) { return new RequestCoordinator( target.getPods().stream() From 4d22a34c4a2ba14114f5339a28f33a6d68d0782c Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Thu, 1 May 2025 19:17:25 +0900 Subject: [PATCH 05/21] Update exception handling based on review feedback --- .../kubernetes/PauseFailedException.java | 11 ++ .../com/scalar/admin/kubernetes/Pauser.java | 162 ++++++++++++------ .../StatusCheckFailedException.java | 11 ++ .../kubernetes/UnpauseFailedException.java | 11 ++ 4 files changed, 143 insertions(+), 52 deletions(-) create mode 100644 lib/src/main/java/com/scalar/admin/kubernetes/PauseFailedException.java create mode 100644 lib/src/main/java/com/scalar/admin/kubernetes/StatusCheckFailedException.java create mode 100644 lib/src/main/java/com/scalar/admin/kubernetes/UnpauseFailedException.java diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/PauseFailedException.java b/lib/src/main/java/com/scalar/admin/kubernetes/PauseFailedException.java new file mode 100644 index 0000000..8462501 --- /dev/null +++ b/lib/src/main/java/com/scalar/admin/kubernetes/PauseFailedException.java @@ -0,0 +1,11 @@ +package com.scalar.admin.kubernetes; + +public class PauseFailedException extends Exception { + public PauseFailedException(String message) { + super(message); + } + + public PauseFailedException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 7cd2daf..f5290bc 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -10,6 +10,7 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.time.Instant; +import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -39,6 +40,14 @@ public class Pauser { private final Logger logger = LoggerFactory.getLogger(Pauser.class); private final TargetSelector targetSelector; + private Instant startTime; + private Instant endTime; + private PauseFailedException pauseFailedException; + private UnpauseFailedException unpauseFailedException; + private StatusCheckFailedException statusCheckFailedException; + private boolean pauseSuccessful = false; + private boolean unpauseSuccessful = false; + private boolean compareTargetSuccessful = false; /** * @param namespace The namespace where the pods are deployed. @@ -72,55 +81,45 @@ public Pauser(String namespace, String helmReleaseName) throws PauserException { * @return The start and end time of the pause operation. */ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) - throws PauserException { + throws PauserException, UnpauseFailedException, PauseFailedException, + StatusCheckFailedException { if (pauseDuration < 1) { throw new IllegalArgumentException( "pauseDuration is required to be greater than 0 millisecond."); } - TargetSnapshot target; + // Get pods and deployment information before pause. + TargetSnapshot targetBeforePause; try { - target = getTarget(); + targetBeforePause = getTarget(); } catch (Exception e) { throw new PauserException("Failed to find the target pods to pause.", e); } - RequestCoordinator coordinator; + // Get RequestCoordinator of Scalar Admin to pause. + RequestCoordinator requestCoordinator; try { - coordinator = getRequestCoordinator(target); + requestCoordinator = getRequestCoordinator(targetBeforePause); } catch (Exception e) { - throw new PauserException("Failed to initialize the coordinator.", e); + throw new PauserException("Failed to initialize the request coordinator.", e); } - Instant startTime; - Instant endTime; + // Run pause operation. try { - coordinator.pause(true, maxPauseWaitTime); - - startTime = Instant.now(); - - Uninterruptibles.sleepUninterruptibly(pauseDuration, TimeUnit.MILLISECONDS); - - endTime = Instant.now(); - - unpauseWithRetry(coordinator, MAX_UNPAUSE_RETRY_COUNT, target); + pauseSuccessful = pauseInternal(requestCoordinator, pauseDuration, maxPauseWaitTime); + } catch (Exception e) { + pauseFailedException = new PauseFailedException("Pause operation failed.", e); + } + // Run unpause operation. + try { + unpauseSuccessful = + unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetBeforePause); } catch (Exception e) { - try { - unpauseWithRetry(coordinator, MAX_UNPAUSE_RETRY_COUNT, target); - } catch (PauserException ex) { - throw new PauserException("unpauseWithRetry() method failed twice.", e); - } catch (Exception ex) { - throw new PauserException( - "unpauseWithRetry() method failed twice due to unexpected exception.", e); - } - throw new PauserException( - "The pause operation failed for some reason. However, the unpause operation succeeded" - + " afterward. Currently, the scalar products are running with the unpause status." - + " You should retry the pause operation to ensure proper backup.", - e); + unpauseFailedException = new UnpauseFailedException("Unpause operation failed.", e); } + // Get pods and deployment information after pause. TargetSnapshot targetAfterPause; try { targetAfterPause = getTarget(); @@ -131,41 +130,85 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) e); } - if (!target.getStatus().equals(targetAfterPause.getStatus())) { - throw new PauserException("The target pods were updated during paused. Please retry."); + // Check if pods and deployment information are the same between before pause and after pause. + try { + compareTargetSuccessful = compareTargetStates(targetBeforePause, targetAfterPause); + } catch (Exception e) { + statusCheckFailedException = new StatusCheckFailedException("Status check failed.", e); } - return new PausedDuration(startTime, endTime); + // If both the pause operation and status check succeeded, you can use the backup that was taken + // during the pause duration. + boolean backupOk = pauseSuccessful && compareTargetSuccessful; + + // Return the final result based on each process. + if (backupOk) { // Backup OK + if (unpauseSuccessful) { // Backup OK and Unpause OK + return new PausedDuration(startTime, endTime); + } else { // Backup OK but Unpause NG + String errorMessage = + String.format( + "Unpause operation failed. Scalar products might still be in a paused state. You" + + " must restart related pods by using the `kubectl rollout restart deployment" + + " %s` command to unpause all pods. However, the pause operations for taking" + + " backup succeeded. You can use a backup that was taken during this pause" + + " duration: Start Time = %s, End Time = %s.", + Objects.requireNonNull(targetBeforePause.getDeployment().getMetadata()).getName(), + startTime, + endTime); + // Users who directly utilize this library, bypassing our CLI, are responsible for proper + // exception handling. However, this scenario represents a critical issue. Consequently, + // we output the error message here regardless of whether the calling code handles the + // exception. + logger.error(errorMessage); + throw new UnpauseFailedException(errorMessage, unpauseFailedException); + } + } else { // Backup NG + if (unpauseSuccessful) { // Backup NG but Unpause OK + if (!pauseSuccessful) { // Backup NG (Pause operation failed) but Unpause OK + String errorMessage = + String.format( + "Pause operation failed. You cannot use the backup that was taken during this" + + " pause duration. You need to retry the pause operation from the beginning" + + " to take a backup."); + throw new PauseFailedException(errorMessage, pauseFailedException); + } else { // Backup NG (Status check failed) but Unpause OK + String errorMessage = + String.format( + "Status check failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to" + + " take a backup."); + throw new StatusCheckFailedException(errorMessage, statusCheckFailedException); + } + } else { // Backup NG and Unpause NG + String errorMessage = + String.format( + "Pause and unpause operation failed. Scalar products might still be in a paused" + + " state. You must restart related pods by using the `kubectl rollout restart" + + " deployment %s` command to unpause all pods.", + Objects.requireNonNull(targetBeforePause.getDeployment().getMetadata()).getName()); + // Users who directly utilize this library, bypassing our CLI, are responsible for proper + // exception handling. However, this scenario represents a critical issue. Consequently, + // we output the error message here regardless of whether the calling code handles the + // exception. + logger.error(errorMessage); + throw new UnpauseFailedException(errorMessage, unpauseFailedException); + } + } } @VisibleForTesting - void unpauseWithRetry(RequestCoordinator coordinator, int maxRetryCount, TargetSnapshot target) + boolean unpauseWithRetry(RequestCoordinator coordinator, int maxRetryCount, TargetSnapshot target) throws PauserException { int retryCounter = 0; while (true) { try { coordinator.unpause(); - return; + return true; } catch (Exception e) { if (++retryCounter >= maxRetryCount) { - // Users who directly utilize this library, bypassing our CLI, are responsible for proper - // exception handling. However, this scenario represents a critical issue. Consequently, - // we output the error message here regardless of whether the calling code handles the - // exception. - logger.error( - "Failed to unpause Scalar product. They are still in paused. You must restart related" - + " pods by using the `kubectl rollout restart deployment {}`" - + " command to unpause all pods.", - target.getDeployment().getMetadata().getName()); - // In our CLI, we catch this exception and output the message as an error on the CLI side. - throw new PauserException( - String.format( - "Failed to unpause Scalar product. They are still in paused. You must restart" - + " related pods by using the `kubectl rollout restart deployment %s` command" - + " to unpause all pods.", - target.getDeployment().getMetadata().getName()), - e); + throw e; } } } @@ -183,4 +226,19 @@ RequestCoordinator getRequestCoordinator(TargetSnapshot target) { .map(p -> new InetSocketAddress(p.getStatus().getPodIP(), target.getAdminPort())) .collect(Collectors.toList())); } + + private boolean pauseInternal( + RequestCoordinator requestCoordinator, int pauseDuration, @Nullable Long maxPauseWaitTime) { + + requestCoordinator.pause(true, maxPauseWaitTime); + startTime = Instant.now(); + Uninterruptibles.sleepUninterruptibly(pauseDuration, TimeUnit.MILLISECONDS); + endTime = Instant.now(); + + return true; + } + + private boolean compareTargetStates(TargetSnapshot before, TargetSnapshot after) { + return before.getStatus().equals(after.getStatus()); + } } diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/StatusCheckFailedException.java b/lib/src/main/java/com/scalar/admin/kubernetes/StatusCheckFailedException.java new file mode 100644 index 0000000..4890ac7 --- /dev/null +++ b/lib/src/main/java/com/scalar/admin/kubernetes/StatusCheckFailedException.java @@ -0,0 +1,11 @@ +package com.scalar.admin.kubernetes; + +public class StatusCheckFailedException extends Exception { + public StatusCheckFailedException(String message) { + super(message); + } + + public StatusCheckFailedException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/UnpauseFailedException.java b/lib/src/main/java/com/scalar/admin/kubernetes/UnpauseFailedException.java new file mode 100644 index 0000000..a3fef44 --- /dev/null +++ b/lib/src/main/java/com/scalar/admin/kubernetes/UnpauseFailedException.java @@ -0,0 +1,11 @@ +package com.scalar.admin.kubernetes; + +public class UnpauseFailedException extends Exception { + public UnpauseFailedException(String message) { + super(message); + } + + public UnpauseFailedException(String message, Throwable cause) { + super(message, cause); + } +} From 42f1a303bf1b7912264287697e49bc8c34fa32c3 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Fri, 2 May 2025 19:42:27 +0900 Subject: [PATCH 06/21] Update pauser logic based on feedback and discussion --- .../kubernetes/PauseFailedException.java | 2 +- .../com/scalar/admin/kubernetes/Pauser.java | 138 +++++++++--------- .../StatusCheckFailedException.java | 2 +- .../kubernetes/UnpauseFailedException.java | 2 +- 4 files changed, 68 insertions(+), 76 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/PauseFailedException.java b/lib/src/main/java/com/scalar/admin/kubernetes/PauseFailedException.java index 8462501..0641edb 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/PauseFailedException.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/PauseFailedException.java @@ -1,6 +1,6 @@ package com.scalar.admin.kubernetes; -public class PauseFailedException extends Exception { +public class PauseFailedException extends PauserException { public PauseFailedException(String message) { super(message); } diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index f5290bc..0a16724 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -10,7 +10,6 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.time.Instant; -import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -42,12 +41,6 @@ public class Pauser { private final TargetSelector targetSelector; private Instant startTime; private Instant endTime; - private PauseFailedException pauseFailedException; - private UnpauseFailedException unpauseFailedException; - private StatusCheckFailedException statusCheckFailedException; - private boolean pauseSuccessful = false; - private boolean unpauseSuccessful = false; - private boolean compareTargetSuccessful = false; /** * @param namespace The namespace where the pods are deployed. @@ -105,16 +98,17 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) } // Run pause operation. + PauseFailedException pauseFailedException = null; try { - pauseSuccessful = pauseInternal(requestCoordinator, pauseDuration, maxPauseWaitTime); + pauseInternal(requestCoordinator, pauseDuration, maxPauseWaitTime); } catch (Exception e) { pauseFailedException = new PauseFailedException("Pause operation failed.", e); } // Run unpause operation. + UnpauseFailedException unpauseFailedException = null; try { - unpauseSuccessful = - unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetBeforePause); + unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); } catch (Exception e) { unpauseFailedException = new UnpauseFailedException("Unpause operation failed.", e); } @@ -130,82 +124,82 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) e); } + // Prepare error messages for each process. + String unpauseErrorMessage = + String.format( + "Unpause operation failed. Scalar products might still be in a paused state. You" + + " must restart related pods by using the `kubectl rollout restart deployment" + + " %s` command to unpause all pods. ", + targetBeforePause.getDeployment().getMetadata().getName()); + String pauseErrorMessage = + "Pause operation failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to" + + " take a backup. "; + String statusCheckErrorMessage = + "Status check failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to" + + " take a backup. "; + String statusDifferentErrorMessage = + "The target pods were updated during the pause duration. You cannot use the backup that" + + " was taken during this pause duration. "; + String unpauseFailedButBackupOkErrorMessage = + String.format( + "Note that the pause operations for taking backup succeeded. You can use a backup that" + + " was taken during this pause duration: Start Time = %s, End Time = %s. ", + startTime, endTime); + // Check if pods and deployment information are the same between before pause and after pause. + boolean compareTargetSuccessful; try { - compareTargetSuccessful = compareTargetStates(targetBeforePause, targetAfterPause); + compareTargetSuccessful = compareTargetStatus(targetBeforePause, targetAfterPause); } catch (Exception e) { - statusCheckFailedException = new StatusCheckFailedException("Status check failed.", e); + if (unpauseFailedException == null) { + throw new StatusCheckFailedException(statusCheckErrorMessage, e); + } else { + throw new UnpauseFailedException(unpauseErrorMessage, e); + } } // If both the pause operation and status check succeeded, you can use the backup that was taken // during the pause duration. - boolean backupOk = pauseSuccessful && compareTargetSuccessful; + boolean backupOk = (pauseFailedException == null) && compareTargetSuccessful; + + // Create error message if any of the operations failed. + StringBuilder errorMessageBuilder = new StringBuilder(); + if (unpauseFailedException != null) { + errorMessageBuilder.append(unpauseErrorMessage); + if (backupOk) { + errorMessageBuilder.append(unpauseFailedButBackupOkErrorMessage); + } + } + if (pauseFailedException != null) { + errorMessageBuilder.append(pauseErrorMessage); + } + if (!compareTargetSuccessful) { + errorMessageBuilder.append(statusDifferentErrorMessage); + } + String errorMessage = errorMessageBuilder.toString(); // Return the final result based on each process. - if (backupOk) { // Backup OK - if (unpauseSuccessful) { // Backup OK and Unpause OK - return new PausedDuration(startTime, endTime); - } else { // Backup OK but Unpause NG - String errorMessage = - String.format( - "Unpause operation failed. Scalar products might still be in a paused state. You" - + " must restart related pods by using the `kubectl rollout restart deployment" - + " %s` command to unpause all pods. However, the pause operations for taking" - + " backup succeeded. You can use a backup that was taken during this pause" - + " duration: Start Time = %s, End Time = %s.", - Objects.requireNonNull(targetBeforePause.getDeployment().getMetadata()).getName(), - startTime, - endTime); - // Users who directly utilize this library, bypassing our CLI, are responsible for proper - // exception handling. However, this scenario represents a critical issue. Consequently, - // we output the error message here regardless of whether the calling code handles the - // exception. - logger.error(errorMessage); - throw new UnpauseFailedException(errorMessage, unpauseFailedException); - } - } else { // Backup NG - if (unpauseSuccessful) { // Backup NG but Unpause OK - if (!pauseSuccessful) { // Backup NG (Pause operation failed) but Unpause OK - String errorMessage = - String.format( - "Pause operation failed. You cannot use the backup that was taken during this" - + " pause duration. You need to retry the pause operation from the beginning" - + " to take a backup."); - throw new PauseFailedException(errorMessage, pauseFailedException); - } else { // Backup NG (Status check failed) but Unpause OK - String errorMessage = - String.format( - "Status check failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to" - + " take a backup."); - throw new StatusCheckFailedException(errorMessage, statusCheckFailedException); - } - } else { // Backup NG and Unpause NG - String errorMessage = - String.format( - "Pause and unpause operation failed. Scalar products might still be in a paused" - + " state. You must restart related pods by using the `kubectl rollout restart" - + " deployment %s` command to unpause all pods.", - Objects.requireNonNull(targetBeforePause.getDeployment().getMetadata()).getName()); - // Users who directly utilize this library, bypassing our CLI, are responsible for proper - // exception handling. However, this scenario represents a critical issue. Consequently, - // we output the error message here regardless of whether the calling code handles the - // exception. - logger.error(errorMessage); - throw new UnpauseFailedException(errorMessage, unpauseFailedException); - } + if (unpauseFailedException != null) { // Unpause issue is the most critical. + throw new UnpauseFailedException(errorMessage, unpauseFailedException); + } else if (pauseFailedException + != null) { // Pause issue might be caused by configuration error. + throw new PauseFailedException(errorMessage, pauseFailedException); + } else if (!compareTargetSuccessful) { // Status check issue might be caused by temporary issue. + throw new PauseFailedException(errorMessage); + } else { // All operations succeeded. + return new PausedDuration(startTime, endTime); } } @VisibleForTesting - boolean unpauseWithRetry(RequestCoordinator coordinator, int maxRetryCount, TargetSnapshot target) - throws PauserException { + void unpauseWithRetry(RequestCoordinator coordinator, int maxRetryCount) { int retryCounter = 0; - while (true) { try { coordinator.unpause(); - return true; + return; } catch (Exception e) { if (++retryCounter >= maxRetryCount) { throw e; @@ -227,18 +221,16 @@ RequestCoordinator getRequestCoordinator(TargetSnapshot target) { .collect(Collectors.toList())); } - private boolean pauseInternal( + void pauseInternal( RequestCoordinator requestCoordinator, int pauseDuration, @Nullable Long maxPauseWaitTime) { requestCoordinator.pause(true, maxPauseWaitTime); startTime = Instant.now(); Uninterruptibles.sleepUninterruptibly(pauseDuration, TimeUnit.MILLISECONDS); endTime = Instant.now(); - - return true; } - private boolean compareTargetStates(TargetSnapshot before, TargetSnapshot after) { + boolean compareTargetStatus(TargetSnapshot before, TargetSnapshot after) { return before.getStatus().equals(after.getStatus()); } } diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/StatusCheckFailedException.java b/lib/src/main/java/com/scalar/admin/kubernetes/StatusCheckFailedException.java index 4890ac7..c30ed07 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/StatusCheckFailedException.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/StatusCheckFailedException.java @@ -1,6 +1,6 @@ package com.scalar.admin.kubernetes; -public class StatusCheckFailedException extends Exception { +public class StatusCheckFailedException extends PauserException { public StatusCheckFailedException(String message) { super(message); } diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/UnpauseFailedException.java b/lib/src/main/java/com/scalar/admin/kubernetes/UnpauseFailedException.java index a3fef44..bccd72b 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/UnpauseFailedException.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/UnpauseFailedException.java @@ -1,6 +1,6 @@ package com.scalar.admin.kubernetes; -public class UnpauseFailedException extends Exception { +public class UnpauseFailedException extends PauserException { public UnpauseFailedException(String message) { super(message); } From 1d17af978f7980b91f899b3eee0d5f9a57eb0eff Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Wed, 7 May 2025 15:57:31 +0900 Subject: [PATCH 07/21] Update unit tests --- .../com/scalar/admin/kubernetes/Pauser.java | 23 +- .../scalar/admin/kubernetes/PauserTest.java | 498 +++++++++++------- 2 files changed, 331 insertions(+), 190 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 0a16724..9645759 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -14,8 +14,6 @@ import java.util.stream.Collectors; import javax.annotation.Nullable; import javax.annotation.concurrent.NotThreadSafe; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * This class implements a pause operation for Scalar product pods in a Kubernetes cluster. The @@ -37,7 +35,6 @@ public class Pauser { @VisibleForTesting static final int MAX_UNPAUSE_RETRY_COUNT = 3; - private final Logger logger = LoggerFactory.getLogger(Pauser.class); private final TargetSelector targetSelector; private Instant startTime; private Instant endTime; @@ -74,8 +71,7 @@ public Pauser(String namespace, String helmReleaseName) throws PauserException { * @return The start and end time of the pause operation. */ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) - throws PauserException, UnpauseFailedException, PauseFailedException, - StatusCheckFailedException { + throws PauserException { if (pauseDuration < 1) { throw new IllegalArgumentException( "pauseDuration is required to be greater than 0 millisecond."); @@ -150,13 +146,15 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) // Check if pods and deployment information are the same between before pause and after pause. boolean compareTargetSuccessful; + StatusCheckFailedException statusCheckFailedException; try { compareTargetSuccessful = compareTargetStatus(targetBeforePause, targetAfterPause); } catch (Exception e) { + statusCheckFailedException = new StatusCheckFailedException(statusCheckErrorMessage, e); if (unpauseFailedException == null) { - throw new StatusCheckFailedException(statusCheckErrorMessage, e); + throw statusCheckFailedException; } else { - throw new UnpauseFailedException(unpauseErrorMessage, e); + throw new UnpauseFailedException(unpauseErrorMessage, statusCheckFailedException); } } @@ -181,12 +179,15 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) String errorMessage = errorMessageBuilder.toString(); // Return the final result based on each process. - if (unpauseFailedException != null) { // Unpause issue is the most critical. + if (unpauseFailedException + != null) { // Unpause issue is the most critical because it might cause system down. throw new UnpauseFailedException(errorMessage, unpauseFailedException); } else if (pauseFailedException - != null) { // Pause issue might be caused by configuration error. + != null) { // Pause Failed is second priority because pause issue might be caused by + // configuration error. throw new PauseFailedException(errorMessage, pauseFailedException); - } else if (!compareTargetSuccessful) { // Status check issue might be caused by temporary issue. + } else if (!compareTargetSuccessful) { // Status check failed is third priority because this + // issue might be caused by temporary issue, for example, pod restarts. throw new PauseFailedException(errorMessage); } else { // All operations succeeded. return new PausedDuration(startTime, endTime); @@ -221,6 +222,7 @@ RequestCoordinator getRequestCoordinator(TargetSnapshot target) { .collect(Collectors.toList())); } + @VisibleForTesting void pauseInternal( RequestCoordinator requestCoordinator, int pauseDuration, @Nullable Long maxPauseWaitTime) { @@ -230,6 +232,7 @@ void pauseInternal( endTime = Instant.now(); } + @VisibleForTesting boolean compareTargetStatus(TargetSnapshot before, TargetSnapshot after) { return before.getStatus().equals(after.getStatus()); } diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index 9ed9c99..48f94c2 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -25,8 +25,8 @@ class PauserTest { private MockedStatic mockedConfig; private RequestCoordinator requestCoordinator; - private TargetSnapshot targetSnapshot; - private TargetSelector targetSelector; + private TargetSnapshot targetBeforePause; + private TargetSnapshot targetAfterPause; private V1Deployment deployment; private V1ObjectMeta objectMeta; private final String dummyObjectName = "dummyObjectName"; @@ -36,15 +36,15 @@ void beforeEach() throws PauserException { mockedConfig = mockStatic(Config.class); mockedConfig.when(() -> Config.defaultClient()).thenReturn(null); requestCoordinator = mock(RequestCoordinator.class); - targetSnapshot = mock(TargetSnapshot.class); - targetSelector = mock(TargetSelector.class); deployment = mock(V1Deployment.class); objectMeta = mock(V1ObjectMeta.class); + targetBeforePause = mock(TargetSnapshot.class); + targetAfterPause = mock(TargetSnapshot.class); - when(targetSnapshot.getDeployment()).thenReturn(deployment); - when(deployment.getMetadata()).thenReturn(objectMeta); - when(objectMeta.getName()).thenReturn(dummyObjectName); - when(targetSelector.select()).thenReturn(targetSnapshot); + doReturn(deployment).when(targetBeforePause).getDeployment(); + doReturn(deployment).when(targetAfterPause).getDeployment(); + doReturn(objectMeta).when(deployment).getMetadata(); + doReturn(dummyObjectName).when(objectMeta).getName(); } @AfterEach @@ -53,9 +53,9 @@ void afterEach() { } @Nested - class ConstructorPauser { + class Constructor { @Test - void WithCorrectArgsReturnPauser() throws PauserException, IOException { + void constructor_WithCorrectArgs_ReturnPauser() throws PauserException, IOException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; @@ -65,7 +65,7 @@ void WithCorrectArgsReturnPauser() throws PauserException, IOException { } @Test - void WithNullNamespaceShouldThrowException() { + void constructor_WithNullNamespace_ShouldThrowIllegalArgumentException() { // Arrange String namespace = null; String helmReleaseName = "dummyRelease"; @@ -78,7 +78,7 @@ void WithNullNamespaceShouldThrowException() { } @Test - void WithNullHelmReleaseNameShouldThrowException() { + void constructor_WithNullHelmReleaseName_ShouldThrowIllegalArgumentException() { // Arrange String namespace = "dummyNs"; String helmReleaseName = null; @@ -91,7 +91,7 @@ void WithNullHelmReleaseNameShouldThrowException() { } @Test - void WhenSetDefaultApiClientThrowIOExceptionShouldThrowException() { + void constructor_WhenSetDefaultApiClientThrowIOException_ShouldThrowPauserException() { // Arrange mockedConfig.when(() -> Config.defaultClient()).thenThrow(IOException.class); String namespace = "dummyNs"; @@ -105,9 +105,55 @@ void WhenSetDefaultApiClientThrowIOExceptionShouldThrowException() { } @Nested - class MethodPause { + class Pause { + @Test - void LessThanOnePauseDurationShouldThrowException() throws PauserException { + void pause_WhenPauseSucceeded_ReturnPausedDuration() throws PauserException { + Map podRestartCounts = + new HashMap() { + { + put("dummyKey", 1); + } + }; + Map podResourceVersions = + new HashMap() { + { + put("dummyKey", "dummyValue"); + } + }; + TargetStatus beforeTargetStatus = + new TargetStatus(podRestartCounts, podResourceVersions, "sameValue"); + TargetStatus afterTargetStatus = + new TargetStatus(podRestartCounts, podResourceVersions, "sameValue"); + doReturn(beforeTargetStatus).when(targetBeforePause).getStatus(); + doReturn(afterTargetStatus).when(targetAfterPause).getStatus(); + + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + MockedStatic mockedTime = mockStatic(Instant.class); + mockedTime.when(() -> Instant.now()).thenReturn(startTime).thenReturn(endTime); + + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(requestCoordinator).pause(true, null); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + + // Act & Assert + PausedDuration actual = assertDoesNotThrow(() -> pauser.pause(pauseDuration, 3000L)); + PausedDuration expected = new PausedDuration(startTime, endTime); + assertEquals(actual.getStartTime(), expected.getStartTime()); + assertEquals(actual.getEndTime(), expected.getEndTime()); + + mockedTime.close(); + } + + @Test + void pause_LessThanOnePauseDuration_ShouldThrowIllegalArgumentException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; @@ -122,13 +168,14 @@ void LessThanOnePauseDurationShouldThrowException() throws PauserException { } @Test - void WhenFirstTargetSelectorThrowExceptionShouldThrowException() throws PauserException { + void pause_WhenFirstGetTargetThrowException_ShouldThrowPauserException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; - Pauser pauser = new Pauser(namespace, helmReleaseName); - when(targetSelector.select()).thenThrow(RuntimeException.class); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doThrow(RuntimeException.class).when(pauser).getTarget(); // Act & Assert PauserException thrown = @@ -137,50 +184,75 @@ void WhenFirstTargetSelectorThrowExceptionShouldThrowException() throws PauserEx } @Test - void WhenGetRequestCoordinatorThrowExceptionShouldThrowException() throws PauserException { + void pause_WhenGetRequestCoordinatorThrowException_ShouldThrowPauserException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - doReturn(targetSnapshot).when(pauser).getTarget(); - doThrow(RuntimeException.class).when(pauser).getRequestCoordinator(targetSnapshot); + doReturn(targetBeforePause).when(pauser).getTarget(); + doThrow(RuntimeException.class).when(pauser).getRequestCoordinator(targetBeforePause); // Act & Assert PauserException thrown = assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals("Failed to initialize the coordinator.", thrown.getMessage()); + assertEquals("Failed to initialize the request coordinator.", thrown.getMessage()); + } + + @Test + void pause_WhenPauseInternalThrowException_ShouldThrowPauseFailedException() + throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doThrow(RuntimeException.class) + .when(pauser) + .pauseInternal(requestCoordinator, pauseDuration, null); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + doReturn(true).when(pauser).compareTargetStatus(any(), any()); + + // Act & Assert + PauseFailedException thrown = + assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "Pause operation failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to take a" + + " backup. ", + thrown.getMessage()); } @Test - void WhenCoordinatorPauseThrowExceptionShouldRunUnpause() throws PauserException { + void pause_WhenRequestCoordinatorPauseThrowException_ShouldThrowPauseFailedException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - doReturn(targetSnapshot).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doThrow(RuntimeException.class).when(requestCoordinator).pause(true, null); - doNothing().when(requestCoordinator).unpause(); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + doReturn(true).when(pauser).compareTargetStatus(any(), any()); // Act & Assert - PauserException thrown = - assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + PauseFailedException thrown = + assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - "The pause operation failed for some reason. However, the unpause operation succeeded" - + " afterward. Currently, the scalar products are running with the unpause status." - + " You should retry the pause operation to ensure proper backup.", + "Pause operation failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to take a" + + " backup. ", thrown.getMessage()); - verify(pauser, times(1)) - .unpauseWithRetry( - any(RequestCoordinator.class), - eq(MAX_UNPAUSE_RETRY_COUNT), - any(TargetSnapshot.class)); } @Test - void WhenInstantNowThrowExceptionShouldRunUnpause() throws PauserException { + void pause_WhenInstantNowThrowException_ShouldThrowPauseFailedException() + throws PauserException { // Arrange MockedStatic mockedTime = mockStatic(Instant.class); mockedTime.when(() -> Instant.now()).thenThrow(RuntimeException.class); @@ -188,30 +260,25 @@ void WhenInstantNowThrowExceptionShouldRunUnpause() throws PauserException { String helmReleaseName = "dummyRelease"; int pauseDuration = 1; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - doReturn(targetSnapshot).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).pause(true, null); - doNothing().when(requestCoordinator).unpause(); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + doReturn(true).when(pauser).compareTargetStatus(any(), any()); // Act & Assert - PauserException thrown = - assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + PauseFailedException thrown = + assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - "The pause operation failed for some reason. However, the unpause operation succeeded" - + " afterward. Currently, the scalar products are running with the unpause status." - + " You should retry the pause operation to ensure proper backup.", + "Pause operation failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to take a" + + " backup. ", thrown.getMessage()); - verify(pauser, times(1)) - .unpauseWithRetry( - any(RequestCoordinator.class), - eq(MAX_UNPAUSE_RETRY_COUNT), - any(TargetSnapshot.class)); mockedTime.close(); } @Test - void WhenSleepThrowExceptionShouldRunUnpause() throws PauserException { + void pause_WhenSleepThrowException_ShouldThrowPauseFailedException() throws PauserException { // Arrange int pauseDuration = 1; MockedStatic mockedSleep = mockStatic(Uninterruptibles.class); @@ -221,138 +288,145 @@ void WhenSleepThrowExceptionShouldRunUnpause() throws PauserException { String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - doReturn(targetSnapshot).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).pause(true, null); - doNothing().when(requestCoordinator).unpause(); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + doReturn(true).when(pauser).compareTargetStatus(any(), any()); // Act & Assert - PauserException thrown = - assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + PauseFailedException thrown = + assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - "The pause operation failed for some reason. However, the unpause operation succeeded" - + " afterward. Currently, the scalar products are running with the unpause status." - + " You should retry the pause operation to ensure proper backup.", + "Pause operation failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to take a" + + " backup. ", thrown.getMessage()); - verify(pauser, times(1)) - .unpauseWithRetry( - any(RequestCoordinator.class), - eq(MAX_UNPAUSE_RETRY_COUNT), - any(TargetSnapshot.class)); mockedSleep.close(); } @Test - void WhenUnpauseThrowExceptionShouldRunUnpauseAgain() throws PauserException { + void pause_WhenUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; + + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + MockedStatic mockedTime = mockStatic(Instant.class); + mockedTime.when(() -> Instant.now()).thenReturn(startTime).thenReturn(endTime); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - doReturn(targetSnapshot).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).pause(true, null); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); doThrow(RuntimeException.class) - .doNothing() .when(pauser) - .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot); + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); + doReturn(true).when(pauser).compareTargetStatus(any(), any()); // Act & Assert - PauserException thrown = - assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + UnpauseFailedException thrown = + assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - "The pause operation failed for some reason. However, the unpause operation succeeded" - + " afterward. Currently, the scalar products are running with the unpause status." - + " You should retry the pause operation to ensure proper backup.", + String.format( + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment %s`" + + " command to unpause all pods. Note that the pause operations for taking backup" + + " succeeded. You can use a backup that was taken during this pause duration:" + + " Start Time = %s, End Time = %s. ", + dummyObjectName, startTime, endTime), thrown.getMessage()); - verify(pauser, times(2)) - .unpauseWithRetry( - any(RequestCoordinator.class), - eq(MAX_UNPAUSE_RETRY_COUNT), - any(TargetSnapshot.class)); + + mockedTime.close(); } @Test - void WhenUnpauseThrowPauserExceptionTwiceShouldRunUnpauseAgain() throws PauserException { + void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - doReturn(targetSnapshot).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).pause(true, null); - doThrow(PauserException.class) - .doThrow(PauserException.class) - .when(pauser) - .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot); + doReturn(targetBeforePause).doThrow(RuntimeException.class).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); // Act & Assert PauserException thrown = assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals("unpauseWithRetry() method failed twice.", thrown.getMessage()); - verify(pauser, times(2)) - .unpauseWithRetry( - any(RequestCoordinator.class), - eq(MAX_UNPAUSE_RETRY_COUNT), - any(TargetSnapshot.class)); + assertEquals( + "Failed to find the target pods to examine if the targets pods were updated during" + + " paused.", + thrown.getMessage()); } @Test - void WhenUnpauseThrowUnexpectedExceptionTwiceShouldRunUnpauseAgain() throws PauserException { + void pause_WhenCompareTargetStatusThrowException_ShouldThrowStatusCheckFailedException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - doReturn(targetSnapshot).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).pause(true, null); - doThrow(RuntimeException.class) - .doThrow(RuntimeException.class) - .when(pauser) - .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + doThrow(RuntimeException.class).when(pauser).compareTargetStatus(any(), any()); // Act & Assert - PauserException thrown = - assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + StatusCheckFailedException thrown = + assertThrows(StatusCheckFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - "unpauseWithRetry() method failed twice due to unexpected exception.", + "Status check failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to take a" + + " backup. ", thrown.getMessage()); - verify(pauser, times(2)) - .unpauseWithRetry( - any(RequestCoordinator.class), - eq(MAX_UNPAUSE_RETRY_COUNT), - any(TargetSnapshot.class)); } @Test - void WhenSecondTargetSelectorThrowExceptionShouldThrowException() throws PauserException { + void + pause_WhenCompareTargetStatusAndUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - doReturn(targetSnapshot).doThrow(RuntimeException.class).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).pause(true, null); - doNothing().when(requestCoordinator).unpause(); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doThrow(RuntimeException.class) + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); + doThrow(RuntimeException.class).when(pauser).compareTargetStatus(any(), any()); // Act & Assert - PauserException thrown = - assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + UnpauseFailedException thrown = + assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - "Failed to find the target pods to examine if the targets pods were updated during" - + " paused.", + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment" + + " dummyObjectName` command to unpause all pods. ", thrown.getMessage()); } @Test - void WhenTargetPodStatusChangedShouldThrowException() throws PauserException { + void pause_WhenTargetPodStatusChanged_ShouldThrowStatusCheckFailedException() + throws PauserException { // Arrange - TargetSnapshot targetBeforePause = mock(TargetSnapshot.class); - TargetSnapshot targetAfterPause = mock(TargetSnapshot.class); + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + Map podRestartCounts = new HashMap() { { @@ -371,73 +445,147 @@ void WhenTargetPodStatusChangedShouldThrowException() throws PauserException { new TargetStatus(podRestartCounts, podResourceVersions, "afterDifferentValue"); doReturn(beforeTargetStatus).when(targetBeforePause).getStatus(); doReturn(afterTargetStatus).when(targetAfterPause).getStatus(); + + doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + + // Act & Assert + PauseFailedException thrown = + assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "The target pods were updated during the pause duration. You cannot use the backup that" + + " was taken during this pause duration. ", + thrown.getMessage()); + } + + @Test + void pause_WhenPauseAndUnpauseFailed_ShouldThrowUnpauseFailedException() + throws PauserException { + // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).pause(true, null); - doNothing().when(requestCoordinator).unpause(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doThrow(RuntimeException.class) + .when(pauser) + .pauseInternal(requestCoordinator, pauseDuration, null); + doThrow(RuntimeException.class) + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); + doReturn(true).when(pauser).compareTargetStatus(any(), any()); // Act & Assert - PauserException thrown = - assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); + UnpauseFailedException thrown = + assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - "The target pods were updated during paused. Please retry.", thrown.getMessage()); + String.format( + "Unpause operation failed. Scalar products might still be in a paused state. You" + + " must restart related pods by using the `kubectl rollout restart deployment" + + " %s` command to unpause all pods. Pause operation failed. You cannot use the" + + " backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to" + + " take a backup. ", + dummyObjectName), + thrown.getMessage()); } @Test - void WhenPauseSucceededReturnPausedDuration() throws PauserException { - TargetSnapshot targetBeforePause = mock(TargetSnapshot.class); - TargetSnapshot targetAfterPause = mock(TargetSnapshot.class); - Map podRestartCounts = - new HashMap() { - { - put("dummyKey", 1); - } - }; - Map podResourceVersions = - new HashMap() { - { - put("dummyKey", "dummyValue"); - } - }; - TargetStatus beforeTargetStatus = - new TargetStatus(podRestartCounts, podResourceVersions, "sameValue"); - TargetStatus afterTargetStatus = - new TargetStatus(podRestartCounts, podResourceVersions, "sameValue"); - doReturn(beforeTargetStatus).when(targetBeforePause).getStatus(); - doReturn(afterTargetStatus).when(targetAfterPause).getStatus(); + void pause_WhenUnpauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedException() + throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doThrow(RuntimeException.class) + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); + doReturn(false).when(pauser).compareTargetStatus(any(), any()); - Instant startTime = Instant.now().minus(5, SECONDS); - Instant endTime = Instant.now().plus(5, SECONDS); - MockedStatic mockedTime = mockStatic(Instant.class); - mockedTime.when(() -> Instant.now()).thenReturn(startTime).thenReturn(endTime); + // Act & Assert + UnpauseFailedException thrown = + assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + String.format( + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment %s`" + + " command to unpause all pods. The target pods were updated during the pause" + + " duration. You cannot use the backup that was taken during this pause" + + " duration. ", + dummyObjectName), + thrown.getMessage()); + } + @Test + void pause_WhenPauseAndUnpauseAndTargetPodStatusChanged_ShouldThrowUnpauseFailedException() + throws PauserException { + // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); - doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetSnapshot); - doNothing().when(requestCoordinator).pause(true, null); - doNothing().when(requestCoordinator).unpause(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doThrow(RuntimeException.class) + .when(pauser) + .pauseInternal(requestCoordinator, pauseDuration, null); + doThrow(RuntimeException.class) + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); + doReturn(false).when(pauser).compareTargetStatus(any(), any()); // Act & Assert - PausedDuration actual = assertDoesNotThrow(() -> pauser.pause(pauseDuration, 3000L)); - PausedDuration expected = new PausedDuration(startTime, endTime); - assertEquals(actual.getStartTime(), expected.getStartTime()); - assertEquals(actual.getEndTime(), expected.getEndTime()); + UnpauseFailedException thrown = + assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + String.format( + "Unpause operation failed. Scalar products might still be in a paused state. You" + + " must restart related pods by using the `kubectl rollout restart deployment" + + " %s` command to unpause all pods. Pause operation failed. You cannot use the" + + " backup that was taken during this pause duration. You need to retry the pause" + + " operation from the beginning to take a backup. The target pods were updated" + + " during the pause duration. You cannot use the backup that was taken during" + + " this pause duration. ", + dummyObjectName), + thrown.getMessage()); + } - mockedTime.close(); + @Test + void pause_WhenPauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedException() + throws PauserException { // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doThrow(RuntimeException.class) + .when(pauser) + .pauseInternal(requestCoordinator, pauseDuration, null); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + doReturn(false).when(pauser).compareTargetStatus(any(), any()); + + // Act & Assert + PauseFailedException thrown = + assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + "Pause operation failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to take a" + + " backup. The target pods were updated during the pause duration. You cannot use" + + " the backup that was taken during this pause duration. ", + thrown.getMessage()); } } @Nested - class MethodUnpauseWithRetry { + class UnpauseWithRetry { @Test - void WhenUnpauseSucceededReturnWithoutException() throws PauserException { + void unpauseWithRetry_WhenUnpauseSucceeded_ReturnWithoutException() throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; @@ -446,12 +594,11 @@ void WhenUnpauseSucceededReturnWithoutException() throws PauserException { // Act & Assert assertDoesNotThrow( - () -> - pauser.unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot)); + () -> pauser.unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT)); } @Test - void WhenExceptionOccurShouldRetryThreeTimes() throws PauserException { + void unpauseWithRetry_WhenExceptionOccur_ShouldRetryThreeTimes() throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; @@ -459,19 +606,10 @@ void WhenExceptionOccurShouldRetryThreeTimes() throws PauserException { doThrow(RuntimeException.class).when(requestCoordinator).unpause(); // Act & Assert - PauserException thrown = + RuntimeException thrown = assertThrows( - PauserException.class, - () -> - pauser.unpauseWithRetry( - requestCoordinator, MAX_UNPAUSE_RETRY_COUNT, targetSnapshot)); - assertEquals( - "Failed to unpause Scalar product. They are still in paused. You must restart related" - + " pods by using the `kubectl rollout restart deployment " - + dummyObjectName - + "` command to" - + " unpause all pods.", - thrown.getMessage()); + RuntimeException.class, + () -> pauser.unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT)); verify(requestCoordinator, times(MAX_UNPAUSE_RETRY_COUNT)).unpause(); } } From f0870772cceb576f2df988d1f7acb870fb83d847 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Wed, 7 May 2025 16:09:04 +0900 Subject: [PATCH 08/21] Update dummy object name variable to static constant --- .../scalar/admin/kubernetes/PauserTest.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index 48f94c2..66ae07c 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -29,7 +29,7 @@ class PauserTest { private TargetSnapshot targetAfterPause; private V1Deployment deployment; private V1ObjectMeta objectMeta; - private final String dummyObjectName = "dummyObjectName"; + private static final String DUMMY_OBJECT_NAME = "dummyObjectName"; @BeforeEach void beforeEach() throws PauserException { @@ -44,7 +44,7 @@ void beforeEach() throws PauserException { doReturn(deployment).when(targetBeforePause).getDeployment(); doReturn(deployment).when(targetAfterPause).getDeployment(); doReturn(objectMeta).when(deployment).getMetadata(); - doReturn(dummyObjectName).when(objectMeta).getName(); + doReturn(DUMMY_OBJECT_NAME).when(objectMeta).getName(); } @AfterEach @@ -337,7 +337,7 @@ void pause_WhenUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException( + " command to unpause all pods. Note that the pause operations for taking backup" + " succeeded. You can use a backup that was taken during this pause duration:" + " Start Time = %s, End Time = %s. ", - dummyObjectName, startTime, endTime), + DUMMY_OBJECT_NAME, startTime, endTime), thrown.getMessage()); mockedTime.close(); @@ -410,9 +410,11 @@ void pause_WhenCompareTargetStatusThrowException_ShouldThrowStatusCheckFailedExc UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment" - + " dummyObjectName` command to unpause all pods. ", + String.format( + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment" + + " %s` command to unpause all pods. ", + DUMMY_OBJECT_NAME), thrown.getMessage()); } @@ -487,7 +489,7 @@ void pause_WhenPauseAndUnpauseFailed_ShouldThrowUnpauseFailedException() + " backup that was taken during this pause" + " duration. You need to retry the pause operation from the beginning to" + " take a backup. ", - dummyObjectName), + DUMMY_OBJECT_NAME), thrown.getMessage()); } @@ -517,7 +519,7 @@ void pause_WhenUnpauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedEx + " command to unpause all pods. The target pods were updated during the pause" + " duration. You cannot use the backup that was taken during this pause" + " duration. ", - dummyObjectName), + DUMMY_OBJECT_NAME), thrown.getMessage()); } @@ -551,7 +553,7 @@ void pause_WhenPauseAndUnpauseAndTargetPodStatusChanged_ShouldThrowUnpauseFailed + " operation from the beginning to take a backup. The target pods were updated" + " during the pause duration. You cannot use the backup that was taken during" + " this pause duration. ", - dummyObjectName), + DUMMY_OBJECT_NAME), thrown.getMessage()); } From d3b54c092b47410dfc9383424f9b7b608df1df7e Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Thu, 8 May 2025 09:27:32 +0900 Subject: [PATCH 09/21] Add exception handling for getTarget() after pause --- .../com/scalar/admin/kubernetes/Pauser.java | 28 ++++++++++------- .../scalar/admin/kubernetes/PauserTest.java | 30 ++++++++++++++++++- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 9645759..ff67175 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -109,17 +109,6 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) unpauseFailedException = new UnpauseFailedException("Unpause operation failed.", e); } - // Get pods and deployment information after pause. - TargetSnapshot targetAfterPause; - try { - targetAfterPause = getTarget(); - } catch (Exception e) { - throw new PauserException( - "Failed to find the target pods to examine if the targets pods were updated during" - + " paused.", - e); - } - // Prepare error messages for each process. String unpauseErrorMessage = String.format( @@ -131,6 +120,9 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) "Pause operation failed. You cannot use the backup that was taken during this pause" + " duration. You need to retry the pause operation from the beginning to" + " take a backup. "; + String getTargetAfterPauseErrorMessage = + "Failed to find the target pods to examine if the targets pods were updated during" + + " paused. "; String statusCheckErrorMessage = "Status check failed. You cannot use the backup that was taken during this pause" + " duration. You need to retry the pause operation from the beginning to" @@ -144,6 +136,20 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) + " was taken during this pause duration: Start Time = %s, End Time = %s. ", startTime, endTime); + // Get pods and deployment information after pause. + TargetSnapshot targetAfterPause; + PauserException pauserException; + try { + targetAfterPause = getTarget(); + } catch (Exception e) { + pauserException = new PauserException(getTargetAfterPauseErrorMessage, e); + if (unpauseFailedException == null) { + throw pauserException; + } else { + throw new UnpauseFailedException(unpauseErrorMessage, pauserException); + } + } + // Check if pods and deployment information are the same between before pause and after pause. boolean compareTargetSuccessful; StatusCheckFailedException statusCheckFailedException; diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index 66ae07c..ed7ad6d 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -361,7 +361,35 @@ void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( "Failed to find the target pods to examine if the targets pods were updated during" - + " paused.", + + " paused. ", + thrown.getMessage()); + } + + @Test + void + pause_WhenSecondGetTargetAndUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException() + throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + int pauseDuration = 1; + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + doReturn(targetBeforePause).doThrow(RuntimeException.class).when(pauser).getTarget(); + doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doThrow(RuntimeException.class) + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); + + // Act & Assert + UnpauseFailedException thrown = + assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals( + String.format( + "Unpause operation failed. Scalar products might still be in a paused state. You" + + " must restart related pods by using the `kubectl rollout restart deployment" + + " %s` command to unpause all pods. ", + DUMMY_OBJECT_NAME), thrown.getMessage()); } From 55ba29c9514ca8a4940fb0f352f1a04dc497ae62 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Thu, 8 May 2025 13:17:33 +0900 Subject: [PATCH 10/21] Apply suggestions from code review Co-authored-by: Hiroyuki Yamada --- lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index ff67175..246c66c 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -93,7 +93,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) throw new PauserException("Failed to initialize the request coordinator.", e); } - // Run pause operation. + // Run a pause operation. PauseFailedException pauseFailedException = null; try { pauseInternal(requestCoordinator, pauseDuration, maxPauseWaitTime); @@ -101,7 +101,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) pauseFailedException = new PauseFailedException("Pause operation failed.", e); } - // Run unpause operation. + // Run an unpause operation. UnpauseFailedException unpauseFailedException = null; try { unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); @@ -168,7 +168,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) // during the pause duration. boolean backupOk = (pauseFailedException == null) && compareTargetSuccessful; - // Create error message if any of the operations failed. + // Create an error message if any of the operations failed. StringBuilder errorMessageBuilder = new StringBuilder(); if (unpauseFailedException != null) { errorMessageBuilder.append(unpauseErrorMessage); From 891beb85703faec5f8d82c18fb1892809d287a63 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Thu, 8 May 2025 14:07:51 +0900 Subject: [PATCH 11/21] Change variable name from backupOk to isPauseOk --- lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 246c66c..8ca8156 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -130,7 +130,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) String statusDifferentErrorMessage = "The target pods were updated during the pause duration. You cannot use the backup that" + " was taken during this pause duration. "; - String unpauseFailedButBackupOkErrorMessage = + String unpauseFailedButPauseOkErrorMessage = String.format( "Note that the pause operations for taking backup succeeded. You can use a backup that" + " was taken during this pause duration: Start Time = %s, End Time = %s. ", @@ -166,14 +166,14 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) // If both the pause operation and status check succeeded, you can use the backup that was taken // during the pause duration. - boolean backupOk = (pauseFailedException == null) && compareTargetSuccessful; + boolean isPauseOk = (pauseFailedException == null) && compareTargetSuccessful; // Create an error message if any of the operations failed. StringBuilder errorMessageBuilder = new StringBuilder(); if (unpauseFailedException != null) { errorMessageBuilder.append(unpauseErrorMessage); - if (backupOk) { - errorMessageBuilder.append(unpauseFailedButBackupOkErrorMessage); + if (isPauseOk) { + errorMessageBuilder.append(unpauseFailedButPauseOkErrorMessage); } } if (pauseFailedException != null) { From c233242455c9538d9605fb23bba28176b73a1e99 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Thu, 8 May 2025 14:12:02 +0900 Subject: [PATCH 12/21] Update name of target status variable and method --- .../com/scalar/admin/kubernetes/Pauser.java | 12 +++++----- .../scalar/admin/kubernetes/PauserTest.java | 24 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 8ca8156..c91119f 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -151,10 +151,10 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) } // Check if pods and deployment information are the same between before pause and after pause. - boolean compareTargetSuccessful; + boolean isTargetStatusEqual; StatusCheckFailedException statusCheckFailedException; try { - compareTargetSuccessful = compareTargetStatus(targetBeforePause, targetAfterPause); + isTargetStatusEqual = targetStatusEquals(targetBeforePause, targetAfterPause); } catch (Exception e) { statusCheckFailedException = new StatusCheckFailedException(statusCheckErrorMessage, e); if (unpauseFailedException == null) { @@ -166,7 +166,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) // If both the pause operation and status check succeeded, you can use the backup that was taken // during the pause duration. - boolean isPauseOk = (pauseFailedException == null) && compareTargetSuccessful; + boolean isPauseOk = (pauseFailedException == null) && isTargetStatusEqual; // Create an error message if any of the operations failed. StringBuilder errorMessageBuilder = new StringBuilder(); @@ -179,7 +179,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) if (pauseFailedException != null) { errorMessageBuilder.append(pauseErrorMessage); } - if (!compareTargetSuccessful) { + if (!isTargetStatusEqual) { errorMessageBuilder.append(statusDifferentErrorMessage); } String errorMessage = errorMessageBuilder.toString(); @@ -192,7 +192,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) != null) { // Pause Failed is second priority because pause issue might be caused by // configuration error. throw new PauseFailedException(errorMessage, pauseFailedException); - } else if (!compareTargetSuccessful) { // Status check failed is third priority because this + } else if (!isTargetStatusEqual) { // Status check failed is third priority because this // issue might be caused by temporary issue, for example, pod restarts. throw new PauseFailedException(errorMessage); } else { // All operations succeeded. @@ -239,7 +239,7 @@ void pauseInternal( } @VisibleForTesting - boolean compareTargetStatus(TargetSnapshot before, TargetSnapshot after) { + boolean targetStatusEquals(TargetSnapshot before, TargetSnapshot after) { return before.getStatus().equals(after.getStatus()); } } diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index ed7ad6d..5be8f0d 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -214,7 +214,7 @@ void pause_WhenPauseInternalThrowException_ShouldThrowPauseFailedException() .when(pauser) .pauseInternal(requestCoordinator, pauseDuration, null); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(true).when(pauser).compareTargetStatus(any(), any()); + doReturn(true).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = @@ -238,7 +238,7 @@ void pause_WhenRequestCoordinatorPauseThrowException_ShouldThrowPauseFailedExcep doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doThrow(RuntimeException.class).when(requestCoordinator).pause(true, null); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(true).when(pauser).compareTargetStatus(any(), any()); + doReturn(true).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = @@ -263,7 +263,7 @@ void pause_WhenInstantNowThrowException_ShouldThrowPauseFailedException() doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(true).when(pauser).compareTargetStatus(any(), any()); + doReturn(true).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = @@ -291,7 +291,7 @@ void pause_WhenSleepThrowException_ShouldThrowPauseFailedException() throws Paus doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(true).when(pauser).compareTargetStatus(any(), any()); + doReturn(true).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = @@ -325,7 +325,7 @@ void pause_WhenUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException( doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doReturn(true).when(pauser).compareTargetStatus(any(), any()); + doReturn(true).when(pauser).targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = @@ -394,7 +394,7 @@ void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() } @Test - void pause_WhenCompareTargetStatusThrowException_ShouldThrowStatusCheckFailedException() + void pause_WhenTargetStatusEqualsThrowException_ShouldThrowStatusCheckFailedException() throws PauserException { // Arrange String namespace = "dummyNs"; @@ -405,7 +405,7 @@ void pause_WhenCompareTargetStatusThrowException_ShouldThrowStatusCheckFailedExc doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doThrow(RuntimeException.class).when(pauser).compareTargetStatus(any(), any()); + doThrow(RuntimeException.class).when(pauser).targetStatusEquals(any(), any()); // Act & Assert StatusCheckFailedException thrown = @@ -432,7 +432,7 @@ void pause_WhenCompareTargetStatusThrowException_ShouldThrowStatusCheckFailedExc doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doThrow(RuntimeException.class).when(pauser).compareTargetStatus(any(), any()); + doThrow(RuntimeException.class).when(pauser).targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = @@ -504,7 +504,7 @@ void pause_WhenPauseAndUnpauseFailed_ShouldThrowUnpauseFailedException() doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doReturn(true).when(pauser).compareTargetStatus(any(), any()); + doReturn(true).when(pauser).targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = @@ -535,7 +535,7 @@ void pause_WhenUnpauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedEx doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doReturn(false).when(pauser).compareTargetStatus(any(), any()); + doReturn(false).when(pauser).targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = @@ -567,7 +567,7 @@ void pause_WhenPauseAndUnpauseAndTargetPodStatusChanged_ShouldThrowUnpauseFailed doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doReturn(false).when(pauser).compareTargetStatus(any(), any()); + doReturn(false).when(pauser).targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = @@ -598,7 +598,7 @@ void pause_WhenPauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedExce .when(pauser) .pauseInternal(requestCoordinator, pauseDuration, null); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(false).when(pauser).compareTargetStatus(any(), any()); + doReturn(false).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = From 744da27f63bfaad0aec89d157cd7146ec851c552 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Thu, 8 May 2025 16:01:43 +0900 Subject: [PATCH 13/21] Return PausedDuration from pauseInternal() method --- .../com/scalar/admin/kubernetes/Pauser.java | 21 ++++---- .../scalar/admin/kubernetes/PauserTest.java | 54 ++++++++++++++----- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index c91119f..d8e8287 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -36,8 +36,6 @@ public class Pauser { @VisibleForTesting static final int MAX_UNPAUSE_RETRY_COUNT = 3; private final TargetSelector targetSelector; - private Instant startTime; - private Instant endTime; /** * @param namespace The namespace where the pods are deployed. @@ -94,9 +92,10 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) } // Run a pause operation. + PausedDuration pausedDuration = null; PauseFailedException pauseFailedException = null; try { - pauseInternal(requestCoordinator, pauseDuration, maxPauseWaitTime); + pausedDuration = pauseInternal(requestCoordinator, pauseDuration, maxPauseWaitTime); } catch (Exception e) { pauseFailedException = new PauseFailedException("Pause operation failed.", e); } @@ -131,10 +130,8 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) "The target pods were updated during the pause duration. You cannot use the backup that" + " was taken during this pause duration. "; String unpauseFailedButPauseOkErrorMessage = - String.format( - "Note that the pause operations for taking backup succeeded. You can use a backup that" - + " was taken during this pause duration: Start Time = %s, End Time = %s. ", - startTime, endTime); + "Note that the pause operations for taking backup succeeded. You can use a backup that" + + " was taken during this pause duration. "; // Get pods and deployment information after pause. TargetSnapshot targetAfterPause; @@ -196,7 +193,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) // issue might be caused by temporary issue, for example, pod restarts. throw new PauseFailedException(errorMessage); } else { // All operations succeeded. - return new PausedDuration(startTime, endTime); + return pausedDuration; } } @@ -229,13 +226,13 @@ RequestCoordinator getRequestCoordinator(TargetSnapshot target) { } @VisibleForTesting - void pauseInternal( + PausedDuration pauseInternal( RequestCoordinator requestCoordinator, int pauseDuration, @Nullable Long maxPauseWaitTime) { - requestCoordinator.pause(true, maxPauseWaitTime); - startTime = Instant.now(); + Instant startTime = Instant.now(); Uninterruptibles.sleepUninterruptibly(pauseDuration, TimeUnit.MILLISECONDS); - endTime = Instant.now(); + Instant endTime = Instant.now(); + return new PausedDuration(startTime, endTime); } @VisibleForTesting diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index 5be8f0d..16965f6 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -312,16 +312,15 @@ void pause_WhenUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException( String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; - Instant startTime = Instant.now().minus(5, SECONDS); Instant endTime = Instant.now().plus(5, SECONDS); - MockedStatic mockedTime = mockStatic(Instant.class); - mockedTime.when(() -> Instant.now()).thenReturn(startTime).thenReturn(endTime); Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + PausedDuration pausedDuration = new PausedDuration(startTime, endTime); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); @@ -335,12 +334,9 @@ void pause_WhenUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException( "Unpause operation failed. Scalar products might still be in a paused state. You must" + " restart related pods by using the `kubectl rollout restart deployment %s`" + " command to unpause all pods. Note that the pause operations for taking backup" - + " succeeded. You can use a backup that was taken during this pause duration:" - + " Start Time = %s, End Time = %s. ", + + " succeeded. You can use a backup that was taken during this pause duration. ", DUMMY_OBJECT_NAME, startTime, endTime), thrown.getMessage()); - - mockedTime.close(); } @Test @@ -350,10 +346,15 @@ void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + PausedDuration pausedDuration = new PausedDuration(startTime, endTime); + doReturn(targetBeforePause).doThrow(RuntimeException.class).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); // Act & Assert @@ -373,10 +374,15 @@ void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + PausedDuration pausedDuration = new PausedDuration(startTime, endTime); + doReturn(targetBeforePause).doThrow(RuntimeException.class).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); @@ -400,10 +406,15 @@ void pause_WhenTargetStatusEqualsThrowException_ShouldThrowStatusCheckFailedExce String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + PausedDuration pausedDuration = new PausedDuration(startTime, endTime); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); doThrow(RuntimeException.class).when(pauser).targetStatusEquals(any(), any()); @@ -425,10 +436,15 @@ void pause_WhenTargetStatusEqualsThrowException_ShouldThrowStatusCheckFailedExce String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + PausedDuration pausedDuration = new PausedDuration(startTime, endTime); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); @@ -453,7 +469,12 @@ void pause_WhenTargetPodStatusChanged_ShouldThrowStatusCheckFailedException() String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + PausedDuration pausedDuration = new PausedDuration(startTime, endTime); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); @@ -476,7 +497,7 @@ void pause_WhenTargetPodStatusChanged_ShouldThrowStatusCheckFailedException() doReturn(beforeTargetStatus).when(targetBeforePause).getStatus(); doReturn(afterTargetStatus).when(targetAfterPause).getStatus(); - doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); // Act & Assert @@ -528,10 +549,15 @@ void pause_WhenUnpauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedEx String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + PausedDuration pausedDuration = new PausedDuration(startTime, endTime); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - doNothing().when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); From c413386fb15e3ad65f7339694f0c134ea31dd347 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Thu, 8 May 2025 16:04:25 +0900 Subject: [PATCH 14/21] Move variable definition in catch block --- lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index d8e8287..44c8e54 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -135,11 +135,10 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) // Get pods and deployment information after pause. TargetSnapshot targetAfterPause; - PauserException pauserException; try { targetAfterPause = getTarget(); } catch (Exception e) { - pauserException = new PauserException(getTargetAfterPauseErrorMessage, e); + PauserException pauserException = new PauserException(getTargetAfterPauseErrorMessage, e); if (unpauseFailedException == null) { throw pauserException; } else { @@ -149,11 +148,11 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) // Check if pods and deployment information are the same between before pause and after pause. boolean isTargetStatusEqual; - StatusCheckFailedException statusCheckFailedException; try { isTargetStatusEqual = targetStatusEquals(targetBeforePause, targetAfterPause); } catch (Exception e) { - statusCheckFailedException = new StatusCheckFailedException(statusCheckErrorMessage, e); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(statusCheckErrorMessage, e); if (unpauseFailedException == null) { throw statusCheckFailedException; } else { From 600e7b19fcf4592a0ca3eaa89af38d82fcd815fd Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Fri, 9 May 2025 14:49:29 +0900 Subject: [PATCH 15/21] Make the logic a bit simpler --- .../com/scalar/admin/kubernetes/Pauser.java | 27 +++++++------------ .../scalar/admin/kubernetes/PauserTest.java | 3 +-- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 44c8e54..3f3d028 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -129,9 +129,6 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) String statusDifferentErrorMessage = "The target pods were updated during the pause duration. You cannot use the backup that" + " was taken during this pause duration. "; - String unpauseFailedButPauseOkErrorMessage = - "Note that the pause operations for taking backup succeeded. You can use a backup that" - + " was taken during this pause duration. "; // Get pods and deployment information after pause. TargetSnapshot targetAfterPause; @@ -160,17 +157,10 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) } } - // If both the pause operation and status check succeeded, you can use the backup that was taken - // during the pause duration. - boolean isPauseOk = (pauseFailedException == null) && isTargetStatusEqual; - // Create an error message if any of the operations failed. StringBuilder errorMessageBuilder = new StringBuilder(); if (unpauseFailedException != null) { errorMessageBuilder.append(unpauseErrorMessage); - if (isPauseOk) { - errorMessageBuilder.append(unpauseFailedButPauseOkErrorMessage); - } } if (pauseFailedException != null) { errorMessageBuilder.append(pauseErrorMessage); @@ -181,17 +171,18 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) String errorMessage = errorMessageBuilder.toString(); // Return the final result based on each process. - if (unpauseFailedException - != null) { // Unpause issue is the most critical because it might cause system down. + if (unpauseFailedException != null) { // Unpause Failed. + // Unpause issue is the most critical because it might cause system down. throw new UnpauseFailedException(errorMessage, unpauseFailedException); - } else if (pauseFailedException - != null) { // Pause Failed is second priority because pause issue might be caused by - // configuration error. + } else if (pauseFailedException != null) { // Pause Failed. + // Pause Failed is second priority because pause issue might be caused by configuration error. throw new PauseFailedException(errorMessage, pauseFailedException); - } else if (!isTargetStatusEqual) { // Status check failed is third priority because this - // issue might be caused by temporary issue, for example, pod restarts. + } else if (!isTargetStatusEqual) { // Target status changed. + // Target status changed is third priority because this issue might be caused by temporary + // issue, for example, pod restarts. throw new PauseFailedException(errorMessage); - } else { // All operations succeeded. + } else { + // All operations succeeded. return pausedDuration; } } diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index 16965f6..a1a2a6a 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -333,8 +333,7 @@ void pause_WhenUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException( String.format( "Unpause operation failed. Scalar products might still be in a paused state. You must" + " restart related pods by using the `kubectl rollout restart deployment %s`" - + " command to unpause all pods. Note that the pause operations for taking backup" - + " succeeded. You can use a backup that was taken during this pause duration. ", + + " command to unpause all pods. ", DUMMY_OBJECT_NAME, startTime, endTime), thrown.getMessage()); } From 9e9132dd6c0691164b1423f7edd7807889b12a49 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Fri, 9 May 2025 15:16:34 +0900 Subject: [PATCH 16/21] Update error message generation and definition based on feedback --- .../com/scalar/admin/kubernetes/Pauser.java | 80 ++++++++++--------- .../scalar/admin/kubernetes/PauserTest.java | 65 ++++++--------- 2 files changed, 69 insertions(+), 76 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 3f3d028..f58e054 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -37,6 +37,25 @@ public class Pauser { private final TargetSelector targetSelector; + private static final String UNPAUSE_ERROR_MESSAGE = + "Unpause operation failed. Scalar products might still be in a paused state. You" + + " must restart related pods by using the `kubectl rollout restart deployment" + + " ` command to unpause all pods. "; + private static final String PAUSE_ERROR_MESSAGE = + "Pause operation failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to" + + " take a backup. "; + private static final String GET_TARGET_AFTER_PAUSE_ERROR_MESSAGE = + "Failed to find the target pods to examine if the targets pods were updated during" + + " paused. "; + private static final String STATUS_CHECK_ERROR_MESSAGE = + "Status check failed. You cannot use the backup that was taken during this pause" + + " duration. You need to retry the pause operation from the beginning to" + + " take a backup. "; + private static final String STATUS_DIFFERENT_ERROR_MESSAGE = + "The target pods were updated during the pause duration. You cannot use the backup that" + + " was taken during this pause duration. "; + /** * @param namespace The namespace where the pods are deployed. * @param helmReleaseName The Helm release name used to deploy the pods. @@ -108,38 +127,17 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) unpauseFailedException = new UnpauseFailedException("Unpause operation failed.", e); } - // Prepare error messages for each process. - String unpauseErrorMessage = - String.format( - "Unpause operation failed. Scalar products might still be in a paused state. You" - + " must restart related pods by using the `kubectl rollout restart deployment" - + " %s` command to unpause all pods. ", - targetBeforePause.getDeployment().getMetadata().getName()); - String pauseErrorMessage = - "Pause operation failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to" - + " take a backup. "; - String getTargetAfterPauseErrorMessage = - "Failed to find the target pods to examine if the targets pods were updated during" - + " paused. "; - String statusCheckErrorMessage = - "Status check failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to" - + " take a backup. "; - String statusDifferentErrorMessage = - "The target pods were updated during the pause duration. You cannot use the backup that" - + " was taken during this pause duration. "; - // Get pods and deployment information after pause. TargetSnapshot targetAfterPause; try { targetAfterPause = getTarget(); } catch (Exception e) { - PauserException pauserException = new PauserException(getTargetAfterPauseErrorMessage, e); + PauserException pauserException = + new PauserException(GET_TARGET_AFTER_PAUSE_ERROR_MESSAGE, e); if (unpauseFailedException == null) { throw pauserException; } else { - throw new UnpauseFailedException(unpauseErrorMessage, pauserException); + throw new UnpauseFailedException(UNPAUSE_ERROR_MESSAGE, pauserException); } } @@ -149,26 +147,17 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) isTargetStatusEqual = targetStatusEquals(targetBeforePause, targetAfterPause); } catch (Exception e) { StatusCheckFailedException statusCheckFailedException = - new StatusCheckFailedException(statusCheckErrorMessage, e); + new StatusCheckFailedException(STATUS_CHECK_ERROR_MESSAGE, e); if (unpauseFailedException == null) { throw statusCheckFailedException; } else { - throw new UnpauseFailedException(unpauseErrorMessage, statusCheckFailedException); + throw new UnpauseFailedException(UNPAUSE_ERROR_MESSAGE, statusCheckFailedException); } } // Create an error message if any of the operations failed. - StringBuilder errorMessageBuilder = new StringBuilder(); - if (unpauseFailedException != null) { - errorMessageBuilder.append(unpauseErrorMessage); - } - if (pauseFailedException != null) { - errorMessageBuilder.append(pauseErrorMessage); - } - if (!isTargetStatusEqual) { - errorMessageBuilder.append(statusDifferentErrorMessage); - } - String errorMessage = errorMessageBuilder.toString(); + String errorMessage = + createErrorMessage(unpauseFailedException, pauseFailedException, isTargetStatusEqual); // Return the final result based on each process. if (unpauseFailedException != null) { // Unpause Failed. @@ -229,4 +218,21 @@ PausedDuration pauseInternal( boolean targetStatusEquals(TargetSnapshot before, TargetSnapshot after) { return before.getStatus().equals(after.getStatus()); } + + private String createErrorMessage( + UnpauseFailedException unpauseFailedException, + PauseFailedException pauseFailedException, + boolean isTargetStatusEqual) { + StringBuilder errorMessageBuilder = new StringBuilder(); + if (unpauseFailedException != null) { + errorMessageBuilder.append(UNPAUSE_ERROR_MESSAGE); + } + if (pauseFailedException != null) { + errorMessageBuilder.append(PAUSE_ERROR_MESSAGE); + } + if (!isTargetStatusEqual) { + errorMessageBuilder.append(STATUS_DIFFERENT_ERROR_MESSAGE); + } + return errorMessageBuilder.toString(); + } } diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index a1a2a6a..7dd08cd 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -330,11 +330,9 @@ void pause_WhenUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException( UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - String.format( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment %s`" - + " command to unpause all pods. ", - DUMMY_OBJECT_NAME, startTime, endTime), + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment" + + " ` command to unpause all pods. ", thrown.getMessage()); } @@ -390,11 +388,9 @@ void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - String.format( - "Unpause operation failed. Scalar products might still be in a paused state. You" - + " must restart related pods by using the `kubectl rollout restart deployment" - + " %s` command to unpause all pods. ", - DUMMY_OBJECT_NAME), + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment" + + " ` command to unpause all pods. ", thrown.getMessage()); } @@ -453,11 +449,9 @@ void pause_WhenTargetStatusEqualsThrowException_ShouldThrowStatusCheckFailedExce UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - String.format( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment" - + " %s` command to unpause all pods. ", - DUMMY_OBJECT_NAME), + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment" + + " ` command to unpause all pods. ", thrown.getMessage()); } @@ -530,14 +524,11 @@ void pause_WhenPauseAndUnpauseFailed_ShouldThrowUnpauseFailedException() UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - String.format( - "Unpause operation failed. Scalar products might still be in a paused state. You" - + " must restart related pods by using the `kubectl rollout restart deployment" - + " %s` command to unpause all pods. Pause operation failed. You cannot use the" - + " backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to" - + " take a backup. ", - DUMMY_OBJECT_NAME), + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment" + + " ` command to unpause all pods. Pause operation failed. You" + + " cannot use the backup that was taken during this pause duration. You need to" + + " retry the pause operation from the beginning to take a backup. ", thrown.getMessage()); } @@ -566,13 +557,11 @@ void pause_WhenUnpauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedEx UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - String.format( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment %s`" - + " command to unpause all pods. The target pods were updated during the pause" - + " duration. You cannot use the backup that was taken during this pause" - + " duration. ", - DUMMY_OBJECT_NAME), + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment" + + " ` command to unpause all pods. The target pods were updated" + + " during the pause duration. You cannot use the backup that was taken during" + + " this pause duration. ", thrown.getMessage()); } @@ -598,15 +587,13 @@ void pause_WhenPauseAndUnpauseAndTargetPodStatusChanged_ShouldThrowUnpauseFailed UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); assertEquals( - String.format( - "Unpause operation failed. Scalar products might still be in a paused state. You" - + " must restart related pods by using the `kubectl rollout restart deployment" - + " %s` command to unpause all pods. Pause operation failed. You cannot use the" - + " backup that was taken during this pause duration. You need to retry the pause" - + " operation from the beginning to take a backup. The target pods were updated" - + " during the pause duration. You cannot use the backup that was taken during" - + " this pause duration. ", - DUMMY_OBJECT_NAME), + "Unpause operation failed. Scalar products might still be in a paused state. You must" + + " restart related pods by using the `kubectl rollout restart deployment" + + " ` command to unpause all pods. Pause operation failed. You" + + " cannot use the backup that was taken during this pause duration. You need to" + + " retry the pause operation from the beginning to take a backup. The target" + + " pods were updated during the pause duration. You cannot use the backup that" + + " was taken during this pause duration. ", thrown.getMessage()); } From 909e596fcd0e1772ec5e91b55b34073b822f0ed8 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Fri, 9 May 2025 15:57:38 +0900 Subject: [PATCH 17/21] Add code comments --- .../main/java/com/scalar/admin/kubernetes/Pauser.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index f58e054..051fa0b 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -110,6 +110,10 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) throw new PauserException("Failed to initialize the request coordinator.", e); } + // From here, we cannot throw exceptions right after they occur because we need to take care of + // the unpause operation failure. We will throw the exception after the unpause operation or at + // the end of this method. + // Run a pause operation. PausedDuration pausedDuration = null; PauseFailedException pauseFailedException = null; @@ -159,6 +163,12 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) String errorMessage = createErrorMessage(unpauseFailedException, pauseFailedException, isTargetStatusEqual); + // We use the exceptions as conditions instead of using boolean flags like `isPauseOk`, etc. If + // we use boolean flags, it might cause a bit large number of combinations. For example, if we + // have three flags, they generate 2^3 = 8 combinations. It also might make the nested if + // statement or a lot of branches of the switch statement. That's why we don't use status flags + // for now. + // Return the final result based on each process. if (unpauseFailedException != null) { // Unpause Failed. // Unpause issue is the most critical because it might cause system down. From f9dc445ba1152b9ef6d5f2b08406a646c65bb594 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Tue, 13 May 2025 18:25:56 +0900 Subject: [PATCH 18/21] Implement buildException() based on feedback --- .../GetTargetAfterPauseFailedException.java | 11 + .../com/scalar/admin/kubernetes/Pauser.java | 152 +- .../kubernetes/StatusUnmatchedException.java | 11 + .../scalar/admin/kubernetes/PauserTest.java | 1231 +++++++++++++++-- 4 files changed, 1205 insertions(+), 200 deletions(-) create mode 100644 lib/src/main/java/com/scalar/admin/kubernetes/GetTargetAfterPauseFailedException.java create mode 100644 lib/src/main/java/com/scalar/admin/kubernetes/StatusUnmatchedException.java diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/GetTargetAfterPauseFailedException.java b/lib/src/main/java/com/scalar/admin/kubernetes/GetTargetAfterPauseFailedException.java new file mode 100644 index 0000000..0e9486f --- /dev/null +++ b/lib/src/main/java/com/scalar/admin/kubernetes/GetTargetAfterPauseFailedException.java @@ -0,0 +1,11 @@ +package com.scalar.admin.kubernetes; + +public class GetTargetAfterPauseFailedException extends PauserException { + public GetTargetAfterPauseFailedException(String message) { + super(message); + } + + public GetTargetAfterPauseFailedException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 051fa0b..e87b36f 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -37,24 +37,33 @@ public class Pauser { private final TargetSelector targetSelector; - private static final String UNPAUSE_ERROR_MESSAGE = + @VisibleForTesting + static final String UNPAUSE_ERROR_MESSAGE = "Unpause operation failed. Scalar products might still be in a paused state. You" + " must restart related pods by using the `kubectl rollout restart deployment" - + " ` command to unpause all pods. "; - private static final String PAUSE_ERROR_MESSAGE = + + " ` command to unpause all pods."; + + @VisibleForTesting + static final String PAUSE_ERROR_MESSAGE = "Pause operation failed. You cannot use the backup that was taken during this pause" + " duration. You need to retry the pause operation from the beginning to" - + " take a backup. "; - private static final String GET_TARGET_AFTER_PAUSE_ERROR_MESSAGE = + + " take a backup."; + + @VisibleForTesting + static final String GET_TARGET_AFTER_PAUSE_ERROR_MESSAGE = "Failed to find the target pods to examine if the targets pods were updated during" - + " paused. "; - private static final String STATUS_CHECK_ERROR_MESSAGE = + + " paused."; + + @VisibleForTesting + static final String STATUS_CHECK_ERROR_MESSAGE = "Status check failed. You cannot use the backup that was taken during this pause" + " duration. You need to retry the pause operation from the beginning to" - + " take a backup. "; - private static final String STATUS_DIFFERENT_ERROR_MESSAGE = + + " take a backup."; + + @VisibleForTesting + static final String STATUS_UNMATCHED_ERROR_MESSAGE = "The target pods were updated during the pause duration. You cannot use the backup that" - + " was taken during this pause duration. "; + + " was taken during this pause duration."; /** * @param namespace The namespace where the pods are deployed. @@ -120,7 +129,7 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) try { pausedDuration = pauseInternal(requestCoordinator, pauseDuration, maxPauseWaitTime); } catch (Exception e) { - pauseFailedException = new PauseFailedException("Pause operation failed.", e); + pauseFailedException = new PauseFailedException(PAUSE_ERROR_MESSAGE, e); } // Run an unpause operation. @@ -128,58 +137,45 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) try { unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); } catch (Exception e) { - unpauseFailedException = new UnpauseFailedException("Unpause operation failed.", e); + unpauseFailedException = new UnpauseFailedException(UNPAUSE_ERROR_MESSAGE, e); } // Get pods and deployment information after pause. - TargetSnapshot targetAfterPause; + TargetSnapshot targetAfterPause = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; try { targetAfterPause = getTarget(); } catch (Exception e) { - PauserException pauserException = - new PauserException(GET_TARGET_AFTER_PAUSE_ERROR_MESSAGE, e); - if (unpauseFailedException == null) { - throw pauserException; - } else { - throw new UnpauseFailedException(UNPAUSE_ERROR_MESSAGE, pauserException); - } + getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(GET_TARGET_AFTER_PAUSE_ERROR_MESSAGE, e); } // Check if pods and deployment information are the same between before pause and after pause. - boolean isTargetStatusEqual; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; try { - isTargetStatusEqual = targetStatusEquals(targetBeforePause, targetAfterPause); + statusUnmatchedException = targetStatusEquals(targetBeforePause, targetAfterPause); } catch (Exception e) { - StatusCheckFailedException statusCheckFailedException = - new StatusCheckFailedException(STATUS_CHECK_ERROR_MESSAGE, e); - if (unpauseFailedException == null) { - throw statusCheckFailedException; - } else { - throw new UnpauseFailedException(UNPAUSE_ERROR_MESSAGE, statusCheckFailedException); - } + statusCheckFailedException = new StatusCheckFailedException(STATUS_CHECK_ERROR_MESSAGE, e); } - // Create an error message if any of the operations failed. - String errorMessage = - createErrorMessage(unpauseFailedException, pauseFailedException, isTargetStatusEqual); - // We use the exceptions as conditions instead of using boolean flags like `isPauseOk`, etc. If // we use boolean flags, it might cause a bit large number of combinations. For example, if we // have three flags, they generate 2^3 = 8 combinations. It also might make the nested if // statement or a lot of branches of the switch statement. That's why we don't use status flags // for now. + PauserException pauserException = + buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); // Return the final result based on each process. - if (unpauseFailedException != null) { // Unpause Failed. - // Unpause issue is the most critical because it might cause system down. - throw new UnpauseFailedException(errorMessage, unpauseFailedException); - } else if (pauseFailedException != null) { // Pause Failed. - // Pause Failed is second priority because pause issue might be caused by configuration error. - throw new PauseFailedException(errorMessage, pauseFailedException); - } else if (!isTargetStatusEqual) { // Target status changed. - // Target status changed is third priority because this issue might be caused by temporary - // issue, for example, pod restarts. - throw new PauseFailedException(errorMessage); + if (pauserException != null) { + // Some operations failed. + throw pauserException; } else { // All operations succeeded. return pausedDuration; @@ -225,24 +221,70 @@ PausedDuration pauseInternal( } @VisibleForTesting - boolean targetStatusEquals(TargetSnapshot before, TargetSnapshot after) { - return before.getStatus().equals(after.getStatus()); + @Nullable + StatusUnmatchedException targetStatusEquals( + TargetSnapshot before, @Nullable TargetSnapshot after) { + if (after != null && before.getStatus().equals(after.getStatus())) { + return null; + } else { + return new StatusUnmatchedException(STATUS_UNMATCHED_ERROR_MESSAGE); + } } - private String createErrorMessage( - UnpauseFailedException unpauseFailedException, - PauseFailedException pauseFailedException, - boolean isTargetStatusEqual) { - StringBuilder errorMessageBuilder = new StringBuilder(); + @Nullable + @VisibleForTesting + PauserException buildException( + @Nullable UnpauseFailedException unpauseFailedException, + @Nullable PauseFailedException pauseFailedException, + @Nullable GetTargetAfterPauseFailedException getTargetAfterPauseFailedException, + @Nullable StatusCheckFailedException statusCheckFailedException, + @Nullable StatusUnmatchedException statusUnmatchedException) { + PauserException pauserException = null; + + // Unpause issue is the most critical because it might cause system down. if (unpauseFailedException != null) { - errorMessageBuilder.append(UNPAUSE_ERROR_MESSAGE); + pauserException = unpauseFailedException; } + + // Pause Failed is second priority because pause issue might be caused by configuration error. if (pauseFailedException != null) { - errorMessageBuilder.append(PAUSE_ERROR_MESSAGE); + if (pauserException == null) { + pauserException = pauseFailedException; + } else { + pauserException.addSuppressed(pauseFailedException); + } + } + + // Get target issue is third priority because this issue might be caused by temporary issue, for + // example, network issues. + if (getTargetAfterPauseFailedException != null) { + if (pauserException == null) { + pauserException = getTargetAfterPauseFailedException; + } else { + pauserException.addSuppressed(getTargetAfterPauseFailedException); + } } - if (!isTargetStatusEqual) { - errorMessageBuilder.append(STATUS_DIFFERENT_ERROR_MESSAGE); + + // Status check issue is third priority because this issue might be caused by temporary issue, + // for example, targetAfterPause is null. + if (statusCheckFailedException != null) { + if (pauserException == null) { + pauserException = statusCheckFailedException; + } else { + pauserException.addSuppressed(statusCheckFailedException); + } + } + + // Status unmatched issue is third priority because this issue might be caused by temporary + // issue, for example, pod restarts. + if (statusUnmatchedException != null) { + if (pauserException == null) { + pauserException = statusUnmatchedException; + } else { + pauserException.addSuppressed(statusUnmatchedException); + } } - return errorMessageBuilder.toString(); + + return pauserException; } } diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/StatusUnmatchedException.java b/lib/src/main/java/com/scalar/admin/kubernetes/StatusUnmatchedException.java new file mode 100644 index 0000000..19f9997 --- /dev/null +++ b/lib/src/main/java/com/scalar/admin/kubernetes/StatusUnmatchedException.java @@ -0,0 +1,11 @@ +package com.scalar.admin.kubernetes; + +public class StatusUnmatchedException extends PauserException { + public StatusUnmatchedException(String message) { + super(message); + } + + public StatusUnmatchedException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java index 7dd08cd..712c922 100644 --- a/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java +++ b/lib/src/test/java/com/scalar/admin/kubernetes/PauserTest.java @@ -1,6 +1,6 @@ package com.scalar.admin.kubernetes; -import static com.scalar.admin.kubernetes.Pauser.MAX_UNPAUSE_RETRY_COUNT; +import static com.scalar.admin.kubernetes.Pauser.*; import static java.time.temporal.ChronoUnit.SECONDS; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; @@ -214,16 +214,12 @@ void pause_WhenPauseInternalThrowException_ShouldThrowPauseFailedException() .when(pauser) .pauseInternal(requestCoordinator, pauseDuration, null); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(true).when(pauser).targetStatusEquals(any(), any()); + doReturn(null).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Pause operation failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to take a" - + " backup. ", - thrown.getMessage()); + assertEquals(PAUSE_ERROR_MESSAGE, thrown.getMessage()); } @Test @@ -238,16 +234,12 @@ void pause_WhenRequestCoordinatorPauseThrowException_ShouldThrowPauseFailedExcep doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doThrow(RuntimeException.class).when(requestCoordinator).pause(true, null); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(true).when(pauser).targetStatusEquals(any(), any()); + doReturn(null).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Pause operation failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to take a" - + " backup. ", - thrown.getMessage()); + assertEquals(PAUSE_ERROR_MESSAGE, thrown.getMessage()); } @Test @@ -263,16 +255,12 @@ void pause_WhenInstantNowThrowException_ShouldThrowPauseFailedException() doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(true).when(pauser).targetStatusEquals(any(), any()); + doReturn(null).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Pause operation failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to take a" - + " backup. ", - thrown.getMessage()); + assertEquals(PAUSE_ERROR_MESSAGE, thrown.getMessage()); mockedTime.close(); } @@ -291,16 +279,12 @@ void pause_WhenSleepThrowException_ShouldThrowPauseFailedException() throws Paus doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(true).when(pauser).targetStatusEquals(any(), any()); + doReturn(null).when(pauser).targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Pause operation failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to take a" - + " backup. ", - thrown.getMessage()); + assertEquals(PAUSE_ERROR_MESSAGE, thrown.getMessage()); mockedSleep.close(); } @@ -324,20 +308,16 @@ void pause_WhenUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException( doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doReturn(true).when(pauser).targetStatusEquals(any(), any()); + doReturn(null).when(pauser).targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment" - + " ` command to unpause all pods. ", - thrown.getMessage()); + assertEquals(UNPAUSE_ERROR_MESSAGE, thrown.getMessage()); } @Test - void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() + void pause_WhenSecondGetTargetThrowException_ShouldThrowGetTargetAfterPauseFailedException() throws PauserException { // Arrange String namespace = "dummyNs"; @@ -355,18 +335,15 @@ void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); // Act & Assert - PauserException thrown = - assertThrows(PauserException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Failed to find the target pods to examine if the targets pods were updated during" - + " paused. ", - thrown.getMessage()); + GetTargetAfterPauseFailedException thrown = + assertThrows( + GetTargetAfterPauseFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals(GET_TARGET_AFTER_PAUSE_ERROR_MESSAGE, thrown.getMessage()); } @Test - void - pause_WhenSecondGetTargetAndUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException() - throws PauserException { + void pause_WhenTargetStatusEqualsThrowException_ShouldThrowStatusCheckFailedException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; @@ -377,25 +354,20 @@ void pause_WhenSecondGetTargetThrowException_ShouldThrowPauserException() Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); PausedDuration pausedDuration = new PausedDuration(startTime, endTime); - doReturn(targetBeforePause).doThrow(RuntimeException.class).when(pauser).getTarget(); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); - doThrow(RuntimeException.class) - .when(pauser) - .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); + doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + doThrow(RuntimeException.class).when(pauser).targetStatusEquals(any(), any()); // Act & Assert - UnpauseFailedException thrown = - assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment" - + " ` command to unpause all pods. ", - thrown.getMessage()); + StatusCheckFailedException thrown = + assertThrows(StatusCheckFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals(STATUS_CHECK_ERROR_MESSAGE, thrown.getMessage()); } @Test - void pause_WhenTargetStatusEqualsThrowException_ShouldThrowStatusCheckFailedException() + void pause_WhenTargetPodStatusChanged_ShouldThrowStatusUnmatchedException() throws PauserException { // Arrange String namespace = "dummyNs"; @@ -409,55 +381,64 @@ void pause_WhenTargetStatusEqualsThrowException_ShouldThrowStatusCheckFailedExce doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); + + Map podRestartCounts = + new HashMap() { + { + put("dummyKey", 1); + } + }; + Map podResourceVersions = + new HashMap() { + { + put("dummyKey", "dummyValue"); + } + }; + TargetStatus beforeTargetStatus = + new TargetStatus(podRestartCounts, podResourceVersions, "beforeDifferentValue"); + TargetStatus afterTargetStatus = + new TargetStatus(podRestartCounts, podResourceVersions, "afterDifferentValue"); + doReturn(beforeTargetStatus).when(targetBeforePause).getStatus(); + doReturn(afterTargetStatus).when(targetAfterPause).getStatus(); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doThrow(RuntimeException.class).when(pauser).targetStatusEquals(any(), any()); // Act & Assert - StatusCheckFailedException thrown = - assertThrows(StatusCheckFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Status check failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to take a" - + " backup. ", - thrown.getMessage()); + StatusUnmatchedException thrown = + assertThrows(StatusUnmatchedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals(STATUS_UNMATCHED_ERROR_MESSAGE, thrown.getMessage()); } @Test - void - pause_WhenCompareTargetStatusAndUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException() - throws PauserException { + void pause_WhenPauseAndUnpauseThrowException_ShouldThrowUnpauseFailedException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; - Instant startTime = Instant.now().minus(5, SECONDS); - Instant endTime = Instant.now().plus(5, SECONDS); - Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); - PausedDuration pausedDuration = new PausedDuration(startTime, endTime); - doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); + doThrow(RuntimeException.class) + .when(pauser) + .pauseInternal(requestCoordinator, pauseDuration, null); doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doThrow(RuntimeException.class).when(pauser).targetStatusEquals(any(), any()); + doReturn(null).when(pauser).targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment" - + " ` command to unpause all pods. ", - thrown.getMessage()); + assertEquals(UNPAUSE_ERROR_MESSAGE, thrown.getMessage()); + assertEquals(PauseFailedException.class, thrown.getSuppressed()[0].getClass()); } @Test - void pause_WhenTargetPodStatusChanged_ShouldThrowStatusCheckFailedException() - throws PauserException { + void + pause_WhenSecondGetTargetAndUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; @@ -468,68 +449,47 @@ void pause_WhenTargetPodStatusChanged_ShouldThrowStatusCheckFailedException() Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); PausedDuration pausedDuration = new PausedDuration(startTime, endTime); - doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); + doReturn(targetBeforePause).doThrow(RuntimeException.class).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - - Map podRestartCounts = - new HashMap() { - { - put("dummyKey", 1); - } - }; - Map podResourceVersions = - new HashMap() { - { - put("dummyKey", "dummyValue"); - } - }; - TargetStatus beforeTargetStatus = - new TargetStatus(podRestartCounts, podResourceVersions, "beforeDifferentValue"); - TargetStatus afterTargetStatus = - new TargetStatus(podRestartCounts, podResourceVersions, "afterDifferentValue"); - doReturn(beforeTargetStatus).when(targetBeforePause).getStatus(); - doReturn(afterTargetStatus).when(targetAfterPause).getStatus(); - doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); - doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); + doThrow(RuntimeException.class) + .when(pauser) + .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); // Act & Assert - PauseFailedException thrown = - assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "The target pods were updated during the pause duration. You cannot use the backup that" - + " was taken during this pause duration. ", - thrown.getMessage()); + UnpauseFailedException thrown = + assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); + assertEquals(UNPAUSE_ERROR_MESSAGE, thrown.getMessage()); + assertEquals(GetTargetAfterPauseFailedException.class, thrown.getSuppressed()[0].getClass()); } @Test - void pause_WhenPauseAndUnpauseFailed_ShouldThrowUnpauseFailedException() - throws PauserException { + void + pause_WhenTargetStatusEqualsAndUnpauseWithRetryThrowException_ShouldThrowUnpauseFailedException() + throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; int pauseDuration = 1; + Instant startTime = Instant.now().minus(5, SECONDS); + Instant endTime = Instant.now().plus(5, SECONDS); + Pauser pauser = spy(new Pauser(namespace, helmReleaseName)); + PausedDuration pausedDuration = new PausedDuration(startTime, endTime); + doReturn(targetBeforePause).doReturn(targetAfterPause).when(pauser).getTarget(); doReturn(requestCoordinator).when(pauser).getRequestCoordinator(targetBeforePause); - doThrow(RuntimeException.class) - .when(pauser) - .pauseInternal(requestCoordinator, pauseDuration, null); + doReturn(pausedDuration).when(pauser).pauseInternal(any(), anyInt(), anyLong()); doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doReturn(true).when(pauser).targetStatusEquals(any(), any()); + doThrow(RuntimeException.class).when(pauser).targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment" - + " ` command to unpause all pods. Pause operation failed. You" - + " cannot use the backup that was taken during this pause duration. You need to" - + " retry the pause operation from the beginning to take a backup. ", - thrown.getMessage()); + assertEquals(UNPAUSE_ERROR_MESSAGE, thrown.getMessage()); + assertEquals(StatusCheckFailedException.class, thrown.getSuppressed()[0].getClass()); } @Test @@ -551,18 +511,15 @@ void pause_WhenUnpauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedEx doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doReturn(false).when(pauser).targetStatusEquals(any(), any()); + doReturn(new StatusUnmatchedException(STATUS_UNMATCHED_ERROR_MESSAGE)) + .when(pauser) + .targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment" - + " ` command to unpause all pods. The target pods were updated" - + " during the pause duration. You cannot use the backup that was taken during" - + " this pause duration. ", - thrown.getMessage()); + assertEquals(UNPAUSE_ERROR_MESSAGE, thrown.getMessage()); + assertEquals(StatusUnmatchedException.class, thrown.getSuppressed()[0].getClass()); } @Test @@ -581,24 +538,18 @@ void pause_WhenPauseAndUnpauseAndTargetPodStatusChanged_ShouldThrowUnpauseFailed doThrow(RuntimeException.class) .when(pauser) .unpauseWithRetry(requestCoordinator, MAX_UNPAUSE_RETRY_COUNT); - doReturn(false).when(pauser).targetStatusEquals(any(), any()); + doReturn(new StatusUnmatchedException(STATUS_UNMATCHED_ERROR_MESSAGE)) + .when(pauser) + .targetStatusEquals(any(), any()); // Act & Assert UnpauseFailedException thrown = assertThrows(UnpauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Unpause operation failed. Scalar products might still be in a paused state. You must" - + " restart related pods by using the `kubectl rollout restart deployment" - + " ` command to unpause all pods. Pause operation failed. You" - + " cannot use the backup that was taken during this pause duration. You need to" - + " retry the pause operation from the beginning to take a backup. The target" - + " pods were updated during the pause duration. You cannot use the backup that" - + " was taken during this pause duration. ", - thrown.getMessage()); + assertEquals(UNPAUSE_ERROR_MESSAGE, thrown.getMessage()); } @Test - void pause_WhenPauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedException() + void pause_WhenPauseFailedAndTargetPodStatusChanged_ShouldThrowPauseFailedException() throws PauserException { // Arrange String namespace = "dummyNs"; String helmReleaseName = "dummyRelease"; @@ -610,17 +561,14 @@ void pause_WhenPauseFailedAndTargetPodStatusChanged_ShouldThrowUnpauseFailedExce .when(pauser) .pauseInternal(requestCoordinator, pauseDuration, null); doNothing().when(pauser).unpauseWithRetry(any(), anyInt()); - doReturn(false).when(pauser).targetStatusEquals(any(), any()); + doReturn(new StatusUnmatchedException(STATUS_UNMATCHED_ERROR_MESSAGE)) + .when(pauser) + .targetStatusEquals(any(), any()); // Act & Assert PauseFailedException thrown = assertThrows(PauseFailedException.class, () -> pauser.pause(pauseDuration, null)); - assertEquals( - "Pause operation failed. You cannot use the backup that was taken during this pause" - + " duration. You need to retry the pause operation from the beginning to take a" - + " backup. The target pods were updated during the pause duration. You cannot use" - + " the backup that was taken during this pause duration. ", - thrown.getMessage()); + assertEquals(PAUSE_ERROR_MESSAGE, thrown.getMessage()); } } @@ -655,4 +603,997 @@ void unpauseWithRetry_WhenExceptionOccur_ShouldRetryThreeTimes() throws PauserEx verify(requestCoordinator, times(MAX_UNPAUSE_RETRY_COUNT)).unpause(); } } + + @Nested + class buildException { + @Test + void buildException_00000_ReturnNull() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertNull(actual); + } + + @Test + void buildException_00001_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(StatusUnmatchedException.class, actual.getClass()); + } + + @Test + void buildException_00010_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(StatusCheckFailedException.class, actual.getClass()); + } + + @Test + void buildException_00011_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(StatusCheckFailedException.class, actual.getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_00100_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(GetTargetAfterPauseFailedException.class, actual.getClass()); + } + + @Test + void buildException_00110_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(GetTargetAfterPauseFailedException.class, actual.getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_00101_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(GetTargetAfterPauseFailedException.class, actual.getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_00111_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(GetTargetAfterPauseFailedException.class, actual.getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_01000_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(PauseFailedException.class, actual.getClass()); + } + + @Test + void buildException_01001_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(PauseFailedException.class, actual.getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_01010_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(PauseFailedException.class, actual.getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_01011_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(PauseFailedException.class, actual.getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_01100_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(PauseFailedException.class, actual.getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_01101_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(PauseFailedException.class, actual.getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_01110_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(PauseFailedException.class, actual.getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_01111_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = null; + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(PauseFailedException.class, actual.getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[1].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[2].getClass()); + } + + @Test + void buildException_10000_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + } + + @Test + void buildException_10001_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_10010_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_10011_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_10100_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_10101_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_10110_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_10111_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = null; + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[1].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[2].getClass()); + } + + @Test + void buildException_11000_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(PauseFailedException.class, actual.getSuppressed()[0].getClass()); + } + + @Test + void buildException_11001_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(PauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_11010_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(PauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_11011_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = null; + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(PauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[1].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[2].getClass()); + } + + @Test + void buildException_11100_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(PauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[1].getClass()); + } + + @Test + void buildException_11101_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = null; + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(PauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[1].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[2].getClass()); + } + + @Test + void buildException_11110_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = null; + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(PauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[1].getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[2].getClass()); + } + + @Test + void buildException_11111_ThrowException() throws PauserException { + // Arrange + String namespace = "dummyNs"; + String helmReleaseName = "dummyRelease"; + Pauser pauser = new Pauser(namespace, helmReleaseName); + + String dummyMessage = "dummyMessage"; + + UnpauseFailedException unpauseFailedException = new UnpauseFailedException(dummyMessage); + PauseFailedException pauseFailedException = new PauseFailedException(dummyMessage); + GetTargetAfterPauseFailedException getTargetAfterPauseFailedException = + new GetTargetAfterPauseFailedException(dummyMessage); + StatusCheckFailedException statusCheckFailedException = + new StatusCheckFailedException(dummyMessage); + StatusUnmatchedException statusUnmatchedException = + new StatusUnmatchedException(dummyMessage); + + // Act + Exception actual = + pauser.buildException( + unpauseFailedException, + pauseFailedException, + getTargetAfterPauseFailedException, + statusCheckFailedException, + statusUnmatchedException); + + // Assert + assertEquals(UnpauseFailedException.class, actual.getClass()); + assertEquals(PauseFailedException.class, actual.getSuppressed()[0].getClass()); + assertEquals(GetTargetAfterPauseFailedException.class, actual.getSuppressed()[1].getClass()); + assertEquals(StatusCheckFailedException.class, actual.getSuppressed()[2].getClass()); + assertEquals(StatusUnmatchedException.class, actual.getSuppressed()[3].getClass()); + } + } } From 5cca44c080bffc253678d0dcb89ba3c9ec432d11 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Wed, 14 May 2025 10:45:59 +0900 Subject: [PATCH 19/21] Run targetStatusEquals() only if targetAfterPause is not null --- .../java/com/scalar/admin/kubernetes/Pauser.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index e87b36f..34cbced 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -153,10 +153,12 @@ public PausedDuration pause(int pauseDuration, @Nullable Long maxPauseWaitTime) // Check if pods and deployment information are the same between before pause and after pause. StatusCheckFailedException statusCheckFailedException = null; StatusUnmatchedException statusUnmatchedException = null; - try { - statusUnmatchedException = targetStatusEquals(targetBeforePause, targetAfterPause); - } catch (Exception e) { - statusCheckFailedException = new StatusCheckFailedException(STATUS_CHECK_ERROR_MESSAGE, e); + if (targetAfterPause != null) { + try { + statusUnmatchedException = targetStatusEquals(targetBeforePause, targetAfterPause); + } catch (Exception e) { + statusCheckFailedException = new StatusCheckFailedException(STATUS_CHECK_ERROR_MESSAGE, e); + } } // We use the exceptions as conditions instead of using boolean flags like `isPauseOk`, etc. If @@ -222,9 +224,8 @@ PausedDuration pauseInternal( @VisibleForTesting @Nullable - StatusUnmatchedException targetStatusEquals( - TargetSnapshot before, @Nullable TargetSnapshot after) { - if (after != null && before.getStatus().equals(after.getStatus())) { + StatusUnmatchedException targetStatusEquals(TargetSnapshot before, TargetSnapshot after) { + if (before.getStatus().equals(after.getStatus())) { return null; } else { return new StatusUnmatchedException(STATUS_UNMATCHED_ERROR_MESSAGE); From 639e132e4766b0299b8b91047b8b633f0371072f Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Fri, 16 May 2025 08:53:14 +0900 Subject: [PATCH 20/21] Apply suggestions from code review Co-authored-by: Hiroyuki Yamada --- .../java/com/scalar/admin/kubernetes/Pauser.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 34cbced..12e933b 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -242,12 +242,12 @@ PauserException buildException( @Nullable StatusUnmatchedException statusUnmatchedException) { PauserException pauserException = null; - // Unpause issue is the most critical because it might cause system down. + // Treat the unpause failure as most critical because it might cause system unavailability. if (unpauseFailedException != null) { pauserException = unpauseFailedException; } - // Pause Failed is second priority because pause issue might be caused by configuration error. + // Treat the pause failure as the second priority because the issue might be caused by a configuration mistake. if (pauseFailedException != null) { if (pauserException == null) { pauserException = pauseFailedException; @@ -256,8 +256,8 @@ PauserException buildException( } } - // Get target issue is third priority because this issue might be caused by temporary issue, for - // example, network issues. + // Treat the getting target failure as the third priority because this issue might be caused by a temporary glitch, for + // example, network failures. if (getTargetAfterPauseFailedException != null) { if (pauserException == null) { pauserException = getTargetAfterPauseFailedException; @@ -266,7 +266,7 @@ PauserException buildException( } } - // Status check issue is third priority because this issue might be caused by temporary issue, + // Treat the status checking failure as third priority because this issue might be caused by a temporary glitch, // for example, targetAfterPause is null. if (statusCheckFailedException != null) { if (pauserException == null) { @@ -276,8 +276,8 @@ PauserException buildException( } } - // Status unmatched issue is third priority because this issue might be caused by temporary - // issue, for example, pod restarts. + // Treat the status unmatched issue as third priority because this issue might be caused by temporary + // glitch, for example, pod restarts. if (statusUnmatchedException != null) { if (pauserException == null) { pauserException = statusUnmatchedException; From b93c054f00233d3d37ebb1571c9031f6d68e7747 Mon Sep 17 00:00:00 2001 From: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com> Date: Fri, 16 May 2025 21:25:39 +0900 Subject: [PATCH 21/21] Fix code comments based on feedback --- .../java/com/scalar/admin/kubernetes/Pauser.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java index 12e933b..6730fb4 100644 --- a/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java +++ b/lib/src/main/java/com/scalar/admin/kubernetes/Pauser.java @@ -247,7 +247,8 @@ PauserException buildException( pauserException = unpauseFailedException; } - // Treat the pause failure as the second priority because the issue might be caused by a configuration mistake. + // Treat the pause failure as the second priority because the issue might be caused by a + // configuration mistake. if (pauseFailedException != null) { if (pauserException == null) { pauserException = pauseFailedException; @@ -256,8 +257,8 @@ PauserException buildException( } } - // Treat the getting target failure as the third priority because this issue might be caused by a temporary glitch, for - // example, network failures. + // Treat the getting target failure as the third priority because this issue might be caused by + // a temporary glitch, for example, network failures. if (getTargetAfterPauseFailedException != null) { if (pauserException == null) { pauserException = getTargetAfterPauseFailedException; @@ -266,8 +267,9 @@ PauserException buildException( } } - // Treat the status checking failure as third priority because this issue might be caused by a temporary glitch, - // for example, targetAfterPause is null. + // Treat the status checking failure as the third priority because this issue might be caused by + // a temporary glitch, for example, getting target information by using the Kubernetes API fails + // after the pause operation. if (statusCheckFailedException != null) { if (pauserException == null) { pauserException = statusCheckFailedException; @@ -276,8 +278,8 @@ PauserException buildException( } } - // Treat the status unmatched issue as third priority because this issue might be caused by temporary - // glitch, for example, pod restarts. + // Treat the status unmatched issue as the third priority because this issue might be caused by + // temporary glitch, for example, pod restarts. if (statusUnmatchedException != null) { if (pauserException == null) { pauserException = statusUnmatchedException;