Add command/arg support to dockerimage tool in devfile

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
eclipse-che · Mar 8, 2019 · d939959 · d939959
1 parent d7ab895
commit d939959
Show file tree

Hide file tree

Showing 13 changed files with 262 additions and 17 deletions.
diff --git a/...kspace/infrastructure/kubernetes/environment/convert/DockerImageEnvironmentConverter.java b/...kspace/infrastructure/kubernetes/environment/convert/DockerImageEnvironmentConverter.java
@@ -20,6 +20,7 @@
 import io.fabric8.kubernetes.api.model.PodBuilder;
 import java.util.HashMap;
 import java.util.Map;
+import javax.inject.Inject;
 import javax.inject.Singleton;
 import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
 import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig;
@@ -40,7 +41,12 @@ public class DockerImageEnvironmentConverter {
   static final String POD_NAME = "dockerimage";
   static final String CONTAINER_NAME = "container";
 
-  private EntryPointParser entryPointParser = new EntryPointParser();
+  private final EntryPointParser entryPointParser;
+
+  @Inject
+  public DockerImageEnvironmentConverter(EntryPointParser entryPointParser) {
+    this.entryPointParser = entryPointParser;
+  }
 
   public KubernetesEnvironment convert(DockerImageEnvironment environment)
       throws InfrastructureException {

diff --git a/...rg/eclipse/che/workspace/infrastructure/kubernetes/environment/util/EntryPointParser.java b/...rg/eclipse/che/workspace/infrastructure/kubernetes/environment/util/EntryPointParser.java
@@ -14,6 +14,7 @@
 import static java.lang.String.format;
 import static java.util.Collections.emptyList;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.dataformat.yaml.YAMLMapper;
 import java.io.IOException;
@@ -23,7 +24,7 @@
 import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
 
 /** Can be used to parse container entry-point definition specified as a YAML list of strings. */
-public final class EntryPointParser {
+public class EntryPointParser {
   private final YAMLMapper mapper = new YAMLMapper();
 
   /**
@@ -52,6 +53,22 @@ public EntryPoint parse(Map<String, String> machineAttributes) throws Infrastruc
     return new EntryPoint(commandList, argList);
   }
 
+  /**
+   * Serializes an entry (that might have been produced from {@link #parse(Map)}) back to a string
+   * representation.
+   *
+   * @param entry the command or args entry
+   * @return a serialized representation of the entry
+   */
+  public String serializeEntry(List<String> entry) {
+    try {
+      return mapper.writer().writeValueAsString(entry);
+    } catch (JsonProcessingException e) {
+      throw new IllegalStateException(
+          format("Failed to serialize list of strings %s to YAML", entry), e);
+    }
+  }
+
   private List<String> parseAsList(String data, String attributeName)
       throws InfrastructureException {
     try {

diff --git a/...ce/infrastructure/kubernetes/environment/convert/DockerImageEnvironmentConverterTest.java b/...ce/infrastructure/kubernetes/environment/convert/DockerImageEnvironmentConverterTest.java
@@ -13,10 +13,13 @@
 
 import static java.lang.String.format;
 import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.MACHINE_NAME_ANNOTATION_FMT;
 import static org.eclipse.che.workspace.infrastructure.kubernetes.environment.convert.DockerImageEnvironmentConverter.CONTAINER_NAME;
 import static org.eclipse.che.workspace.infrastructure.kubernetes.environment.convert.DockerImageEnvironmentConverter.POD_NAME;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -29,11 +32,12 @@
 import io.fabric8.kubernetes.api.model.PodBuilder;
 import java.util.HashMap;
 import java.util.Map;
-import org.eclipse.che.api.core.model.workspace.config.MachineConfig;
 import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig;
 import org.eclipse.che.api.workspace.server.spi.environment.InternalRecipe;
 import org.eclipse.che.workspace.infrastructure.docker.environment.dockerimage.DockerImageEnvironment;
 import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment;
+import org.eclipse.che.workspace.infrastructure.kubernetes.environment.util.EntryPoint;
+import org.eclipse.che.workspace.infrastructure.kubernetes.environment.util.EntryPointParser;
 import org.mockito.Mock;
 import org.mockito.testng.MockitoTestNGListener;
 import org.testng.annotations.BeforeMethod;
@@ -50,14 +54,20 @@ public class DockerImageEnvironmentConverterTest {
 
   @Mock DockerImageEnvironment dockerEnv;
   @Mock InternalRecipe recipe;
+  @Mock private EntryPointParser entryPointParser;
 
   private Pod pod;
   private Map<String, InternalMachineConfig> machines;
   private DockerImageEnvironmentConverter converter;
 
   @BeforeMethod
   public void setup() throws Exception {
-    converter = new DockerImageEnvironmentConverter();
+    converter = new DockerImageEnvironmentConverter(entryPointParser);
+
+    lenient()
+        .when(entryPointParser.parse(any()))
+        .thenReturn(new EntryPoint(emptyList(), emptyList()));
+
     lenient().when(recipe.getContent()).thenReturn(RECIPE_CONTENT);
     lenient().when(recipe.getType()).thenReturn(RECIPE_TYPE);
     machines = ImmutableMap.of(MACHINE_NAME, mock(InternalMachineConfig.class));
@@ -95,12 +105,11 @@ public void testConvertsDockerImageEnvironment2KubernetesEnvironment() throws Ex
   @Test
   public void shouldUseMachineConfigIfProvided() throws Exception {
     // given
-    Map<String, String> attributes = new HashMap<>(2);
-    attributes.put(MachineConfig.CONTAINER_COMMAND_ATTRIBUTE, "[/teh/script]");
-    attributes.put(MachineConfig.CONTAINER_ARGS_ATTRIBUTE, "['teh', 'argz']");
+    doReturn(new EntryPoint(singletonList("/teh/script"), asList("teh", "argz")))
+        .when(entryPointParser)
+        .parse(any());
 
     InternalMachineConfig machineConfig = mock(InternalMachineConfig.class);
-    when(machineConfig.getAttributes()).thenReturn(attributes);
 
     Map<String, InternalMachineConfig> machines = new HashMap<>(1);
     machines.put(MACHINE_NAME, machineConfig);

diff --git a/...clipse/che/workspace/infrastructure/kubernetes/environment/util/EntryPointParserTest.java b/...clipse/che/workspace/infrastructure/kubernetes/environment/util/EntryPointParserTest.java
@@ -16,6 +16,7 @@
 import static org.testng.AssertJUnit.assertEquals;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import org.eclipse.che.api.core.model.workspace.config.MachineConfig;
 import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
@@ -83,4 +84,21 @@ public static Object[][] invalidEntryProvider() {
       new String[] {"[a, b, [c]]"}
     };
   }
+
+  @Test
+  public void shouldSerializeValidData() {
+    // given
+    List<String> data = asList("/bin/sh", "-c");
+
+    EntryPointParser parser = new EntryPointParser();
+
+    // when
+    String serialized = parser.serializeEntry(data);
+
+    // then
+
+    // this is dependent on the configuration of the YAML generator used by the YAMLMapper used in
+    // the EntryPointParser so this may start failing on jackson-dataformat-yaml library upgrade
+    assertEquals(serialized, "---\n- \"/bin/sh\"\n- \"-c\"\n");
+  }
 }
diff --git a/wsmaster/che-core-api-devfile/README.md b/wsmaster/che-core-api-devfile/README.md
@@ -148,7 +148,7 @@ Devfile can only contain one tool with `dockerimage` type.
      type: dockerimage
      image: eclipe/maven-jdk8:latest
      volumes:
-       - name: maven-repo
+       - name: mavenrepo
          containerPath: /root/.m2
      env:
        - name: ENV_VAR
@@ -162,6 +162,8 @@ Devfile can only contain one tool with `dockerimage` type.
            public: 'true'
            discoverable: 'false'
      memoryLimit: 1536M
+     command: ['tail']
+     args: ['-f', '/dev/null']
 ```
 
 ### Commands expanded
@@ -184,4 +186,4 @@ Devfile allows to specify commands set to be available for execution in workspac
     //TODO
 
 ### Planned features
-There is still a lot of plans to extend Devfile possibilities, such as support multiple kubernetes/openshift tools etc
+There is still a lot of plans to extend Devfile possibilities, such as support multiple kubernetes/openshift tools etc
diff --git a/...g/eclipse/che/api/devfile/server/convert/tool/dockerimage/DockerimageToolProvisioner.java b/...g/eclipse/che/api/devfile/server/convert/tool/dockerimage/DockerimageToolProvisioner.java
@@ -21,6 +21,8 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.stream.Collectors;
+import javax.inject.Inject;
+import org.eclipse.che.api.core.model.workspace.config.MachineConfig;
 import org.eclipse.che.api.devfile.model.Devfile;
 import org.eclipse.che.api.devfile.model.Endpoint;
 import org.eclipse.che.api.devfile.model.Env;
@@ -34,7 +36,10 @@
 import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl;
 import org.eclipse.che.api.workspace.server.model.impl.VolumeImpl;
 import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
+import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
 import org.eclipse.che.workspace.infrastructure.docker.environment.dockerimage.DockerImageEnvironment;
+import org.eclipse.che.workspace.infrastructure.kubernetes.environment.util.EntryPoint;
+import org.eclipse.che.workspace.infrastructure.kubernetes.environment.util.EntryPointParser;
 
 /**
  * Provision dockerimage tool in {@link Devfile} according to the value of environment with
@@ -44,6 +49,13 @@
  */
 public class DockerimageToolProvisioner implements ToolProvisioner {
 
+  private final EntryPointParser entryPointParser;
+
+  @Inject
+  public DockerimageToolProvisioner(EntryPointParser entryPointParser) {
+    this.entryPointParser = entryPointParser;
+  }
+
   /**
    * Provision dockerimage tool in {@link Devfile} according to the value of environment with
    * dockerimage recipe if the specified {@link WorkspaceConfigImpl} has such.
@@ -118,6 +130,10 @@ public void provision(Devfile devfile, WorkspaceConfigImpl workspaceConfig)
 
     dockerimageTool.setMemoryLimit(machineConfig.getAttributes().get(MEMORY_LIMIT_ATTRIBUTE));
 
+    EntryPoint ep = toEntryPoint(machineConfig);
+    dockerimageTool.setCommand(ep.getCommand());
+    dockerimageTool.setArgs(ep.getArguments());
+
     machineConfig
         .getEnv()
         .entrySet()
@@ -128,6 +144,14 @@ public void provision(Devfile devfile, WorkspaceConfigImpl workspaceConfig)
     devfile.getTools().add(dockerimageTool);
   }
 
+  private EntryPoint toEntryPoint(MachineConfig machineConfig) throws WorkspaceExportException {
+    try {
+      return entryPointParser.parse(machineConfig.getAttributes());
+    } catch (InfrastructureException e) {
+      throw new WorkspaceExportException(e.getMessage());
+    }
+  }
+
   private Volume toDevfileVolume(String name, VolumeImpl volume) {
     return new Volume().withName(name).withContainerPath(volume.getPath());
   }

diff --git a/...se/che/api/devfile/server/convert/tool/dockerimage/DockerimageToolToWorkspaceApplier.java b/...se/che/api/devfile/server/convert/tool/dockerimage/DockerimageToolToWorkspaceApplier.java
@@ -15,12 +15,15 @@
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.lang.String.format;
 import static org.eclipse.che.api.core.model.workspace.config.Command.MACHINE_NAME_ATTRIBUTE;
+import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CONTAINER_ARGS_ATTRIBUTE;
+import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.CONTAINER_COMMAND_ATTRIBUTE;
 import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE;
 import static org.eclipse.che.api.devfile.server.Constants.DOCKERIMAGE_TOOL_TYPE;
 import static org.eclipse.che.api.workspace.shared.Constants.PROJECTS_VOLUME_NAME;
 
 import com.google.common.collect.ImmutableMap;
 import java.util.HashMap;
+import java.util.List;
 import javax.inject.Inject;
 import javax.inject.Named;
 import org.eclipse.che.api.devfile.model.Endpoint;
@@ -35,6 +38,7 @@
 import org.eclipse.che.api.workspace.server.model.impl.VolumeImpl;
 import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
 import org.eclipse.che.workspace.infrastructure.docker.environment.dockerimage.DockerImageEnvironment;
+import org.eclipse.che.workspace.infrastructure.kubernetes.environment.util.EntryPointParser;
 import org.eclipse.che.workspace.infrastructure.kubernetes.util.KubernetesSize;
 
 /**
@@ -45,11 +49,14 @@
 public class DockerimageToolToWorkspaceApplier implements ToolToWorkspaceApplier {
 
   private final String projectFolderPath;
+  private final EntryPointParser entryPointParser;
 
   @Inject
   public DockerimageToolToWorkspaceApplier(
-      @Named("che.workspace.projects.storage") String projectFolderPath) {
+      @Named("che.workspace.projects.storage") String projectFolderPath,
+      EntryPointParser entryPointParser) {
     this.projectFolderPath = projectFolderPath;
+    this.entryPointParser = entryPointParser;
   }
 
   /**
@@ -116,6 +123,10 @@ public void apply(
             MEMORY_LIMIT_ATTRIBUTE,
             Long.toString(KubernetesSize.toBytes(dockerimageTool.getMemoryLimit())));
 
+    setEntryPointAttribute(
+        machineConfig, CONTAINER_COMMAND_ATTRIBUTE, dockerimageTool.getCommand());
+    setEntryPointAttribute(machineConfig, CONTAINER_ARGS_ATTRIBUTE, dockerimageTool.getArgs());
+
     RecipeImpl recipe =
         new RecipeImpl(DockerImageEnvironment.TYPE, null, dockerimageTool.getImage(), null);
     EnvironmentImpl environment =
@@ -133,4 +144,16 @@ public void apply(
                     .equals(c.getAttributes().get(Constants.TOOL_NAME_COMMAND_ATTRIBUTE)))
         .forEach(c -> c.getAttributes().put(MACHINE_NAME_ATTRIBUTE, machineName));
   }
+
+  private void setEntryPointAttribute(
+      MachineConfigImpl machineConfig, String attributeName, List<String> attributeValue) {
+
+    if (attributeValue == null) {
+      return;
+    }
+
+    String val = entryPointParser.serializeEntry(attributeValue);
+
+    machineConfig.getAttributes().put(attributeName, val);
+  }
 }
diff --git a/wsmaster/che-core-api-devfile/src/main/resources/schema/devfile.json b/wsmaster/che-core-api-devfile/src/main/resources/schema/devfile.json
@@ -162,7 +162,9 @@
               "mountSources": {},
               "volumes": {},
               "env": {},
-              "endpoints": {}
+              "endpoints": {},
+              "command": {},
+              "args": {}
             },
             "required": [
               "image",
@@ -237,6 +239,26 @@
             "description": "Describes whether projects sources should be mount to the tool. `CHE_PROJECTS_ROOT` environment variable should contains a path where projects sources are mount",
             "default": "false"
           },
+          "command": {
+            "type": "array",
+            "items": {
+              "type": "string"
+            },
+            "description": "The command to run in the dockerimage tool instead of the default one provided in the image.",
+            "examples": [
+              "['/bin/sh', '-c']"
+            ]
+          },
+          "args": {
+            "type": "array",
+            "items": {
+              "type": "string"
+            },
+            "description": "The arguments to supply to the command running the dockerimage tool. The arguments are supplied either to the default command provided in the image or to the overridden command.",
+            "examples": [
+              "['-R', '-f']"
+            ]
+          },
           "volumes": {
             "type": "array",
             "description": "Describes volumes which should be mount to tool",