From 7276a81ac23b2c592a862652be8acc3bb1050fc7 Mon Sep 17 00:00:00 2001 From: Dmitry Kaukov Date: Mon, 16 Dec 2024 14:35:53 +1100 Subject: [PATCH] Change rolling restart/pause/resume to use rfc6902 JSON patch Fixes https://github.com/fabric8io/kubernetes-client/issues/6658 --- .../dsl/internal/apps/v1/RollingUpdater.java | 50 +++++----- .../client/mock/DeploymentCrudTest.java | 91 +++++++++++++++++++ .../client/mock/DeploymentTest.java | 4 +- 3 files changed, 115 insertions(+), 30 deletions(-) diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/apps/v1/RollingUpdater.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/apps/v1/RollingUpdater.java index c21a8f22aa4..56b34f1f5ea 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/apps/v1/RollingUpdater.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/apps/v1/RollingUpdater.java @@ -45,10 +45,10 @@ import java.security.NoSuchAlgorithmException; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; @@ -169,8 +169,8 @@ public T rollUpdate(T oldObj, T newObj) { } } - private static T applyPatch(Resource resource, Map map, KubernetesSerialization serialization) { - return resource.patch(PatchContext.of(PatchType.STRATEGIC_MERGE), serialization.asJson(map)); + private static T applyPatch(Resource resource, List list, KubernetesSerialization serialization) { + return resource.patch(PatchContext.of(PatchType.JSON), serialization.asJson(list)); } public static T resume(RollableScalableResourceOperation resource) { @@ -185,35 +185,29 @@ public static T restart(RollableScalableResourceOperatio return applyPatch(resource, RollingUpdater.requestPayLoadForRolloutRestart(), resource.getKubernetesSerialization()); } - public static Map requestPayLoadForRolloutPause() { - Map jsonPatchPayload = new HashMap<>(); - Map spec = new HashMap<>(); - spec.put("paused", true); - jsonPatchPayload.put("spec", spec); - return jsonPatchPayload; + public static List requestPayLoadForRolloutPause() { + HashMap patch = new HashMap<>(); + patch.put("op", "add"); + patch.put("path", "/spec/paused"); + patch.put("value", true); + return Collections.singletonList(patch); } - public static Map requestPayLoadForRolloutResume() { - Map jsonPatchPayload = new HashMap<>(); - Map spec = new HashMap<>(); - spec.put("paused", null); - jsonPatchPayload.put("spec", spec); - return jsonPatchPayload; + public static List requestPayLoadForRolloutResume() { + HashMap patch = new HashMap<>(); + patch.put("op", "remove"); + patch.put("path", "/spec/paused"); + return Collections.singletonList(patch); } - public static Map requestPayLoadForRolloutRestart() { - Map jsonPatchPayload = new HashMap<>(); - Map annotations = new HashMap<>(); - annotations.put("kubectl.kubernetes.io/restartedAt", - new Date().toInstant().atOffset(ZoneOffset.UTC).format(DateTimeFormatter.ISO_LOCAL_DATE_TIME)); - Map templateMetadata = new HashMap<>(); - templateMetadata.put("annotations", annotations); - Map template = new HashMap<>(); - template.put("metadata", templateMetadata); - Map deploymentSpec = new HashMap<>(); - deploymentSpec.put("template", template); - jsonPatchPayload.put("spec", deploymentSpec); - return jsonPatchPayload; + public static List requestPayLoadForRolloutRestart() { + HashMap patch = new HashMap<>(); + HashMap value = new HashMap<>(); + patch.put("op", "replace"); + patch.put("path", "/spec/template/metadata/annotations"); + value.put("kubectl.kubernetes.io/restartedAt", new Date().toInstant().atOffset(ZoneOffset.UTC).format(DateTimeFormatter.ISO_LOCAL_DATE_TIME)); + patch.put("value", value); + return Collections.singletonList(patch); } /** diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentCrudTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentCrudTest.java index 4a9d0f6f163..1ba51d29688 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentCrudTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentCrudTest.java @@ -30,6 +30,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @EnableKubernetesMockClient(crud = true) @@ -125,4 +126,94 @@ void testReplace() { assertFalse(replacedDeployment.getMetadata().getLabels().isEmpty()); assertEquals("one", replacedDeployment.getMetadata().getLabels().get("testKey")); } + + @Test + @DisplayName("Should pause resource") + void testPause() { + // Given + Deployment deployment1 = new DeploymentBuilder().withNewMetadata() + .withName("d1") + .withNamespace("ns1") + .addToLabels("testKey", "testValue") + .endMetadata() + .withNewSpec() + .endSpec() + .build(); + client.apps().deployments().inNamespace("ns1").create(deployment1); + + // When + client.apps() + .deployments() + .inNamespace("ns1") + .withName("d1") + .rolling() + .pause(); + Deployment updatedDeployment = client.apps().deployments().inNamespace("ns1").withName("d1").get(); + + // Then + assertNotNull(updatedDeployment); + assertEquals(true, updatedDeployment.getSpec().getPaused()); + } + + @Test + @DisplayName("Should resume rollout") + void testRolloutResume() throws InterruptedException { + // Given + Deployment deployment1 = new DeploymentBuilder().withNewMetadata() + .withName("d1") + .withNamespace("ns1") + .addToLabels("testKey", "testValue") + .endMetadata() + .withNewSpec() + .withPaused(true) + .endSpec() + .build(); + client.apps().deployments().inNamespace("ns1").create(deployment1); + + // When + client.apps() + .deployments() + .inNamespace("ns1") + .withName("d1") + .rolling() + .resume(); + Deployment updatedDeployment = client.apps().deployments().inNamespace("ns1").withName("d1").get(); + + // Then + assertNotNull(updatedDeployment); + assertNull(updatedDeployment.getSpec().getPaused()); + } + + @Test + @DisplayName("Should restart rollout") + void testRolloutRestart() throws InterruptedException { + // Given + Deployment deployment1 = new DeploymentBuilder().withNewMetadata() + .withName("d1") + .withNamespace("ns1") + .addToLabels("testKey", "testValue") + .endMetadata() + .withNewSpec() + .withNewTemplate() + .withNewMetadata() + .addToAnnotations("", "") + .endMetadata() + .endTemplate() + .endSpec() + .build(); + client.apps().deployments().inNamespace("ns1").create(deployment1); + + // When + client.apps() + .deployments() + .inNamespace("ns1") + .withName("d1") + .rolling() + .restart(); + Deployment updatedDeployment = client.apps().deployments().inNamespace("ns1").withName("d1").get(); + + // Then + assertNotNull(updatedDeployment); + assertNotNull(updatedDeployment.getSpec().getTemplate().getMetadata().getAnnotations().get("kubectl.kubernetes.io/restartedAt")); + } } diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentTest.java index c2fc47e1fd5..0b343d01abd 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentTest.java @@ -622,7 +622,7 @@ void testRolloutPause() throws InterruptedException { // Then RecordedRequest recordedRequest = server.getLastRequest(); assertEquals("PATCH", recordedRequest.getMethod()); - assertEquals("{\"spec\":{\"paused\":true}}", recordedRequest.getBody().readUtf8()); + assertEquals("[{\"op\":\"add\",\"path\":\"/spec/paused\",\"value\":true}]", recordedRequest.getBody().readUtf8()); } @Test @@ -652,7 +652,7 @@ void testRolloutResume() throws InterruptedException { RecordedRequest recordedRequest = server.getLastRequest(); assertNotNull(deployment); assertEquals("PATCH", recordedRequest.getMethod()); - assertEquals("{\"spec\":{\"paused\":null}}", recordedRequest.getBody().readUtf8()); + assertEquals("[{\"op\":\"remove\",\"path\":\"/spec/paused\"}]", recordedRequest.getBody().readUtf8()); } @Test