Fix local deployer issues

- add switch to allow testing with docker images instead of Maven artifacts

- use SPRING_CLOUD_DEPLOYER_SPI_TEST_USE_DOCKER=true

- set instance env vars like INSTANCE_INDEX for app when deployed as docker image

- pass command line args to app when deployed as docker image

- for docker, expose the port used by the container instead of 8080

- support deploying Boot 2.0 apps by changing to pass app properties as command line args

Resolves #62
Resolves #65
Resolves #66
Resolves #67

Add support for passing app props as SPRING_APPLICATION_JSON

- moving deployer provided env vars out of the LocalDeployerProperties

Rename test to remove Abstract prefix

spring-cloud-deployer-local/pom.xml

@@ -35,6 +35,10 @@
 			<groupId>org.springframework</groupId>
 			<artifactId>spring-web</artifactId>
 		</dependency>
+		<dependency>
+			<groupId>com.fasterxml.jackson.core</groupId>
+			<artifactId>jackson-databind</artifactId>
+		</dependency>
 		<dependency>
 			<groupId>org.springframework.boot</groupId>
 			<artifactId>spring-boot-configuration-processor</artifactId>

spring-cloud-deployer-local/src/main/java/org/springframework/cloud/deployer/spi/local/AbstractLocalDeployerSupport.java

@@ -17,12 +17,19 @@
 package org.springframework.cloud.deployer.spi.local;
 
 import java.net.URL;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Pattern;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.cloud.deployer.resource.docker.DockerResource;
