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

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

commit f215ed32a14233e883b9db6badd57209637a6dd2
Author: iuliana <[email protected]>
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     |  5 +-
 5 files changed, 126 insertions(+), 19 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..392039193b 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;
@@ -124,8 +125,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();

Reply via email to