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); } }