You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by iu...@apache.org on 2022/06/27 15:07:49 UTC

[brooklyn-server] branch master updated: Addressed Alex's comments

This is an automated email from the ASF dual-hosted git repository.

iuliana pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git


The following commit(s) were added to refs/heads/master by this push:
     new 7e88d3afee Addressed Alex's comments
     new c852fd77c1 Merge remote-tracking branch 'mine/feature/container-components'
7e88d3afee is described below

commit 7e88d3afee8fc141a89330cf24ade85f17cfae41
Author: iuliana <iu...@cloudsoft.io>
AuthorDate: Mon Jun 27 15:07:28 2022 +0100

    Addressed Alex's comments
---
 .../brooklyn/tasks/kubectl/ContainerCommons.java   | 15 ++--
 .../tasks/kubectl/ContainerTaskFactory.java        | 12 ++-
 .../apache/brooklyn/tasks/kubectl/JobBuilder.java  | 24 ++++--
 .../brooklyn/tasks/kubectl/DockerTaskTest.java     | 89 ++++++++++++++++++++++
 .../brooklyn/tasks/kubectl/JobBuilderTest.java     |  6 +-
 5 files changed, 126 insertions(+), 20 deletions(-)

diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerCommons.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerCommons.java
index ecf2c50d30..e2f5e533cc 100644
--- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerCommons.java
+++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerCommons.java
@@ -20,13 +20,13 @@ package org.apache.brooklyn.tasks.kubectl;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.common.reflect.TypeToken;
 import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.BasicConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.config.SetConfigKey;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 @SuppressWarnings({ "rawtypes"})
 public interface ContainerCommons {
@@ -39,8 +39,11 @@ public interface ContainerCommons {
     ConfigKey<List> ARGUMENTS = ConfigKeys.newConfigKey(List.class,"args", "Arguments to execute for container", Lists.newArrayList());
 
     ConfigKey<String> WORKING_DIR = ConfigKeys.newStringConfigKey("workingDir", "Location where the container commands are executed");
-    ConfigKey<List<Map<String,String>>> VOLUME_MOUNTS = ConfigKeys.newConfigKey("volumeMounts", "Configuration to  mount a volume  into a container.", Lists.newArrayList());
-    ConfigKey<List<Map<String, Object>>> VOLUMES = ConfigKeys.newConfigKey("volumes", "List of directories with data that is accessible across multiple containers", Lists.newArrayList());
+    BasicConfigKey<Map<String,String>> VOLUME_MOUNTS = SetConfigKey.builder(new TypeToken<Map<String,String>>()  {}, "volumeMounts")
+            .description("Configuration to mount a volume into a container.").defaultValue(null).build();
+
+    BasicConfigKey<Map<String,Object>> VOLUMES = SetConfigKey.builder(new TypeToken<Map<String,Object>>()  {}, "volumes")
+            .description("List of directories with data that is accessible across multiple containers").defaultValue(null).build();
 
     String NAMESPACE_CREATE_CMD = "kubectl create namespace brooklyn-%s"; // namespace name
     String NAMESPACE_SET_CMD = "kubectl config set-context --current --namespace=brooklyn-%s"; // namespace name
diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskFactory.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskFactory.java
index 711c7e2390..4c7c6f04b3 100644
--- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskFactory.java
+++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskFactory.java
@@ -35,6 +35,7 @@ import org.apache.brooklyn.util.text.Strings;
 
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static org.apache.brooklyn.tasks.kubectl.ContainerCommons.*;
 
@@ -55,17 +56,19 @@ public class ContainerTaskFactory<T extends ContainerTaskFactory<T,RET>,RET>  im
         Boolean devMode = EntityInitializers.resolve(configBag, KEEP_CONTAINER_FOR_DEBUGGING);
 
         String workingDir = EntityInitializers.resolve(configBag, WORKING_DIR);
-        List<Map<String,String>> volumeMounts = EntityInitializers.resolve(configBag, VOLUME_MOUNTS);
-        List<Map<String, Object>> volumes = EntityInitializers.resolve(configBag, VOLUMES);
+        Set<Map<String,String>> volumeMounts = (Set<Map<String,String>>) EntityInitializers.resolve(configBag, VOLUME_MOUNTS);
+        Set<Map<String, Object>> volumes = (Set<Map<String, Object>>) EntityInitializers.resolve(configBag, VOLUMES);
 
         if(Strings.isBlank(containerImage)) {
             throw new IllegalStateException("You must specify containerImage when using " + this.getClass().getSimpleName());
         }
 
-
         final String containerName = (Strings.isBlank(containerNameFromCfg)
                 ? ( (Strings.isNonBlank(this.tag) ? this.tag + "-" : "").concat(containerImage).concat("-").concat(Strings.makeRandomId(10)))
-                : containerNameFromCfg).replace(" ", "-").replace("/", "-").toLowerCase();
+                : containerNameFromCfg).replace(" ", "-")
+                .replace("/", "-")
+                .replace("_", "-")
+                .toLowerCase();
 
         final String jobYamlLocation =  new JobBuilder()
                 .withImage(containerImage)
@@ -75,6 +78,7 @@ public class ContainerTaskFactory<T extends ContainerTaskFactory<T,RET>,RET>  im
                 .withCommands(Lists.newArrayList(commandsCfg))
                 .withVolumeMounts(volumeMounts)
                 .withVolumes(volumes)
+                .withWorkingDir(workingDir)
                 .build();
 
 
diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/JobBuilder.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/JobBuilder.java
index cbf932117c..7c33ee9fe4 100644
--- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/JobBuilder.java
+++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/JobBuilder.java
@@ -68,22 +68,30 @@ public class JobBuilder {
     }
 
     public JobBuilder withCommands(final List<String> commandsArg){
-        this.commands.addAll(commandsArg);
+        if (commandsArg != null) {
+            this.commands.addAll(commandsArg);
+        }
         return this;
     }
 
     public JobBuilder withArgs(final List<String> args){
-        this.args.addAll(args);
+        if (args != null) {
+            this.args.addAll(args);
+        }
         return this;
     }
 
-    public JobBuilder withVolumeMounts(final List<Map<String,String>> volumeMounts) {
-        this.volumeMounts.addAll(volumeMounts);
+    public JobBuilder withVolumeMounts(final Set<Map<String,String>> volumeMounts) {
+        if (volumeMounts != null) {
+            this.volumeMounts.addAll(volumeMounts);
+        }
         return this;
     }
 
-    public JobBuilder withVolumes(final List<Map<String, Object>> volumes) {
-        this.volumes.addAll(volumes);
+    public JobBuilder withVolumes(final Set<Map<String, Object>> volumes) {
+        if (volumes != null) {
+            this.volumes.addAll(volumes);
+        }
         return this;
     }
 
@@ -98,7 +106,9 @@ public class JobBuilder {
     }
 
     public JobBuilder withEnv(final Map<String,Object> env){
-        this.env.putAll(env);
+        if (env != null) {
+            this.env.putAll(env);
+        }
         return this;
     }
 
diff --git a/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/DockerTaskTest.java b/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/DockerTaskTest.java
new file mode 100644
index 0000000000..31084e8a9e
--- /dev/null
+++ b/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/DockerTaskTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.tasks.kubectl;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.mgmt.Task;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
+import org.apache.brooklyn.core.test.entity.TestEntity;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.time.Duration;
+import org.testng.annotations.Test;
+
+import java.util.HashMap;
+import java.util.List;
+
+import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import static org.testng.AssertJUnit.assertTrue;
+
+public class DockerTaskTest extends BrooklynAppUnitTestSupport {
+
+    @Test
+    public void testSuccessfulDockerTask() {
+        TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+
+        Map<String,Object> configBag = new HashMap<>();
+        configBag.put("name", "test-docker-task");
+        configBag.put("image", "perl");
+        configBag.put("commands", Lists.newArrayList("/bin/bash", "-c","echo 'hello test'"));
+
+        Task<String> dockerTask =  new ContainerTaskFactory.ConcreteContainerTaskFactory<String>()
+                .summary("Running docker task")
+                .configure(configBag)
+                .newTask();
+        DynamicTasks.queueIfPossible(dockerTask).orSubmitAsync(entity);
+        Object result = dockerTask.getUnchecked(Duration.of(5, TimeUnit.MINUTES));
+        List<String> res = (List<String>) result;
+        while(!res.isEmpty() && Iterables.getLast(res).matches("namespace .* deleted\\s*")) res = res.subList(0, res.size()-1);
+
+        String res2 = res.isEmpty() ? null : Iterables.getLast(res);
+        assertTrue(res2.startsWith("hello test"));
+    }
+
+    @Test
+    public void testFailingDockerTask() {
+        TestEntity entity = app.createAndManageChild(EntitySpec.create(TestEntity.class));
+
+        List<String> commands = MutableList.of("/bin/bash", "-c","echo 'hello test'", "exit 1");
+
+        Map<String,Object> configBag = new HashMap<>();
+        configBag.put("name", "test-docker-task");
+        configBag.put("image", "perl");
+        configBag.put("commands", commands);
+
+        Task<String> dockerTask =  new ContainerTaskFactory.ConcreteContainerTaskFactory<String>()
+                .summary("Running docker task")
+                .configure(configBag)
+                .newTask();
+
+        try {
+            DynamicTasks.queueIfPossible(dockerTask).orSubmitAsync(entity);
+        } catch (Exception e) {
+            assertTrue(e.getCause() instanceof ExecutionException);
+            assertTrue(e.getCause().getMessage().contains("Process task ended with exit code 1 when 0 was required"));
+        }
+    }
+
+}
diff --git a/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/JobBuilderTest.java b/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/JobBuilderTest.java
index da37d36d1b..88cd795c48 100644
--- a/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/JobBuilderTest.java
+++ b/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/JobBuilderTest.java
@@ -20,6 +20,7 @@ package org.apache.brooklyn.tasks.kubectl;
 
 import com.beust.jcommander.internal.Maps;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.annotations.Test;
@@ -27,7 +28,6 @@ import org.testng.annotations.Test;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
@@ -124,8 +124,8 @@ public class JobBuilderTest {
         volumes.put("hostPath", Maps.newHashMap("path", "/tfws"));
         String yamlJobLocation =
                 new JobBuilder().withImage("hashicorp/terraform").withName("tf-version")
-                        .withVolumes(Lists.newArrayList(volumes))
-                        .withVolumeMounts(Lists.newArrayList(Maps.newHashMap("name", "tf-ws", "mountPath", "/tfws")))
+                        .withVolumes(Sets.newHashSet(volumes))
+                        .withVolumeMounts(Sets.newHashSet(Maps.newHashMap("name", "tf-ws", "mountPath", "/tfws")))
                         .withCommands(Lists.newArrayList("terraform", "version"))
                         .withWorkingDir("/tfws/app1")
                         .build();