From a833b7d6209559522344c8ac8dc7b944a5db5cfb Mon Sep 17 00:00:00 2001 From: Amir Alavi Date: Thu, 3 Aug 2023 21:56:37 -0400 Subject: [PATCH 1/2] feat(provider/kubernetes): support for kubectl server-side-apply strategy kubernetes server-side apply (SSA) was released back in 1.14 and became GA In 1.22. This new strategy will use the new merging algorithm, as well as tracking field ownership at the kubernetes api-server Signed-off-by: Amir Alavi --- .../kubernetes/it/DeployManifestIT.java | 109 ++++++++++++++++++ .../manifest/KubernetesManifestStrategy.java | 6 +- .../kubernetes/op/handler/CanDeploy.java | 3 + .../kubernetes/op/job/KubectlJobExecutor.java | 7 +- .../security/KubernetesCredentials.java | 5 +- .../KubernetesManifestStrategyTest.java | 8 ++ .../kubernetes/op/handler/CanDeployTest.java | 10 ++ 7 files changed, 144 insertions(+), 4 deletions(-) diff --git a/clouddriver-kubernetes/src/integration/java/com/netflix/spinnaker/clouddriver/kubernetes/it/DeployManifestIT.java b/clouddriver-kubernetes/src/integration/java/com/netflix/spinnaker/clouddriver/kubernetes/it/DeployManifestIT.java index 9075054887a..b1f6b879d40 100644 --- a/clouddriver-kubernetes/src/integration/java/com/netflix/spinnaker/clouddriver/kubernetes/it/DeployManifestIT.java +++ b/clouddriver-kubernetes/src/integration/java/com/netflix/spinnaker/clouddriver/kubernetes/it/DeployManifestIT.java @@ -1785,4 +1785,113 @@ public void shouldUseSourceCapacityVersioned() throws IOException, InterruptedEx + " -o=jsonpath='{.spec.template.spec.containers[0].env[0].value}'"); assertEquals("test", envVarValue, "Expected update env var for " + appName + " replicaset.\n"); } + + @DisplayName( + ".\n===\n" + + "Given a deployment manifest with server-side-apply strategy set\n" + + "When sending deploy manifest request\n" + + "Then a deployment is created using server-side apply\n===") + @Test + public void shouldDeployUsingServerSideApply() throws IOException, InterruptedException { + // ------------------------- given -------------------------- + String appName = "server-side-apply"; + System.out.println("> Using namespace: " + account1Ns + ", appName: " + appName); + List> manifest = + KubeTestUtils.loadYaml("classpath:manifests/deployment.yml") + .withValue("metadata.namespace", account1Ns) + .withValue("metadata.name", DEPLOYMENT_1_NAME) + .withValue( + "metadata.annotations", + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "true")) + .asList(); + + // ------------------------- when -------------------------- + List> body = + KubeTestUtils.loadJson("classpath:requests/deploy_manifest.json") + .withValue("deployManifest.account", ACCOUNT1_NAME) + .withValue("deployManifest.moniker.app", appName) + .withValue("deployManifest.manifests", manifest) + .asList(); + KubeTestUtils.deployAndWaitStable( + baseUrl(), body, account1Ns, "deployment " + DEPLOYMENT_1_NAME); + + // ------------------------- then -------------------------- + /* Expecting: + metadata: + managedFields: + - manager: kubectl + operation: Apply + fieldsType: FieldsV1 + */ + String managedFields = + kubeCluster.execKubectl( + "-n " + + account1Ns + + " get deployment " + + DEPLOYMENT_1_NAME + + " -o=jsonpath='{.metadata.managedFields}'"); + assertTrue( + Strings.isNotEmpty(managedFields), + "Expected managedFields for " + + DEPLOYMENT_1_NAME + + " deployment to exist and be managed server-side. managedFields:\n" + + managedFields); + + String applyManager = + kubeCluster.execKubectl( + "-n " + + account1Ns + + " get deployment " + + DEPLOYMENT_1_NAME + + " -o=jsonpath='{.metadata.managedFields[?(@.operation==\"Apply\")].manager}'"); + assertEquals( + "kubectl", + applyManager, + "Expected apply manager for " + + DEPLOYMENT_1_NAME + + " deployment to be managed server-side. managedFields:\n" + + managedFields); + } + + @DisplayName( + ".\n===\n" + + "Given a deployment manifest without a strategy set\n" + + "When sending deploy manifest request\n" + + "Then a deployment is created using client-side apply\n===") + @Test + public void shouldDeployUsingClientApply() throws IOException, InterruptedException { + // ------------------------- given -------------------------- + String appName = "client-side-apply"; + System.out.println("> Using namespace: " + account1Ns + ", appName: " + appName); + List> manifest = + KubeTestUtils.loadYaml("classpath:manifests/deployment.yml") + .withValue("metadata.namespace", account1Ns) + .withValue("metadata.name", DEPLOYMENT_1_NAME) + .asList(); + + // ------------------------- when -------------------------- + List> body = + KubeTestUtils.loadJson("classpath:requests/deploy_manifest.json") + .withValue("deployManifest.account", ACCOUNT1_NAME) + .withValue("deployManifest.moniker.app", appName) + .withValue("deployManifest.manifests", manifest) + .asList(); + KubeTestUtils.deployAndWaitStable( + baseUrl(), body, account1Ns, "deployment " + DEPLOYMENT_1_NAME); + + // ------------------------- then -------------------------- + String lastAppliedConfiguration = + kubeCluster.execKubectl( + "-n " + + account1Ns + + " get deployment " + + DEPLOYMENT_1_NAME + + " -o=jsonpath='{.metadata.annotations.kubectl\\.kubernetes\\.io/last-applied-configuration}'"); + assertTrue( + Strings.isNotEmpty(lastAppliedConfiguration), + "Expected last-applied-configuration for " + + DEPLOYMENT_1_NAME + + " deployment to exist and be managed client-side. fields:\n" + + lastAppliedConfiguration); + } } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategy.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategy.java index 79809bbbdb1..327ca7bef34 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategy.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategy.java @@ -107,7 +107,8 @@ ImmutableMap toAnnotations() { public enum DeployStrategy { APPLY(null), RECREATE(STRATEGY_ANNOTATION_PREFIX + "/recreate"), - REPLACE(STRATEGY_ANNOTATION_PREFIX + "/replace"); + REPLACE(STRATEGY_ANNOTATION_PREFIX + "/replace"), + SERVER_SIDE_APPLY(STRATEGY_ANNOTATION_PREFIX + "/server-side-apply"); @Nullable private final String annotation; @@ -122,6 +123,9 @@ static DeployStrategy fromAnnotations(Map annotations) { if (Boolean.parseBoolean(annotations.get(REPLACE.annotation))) { return REPLACE; } + if (Boolean.parseBoolean(annotations.get(SERVER_SIDE_APPLY.annotation))) { + return SERVER_SIDE_APPLY; + } return APPLY; } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeploy.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeploy.java index 84f80769e31..4af67d6c8f6 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeploy.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeploy.java @@ -59,6 +59,9 @@ default OperationResult deploy( case REPLACE: deployedManifest = credentials.createOrReplace(manifest, task, opName); break; + case SERVER_SIDE_APPLY: + deployedManifest = credentials.deploy(manifest, task, opName, "--server-side"); + break; case APPLY: deployedManifest = credentials.deploy(manifest, task, opName); break; diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/job/KubectlJobExecutor.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/job/KubectlJobExecutor.java index 3ffe7d9a32c..7032ca4cf35 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/job/KubectlJobExecutor.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/job/KubectlJobExecutor.java @@ -587,12 +587,17 @@ public ImmutableList list( } public KubernetesManifest deploy( - KubernetesCredentials credentials, KubernetesManifest manifest, Task task, String opName) { + KubernetesCredentials credentials, + KubernetesManifest manifest, + Task task, + String opName, + String... cmdArgs) { log.info("Deploying manifest {}", manifest.getFullResourceName()); List command = kubectlAuthPrefix(credentials); // Read from stdin command.add("apply"); + command.addAll(List.of(cmdArgs)); command.add("-o"); command.add("json"); command.add("-f"); diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java index 07e061521fa..ede2c1c299e 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/security/KubernetesCredentials.java @@ -561,12 +561,13 @@ public Collection topPod(KubernetesCoordinates coords) { () -> jobExecutor.topPod(this, coords.getNamespace(), coords.getName())); } - public KubernetesManifest deploy(KubernetesManifest manifest, Task task, String opName) { + public KubernetesManifest deploy( + KubernetesManifest manifest, Task task, String opName, String... cmdArgs) { return runAndRecordMetrics( "deploy", manifest.getKind(), manifest.getNamespace(), - () -> jobExecutor.deploy(this, manifest, task, opName)); + () -> jobExecutor.deploy(this, manifest, task, opName, cmdArgs)); } private KubernetesManifest replace(KubernetesManifest manifest, Task task, String opName) { diff --git a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategyTest.java b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategyTest.java index 64bd00c6355..2c2dd99d215 100644 --- a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategyTest.java +++ b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategyTest.java @@ -64,6 +64,14 @@ void replaceStrategy() { assertThat(strategy).isEqualTo(DeployStrategy.REPLACE); } + @Test + void serverSideApplyStrategy() { + KubernetesManifestStrategy.DeployStrategy strategy = + KubernetesManifestStrategy.DeployStrategy.fromAnnotations( + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "true")); + assertThat(strategy).isEqualTo(DeployStrategy.SERVER_SIDE_APPLY); + } + @Test void nonBooleanValue() { KubernetesManifestStrategy.DeployStrategy strategy = diff --git a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeployTest.java b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeployTest.java index f052311e562..c0d10b2dc38 100644 --- a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeployTest.java +++ b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeployTest.java @@ -61,6 +61,16 @@ void applyReturnValue() { assertThat(result.getManifests()).containsExactlyInAnyOrder(manifest); } + @Test + void applyServerSideMutations() { + KubernetesCredentials credentials = mock(KubernetesCredentials.class); + KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); + when(credentials.deploy(manifest, task, OP_NAME, "--server-side")).thenReturn(manifest); + handler.deploy(credentials, manifest, DeployStrategy.SERVER_SIDE_APPLY, task, OP_NAME); + verify(credentials).deploy(manifest, task, OP_NAME, "--server-side"); + verifyNoMoreInteractions(credentials); + } + @Test void replaceMutations() { KubernetesCredentials credentials = mock(KubernetesCredentials.class); From 4b5dad6a7ccf7e0331473ad3e0f3de26eb86d12d Mon Sep 17 00:00:00 2001 From: Amir Alavi Date: Thu, 16 Nov 2023 22:19:44 -0500 Subject: [PATCH 2/2] server-side-apply: support force-conflicts Signed-off-by: Amir Alavi --- .../kubernetes/it/DeployManifestIT.java | 48 +++++++++- .../manifest/KubernetesManifestStrategy.java | 47 ++++++++- .../kubernetes/op/handler/CanDeploy.java | 12 ++- .../KubernetesDeployManifestOperation.java | 7 +- .../KubernetesManifestStrategyTest.java | 62 +++++++++++- .../kubernetes/op/handler/CanDeployTest.java | 96 ++++++++++++++++--- 6 files changed, 254 insertions(+), 18 deletions(-) diff --git a/clouddriver-kubernetes/src/integration/java/com/netflix/spinnaker/clouddriver/kubernetes/it/DeployManifestIT.java b/clouddriver-kubernetes/src/integration/java/com/netflix/spinnaker/clouddriver/kubernetes/it/DeployManifestIT.java index b1f6b879d40..ee67bf0f30c 100644 --- a/clouddriver-kubernetes/src/integration/java/com/netflix/spinnaker/clouddriver/kubernetes/it/DeployManifestIT.java +++ b/clouddriver-kubernetes/src/integration/java/com/netflix/spinnaker/clouddriver/kubernetes/it/DeployManifestIT.java @@ -1802,7 +1802,7 @@ public void shouldDeployUsingServerSideApply() throws IOException, InterruptedEx .withValue("metadata.name", DEPLOYMENT_1_NAME) .withValue( "metadata.annotations", - ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "true")) + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "force-conflicts")) .asList(); // ------------------------- when -------------------------- @@ -1853,6 +1853,52 @@ public void shouldDeployUsingServerSideApply() throws IOException, InterruptedEx + managedFields); } + @DisplayName( + ".\n===\n" + + "Given a deployment manifest with server-side-apply disabled set\n" + + "When sending deploy manifest request\n" + + "Then a deployment is created using client-side apply\n===") + @Test + public void shouldDeployUsingApplyWithServerSideApplyDisabled() + throws IOException, InterruptedException { + // ------------------------- given -------------------------- + String appName = "server-side-apply-disabled"; + System.out.println("> Using namespace: " + account1Ns + ", appName: " + appName); + List> manifest = + KubeTestUtils.loadYaml("classpath:manifests/deployment.yml") + .withValue("metadata.namespace", account1Ns) + .withValue("metadata.name", DEPLOYMENT_1_NAME) + .withValue( + "metadata.annotations", + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "false")) + .asList(); + + // ------------------------- when -------------------------- + List> body = + KubeTestUtils.loadJson("classpath:requests/deploy_manifest.json") + .withValue("deployManifest.account", ACCOUNT1_NAME) + .withValue("deployManifest.moniker.app", appName) + .withValue("deployManifest.manifests", manifest) + .asList(); + KubeTestUtils.deployAndWaitStable( + baseUrl(), body, account1Ns, "deployment " + DEPLOYMENT_1_NAME); + + // ------------------------- then -------------------------- + String lastAppliedConfiguration = + kubeCluster.execKubectl( + "-n " + + account1Ns + + " get deployment " + + DEPLOYMENT_1_NAME + + " -o=jsonpath='{.metadata.annotations.kubectl\\.kubernetes\\.io/last-applied-configuration}'"); + assertTrue( + Strings.isNotEmpty(lastAppliedConfiguration), + "Expected last-applied-configuration for " + + DEPLOYMENT_1_NAME + + " deployment to exist and be managed client-side. fields:\n" + + lastAppliedConfiguration); + } + @DisplayName( ".\n===\n" + "Given a deployment manifest without a strategy set\n" diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategy.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategy.java index 327ca7bef34..419ce154f59 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategy.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategy.java @@ -40,10 +40,15 @@ public final class KubernetesManifestStrategy { private static final String USE_SOURCE_CAPACITY = STRATEGY_ANNOTATION_PREFIX + "/use-source-capacity"; + private static final String SERVER_SIDE_APPLY_STRATEGY = + STRATEGY_ANNOTATION_PREFIX + "/server-side-apply"; + private static final String SERVER_SIDE_APPLY_FORCE_CONFLICTS = "force-conflicts"; + private final DeployStrategy deployStrategy; private final Versioned versioned; private final OptionalInt maxVersionHistory; private final boolean useSourceCapacity; + private final ServerSideApplyStrategy serverSideApplyStrategy; @Builder @ParametersAreNullableByDefault @@ -51,18 +56,22 @@ private KubernetesManifestStrategy( DeployStrategy deployStrategy, Versioned versioned, Integer maxVersionHistory, - boolean useSourceCapacity) { + boolean useSourceCapacity, + ServerSideApplyStrategy serverSideApplyStrategy) { this.deployStrategy = Optional.ofNullable(deployStrategy).orElse(DeployStrategy.APPLY); this.versioned = Optional.ofNullable(versioned).orElse(Versioned.DEFAULT); this.maxVersionHistory = maxVersionHistory == null ? OptionalInt.empty() : OptionalInt.of(maxVersionHistory); this.useSourceCapacity = useSourceCapacity; + this.serverSideApplyStrategy = + Optional.ofNullable(serverSideApplyStrategy).orElse(ServerSideApplyStrategy.DEFAULT); } static KubernetesManifestStrategy fromAnnotations(Map annotations) { return KubernetesManifestStrategy.builder() .versioned(Versioned.fromAnnotations(annotations)) .deployStrategy(DeployStrategy.fromAnnotations(annotations)) + .serverSideApplyStrategy(ServerSideApplyStrategy.fromAnnotations(annotations)) .useSourceCapacity(Boolean.parseBoolean(annotations.get(USE_SOURCE_CAPACITY))) .maxVersionHistory(Ints.tryParse(annotations.getOrDefault(MAX_VERSION_HISTORY, ""))) .build(); @@ -72,6 +81,7 @@ ImmutableMap toAnnotations() { ImmutableMap.Builder builder = ImmutableMap.builder(); builder.putAll(deployStrategy.toAnnotations()); builder.putAll(versioned.toAnnotations()); + builder.putAll(serverSideApplyStrategy.toAnnotations()); if (maxVersionHistory.isPresent()) { builder.put(MAX_VERSION_HISTORY, Integer.toString(maxVersionHistory.getAsInt())); } @@ -108,7 +118,7 @@ public enum DeployStrategy { APPLY(null), RECREATE(STRATEGY_ANNOTATION_PREFIX + "/recreate"), REPLACE(STRATEGY_ANNOTATION_PREFIX + "/replace"), - SERVER_SIDE_APPLY(STRATEGY_ANNOTATION_PREFIX + "/server-side-apply"); + SERVER_SIDE_APPLY(SERVER_SIDE_APPLY_STRATEGY); @Nullable private final String annotation; @@ -123,7 +133,9 @@ static DeployStrategy fromAnnotations(Map annotations) { if (Boolean.parseBoolean(annotations.get(REPLACE.annotation))) { return REPLACE; } - if (Boolean.parseBoolean(annotations.get(SERVER_SIDE_APPLY.annotation))) { + if (annotations.containsKey(SERVER_SIDE_APPLY.annotation) + && ServerSideApplyStrategy.fromAnnotations(annotations) + != ServerSideApplyStrategy.DISABLED) { return SERVER_SIDE_APPLY; } return APPLY; @@ -146,4 +158,33 @@ void setAnnotations(Map annotations) { annotations.putAll(toAnnotations()); } } + + public enum ServerSideApplyStrategy { + FORCE_CONFLICTS(ImmutableMap.of(SERVER_SIDE_APPLY_STRATEGY, SERVER_SIDE_APPLY_FORCE_CONFLICTS)), + DISABLED(ImmutableMap.of(SERVER_SIDE_APPLY_STRATEGY, Boolean.FALSE.toString())), + DEFAULT(ImmutableMap.of()); + private final ImmutableMap annotations; + + ServerSideApplyStrategy(ImmutableMap annotations) { + this.annotations = annotations; + } + + static ServerSideApplyStrategy fromAnnotations(Map annotations) { + if (annotations.containsKey(SERVER_SIDE_APPLY_STRATEGY)) { + String strategy = annotations.get(SERVER_SIDE_APPLY_STRATEGY); + if (Boolean.parseBoolean(strategy)) { + return DEFAULT; + } + + if (strategy.equals(SERVER_SIDE_APPLY_FORCE_CONFLICTS)) { + return FORCE_CONFLICTS; + } + } + return DISABLED; + } + + ImmutableMap toAnnotations() { + return annotations; + } + } } diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeploy.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeploy.java index 4af67d6c8f6..8932c7e0310 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeploy.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeploy.java @@ -25,12 +25,15 @@ import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials; import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesSelectorList; import io.kubernetes.client.openapi.models.V1DeleteOptions; +import java.util.ArrayList; +import java.util.List; public interface CanDeploy { default OperationResult deploy( KubernetesCredentials credentials, KubernetesManifest manifest, KubernetesManifestStrategy.DeployStrategy deployStrategy, + KubernetesManifestStrategy.ServerSideApplyStrategy serverSideApplyStrategy, Task task, String opName) { // If the manifest has a generateName, we must apply with kubectl create as all other operations @@ -60,7 +63,14 @@ default OperationResult deploy( deployedManifest = credentials.createOrReplace(manifest, task, opName); break; case SERVER_SIDE_APPLY: - deployedManifest = credentials.deploy(manifest, task, opName, "--server-side"); + List cmdArgs = new ArrayList<>(); + cmdArgs.add("--server-side=true"); + if (serverSideApplyStrategy.equals( + KubernetesManifestStrategy.ServerSideApplyStrategy.FORCE_CONFLICTS)) { + cmdArgs.add("--force-conflicts=true"); + } + deployedManifest = + credentials.deploy(manifest, task, opName, cmdArgs.toArray(new String[cmdArgs.size()])); break; case APPLY: deployedManifest = credentials.deploy(manifest, task, opName); diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/manifest/KubernetesDeployManifestOperation.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/manifest/KubernetesDeployManifestOperation.java index ebfe105c360..f735e31b511 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/manifest/KubernetesDeployManifestOperation.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/manifest/KubernetesDeployManifestOperation.java @@ -158,7 +158,12 @@ public OperationResult operate(List _unused) { + " to kubernetes master..."); result.merge( deployer.deploy( - credentials, holder.manifest, strategy.getDeployStrategy(), getTask(), OP_NAME)); + credentials, + holder.manifest, + strategy.getDeployStrategy(), + strategy.getServerSideApplyStrategy(), + getTask(), + OP_NAME)); result.getCreatedArtifacts().add(holder.artifact); getTask() diff --git a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategyTest.java b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategyTest.java index 2c2dd99d215..d4b89a8b946 100644 --- a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategyTest.java +++ b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestStrategyTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableMap; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifestStrategy.DeployStrategy; +import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifestStrategy.ServerSideApplyStrategy; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifestStrategy.Versioned; import java.util.HashMap; import java.util.Map; @@ -65,13 +66,62 @@ void replaceStrategy() { } @Test - void serverSideApplyStrategy() { + void deployStrategysSrverSideApplyForce() { + KubernetesManifestStrategy.DeployStrategy strategy = + KubernetesManifestStrategy.DeployStrategy.fromAnnotations( + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "force-conflicts")); + assertThat(strategy).isEqualTo(DeployStrategy.SERVER_SIDE_APPLY); + } + + @Test + void deployStrategyServerSideApplyDefault() { KubernetesManifestStrategy.DeployStrategy strategy = KubernetesManifestStrategy.DeployStrategy.fromAnnotations( ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "true")); assertThat(strategy).isEqualTo(DeployStrategy.SERVER_SIDE_APPLY); } + @Test + void deployStrategyServerSideApplyDisabled() { + KubernetesManifestStrategy.DeployStrategy strategy = + KubernetesManifestStrategy.DeployStrategy.fromAnnotations( + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "false")); + assertThat(strategy).isEqualTo(DeployStrategy.APPLY); + } + + @Test + void serverSideApplyStrategyForceConflict() { + KubernetesManifestStrategy.ServerSideApplyStrategy conflictResolution = + KubernetesManifestStrategy.ServerSideApplyStrategy.fromAnnotations( + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "force-conflicts")); + assertThat(conflictResolution) + .isEqualTo(KubernetesManifestStrategy.ServerSideApplyStrategy.FORCE_CONFLICTS); + } + + @Test + void serverSideApplyStrategyDefault() { + KubernetesManifestStrategy.ServerSideApplyStrategy conflictResolution = + KubernetesManifestStrategy.ServerSideApplyStrategy.fromAnnotations( + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "true")); + assertThat(conflictResolution).isEqualTo(ServerSideApplyStrategy.DEFAULT); + } + + @Test + void serverSideApplyStrategyDisabled() { + KubernetesManifestStrategy.ServerSideApplyStrategy conflictResolution = + KubernetesManifestStrategy.ServerSideApplyStrategy.fromAnnotations( + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "false")); + assertThat(conflictResolution).isEqualTo(ServerSideApplyStrategy.DISABLED); + } + + @Test + void serverSideApplyStrategyInvalidValue() { + KubernetesManifestStrategy.ServerSideApplyStrategy conflictResolution = + KubernetesManifestStrategy.ServerSideApplyStrategy.fromAnnotations( + ImmutableMap.of("strategy.spinnaker.io/server-side-apply", "zzzz")); + assertThat(conflictResolution).isEqualTo(ServerSideApplyStrategy.DISABLED); + } + @Test void nonBooleanValue() { KubernetesManifestStrategy.DeployStrategy strategy = @@ -90,6 +140,16 @@ void recreatePreferredOverReplace() { assertThat(strategy).isEqualTo(DeployStrategy.RECREATE); } + @Test + void replacePreferredOverServerSideApply() { + KubernetesManifestStrategy.DeployStrategy strategy = + KubernetesManifestStrategy.DeployStrategy.fromAnnotations( + ImmutableMap.of( + "strategy.spinnaker.io/replace", "true", + "strategy.spinnaker.io/server-side-apply", "true")); + assertThat(strategy).isEqualTo(DeployStrategy.REPLACE); + } + @Test void applyToAnnotations() { Map annotations = DeployStrategy.APPLY.toAnnotations(); diff --git a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeployTest.java b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeployTest.java index c0d10b2dc38..3df195add8f 100644 --- a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeployTest.java +++ b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/handler/CanDeployTest.java @@ -30,6 +30,7 @@ import com.netflix.spinnaker.clouddriver.data.task.Task; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifestStrategy.DeployStrategy; +import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifestStrategy.ServerSideApplyStrategy; import com.netflix.spinnaker.clouddriver.kubernetes.op.OperationResult; import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials; import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesSelectorList; @@ -46,7 +47,13 @@ void applyMutations() { KubernetesCredentials credentials = mock(KubernetesCredentials.class); KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); when(credentials.deploy(manifest, task, OP_NAME)).thenReturn(manifest); - handler.deploy(credentials, manifest, DeployStrategy.APPLY, task, OP_NAME); + handler.deploy( + credentials, + manifest, + DeployStrategy.APPLY, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); verify(credentials).deploy(manifest, task, OP_NAME); verifyNoMoreInteractions(credentials); } @@ -57,7 +64,13 @@ void applyReturnValue() { KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); when(credentials.deploy(manifest, task, OP_NAME)).thenReturn(manifest); OperationResult result = - handler.deploy(credentials, manifest, DeployStrategy.APPLY, task, OP_NAME); + handler.deploy( + credentials, + manifest, + DeployStrategy.APPLY, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); assertThat(result.getManifests()).containsExactlyInAnyOrder(manifest); } @@ -65,9 +78,34 @@ void applyReturnValue() { void applyServerSideMutations() { KubernetesCredentials credentials = mock(KubernetesCredentials.class); KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); - when(credentials.deploy(manifest, task, OP_NAME, "--server-side")).thenReturn(manifest); - handler.deploy(credentials, manifest, DeployStrategy.SERVER_SIDE_APPLY, task, OP_NAME); - verify(credentials).deploy(manifest, task, OP_NAME, "--server-side"); + when(credentials.deploy(manifest, task, OP_NAME, "--server-side=true")).thenReturn(manifest); + handler.deploy( + credentials, + manifest, + DeployStrategy.SERVER_SIDE_APPLY, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); + verify(credentials).deploy(manifest, task, OP_NAME, "--server-side=true"); + verifyNoMoreInteractions(credentials); + } + + @Test + void applyServerSideForceConflictMutations() { + KubernetesCredentials credentials = mock(KubernetesCredentials.class); + KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); + when(credentials.deploy( + manifest, task, OP_NAME, "--server-side=true", "--force-conflicts=true")) + .thenReturn(manifest); + handler.deploy( + credentials, + manifest, + DeployStrategy.SERVER_SIDE_APPLY, + ServerSideApplyStrategy.FORCE_CONFLICTS, + task, + OP_NAME); + verify(credentials) + .deploy(manifest, task, OP_NAME, "--server-side=true", "--force-conflicts=true"); verifyNoMoreInteractions(credentials); } @@ -76,7 +114,13 @@ void replaceMutations() { KubernetesCredentials credentials = mock(KubernetesCredentials.class); KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); when(credentials.createOrReplace(manifest, task, OP_NAME)).thenReturn(manifest); - handler.deploy(credentials, manifest, DeployStrategy.REPLACE, task, OP_NAME); + handler.deploy( + credentials, + manifest, + DeployStrategy.REPLACE, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); verify(credentials).createOrReplace(manifest, task, OP_NAME); verifyNoMoreInteractions(credentials); } @@ -87,7 +131,13 @@ void replaceReturnValue() { KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); when(credentials.createOrReplace(manifest, task, OP_NAME)).thenReturn(manifest); OperationResult result = - handler.deploy(credentials, manifest, DeployStrategy.REPLACE, task, OP_NAME); + handler.deploy( + credentials, + manifest, + DeployStrategy.REPLACE, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); assertThat(result.getManifests()).containsExactlyInAnyOrder(manifest); } @@ -96,7 +146,13 @@ void recreateMutations() { KubernetesCredentials credentials = mock(KubernetesCredentials.class); KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); when(credentials.deploy(manifest, task, OP_NAME)).thenReturn(manifest); - handler.deploy(credentials, manifest, DeployStrategy.RECREATE, task, OP_NAME); + handler.deploy( + credentials, + manifest, + DeployStrategy.RECREATE, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); verify(credentials).deploy(manifest, task, OP_NAME); verify(credentials) .delete( @@ -116,7 +172,13 @@ void recreateReturnValue() { KubernetesManifest manifest = ManifestFetcher.getManifest("candeploy/deployment.yml"); when(credentials.deploy(manifest, task, OP_NAME)).thenReturn(manifest); OperationResult result = - handler.deploy(credentials, manifest, DeployStrategy.RECREATE, task, OP_NAME); + handler.deploy( + credentials, + manifest, + DeployStrategy.RECREATE, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); assertThat(result.getManifests()).containsExactlyInAnyOrder(manifest); } @@ -128,7 +190,13 @@ void createMutation() { KubernetesManifest createResult = ManifestFetcher.getManifest("candeploy/deployment-generate-name-result.yml"); when(credentials.create(manifest, task, OP_NAME)).thenReturn(createResult); - handler.deploy(credentials, manifest, DeployStrategy.APPLY, task, OP_NAME); + handler.deploy( + credentials, + manifest, + DeployStrategy.APPLY, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); verify(credentials).create(manifest, task, OP_NAME); verifyNoMoreInteractions(credentials); } @@ -142,7 +210,13 @@ void createReturnValue() { ManifestFetcher.getManifest("candeploy/deployment-generate-name-result.yml"); when(credentials.create(manifest, task, OP_NAME)).thenReturn(createResult); OperationResult result = - handler.deploy(credentials, manifest, DeployStrategy.APPLY, task, OP_NAME); + handler.deploy( + credentials, + manifest, + DeployStrategy.APPLY, + ServerSideApplyStrategy.DEFAULT, + task, + OP_NAME); assertThat(result.getManifests()).containsExactlyInAnyOrder(createResult); } }