@@ -44,6 +51,11 @@
  */
 public abstract class AbstractLocalDeployerSupport {
 
+	public static final String USE_SPRING_APPLICATION_JSON_KEY =
+			LocalDeployerProperties.PREFIX + ".use-spring-application-json";
+
+	private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+
 	protected final Logger logger = LoggerFactory.getLogger(getClass());
 
 	private final LocalDeployerProperties properties;
@@ -54,6 +66,9 @@ public abstract class AbstractLocalDeployerSupport {
 
 	private final DockerCommandBuilder dockerCommandBuilder;
 
+	private String[] envVarsSetByDeployer =
+			{"SPRING_CLOUD_APPLICATION_GUID", "SPRING_APPLICATION_INDEX", "INSTANCE_INDEX"};
+
 	/**
 	 * Instantiates a new abstract deployer support.
 	 *
@@ -100,7 +115,9 @@ final protected LocalDeployerProperties getLocalDeployerProperties() {
 	 * @param vars set of environment variable strings
 	 */
 	protected void retainEnvVars(Set<String> vars) {
-		String[] patterns = getLocalDeployerProperties().getEnvVarsToInherit();
+
+		List<String> patterns = new ArrayList<>(Arrays.asList(getLocalDeployerProperties().getEnvVarsToInherit()));
+		patterns.addAll(Arrays.asList(this.envVarsSetByDeployer));
 
 		for (Iterator<String> iterator = vars.iterator(); iterator.hasNext();) {
 			String var = iterator.next();
@@ -121,18 +138,25 @@ protected void retainEnvVars(Set<String> vars) {
 	 * Builds the process builder.
 	 *
 	 * @param request the request
-	 * @param args the args
+	 * @param appInstanceEnv the instance environment variables
+	 * @param appProperties the app properties
 	 * @return the process builder
 	 */
-	protected ProcessBuilder buildProcessBuilder(AppDeploymentRequest request, Map<String, String> args, Optional<Integer> appInstanceNumber) {
+	protected ProcessBuilder buildProcessBuilder(AppDeploymentRequest request, Map<String, String> appInstanceEnv,
+												 Map<String, String> appProperties, Optional<Integer> appInstanceNumber) {
 		Assert.notNull(request, "AppDeploymentRequest must be set");
-		Assert.notNull(args, "Args must be set");
+		Assert.notNull(appProperties, "Args must be set");
 		String[] commands = null;
+		Map<String, String> appInstanceEnvToUse = new HashMap<>(appInstanceEnv);
+		Map<String, String> appPropertiesToUse = new HashMap<>();
+		handleAppPropertiesPassing(request, appProperties, appInstanceEnvToUse, appPropertiesToUse);
 		if (request.getResource() instanceof DockerResource) {
-			commands = this.dockerCommandBuilder.buildExecutionCommand(request, args, appInstanceNumber);
+			commands = this.dockerCommandBuilder.buildExecutionCommand(request,
+					appInstanceEnvToUse, appPropertiesToUse, appInstanceNumber);
 		}
 		else {
-			commands = this.javaCommandBuilder.buildExecutionCommand(request, args, appInstanceNumber);
+			commands = this.javaCommandBuilder.buildExecutionCommand(request,
+					appInstanceEnvToUse, appPropertiesToUse, appInstanceNumber);
 		}
 
 		// tweak escaping double quotes needed for windows
@@ -143,11 +167,35 @@ protected ProcessBuilder buildProcessBuilder(AppDeploymentRequest request, Map<S
 		}
 
 		ProcessBuilder builder = new ProcessBuilder(commands);
+		if (!(request.getResource() instanceof DockerResource)) {
+			builder.environment().putAll(appInstanceEnv);
+		}
 		retainEnvVars(builder.environment().keySet());
-		builder.environment().putAll(args);
 		return builder;
 	}
 
+	protected void handleAppPropertiesPassing(AppDeploymentRequest request, Map<String, String> appProperties,
+											  Map<String, String> appInstanceEnvToUse,
+											  Map<String, String> appPropertiesToUse) {
+		if (useSpringApplicationJson(request)) {
+			try {
+				appInstanceEnvToUse.putAll(Collections.singletonMap(
+						"SPRING_APPLICATION_JSON", OBJECT_MAPPER.writeValueAsString(appProperties)));
+			} catch (JsonProcessingException e) {
+				throw new RuntimeException(e);
+			}
+		}
+		else {
+			appPropertiesToUse.putAll(appProperties);
+		}
+	}
+
+	private boolean useSpringApplicationJson(AppDeploymentRequest request) {
+		return Optional.ofNullable(request.getDeploymentProperties().get(USE_SPRING_APPLICATION_JSON_KEY))
+				.map(Boolean::valueOf)
+				.orElse(this.properties.isUseSpringApplicationJson());
+	}
+
 	/**
 	 * Shut down the {@link Process} backing the application {@link Instance}.
 	 * If the application exposes a {@code /shutdown} endpoint, that will be

spring-cloud-deployer-local/src/main/java/org/springframework/cloud/deployer/spi/local/CommandBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 the original author or authors.
+ * Copyright 2016-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,15 +24,20 @@
  * Strategy interface for Execution Command builder.
  *
  * @author Ilayaperumal Gopinathan
+ * @author Thomas Risberg
  */
 public interface CommandBuilder {
 
 	/**
 	 * Builds the execution command for an application.
 	 *
 	 * @param request the request for the application to execute
-	 * @param args the properties to use when building the execution command
+	 * @param appInstanceEnv the env vars tha might be needed when building the execution command
+	 * @param appProperties the app properties to use when building the execution command
 	 * @return the build command as a string array
 	 */
-	String[] buildExecutionCommand(AppDeploymentRequest request, Map<String, String> args, Optional<Integer> appInstanceNumber);
+	String[] buildExecutionCommand(AppDeploymentRequest request,
+								   Map<String, String> appInstanceEnv,
+								   Map<String, String> appProperties,
+								   Optional<Integer> appInstanceNumber);
 }

spring-cloud-deployer-local/src/main/java/org/springframework/cloud/deployer/spi/local/DockerCommandBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 the original author or authors.
+ * Copyright 2016-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@
  * @author Ilayaperumal Gopinathan
  * @author Eric Bottard
  * @author Henryk Konsek
+ * @author Thomas Risberg
  */
 public class DockerCommandBuilder implements CommandBuilder {
 
@@ -45,26 +46,32 @@ public class DockerCommandBuilder implements CommandBuilder {
 	private final Logger logger = LoggerFactory.getLogger(getClass());
 
 	@Override
-	public String[] buildExecutionCommand(AppDeploymentRequest request, Map<String, String> args, Optional<Integer> appInstanceNumber) {
-		List<String> commands = addDockerOptions(request, args, appInstanceNumber);
+	public String[] buildExecutionCommand(AppDeploymentRequest request, Map<String, String> appInstanceEnv,
+										  Map<String, String> appProperties, Optional<Integer> appInstanceNumber) {
+		List<String> commands = addDockerOptions(request, appInstanceEnv, appProperties, appInstanceNumber);
+		// Add appProperties
+		for (String prop : appProperties.keySet()) {
+			commands.add(String.format("--%s=%s", prop, appProperties.get(prop)));
+		}
+		commands.addAll(request.getCommandlineArguments());
 		logger.debug("Docker Command = " + commands);
 		return commands.toArray(new String[0]);
 	}
 
-	private List<String> addDockerOptions(AppDeploymentRequest request, Map<String, String> args, Optional<Integer> appInstanceNumber) {
+	private List<String> addDockerOptions(AppDeploymentRequest request, Map<String, String> appInstanceEnv,
+										  Map<String, String> appProperties, Optional<Integer> appInstanceNumber) {
 		List<String> commands = new ArrayList<>();
 		commands.add("docker");
 		commands.add("run");
-		DockerResource dockerResource = (DockerResource) request.getResource();
-		for (Map.Entry<String, String> entry : args.entrySet()) {
-			if (entry.getKey().equals(LocalAppDeployer.SERVER_PORT_KEY)) {
-				commands.add("-p");
-				commands.add(String.format("%s:8080", args.get(entry.getKey())));
-			}
-			else {
-				commands.add("-e");
-				commands.add(String.format("%s=%s", entry.getKey(), entry.getValue()));
-			}
+		// Add env vars
+		for (String env : appInstanceEnv.keySet()) {
+			commands.add("-e");
+			commands.add(String.format("%s=%s", env, appInstanceEnv.get(env)));
+		}
+		if (appProperties.containsKey(LocalAppDeployer.SERVER_PORT_KEY)) {
+			String port = appProperties.get(LocalAppDeployer.SERVER_PORT_KEY);
+			commands.add("-p");
+			commands.add(String.format("%s:%s", port, port));
 		}
 		if(request.getDeploymentProperties().containsKey(DOCKER_CONTAINER_NAME_KEY)) {
 			if(appInstanceNumber.isPresent()) {
@@ -73,6 +80,7 @@ private List<String> addDockerOptions(AppDeploymentRequest request, Map<String,
 				commands.add(String.format("--name=%s", request.getDeploymentProperties().get(DOCKER_CONTAINER_NAME_KEY)));
 			}
 		}
+		DockerResource dockerResource = (DockerResource) request.getResource();
 		try {
 			String dockerImageURI = dockerResource.getURI().toString();
 			commands.add(dockerImageURI.substring("docker:".length()));

spring-cloud-deployer-local/src/main/java/org/springframework/cloud/deployer/spi/local/JavaCommandBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 the original author or authors.
+ * Copyright 2016-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -38,6 +38,7 @@
 /**
  * @author Mark Pollack
  * @author Ilayaperumal Gopinathan
+ * @author Thomas Risberg
  */
 public class JavaCommandBuilder implements CommandBuilder {
 
@@ -54,13 +55,18 @@ public JavaCommandBuilder(LocalDeployerProperties properties) {
 	}
 
 	@Override
-	public String[] buildExecutionCommand(AppDeploymentRequest request, Map<String, String> args, Optional<Integer> appInstanceNumber) {
+	public String[] buildExecutionCommand(AppDeploymentRequest request, Map<String, String> appInstanceEnv,
+										  Map<String, String> appProperties, Optional<Integer> appInstanceNumber) {
 		ArrayList<String> commands = new ArrayList<String>();
 		Map<String, String> deploymentProperties = request.getDeploymentProperties();
 		commands.add(properties.getJavaCmd());
 		// Add Java System Properties (ie -Dmy.prop=val) before main class or -jar
 		addJavaOptions(commands, deploymentProperties, properties);
 		addJavaExecutionOptions(commands, request);
+		// Add appProperties
+		for (String prop : appProperties.keySet()) {
+			commands.add(String.format("--%s=%s", prop, appProperties.get(prop)));
+		}
 		commands.addAll(request.getCommandlineArguments());
 		logger.debug("Java Command = " + commands);
 		return commands.toArray(new String[0]);

spring-cloud-deployer-local/src/main/java/org/springframework/cloud/deployer/spi/local/LocalAppDeployer.java

@@ -134,9 +134,10 @@ public String deploy(AppDeploymentRequest request) {
 				if (useDynamicPort) {
 					args.put(SERVER_PORT_KEY, String.valueOf(port));
 				}
-				ProcessBuilder builder = buildProcessBuilder(request, args, Optional.of(i)).inheritIO();
+				Map<String, String> appInstanceEnv = new HashMap<>();
+				AppInstance instance = new AppInstance(deploymentId, i, appInstanceEnv, port);
+				ProcessBuilder builder = buildProcessBuilder(request, appInstanceEnv, args, Optional.of(i)).inheritIO();
 				builder.directory(workDir.toFile());
-				AppInstance instance = new AppInstance(deploymentId, i, builder, port);
 				if (this.shouldInheritLogging(request)){
 					instance.start(builder, workDir);
 					logger.info("Deploying app with deploymentId {} instance {}.\n   Logs will be inherited.", deploymentId, i);
@@ -228,16 +229,16 @@ private static class AppInstance implements Instance, AppInstanceStatus {
 
 		private final Map<String, String> attributes = new TreeMap<>();
 
-		private AppInstance(String deploymentId, int instanceNumber, ProcessBuilder builder, int port) throws IOException {
+		private AppInstance(String deploymentId, int instanceNumber, Map<String, String> appInstanceEnv, int port) throws IOException {
 			this.deploymentId = deploymentId;
 			this.instanceNumber = instanceNumber;
 			attributes.put("port", Integer.toString(port));
 			attributes.put("guid", Integer.toString(port));
 			this.baseUrl = new URL("http", Inet4Address.getLocalHost().getHostAddress(), port, "");
 			attributes.put("url", baseUrl.toString());
-			builder.environment().put("INSTANCE_INDEX", Integer.toString(instanceNumber));
-			builder.environment().put("SPRING_APPLICATION_INDEX", Integer.toString(instanceNumber));
-			builder.environment().put("SPRING_CLOUD_APPLICATION_GUID", Integer.toString(port));
+			appInstanceEnv.put("INSTANCE_INDEX", Integer.toString(instanceNumber));
+			appInstanceEnv.put("SPRING_APPLICATION_INDEX", Integer.toString(instanceNumber));
+			appInstanceEnv.put("SPRING_CLOUD_APPLICATION_GUID", Integer.toString(port));
 		}
 
 		@Override

spring-cloud-deployer-local/src/main/java/org/springframework/cloud/deployer/spi/local/LocalDeployerProperties.java

@@ -85,6 +85,12 @@ public class LocalDeployerProperties {
 	 */
 	private String javaOpts;
 
+	/**
+	 * Flag to indicate whether application properties are passed as command line args or in a
+	 * SPRING_APPLICATION_JSON environment variable.
+	 */
+	private boolean useSpringApplicationJson = false;
+
 
 	public String getJavaCmd() {
 		return javaCmd;
@@ -135,6 +141,14 @@ public void setJavaOpts(String javaOpts) {
 		this.javaOpts = javaOpts;
 	}
 
+	public boolean isUseSpringApplicationJson() {
+		return useSpringApplicationJson;
+	}
+
+	public void setUseSpringApplicationJson(boolean useSpringApplicationJson) {
+		this.useSpringApplicationJson = useSpringApplicationJson;
+	}
+
 	private String deduceJavaCommand() {
 		String javaExecutablePath = JAVA_COMMAND;
 		String javaHome = System.getProperty("java.home");

spring-cloud-deployer-local/src/main/java/org/springframework/cloud/deployer/spi/local/LocalTaskLauncher.java

@@ -109,7 +109,8 @@ public String launch(AppDeploymentRequest request) {
 			if (useDynamicPort) {
 				args.put(SERVER_PORT_KEY, String.valueOf(port));
 			}
-			ProcessBuilder builder = buildProcessBuilder(request, args, Optional.empty());
+			Map<String, String> appInstanceEnv = new HashMap<>();
+			ProcessBuilder builder = buildProcessBuilder(request, appInstanceEnv, args, Optional.empty());
 			TaskInstance instance = new TaskInstance(builder, workDir, port);
 			running.put(taskLaunchId, instance);
 			if (getLocalDeployerProperties().isDeleteFilesOnExit()) {

spring-cloud-deployer-local/src/test/java/org/springframework/cloud/deployer/spi/local/DockerCommandBuilderTests.java

@@ -47,7 +47,7 @@ public void testContainerName() {
 		Resource resource = new DockerResource("foo/bar");
 		Map<String, String> deploymentProperties = Collections.singletonMap(DockerCommandBuilder.DOCKER_CONTAINER_NAME_KEY, "gogo");
 		AppDeploymentRequest request = new AppDeploymentRequest(appDefinition, resource, deploymentProperties);
-		String[] command = commandBuilder.buildExecutionCommand(request, Collections.emptyMap(), Optional.of(1));
+		String[] command = commandBuilder.buildExecutionCommand(request, Collections.emptyMap(), Collections.emptyMap(), Optional.of(1));
 
 		assertThat(command, arrayContaining("docker", "run", "--name=gogo-1", "foo/bar"));
 	}

spring-cloud-deployer-local/src/test/java/org/springframework/cloud/deployer/spi/local/LocalAppDeployerIntegrationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 the original author or authors.
+ * Copyright 2016-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,8 +32,10 @@
 import org.hamcrest.Matchers;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.cloud.deployer.resource.docker.DockerResource;
 import org.springframework.cloud.deployer.spi.app.AppDeployer;
 import org.springframework.cloud.deployer.spi.app.AppInstanceStatus;
 import org.springframework.cloud.deployer.spi.app.AppStatus;
@@ -50,6 +52,10 @@
 /**
  * Integration tests for {@link LocalAppDeployer}.
  *
+ * Now supports running with Docker images for tests, just set this env var:
+ *
+ *   SPRING_CLOUD_DEPLOYER_SPI_TEST_USE_DOCKER=true
+ *
  * @author Eric Bottard
  * @author Mark Fisher
  * @author Oleg Zhurakousky
@@ -61,11 +67,23 @@ public class LocalAppDeployerIntegrationTests extends AbstractAppDeployerIntegra
 	@Autowired
 	private AppDeployer appDeployer;
 
+	@Value("${spring-cloud-deployer-spi-test-use-docker:false}")
+	private boolean useDocker;
+
 	@Override
 	protected AppDeployer provideAppDeployer() {
 		return appDeployer;
 	}
 
+	@Override
+	protected Resource testApplication() {
+		if (useDocker) {
+			log.info("Using Docker image for tests");
+			return new DockerResource("springcloud/spring-cloud-deployer-spi-test-app:latest");
+		}
+		return super.testApplication();
+	}
+
 	@Test
 	public void testFailureToCallShutdownOnUndeploy() {
 		Map<String, String> properties = new HashMap<>();