diff --git a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/manifest/KubernetesDeleteManifestOperation.java b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/manifest/KubernetesDeleteManifestOperation.java index e23c43bd797..1009d7ebb1a 100644 --- a/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/manifest/KubernetesDeleteManifestOperation.java +++ b/clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/manifest/KubernetesDeleteManifestOperation.java @@ -22,6 +22,7 @@ import com.netflix.spinnaker.clouddriver.data.task.TaskRepository; import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesCoordinates; import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesDeleteManifestDescription; +import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind; import com.netflix.spinnaker.clouddriver.kubernetes.op.OperationResult; import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler; import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials; @@ -86,7 +87,15 @@ public OperationResult operate(List priorOutputs) { .updateStatus(OP_NAME, "Looking up resource properties for " + c.getKind() + "..."); KubernetesHandler deployer = credentials.getResourcePropertyRegistry().get(c.getKind()).getHandler(); - getTask().updateStatus(OP_NAME, "Calling delete operation..."); + if (deployer.kind().equals(KubernetesKind.NONE)) { + // No point in invoking kubectl with a resource type of none, as it fails with + // + // error: the server doesn't have a resource type "none" + // + // Throw an exception to fail the stage, similar to what happens when kubectl fails. + throw new IllegalStateException("Unable to delete unknown kind '" + c.getKind() + "'"); + } + getTask().updateStatus(OP_NAME, "Calling delete operation for resource" + c + "..."); result.merge( deployer.delete( credentials, @@ -94,10 +103,12 @@ public OperationResult operate(List priorOutputs) { c.getName(), description.getLabelSelectors(), deleteOptions)); - getTask() - .updateStatus(OP_NAME, " delete operation completed successfully for " + c.getName()); + getTask().updateStatus(OP_NAME, "Delete operation completed successfully for " + c); }); + getTask() + .updateStatus( + OP_NAME, "Delete operation completed successfully for all applicable resources"); return result; } } diff --git a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/KubernetesDeleteManifestOperationTest.java b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/KubernetesDeleteManifestOperationTest.java index a0d9ff286e7..9be019eaead 100644 --- a/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/KubernetesDeleteManifestOperationTest.java +++ b/clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/op/KubernetesDeleteManifestOperationTest.java @@ -17,13 +17,16 @@ package com.netflix.spinnaker.clouddriver.kubernetes.op; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -40,6 +43,7 @@ import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind; import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesCustomResourceDefinitionHandler; import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesCustomResourceHandler; +import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesDeploymentHandler; import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler; import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesReplicaSetHandler; import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesServiceHandler; @@ -75,6 +79,7 @@ public class KubernetesDeleteManifestOperationTest { private static final GlobalResourcePropertyRegistry resourcePropertyRegistry = new GlobalResourcePropertyRegistry( ImmutableList.of( + new KubernetesDeploymentHandler(), new KubernetesReplicaSetHandler(), new KubernetesServiceHandler(), new KubernetesCustomResourceDefinitionHandler()), @@ -227,6 +232,53 @@ public void deleteUnregisteredCustomResourceViaLabelSelector() throws IOExceptio labelSelectorsCaptor.getValue()); } + @Test + public void deleteReallyUnregisteredCustomResourceViaManifestName() throws IOException { + // Beyond not configuring a spinnaker account with a particular kind (which + // is one definition of unregistered), this test exercises what happens when + // spinnaker has no idea about a particular kind, via configuration or + // discovery at runtime. In other words, nothing has called + // resourcePropertyRegistry.updateCrdProperties with information about the + // custom resource this test attempts to delete. + String unknownKind = "unknown.bar.com"; + String pipelineJSON = + "{ " + + " \"account\": \"kubernetes-account\"," + + " \"manifestName\": \"" + + unknownKind + + " my-unknown-custom-resource\"" + + " }"; + + Map pipeline = mapper.readValue(pipelineJSON, mapType); + + KubernetesDeleteManifestDescription description = buildDeleteManifestDescription(pipeline); + + KubernetesCredentials mockKubernetesCreds = description.getCredentials().getCredentials(); + + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, + () -> new KubernetesDeleteManifestOperation(description).operate(ImmutableList.of())); + + assertThat(thrown.getMessage()).contains("Unable to delete unknown kind '" + unknownKind); + + // With no handler for the given kind, GlobalResourcePropertyRegistry + // returns KubernetesUnregisteredCustomResourceHandler which has a kind of + // KubernetesKind.NONE. kubectl always fails with an invocation like: + // + // $ kubectl delete none + // error: the server doesn't have a resource type "none" + // + // so better to not invoke it and give a more helpful error message. + verify(mockKubernetesCreds, never()) + .delete( + any(KubernetesKind.class), + anyString(), + anyString(), + any(KubernetesSelectorList.class), + any(V1DeleteOptions.class)); + } + @Test public void orphanDependentsTrueTest() throws IOException { String pipelineJSON =