diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingApplication.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingApplication.java index ccdf18a..807f43e 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingApplication.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingApplication.java @@ -60,7 +60,7 @@ public class BackingApplication { private BackingApplication() { } - private BackingApplication(String name, String path, + BackingApplication(String name, String path, Map properties, Map environment, List services, diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/TargetSpec.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/TargetSpec.java index 5d2e2b8..5080b73 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/TargetSpec.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/TargetSpec.java @@ -23,7 +23,7 @@ public class TargetSpec { private TargetSpec() { } - private TargetSpec(String name) { + TargetSpec(String name) { this.name = name; } @@ -44,7 +44,7 @@ public class TargetSpec { private String name; - private TargetSpecBuilder() { + TargetSpecBuilder() { } public TargetSpecBuilder name(String name) { diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployer.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployer.java index 0e83cd0..6f7e1bd 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployer.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployer.java @@ -151,12 +151,12 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .path(getApplication(appResource)) .environmentVariables(getEnvironmentVariables(request.getEnvironment())) .services(request.getServices()) + .instances(instances(deploymentProperties)) + .memory(memory(deploymentProperties)) .disk(diskQuota(deploymentProperties)) .healthCheckType(healthCheck(deploymentProperties)) .healthCheckHttpEndpoint(healthCheckEndpoint(deploymentProperties)) .timeout(healthCheckTimeout(deploymentProperties)) - .instances(instances(deploymentProperties)) - .memory(memory(deploymentProperties)) .noRoute(toggleNoRoute(deploymentProperties)); Optional.ofNullable(host(deploymentProperties)).ifPresent(manifest::host); @@ -252,7 +252,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware return new DefaultApplications( ((DefaultCloudFoundryOperations) this.operations).getCloudFoundryClientPublisher(), ((DefaultCloudFoundryOperations) this.operations).getDopplerClientPublisher(), - (this.operations).spaces().get(GetSpaceRequest.builder().name(space).build()).map(SpaceDetail::getId)); + this.operations.spaces().get(GetSpaceRequest.builder().name(space).build()).map(SpaceDetail::getId)); } private DefaultSpaces createSpaceOperations() { @@ -321,8 +321,8 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware private ApplicationHealthCheck healthCheck(Map properties) { return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.HEALTHCHECK_PROPERTY_KEY)) - .map(this::toApplicationHealthCheck) - .orElse(this.defaultDeploymentProperties.getHealthCheck()); + .map(this::toApplicationHealthCheck) + .orElse(this.defaultDeploymentProperties.getHealthCheck()); } private ApplicationHealthCheck toApplicationHealthCheck(String raw) { @@ -336,19 +336,19 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware private String healthCheckEndpoint(Map properties) { return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY)) - .orElse(this.defaultDeploymentProperties.getHealthCheckHttpEndpoint()); + .orElse(this.defaultDeploymentProperties.getHealthCheckHttpEndpoint()); } private Integer healthCheckTimeout(Map properties) { - String timeoutString = properties - .getOrDefault(CloudFoundryDeploymentProperties.HEALTHCHECK_TIMEOUT_PROPERTY_KEY, this.defaultDeploymentProperties.getHealthCheckTimeout()); - return Integer.parseInt(timeoutString); + return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.HEALTHCHECK_TIMEOUT_PROPERTY_KEY)) + .map(Integer::parseInt) + .orElse(this.defaultDeploymentProperties.getHealthCheckTimeout()); } - private int instances(Map properties) { + private Integer instances(Map properties) { return Optional.ofNullable(properties.get(DeploymentProperties.COUNT_PROPERTY_KEY)) - .map(Integer::parseInt) - .orElse(this.defaultDeploymentProperties.getInstances()); + .map(Integer::parseInt) + .orElse(this.defaultDeploymentProperties.getInstances()); } private String host(Map properties) { @@ -382,30 +382,31 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .orElse(null); } - private int memory(Map properties) { - String withUnit = properties - .getOrDefault(DeploymentProperties.MEMORY_PROPERTY_KEY, this.defaultDeploymentProperties.getMemory()); - return (int) ByteSizeUtils.parseToMebibytes(withUnit); + private Integer memory(Map properties) { + return Optional.ofNullable(properties.get(DeploymentProperties.MEMORY_PROPERTY_KEY)) + .map(ByteSizeUtils::parseToMebibytes) + .orElse(ByteSizeUtils.parseToMebibytes(defaultDeploymentProperties.getMemory())); } - private int diskQuota(Map properties) { - String withUnit = properties - .getOrDefault(DeploymentProperties.DISK_PROPERTY_KEY, this.defaultDeploymentProperties.getDisk()); - return (int) ByteSizeUtils.parseToMebibytes(withUnit); + private Integer diskQuota(Map properties) { + return Optional.ofNullable(properties.get(DeploymentProperties.DISK_PROPERTY_KEY)) + .map(ByteSizeUtils::parseToMebibytes) + .orElse(ByteSizeUtils.parseToMebibytes(defaultDeploymentProperties.getDisk())); } private String buildpack(Map properties) { return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.BUILDPACK_PROPERTY_KEY)) - .orElse(this.defaultDeploymentProperties.getBuildpack()); + .orElse(this.defaultDeploymentProperties.getBuildpack()); } private String javaOpts(Map properties) { return Optional.ofNullable(properties.get(CloudFoundryDeploymentProperties.JAVA_OPTS_PROPERTY_KEY)) - .orElse(this.defaultDeploymentProperties.getJavaOpts()); + .orElse(this.defaultDeploymentProperties.getJavaOpts()); } private Predicate isNotFoundError() { - return t -> t instanceof AbstractCloudFoundryException && ((AbstractCloudFoundryException) t).getStatusCode() == HttpStatus.NOT_FOUND.value(); + return t -> t instanceof AbstractCloudFoundryException && + ((AbstractCloudFoundryException) t).getStatusCode() == HttpStatus.NOT_FOUND.value(); } /** diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java index 538ac2c..770ee92 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/main/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryDeploymentProperties.java @@ -76,37 +76,37 @@ public class CloudFoundryDeploymentProperties { /** * The buildpack to use for deploying the application. */ - private String buildpack = "https://github.com/cloudfoundry/java-buildpack.git#v4.7.1"; + private String buildpack; /** * The amount of memory to allocate, if not overridden per-app. Default unit is mebibytes, 'M' and 'G" suffixes supported. */ - private String memory = "1024m"; + private String memory; /** * The amount of disk space to allocate, if not overridden per-app. Default unit is mebibytes, 'M' and 'G" suffixes supported. */ - private String disk = "1024m"; + private String disk; /** * The type of health check to perform on deployed application, if not overridden per-app. Defaults to PORT */ - private ApplicationHealthCheck healthCheck = ApplicationHealthCheck.PORT; + private ApplicationHealthCheck healthCheck; /** * The path that the http health check will use, defaults to @{code /health} */ - private String healthCheckHttpEndpoint = "/health"; + private String healthCheckHttpEndpoint; /** * The timeout value for health checks in seconds. Defaults to 120 seconds. */ - private String healthCheckTimeout = "120"; + private Integer healthCheckTimeout; /** * The number of instances to run. */ - private int instances = 1; + private Integer instances; /** * Flag to enable prefixing the app name with a random prefix. @@ -179,7 +179,7 @@ public class CloudFoundryDeploymentProperties { this.disk = disk; } - public int getInstances() { + public Integer getInstances() { return instances; } @@ -235,11 +235,11 @@ public class CloudFoundryDeploymentProperties { this.healthCheckHttpEndpoint = healthCheckHttpEndpoint; } - public String getHealthCheckTimeout() { + public Integer getHealthCheckTimeout() { return healthCheckTimeout; } - public void setHealthCheckTimeout(String healthCheckTimeout) { + public void setHealthCheckTimeout(Integer healthCheckTimeout) { this.healthCheckTimeout = healthCheckTimeout; } diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTest.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTest.java index 607245e..4801e34 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTest.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerTest.java @@ -16,9 +16,11 @@ package org.springframework.cloud.appbroker.deployer.cloudfoundry; -import java.util.Collections; +import java.io.File; +import java.util.ArrayList; import org.cloudfoundry.operations.CloudFoundryOperations; +import org.cloudfoundry.operations.applications.ApplicationHealthCheck; import org.cloudfoundry.operations.applications.ApplicationManifest; import org.cloudfoundry.operations.applications.Applications; import org.cloudfoundry.operations.applications.PushApplicationManifestRequest; @@ -28,6 +30,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.cloud.appbroker.deployer.DeploymentProperties; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -45,8 +48,6 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class CloudFoundryAppDeployerTest { - private static final int APP_MEMORY = 1024; - private static final String APP_MEMORY_STRING = "1024M"; private static final String APP_NAME = "test-app"; private static final String APP_PATH = "test.jar"; @@ -60,43 +61,185 @@ class CloudFoundryAppDeployerTest { @Mock private ResourceLoader resourceLoader; + + private CloudFoundryDeploymentProperties deploymentProperties; @BeforeEach void setUp() { - CloudFoundryDeploymentProperties deploymentProperties = new CloudFoundryDeploymentProperties(); - deploymentProperties.setDisk(APP_MEMORY_STRING); + deploymentProperties = new CloudFoundryDeploymentProperties(); - when(applications.pushManifest(any())).thenReturn(Mono.empty()); - when(cloudFoundryOperations.applications()).thenReturn(applications); - when(resourceLoader.getResource(APP_PATH)).thenReturn(new FileSystemResource(APP_PATH)); + when(applications.pushManifest(any())) + .thenReturn(Mono.empty()); + when(cloudFoundryOperations.applications()) + .thenReturn(applications); + when(resourceLoader.getResource(APP_PATH)) + .thenReturn(new FileSystemResource(APP_PATH)); appDeployer = new CloudFoundryAppDeployer( deploymentProperties, cloudFoundryOperations, resourceLoader); } @Test - void shouldDeployAppWithNameAndPath() { - StepVerifier.create(appDeployer.deploy(DeployApplicationRequest.builder() + void deployAppWithPlatformDefaults() { + DeployApplicationRequest request = DeployApplicationRequest.builder() .name(APP_NAME) - .properties(Collections.emptyMap()) .path(APP_PATH) - .build())) + .build(); + + StepVerifier.create(appDeployer.deploy(request)) .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) .verifyComplete(); - verify(applications).pushManifest(argThat(matchesExpectedManifest())); + ApplicationManifest expectedManifest = baseManifest() + .name(APP_NAME) + .path(new File(APP_PATH).toPath()) + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); } - private ArgumentMatcher matchesExpectedManifest() { - return request -> { - if (request.getManifests().size() == 1) { - ApplicationManifest manifest = request.getManifests().get(0); - return APP_NAME.equals(manifest.getName()) - && APP_PATH.equals(manifest.getPath().toString()) - && manifest.getDisk() == APP_MEMORY; + @Test + void deployAppWithPropertiesInRequest() { + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .property(DeploymentProperties.COUNT_PROPERTY_KEY, "3") + .property(DeploymentProperties.MEMORY_PROPERTY_KEY, "2G") + .property(DeploymentProperties.DISK_PROPERTY_KEY, "3G") + .property(CloudFoundryDeploymentProperties.HEALTHCHECK_PROPERTY_KEY, "http") + .property(CloudFoundryDeploymentProperties.HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY, "/healthcheck") + .property(CloudFoundryDeploymentProperties.BUILDPACK_PROPERTY_KEY, "buildpack") + .property(CloudFoundryDeploymentProperties.DOMAIN_PROPERTY, "domain") + .property(CloudFoundryDeploymentProperties.HOST_PROPERTY, "host") + .property(CloudFoundryDeploymentProperties.NO_ROUTE_PROPERTY, "true") + .build(); + + StepVerifier.create(appDeployer.deploy(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + ApplicationManifest expectedManifest = baseManifest() + .name(APP_NAME) + .path(new File(APP_PATH).toPath()) + .instances(3) + .memory(2048) + .disk(3072) + .healthCheckType(ApplicationHealthCheck.HTTP) + .healthCheckHttpEndpoint("/healthcheck") + .buildpack("buildpack") + .domain("domain") + .host("host") + .noRoute(true) + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); + } + + @Test + void deployAppWithDefaultProperties() { + deploymentProperties.setInstances(3); + deploymentProperties.setMemory("2G"); + deploymentProperties.setDisk("3G"); + deploymentProperties.setBuildpack("buildpack"); + deploymentProperties.setHealthCheck(ApplicationHealthCheck.HTTP); + deploymentProperties.setHealthCheckHttpEndpoint("/healthcheck"); + deploymentProperties.setDomain("domain"); + deploymentProperties.setHost("host"); + + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .build(); + + StepVerifier.create(appDeployer.deploy(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + ApplicationManifest expectedManifest = baseManifest() + .name(APP_NAME) + .path(new File(APP_PATH).toPath()) + .instances(3) + .memory(2048) + .disk(3072) + .healthCheckType(ApplicationHealthCheck.HTTP) + .healthCheckHttpEndpoint("/healthcheck") + .buildpack("buildpack") + .domain("domain") + .host("host") + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); + } + + @Test + void deployAppWithRequestOverridingDefaultProperties() { + deploymentProperties.setInstances(3); + deploymentProperties.setMemory("2G"); + deploymentProperties.setDisk("3G"); + deploymentProperties.setBuildpack("buildpack1"); + deploymentProperties.setHealthCheck(ApplicationHealthCheck.HTTP); + deploymentProperties.setHealthCheckHttpEndpoint("/healthcheck1"); + deploymentProperties.setDomain("domain1"); + deploymentProperties.setHost("host1"); + + DeployApplicationRequest request = DeployApplicationRequest.builder() + .name(APP_NAME) + .path(APP_PATH) + .property(DeploymentProperties.COUNT_PROPERTY_KEY, "5") + .property(DeploymentProperties.MEMORY_PROPERTY_KEY, "4G") + .property(DeploymentProperties.DISK_PROPERTY_KEY, "5G") + .property(CloudFoundryDeploymentProperties.HEALTHCHECK_PROPERTY_KEY, "port") + .property(CloudFoundryDeploymentProperties.HEALTHCHECK_HTTP_ENDPOINT_PROPERTY_KEY, "/healthcheck2") + .property(CloudFoundryDeploymentProperties.BUILDPACK_PROPERTY_KEY, "buildpack2") + .property(CloudFoundryDeploymentProperties.DOMAIN_PROPERTY, "domain2") + .property(CloudFoundryDeploymentProperties.HOST_PROPERTY, "host2") + .property(CloudFoundryDeploymentProperties.NO_ROUTE_PROPERTY, "true") + .build(); + + StepVerifier.create(appDeployer.deploy(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + ApplicationManifest expectedManifest = baseManifest() + .name(APP_NAME) + .path(new File(APP_PATH).toPath()) + .instances(5) + .memory(4096) + .disk(5120) + .healthCheckType(ApplicationHealthCheck.PORT) + .healthCheckHttpEndpoint("/healthcheck2") + .buildpack("buildpack2") + .domain("domain2") + .host("host2") + .noRoute(true) + .build(); + + verify(applications).pushManifest(argThat(matchesManifest(expectedManifest))); + } + + private ApplicationManifest.Builder baseManifest() { + return ApplicationManifest.builder() + .environmentVariable("SPRING_APPLICATION_INDEX", "${vcap.application.instance_index}") + .environmentVariable("SPRING_CLOUD_APPLICATION_GUID", "${vcap.application.name}:${vcap.application.instance_index}") + .environmentVariable("SPRING_APPLICATION_JSON", "{}") + .services(new ArrayList<>()); + } + + private ArgumentMatcher matchesManifest(ApplicationManifest expectedManifest) { + return new ArgumentMatcher() { + @Override + public boolean matches(PushApplicationManifestRequest request) { + if (request.getManifests().size() == 1) { + return request.getManifests().get(0).equals(expectedManifest); + } + + return false; } - return false; + @Override + public String toString() { + return expectedManifest.toString(); + } }; } diff --git a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeployApplicationRequest.java b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeployApplicationRequest.java index 57059a3..503c65d 100644 --- a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeployApplicationRequest.java +++ b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/DeployApplicationRequest.java @@ -123,9 +123,6 @@ public class DeployApplicationRequest { } public DeployApplicationRequestBuilder services(List services) { - if (services == null) { - return this; - } this.services.addAll(services); return this; } diff --git a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/UndeployApplicationRequest.java b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/UndeployApplicationRequest.java index 3275b6f..770ef39 100644 --- a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/UndeployApplicationRequest.java +++ b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/UndeployApplicationRequest.java @@ -23,7 +23,7 @@ public class UndeployApplicationRequest { private final String name; private final Map properties; - private UndeployApplicationRequest(String name, Map properties) { + UndeployApplicationRequest(String name, Map properties) { this.name = name; this.properties = properties; } diff --git a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/util/ByteSizeUtils.java b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/util/ByteSizeUtils.java index 4612dc1..952bfd5 100644 --- a/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/util/ByteSizeUtils.java +++ b/spring-cloud-app-broker-deployer/src/main/java/org/springframework/cloud/appbroker/deployer/util/ByteSizeUtils.java @@ -16,6 +16,8 @@ package org.springframework.cloud.appbroker.deployer.util; +import org.springframework.util.StringUtils; + import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -26,7 +28,7 @@ import java.util.regex.Pattern; */ public final class ByteSizeUtils { - private static final Pattern SIZE_PATTERN = Pattern.compile("(?\\d+)(?(m|g)?)", Pattern.CASE_INSENSITIVE); + private static final Pattern SIZE_PATTERN = Pattern.compile("(?\\d+)(?([mg])?)", Pattern.CASE_INSENSITIVE); private ByteSizeUtils() { } @@ -35,13 +37,17 @@ public final class ByteSizeUtils { * Return the number of mebibytes (1024*1024) denoted by the given text, where an optional case-insensitive unit of * 'm' or 'g' can be used to mean mebi- or gebi- bytes, respectively. Lack of unit assumes mebibytes. */ - public static long parseToMebibytes(String text) { + public static Integer parseToMebibytes(String text) { + if (!StringUtils.hasText(text)) { + return null; + } + Matcher matcher = SIZE_PATTERN.matcher(text); if (!matcher.matches()) { throw new IllegalArgumentException(String.format("Could not parse '%s' as a byte size." + " Expected a number with optional 'm' or 'g' suffix", text)); } - long size = Long.parseLong(matcher.group("amount")); + int size = Integer.parseInt(matcher.group("amount")); if (matcher.group("unit").equalsIgnoreCase("g")) { size *= 1024L; }