Fix no error message when creating duplicate Quick resources (#16)

* fixes #15 * made changes according to the PR feedback * changed confusing test names * extended QuickResources interface to easily get resources name+some other PR-changes * feedback changes: missing final, missing docu, output msg adjusted * docs corrected, use of constants in tests
bakdata · May 9, 2022 · f4c49c3 · f4c49c3
1 parent 27d488d
commit f4c49c3
Show file tree

Hide file tree

Showing 14 changed files with 109 additions and 21 deletions.
diff --git a/...ger/src/main/java/com/bakdata/quick/manager/application/KubernetesApplicationService.java b/...ger/src/main/java/com/bakdata/quick/manager/application/KubernetesApplicationService.java
@@ -79,7 +79,7 @@ public Completable deleteApplication(final String name) {
             .map(serviceExists -> {
                 final ApplicationResources applicationResources = this.loader.forDeletion(deploymentName);
                 if (!serviceExists) {
-                    return new ApplicationResources(applicationResources.getDeployment(), Optional.empty());
+                    return new ApplicationResources(name, applicationResources.getDeployment(), Optional.empty());
                 }
                 return applicationResources;
             })

diff --git a/.../main/java/com/bakdata/quick/manager/application/resources/ApplicationResourceLoader.java b/.../main/java/com/bakdata/quick/manager/application/resources/ApplicationResourceLoader.java
@@ -77,7 +77,8 @@ public ApplicationResources forCreation(final ApplicationCreationData appCreatio
             Objects.requireNonNullElse(appCreationData.getReplicas(), this.deploymentConfig.getDefaultReplicas());
         final ImageConfig config = ImageConfig.of(appCreationData.getRegistry(), appCreationData.getImageName(),
             replicas, appCreationData.getTag());
-        final String deploymentName = getDeploymentName(appCreationData.getName());
+        final String applicationName = appCreationData.getName();
+        final String deploymentName = getDeploymentName(applicationName);
         final Map<String, String> arguments = Objects.requireNonNullElse(appCreationData.getArguments(), Map.of());
         final List<String> listArgs = CliArgHandler.convertArgs(arguments, this.kafkaConfig);
 
@@ -88,9 +89,9 @@ public ApplicationResources forCreation(final ApplicationCreationData appCreatio
         if (appCreationData.getPort() != null) {
             final ApplicationService service =
                 new ApplicationService(this.createApplicationService(deploymentName, appCreationData.getPort()));
-            return new ApplicationResources(deployment, Optional.of(service));
+            return new ApplicationResources(applicationName, deployment, Optional.of(service));
         }
-        return new ApplicationResources(deployment, Optional.empty());
+        return new ApplicationResources(applicationName, deployment, Optional.empty());
     }
 
     /**
@@ -109,7 +110,7 @@ public ApplicationResources forDeletion(final String name) {
         final ApplicationService service =
             new ApplicationService(KubernetesResources.forDeletion(Service.class, name));
 
-        return new ApplicationResources(deployment, Optional.of(service));
+        return new ApplicationResources(name, deployment, Optional.of(service));
     }
 
     /**

diff --git a/...r/src/main/java/com/bakdata/quick/manager/application/resources/ApplicationResources.java b/...r/src/main/java/com/bakdata/quick/manager/application/resources/ApplicationResources.java
@@ -32,12 +32,15 @@
  */
 public class ApplicationResources implements QuickResources {
 
+    private final String name;
     @Getter
     private final ApplicationDeployment deployment;
     private final Optional<ApplicationService> service;
 
-    public ApplicationResources(final ApplicationDeployment deployment,
-        final Optional<ApplicationService> service) {
+    public ApplicationResources(final String name,
+                                final ApplicationDeployment deployment,
+                                final Optional<ApplicationService> service) {
+        this.name = name;
         this.deployment = deployment;
         this.service = service;
     }
@@ -47,4 +50,9 @@ public List<QuickResource> listResources() {
         return this.service.map(applicationService -> List.of(this.deployment, applicationService))
             .orElseGet(() -> List.of(this.deployment));
     }
+
+    @Override
+    public String getResourcesName() {
+        return this.name;
+    }
 }
diff --git a/manager/src/main/java/com/bakdata/quick/manager/gateway/resource/GatewayResourceLoader.java b/manager/src/main/java/com/bakdata/quick/manager/gateway/resource/GatewayResourceLoader.java
@@ -105,7 +105,8 @@ public GatewayResources forCreation(final GatewayCreationData gatewayCreationDat
 
         final String dockerRegistry = this.deploymentConfig.getDockerRegistry();
         final ImageConfig imageConfig = ImageConfig.of(dockerRegistry, GATEWAY_IMAGE, imageReplicas, imageTag);
-        final String resourceName = getResourceName(gatewayCreationData.getName());
+        final String gatewayName = gatewayCreationData.getName();
+        final String resourceName = getResourceName(gatewayName);
         final boolean hasFixedTag = gatewayCreationData.getTag() != null;
 
         final GatewayDeployment deployment = new GatewayDeployment(
@@ -124,7 +125,7 @@ public GatewayResources forCreation(final GatewayCreationData gatewayCreationDat
         final GatewayConfigMap configMap =
             new GatewayConfigMap(this.createGatewayConfigMap(resourceName, gatewayCreationData.getSchema()));
 
-        return new GatewayResources(deployment, service, ingress, middleware, configMap);
+        return new GatewayResources(gatewayName, deployment, service, ingress, middleware, configMap);
     }
 
     /**
@@ -150,7 +151,7 @@ public GatewayResources forDeletion(final String name) {
         final GatewayConfigMap configMap =
             new GatewayConfigMap(KubernetesResources.forDeletion(ConfigMap.class, getConfigMapName(name)));
 
-        return new GatewayResources(deployment, service, ingress, middleware, configMap);
+        return new GatewayResources(name, deployment, service, ingress, middleware, configMap);
     }
 
     /**

diff --git a/manager/src/main/java/com/bakdata/quick/manager/gateway/resource/GatewayResources.java b/manager/src/main/java/com/bakdata/quick/manager/gateway/resource/GatewayResources.java
@@ -45,6 +45,7 @@ public class GatewayResources implements QuickResources {
 
     public static final String GATEWAY_IMAGE = "quick-gateway";
 
+    private final String name;
     private final GatewayDeployment deployment;
     private final GatewayService service;
     private final GatewayIngress ingress;
@@ -60,8 +61,10 @@ public class GatewayResources implements QuickResources {
      * @param middleware        For gateway Middleware resource
      * @param configMap         For gateway ConfigMap resource
      */
-    public GatewayResources(final GatewayDeployment gatewayDeployment, final GatewayService gatewayService,
-        final GatewayIngress gatewayIngress, final GatewayMiddleware middleware, final GatewayConfigMap configMap) {
+    public GatewayResources(final String name, final GatewayDeployment gatewayDeployment,
+                            final GatewayService gatewayService, final GatewayIngress gatewayIngress,
+                            final GatewayMiddleware middleware, final GatewayConfigMap configMap) {
+        this.name = name;
         this.deployment = gatewayDeployment;
         this.service = gatewayService;
         this.ingress = gatewayIngress;
@@ -120,4 +123,9 @@ public static Completable handleDefinitionCreationError(final Throwable ex, fina
     public List<QuickResource> listResources() {
         return List.of(this.deployment, this.service, this.ingress, this.middleware, this.configMap);
     }
+
+    @Override
+    public String getResourcesName() {
+        return this.name;
+    }
 }
diff --git a/manager/src/main/java/com/bakdata/quick/manager/k8s/KubernetesManagerClient.java b/manager/src/main/java/com/bakdata/quick/manager/k8s/KubernetesManagerClient.java
@@ -36,6 +36,7 @@
 import io.reactivex.Single;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -79,12 +80,19 @@ public Completable delete(final QuickResources quickResources) {
      * Deploys all the resources passed in.
      *
      * @param quickResources A quick resources object containing one or many kubernetes resources
-     * @return Completable or throws an exception if fail
+     * @return Completable or an exception if the specific resources already exist
      */
     public Completable deploy(final QuickResources quickResources) {
         return Completable.fromCallable(() -> {
             final KubernetesList resourceList = getKubernetesList(quickResources);
-            return this.client.resourceList(resourceList).inNamespace(this.namespace).createOrReplace();
+            final List<HasMetadata> resourcesMetadata = this.client.resourceList(resourceList)
+                    .inNamespace(this.namespace).fromServer().get();
+            if (resourcesMetadata.stream().allMatch(Objects::isNull)) {
+                return this.client.resourceList(resourceList).inNamespace(this.namespace).createOrReplace();
+            } else {
+                final String resourcesName = quickResources.getResourcesName();
+                throw new BadArgumentException(String.format("The resource with the name %s already exists", resourcesName));
+            }
         });
     }
 

diff --git a/manager/src/main/java/com/bakdata/quick/manager/k8s/resource/QuickResources.java b/manager/src/main/java/com/bakdata/quick/manager/k8s/resource/QuickResources.java
@@ -50,4 +50,11 @@ enum ResourcePrefix {
             this.prefix = deploymentPrefix;
         }
     }
+
+    /**
+     * Retrieves the name of resources (without prefix).
+     * @return The name that a user has chosen for a specific type of resources.
+     */
+    String getResourcesName();
+
 }
diff --git a/manager/src/main/java/com/bakdata/quick/manager/mirror/resources/MirrorResourceLoader.java b/manager/src/main/java/com/bakdata/quick/manager/mirror/resources/MirrorResourceLoader.java
@@ -73,7 +73,8 @@ public MirrorResourceLoader(final KubernetesResources kubernetesResources,
      */
     @Override
     public MirrorResources forCreation(final MirrorCreationData mirrorCreationData, final ResourcePrefix prefix) {
-        final String deploymentName = prefix.getPrefix() + mirrorCreationData.getName();
+        final String mirrorName = mirrorCreationData.getName();
+        final String deploymentName = prefix.getPrefix() + mirrorName;
         final String imageTag =
             Objects.requireNonNullElse(mirrorCreationData.getTag(), this.deploymentConfig.getDefaultImageTag());
         final int imageReplicas =
@@ -91,7 +92,7 @@ public MirrorResources forCreation(final MirrorCreationData mirrorCreationData,
 
         final MirrorService service = new MirrorService(this.createMirrorService(deploymentName));
 
-        return new MirrorResources(deployment, service);
+        return new MirrorResources(mirrorName, deployment, service);
     }
 
     /**
@@ -107,7 +108,7 @@ public MirrorResources forDeletion(final String name) {
         final MirrorDeployment deployment =
             new MirrorDeployment(KubernetesResources.forDeletion(Deployment.class, name));
         final MirrorService service = new MirrorService(KubernetesResources.forDeletion(Service.class, name));
-        return new MirrorResources(deployment, service);
+        return new MirrorResources(name, deployment, service);
     }
 
     public static String getDeploymentName(final String name) {

diff --git a/manager/src/main/java/com/bakdata/quick/manager/mirror/resources/MirrorResources.java b/manager/src/main/java/com/bakdata/quick/manager/mirror/resources/MirrorResources.java
@@ -31,11 +31,14 @@
 public class MirrorResources implements QuickResources {
     public static final String MIRROR_IMAGE = "quick-mirror";
 
+    private final String name;
     private final MirrorDeployment deployment;
     private final MirrorService service;
 
-    public MirrorResources(final MirrorDeployment deployment,
-        final MirrorService service) {
+    public MirrorResources(final String name,
+                           final MirrorDeployment deployment,
+                           final MirrorService service) {
+        this.name = name;
         this.deployment = deployment;
         this.service = service;
     }
@@ -44,4 +47,9 @@ public MirrorResources(final MirrorDeployment deployment,
     public List<QuickResource> listResources() {
         return List.of(this.deployment, this.service);
     }
+
+    @Override
+    public String getResourcesName() {
+        return this.name;
+    }
 }
diff --git a/manager/src/test/java/com/bakdata/quick/manager/application/ApplicationControllerTest.java b/manager/src/test/java/com/bakdata/quick/manager/application/ApplicationControllerTest.java
@@ -26,8 +26,8 @@
 import static org.mockito.Mockito.when;
 
 import com.bakdata.quick.common.api.client.ApplicationClient;
-import com.bakdata.quick.common.api.model.manager.creation.ApplicationCreationData;
 import com.bakdata.quick.common.api.model.manager.ApplicationDescription;
+import com.bakdata.quick.common.api.model.manager.creation.ApplicationCreationData;
 import io.micronaut.http.client.RxHttpClient;
 import io.micronaut.http.client.annotation.Client;
 import io.micronaut.http.uri.UriBuilder;

diff --git a/...src/test/java/com/bakdata/quick/manager/application/KubernetesApplicationServiceTest.java b/...src/test/java/com/bakdata/quick/manager/application/KubernetesApplicationServiceTest.java
@@ -20,6 +20,7 @@
 
 import com.bakdata.quick.common.api.model.manager.creation.ApplicationCreationData;
 import com.bakdata.quick.common.config.KafkaConfig;
+import com.bakdata.quick.common.exception.BadArgumentException;
 import com.bakdata.quick.manager.application.resources.ApplicationResourceLoader;
 import com.bakdata.quick.manager.k8s.ImageConfig;
 import com.bakdata.quick.manager.k8s.KubernetesResources;
@@ -134,6 +135,23 @@ void shouldCreateDeletionJob() {
             });
     }
 
+    @Test
+    void shouldRejectDuplicateApplicationCreation() {
+        final ApplicationCreationData applicationCreationData = new ApplicationCreationData(APP_NAME,
+                DOCKER_REGISTRY,
+                IMAGE_NAME,
+                DEFAULT_IMAGE_TAG,
+                1,
+                DEFAULT_PORT,
+                Map.of());
+
+        final Completable firstDeployment = this.service.deployApplication(applicationCreationData);
+        Optional.ofNullable(firstDeployment.blockingGet()).ifPresent(Assertions::fail);
+        final Throwable invalidDeployment = this.service.deployApplication(applicationCreationData).blockingGet();
+        assertThat(invalidDeployment).isInstanceOf(BadArgumentException.class)
+                .hasMessageContaining(String.format("The resource with the name %s already exists", APP_NAME));
+    }
+
     private void deployApplication(@Nullable final Integer port, final Map<String, String> arguments) {
         final ApplicationCreationData applicationCreationData = new ApplicationCreationData(APP_NAME,
             DOCKER_REGISTRY,

diff --git a/manager/src/test/java/com/bakdata/quick/manager/gateway/GatewayControllerTest.java b/manager/src/test/java/com/bakdata/quick/manager/gateway/GatewayControllerTest.java
@@ -29,8 +29,8 @@
 
 import com.bakdata.quick.common.api.client.GatewayClient;
 import com.bakdata.quick.common.api.model.gateway.SchemaData;
-import com.bakdata.quick.common.api.model.manager.creation.GatewayCreationData;
 import com.bakdata.quick.common.api.model.manager.GatewayDescription;
+import com.bakdata.quick.common.api.model.manager.creation.GatewayCreationData;
 import com.bakdata.quick.manager.gateway.GatewayService.SchemaFormat;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;

diff --git a/manager/src/test/java/com/bakdata/quick/manager/gateway/KubernetesGatewayServiceTest.java b/manager/src/test/java/com/bakdata/quick/manager/gateway/KubernetesGatewayServiceTest.java
@@ -22,6 +22,7 @@
 import com.bakdata.quick.common.api.model.gateway.SchemaData;
 import com.bakdata.quick.common.api.model.manager.GatewayDescription;
 import com.bakdata.quick.common.api.model.manager.creation.GatewayCreationData;
+import com.bakdata.quick.common.exception.BadArgumentException;
 import com.bakdata.quick.manager.gateway.resource.GatewayResourceLoader;
 import com.bakdata.quick.manager.graphql.GraphQLToAvroConverter;
 import com.bakdata.quick.manager.k8s.KubernetesResources;
@@ -321,6 +322,16 @@ void shouldDeleteConfigMap() {
         assertThat(this.getConfigMaps()).isNullOrEmpty();
     }
 
+    @Test
+    void shouldRejectDuplicateGatewayCreation() {
+        final GatewayCreationData creationData = new GatewayCreationData(GATEWAY_NAME, 1, null, null);
+        final Throwable firstDeployment = this.gatewayService.createGateway(creationData).blockingGet();
+        assertThat(firstDeployment).isNull();
+        final Throwable invalidDeployment = this.gatewayService.createGateway(creationData).blockingGet();
+        assertThat(invalidDeployment).isInstanceOf(BadArgumentException.class)
+                .hasMessageContaining(String.format("The resource with the name %s already exists", GATEWAY_NAME));
+    }
+
 
     private void deleteGatewayResources() {
         this.gatewayService.deleteGateway(GATEWAY_NAME).blockingAwait();

diff --git a/manager/src/test/java/com/bakdata/quick/manager/mirror/KubernetesMirrorServiceTest.java b/manager/src/test/java/com/bakdata/quick/manager/mirror/KubernetesMirrorServiceTest.java
@@ -18,8 +18,10 @@
 
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 import com.bakdata.quick.common.api.model.manager.creation.MirrorCreationData;
+import com.bakdata.quick.common.exception.BadArgumentException;
 import com.bakdata.quick.manager.k8s.KubernetesResources;
 import com.bakdata.quick.manager.k8s.KubernetesTest;
 import com.bakdata.quick.manager.k8s.resource.QuickResources.ResourcePrefix;
@@ -175,6 +177,21 @@ void shouldCreateDeletionJob() {
             .satisfies(container -> assertThat(container.getArgs()).contains("--input-topics=" + TOPIC_NAME));
     }
 
+    @Test
+    void shouldRejectDuplicateMirrorCreation() {
+        final MirrorCreationData mirrorCreationData = new MirrorCreationData(
+                TOPIC_NAME,
+                TOPIC_NAME,
+                1,
+                null,
+                null);
+        final Throwable firstDeployment = this.mirrorService.createMirror(mirrorCreationData).blockingGet();
+        assertThat(firstDeployment).isNull();
+        final Throwable invalidDeployment = this.mirrorService.createMirror(mirrorCreationData).blockingGet();
+        assertThat(invalidDeployment).isInstanceOf(BadArgumentException.class)
+                .hasMessageContaining(String.format("The resource with the name %s already exists", TOPIC_NAME));
+    }
+
     private void createMirror(final MirrorCreationData mirrorCreationData) {
         final Completable completable = this.mirrorService.createMirror(mirrorCreationData);
         Optional.ofNullable(completable.blockingGet()).ifPresent(Assertions::fail);