From 18aea012e3716f0c0dabdade1ed469836f41da7c Mon Sep 17 00:00:00 2001 From: Alberto Rios Date: Tue, 16 Oct 2018 13:30:13 +0200 Subject: [PATCH] Fixing update flow with SpacePerServiceInstance Added TargetService to update flow Added explicit assertion on backing app containin initial parameters afer an update Updated AT to consider Updates with Target Added ATs for multiple apps Not failing when CF returns a DB error when creating a space Not failing when space does not exist Connected to #93 --- .../CloudFoundryAcceptanceTest.java | 17 ++-- ...nstanceWithMultipleAppsAcceptanceTest.java | 64 ++++++++++++++ ...reateInstanceWithTargetAcceptanceTest.java | 27 +++--- .../UpdateInstanceAcceptanceTest.java | 8 +- ...pdateInstanceWithTargetAcceptanceTest.java | 84 +++++++++++++++++++ .../fixtures/cf/CloudFoundryService.java | 35 +++++--- .../AppBrokerAutoConfiguration.java | 5 +- ...ploymentUpdateServiceInstanceWorkflow.java | 7 +- ...mentUpdateServiceInstanceWorkflowTest.java | 18 +++- .../cloudfoundry/CloudFoundryAppDeployer.java | 46 ++++++---- 10 files changed, 258 insertions(+), 53 deletions(-) create mode 100644 spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CreateInstanceWithMultipleAppsAcceptanceTest.java create mode 100644 spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceWithTargetAcceptanceTest.java diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CloudFoundryAcceptanceTest.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CloudFoundryAcceptanceTest.java index d80a31f..18fefc0 100644 --- a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CloudFoundryAcceptanceTest.java +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CloudFoundryAcceptanceTest.java @@ -60,7 +60,7 @@ class CloudFoundryAcceptanceTest { @BeforeEach void setUp(BrokerProperties brokerProperties) { - initializeBroker(brokerProperties.getProperties()); + blockingSubscribe(initializeBroker(brokerProperties.getProperties())); } @AfterEach @@ -68,14 +68,13 @@ class CloudFoundryAcceptanceTest { blockingSubscribe(cleanup()); } - private void initializeBroker(String... backingAppProperties) { - - blockingSubscribe(cloudFoundryService + private Mono initializeBroker(String... backingAppProperties) { + return cloudFoundryService .getOrCreateDefaultOrganization() .then(cloudFoundryService.getOrCreateDefaultSpace()) .then(cloudFoundryService.pushAppBroker(SAMPLE_BROKER_APP_NAME, getSampleBrokerAppPath(), backingAppProperties)) .then(cloudFoundryService.createServiceBroker(SERVICE_BROKER_NAME, SAMPLE_BROKER_APP_NAME)) - .then(cloudFoundryService.enableServiceBrokerAccess(SERVICE_NAME))); + .then(cloudFoundryService.enableServiceBrokerAccess(SERVICE_NAME)); } private Mono cleanup() { @@ -119,13 +118,17 @@ class CloudFoundryAcceptanceTest { } Optional getApplicationSummaryByNameAndSpace(String appName, String space) { - return cloudFoundryService.getApplicationSummaryByName(appName, space).blockOptional(); + return cloudFoundryService.getApplicationSummaryByNameAndSpace(appName, space).blockOptional(); } ApplicationEnvironments getApplicationEnvironmentByName(String appName) { return cloudFoundryService.getApplicationEnvironmentByAppName(appName).block(); } + ApplicationEnvironments getApplicationEnvironmentByNameAndSpace(String appName, String space) { + return cloudFoundryService.getApplicationEnvironmentByAppNameAndSpace(appName, space).block(); + } + List getSpaces() { return cloudFoundryService.getSpaces().block(); } @@ -134,7 +137,7 @@ class CloudFoundryAcceptanceTest { return Paths.get(acceptanceTestProperties.getSampleBrokerAppPath(), ""); } - private void blockingSubscribe(Mono publisher){ + private void blockingSubscribe(Mono publisher) { CountDownLatch latch = new CountDownLatch(1); publisher.subscribe(System.out::println, t -> { t.printStackTrace(); diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CreateInstanceWithMultipleAppsAcceptanceTest.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CreateInstanceWithMultipleAppsAcceptanceTest.java new file mode 100644 index 0000000..4ad1d39 --- /dev/null +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CreateInstanceWithMultipleAppsAcceptanceTest.java @@ -0,0 +1,64 @@ +/* + * Copyright 2016-2018. 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. + * 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.springframework.cloud.appbroker.acceptance; + +import java.util.Optional; + +import org.cloudfoundry.operations.applications.ApplicationSummary; +import org.cloudfoundry.operations.services.ServiceInstanceSummary; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class CreateInstanceWithMultipleAppsAcceptanceTest extends CloudFoundryAcceptanceTest { + + private static final String BROKER_SAMPLE_APP_CREATE_1 = "broker-app-create-1"; + private static final String BROKER_SAMPLE_APP_CREATE_2 = "broker-app-create-2"; + + @Test + @AppBrokerTestProperties({ + "spring.cloud.appbroker.services[0].service-name=example", + "spring.cloud.appbroker.services[0].plan-name=standard", + "spring.cloud.appbroker.services[0].apps[0].name=" + BROKER_SAMPLE_APP_CREATE_1, + "spring.cloud.appbroker.services[0].apps[0].path=classpath:demo.jar", + "spring.cloud.appbroker.services[0].apps[1].name=" + BROKER_SAMPLE_APP_CREATE_2, + "spring.cloud.appbroker.services[0].apps[1].path=classpath:demo.jar", + }) + void shouldPushMultipleAppsWhenCreateServiceCalled() { + // when a service instance is created + createServiceInstance(); + + Optional serviceInstance = getServiceInstance(); + assertThat(serviceInstance).isNotEmpty(); + + // then the backing applications are deployed + Optional backingApplication1 = getApplicationSummaryByName(BROKER_SAMPLE_APP_CREATE_1); + assertThat(backingApplication1).isNotEmpty(); + Optional backingApplication2 = getApplicationSummaryByName(BROKER_SAMPLE_APP_CREATE_2); + assertThat(backingApplication2).isNotEmpty(); + + // when the service instance is deleted + deleteServiceInstance(); + + // then the backing applications are deleted + Optional backingApplication1AfterDelete = getApplicationSummaryByName(BROKER_SAMPLE_APP_CREATE_1); + assertThat(backingApplication1AfterDelete).isEmpty(); + Optional backingApplication2AfterDelete = getApplicationSummaryByName(BROKER_SAMPLE_APP_CREATE_2); + assertThat(backingApplication2AfterDelete).isEmpty(); + } + +} \ No newline at end of file diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CreateInstanceWithTargetAcceptanceTest.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CreateInstanceWithTargetAcceptanceTest.java index ad82f40..f30ff19 100644 --- a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CreateInstanceWithTargetAcceptanceTest.java +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/CreateInstanceWithTargetAcceptanceTest.java @@ -28,6 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat; class CreateInstanceWithTargetAcceptanceTest extends CloudFoundryAcceptanceTest { private static final String BROKER_SAMPLE_APP_CREATE_WITH_TARGET = "app-with-target"; + private static final String BROKER_SAMPLE_APP_CREATE_WITH_TARGET_OTHER = "app-other"; @Test @AppBrokerTestProperties({ @@ -35,32 +36,34 @@ class CreateInstanceWithTargetAcceptanceTest extends CloudFoundryAcceptanceTest "spring.cloud.appbroker.services[0].plan-name=standard", "spring.cloud.appbroker.services[0].apps[0].name=" + BROKER_SAMPLE_APP_CREATE_WITH_TARGET, "spring.cloud.appbroker.services[0].apps[0].path=classpath:demo.jar", - "spring.cloud.appbroker.services[0].apps[0].target.name=SpacePerServiceInstance" + "spring.cloud.appbroker.services[0].apps[0].target.name=SpacePerServiceInstance", + "spring.cloud.appbroker.services[0].apps[1].name=" + BROKER_SAMPLE_APP_CREATE_WITH_TARGET_OTHER, + "spring.cloud.appbroker.services[0].apps[1].path=classpath:demo.jar", + "spring.cloud.appbroker.services[0].apps[1].target.name=SpacePerServiceInstance" }) - void shouldPushAppWithTargetWhenCreateServiceCalled() { - // when a service instance is created + void shouldCreateMultipleAppsInSpace() { + // when a service instance is created with targets createServiceInstance(); Optional serviceInstance = getServiceInstance(); assertThat(serviceInstance).isNotEmpty(); - // then a backing application is deployed in a space named as the service instance id - String serviceInstanceId = serviceInstance.orElseThrow(RuntimeException::new).getId(); - String spaceName = serviceInstanceId; + // then backing applications are deployed in a space named as the service instance id + String space = serviceInstance.orElseThrow(RuntimeException::new).getId(); + Optional backingApplication = - getApplicationSummaryByNameAndSpace(BROKER_SAMPLE_APP_CREATE_WITH_TARGET, spaceName); + getApplicationSummaryByNameAndSpace(BROKER_SAMPLE_APP_CREATE_WITH_TARGET, space); assertThat(backingApplication).isNotEmpty(); - // and has its route with the service instance id appended to it - ApplicationSummary applicationSummary = backingApplication.orElseThrow(RuntimeException::new); - assertThat(applicationSummary.getUrls()).isNotEmpty(); - assertThat(applicationSummary.getUrls().get(0)).startsWith(BROKER_SAMPLE_APP_CREATE_WITH_TARGET + "-" + spaceName); + Optional backingApplicationOther = + getApplicationSummaryByNameAndSpace(BROKER_SAMPLE_APP_CREATE_WITH_TARGET_OTHER, space); + assertThat(backingApplicationOther).isNotEmpty(); // when the service instance is deleted deleteServiceInstance(); // then the space is deleted List spaces = getSpaces(); - assertThat(spaces).doesNotContain(spaceName); + assertThat(spaces).doesNotContain(space); } } \ No newline at end of file diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceAcceptanceTest.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceAcceptanceTest.java index c1e6b6d..4069a56 100644 --- a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceAcceptanceTest.java +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceAcceptanceTest.java @@ -42,6 +42,7 @@ class UpdateInstanceAcceptanceTest extends CloudFoundryAcceptanceTest { "spring.cloud.appbroker.services[0].apps[0].environment.parameter1=config1", "spring.cloud.appbroker.services[0].apps[0].environment.parameter2=config2", "spring.cloud.appbroker.services[0].apps[0].environment.parameter3=config3", + "spring.cloud.appbroker.services[0].apps[0].environment.parameter4=config4", "spring.cloud.appbroker.services[0].apps[0].parameters-transformers[0].name=EnvironmentMapping", "spring.cloud.appbroker.services[0].apps[0].parameters-transformers[0].args.include=parameter1,parameter3" }) @@ -76,6 +77,11 @@ class UpdateInstanceAcceptanceTest extends CloudFoundryAcceptanceTest { assertThat(applicationEnvironments.getUserProvided().get("SPRING_APPLICATION_JSON")).asString() .contains("\"parameter1\":\"value1\"") .contains("\"parameter2\":\"config2\"") - .contains("\"parameter3\":\"value3\""); + .contains("\"parameter3\":\"value3\"") + .contains("\"parameter4\":\"config4\""); + + // and the backing application contains the initial parameters + assertThat(applicationEnvironments.getUserProvided().get("SPRING_APPLICATION_JSON")).asString() + .contains("\"parameter4\":\"config4\""); } } \ No newline at end of file diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceWithTargetAcceptanceTest.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceWithTargetAcceptanceTest.java new file mode 100644 index 0000000..5ae4d3e --- /dev/null +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceWithTargetAcceptanceTest.java @@ -0,0 +1,84 @@ +/* + * Copyright 2016-2018. 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. + * 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.springframework.cloud.appbroker.acceptance; + +import java.time.Duration; +import java.util.Collections; +import java.util.List; +import java.util.Optional; + +import org.cloudfoundry.operations.applications.ApplicationEnvironments; +import org.cloudfoundry.operations.applications.ApplicationSummary; +import org.cloudfoundry.operations.services.ServiceInstanceSummary; +import org.cloudfoundry.util.DelayUtils; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class UpdateInstanceWithTargetAcceptanceTest extends CloudFoundryAcceptanceTest { + + private static final String BROKER_SAMPLE_APP_CREATE_WITH_TARGET = "app-with-target"; + + @Test + @AppBrokerTestProperties({ + "spring.cloud.appbroker.services[0].service-name=example", + "spring.cloud.appbroker.services[0].plan-name=standard", + "spring.cloud.appbroker.services[0].apps[0].name=" + BROKER_SAMPLE_APP_CREATE_WITH_TARGET, + "spring.cloud.appbroker.services[0].apps[0].path=classpath:demo.jar", + "spring.cloud.appbroker.services[0].apps[0].target.name=SpacePerServiceInstance", + "spring.cloud.appbroker.services[0].apps[0].environment.parameter1=config1" + }) + void shouldCreateAppInTargetWhenAddingNewProperties() { + // when a service instance is created + createServiceInstance(); + Optional serviceInstance = getServiceInstance(); + assertThat(serviceInstance).isNotEmpty(); + + // then a backing application is deployed in a space named as the service instance id + String serviceInstanceId = serviceInstance.orElseThrow(RuntimeException::new).getId(); + String spaceName = serviceInstanceId; + Optional backingApplication = + getApplicationSummaryByNameAndSpace(BROKER_SAMPLE_APP_CREATE_WITH_TARGET, spaceName); + assertThat(backingApplication).isNotEmpty(); + + // and has its route with the service instance id appended to it + ApplicationSummary applicationSummary = backingApplication.orElseThrow(RuntimeException::new); + assertThat(applicationSummary.getUrls()).isNotEmpty(); + assertThat(applicationSummary.getUrls().get(0)).startsWith(BROKER_SAMPLE_APP_CREATE_WITH_TARGET + "-" + spaceName); + + // when the service instance is updated + updateServiceInstance(Collections.singletonMap("parameter2", "config2")); + + getServiceInstanceMono() + .filter(summary -> summary.getLastOperation().contains("completed")) + .repeatWhenEmpty(DelayUtils.exponentialBackOff(Duration.ofSeconds(2), Duration.ofSeconds(15), Duration.ofMinutes(5))) + .blockOptional(); + + // then the service instance has the initial parameters + ApplicationEnvironments backingApplicationAfterUpdate = + getApplicationEnvironmentByNameAndSpace(BROKER_SAMPLE_APP_CREATE_WITH_TARGET, spaceName); + assertThat((String) backingApplicationAfterUpdate.getUserProvided().get("SPRING_APPLICATION_JSON")).contains("parameter1"); + + // when the service instance is deleted + deleteServiceInstance(); + + // then the space is deleted + List spaces = getSpaces(); + assertThat(spaces).doesNotContain(spaceName); + } + +} \ No newline at end of file diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/fixtures/cf/CloudFoundryService.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/fixtures/cf/CloudFoundryService.java index ebd8091..2122581 100644 --- a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/fixtures/cf/CloudFoundryService.java +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/fixtures/cf/CloudFoundryService.java @@ -128,14 +128,16 @@ public class CloudFoundryService { return loggingMono( cloudFoundryOperations .applications() - .delete(DeleteApplicationRequest.builder().name(appName).build())); + .delete(DeleteApplicationRequest.builder().name(appName).build())) + .onErrorResume(e -> Mono.empty()); } public Mono deleteServiceBroker(String brokerName) { return loggingMono( cloudFoundryOperations .serviceAdmin() - .delete(DeleteServiceBrokerRequest.builder().name(brokerName).build())); + .delete(DeleteServiceBrokerRequest.builder().name(brokerName).build())) + .onErrorResume(e -> Mono.empty()); } public Mono deleteServiceInstance(String serviceInstanceName) { @@ -144,7 +146,8 @@ public class CloudFoundryService { .flatMap(si -> cloudFoundryOperations .services() - .deleteInstance(DeleteServiceInstanceRequest.builder().name(si.getName()).build()))); + .deleteInstance(DeleteServiceInstanceRequest.builder().name(si.getName()).build()))) + .onErrorResume(e -> Mono.empty()); } private Mono getServiceInstanceFromList(String serviceInstanceName) { @@ -196,13 +199,9 @@ public class CloudFoundryService { return this.cloudFoundryOperations.spaces().list().map(SpaceSummary::getName).collectList(); } - public Mono getApplicationSummaryByName(String appName, String space) { + public Mono getApplicationSummaryByNameAndSpace(String appName, String space) { final String defaultOrg = cloudFoundryProperties.getDefaultOrg(); - return loggingFlux(DefaultCloudFoundryOperations.builder() - .cloudFoundryClient(cloudFoundryClient) - .organization(defaultOrg) - .space(space) - .build() + return loggingFlux(createOperationsForSpace(space, defaultOrg) .applications() .list() .filter(applicationSummary -> applicationSummary.getName().equals(appName)) @@ -216,6 +215,13 @@ public class CloudFoundryService { .getEnvironments(GetApplicationEnvironmentsRequest.builder().name(appName).build())); } + public Mono getApplicationEnvironmentByAppNameAndSpace(String appName, String space) { + final String defaultOrg = cloudFoundryProperties.getDefaultOrg(); + return loggingMono( + createOperationsForSpace(space, defaultOrg) + .applications() + .getEnvironments(GetApplicationEnvironmentsRequest.builder().name(appName).build())); + } public Mono getOrCreateDefaultSpace() { final String defaultOrg = cloudFoundryProperties.getDefaultOrg(); @@ -253,6 +259,14 @@ public class CloudFoundryService { .then(getDefaultOrg(organizationOperations)))); } + private DefaultCloudFoundryOperations createOperationsForSpace(String space, String defaultOrg) { + return DefaultCloudFoundryOperations.builder() + .cloudFoundryClient(cloudFoundryClient) + .organization(defaultOrg) + .space(space) + .build(); + } + private Mono getDefaultOrg(DefaultOrganizations orgOperations) { return orgOperations .list() @@ -306,7 +320,8 @@ public class CloudFoundryService { final String[] appPropertyKeyValue = appProperty.split("="); if (appPropertyKeyValue.length == 2) { backingAppVariables.put(appPropertyKeyValue[0], appPropertyKeyValue[1]); - } else { + } + else { throw new IllegalArgumentException(format("Backing app property '%s' is incorrectly formatted", Arrays.toString(appPropertyKeyValue))); } } diff --git a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfiguration.java b/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfiguration.java index ae8d256..a5bbb95 100644 --- a/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfiguration.java +++ b/spring-cloud-app-broker-autoconfigure/src/main/java/org/springframework/cloud/appbroker/autoconfigure/AppBrokerAutoConfiguration.java @@ -139,8 +139,9 @@ public class AppBrokerAutoConfiguration { @Bean public UpdateServiceInstanceWorkflow updateServiceInstanceWorkflow(BrokeredServices brokeredServices, BackingAppDeploymentService backingAppDeploymentService, - ParametersTransformationService parametersTransformationService) { - return new AppDeploymentUpdateServiceInstanceWorkflow(brokeredServices, backingAppDeploymentService, parametersTransformationService); + ParametersTransformationService parametersTransformationService, + TargetService targetService) { + return new AppDeploymentUpdateServiceInstanceWorkflow(brokeredServices, backingAppDeploymentService, parametersTransformationService, targetService); } @Bean diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflow.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflow.java index 7303a76..091729b 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflow.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflow.java @@ -24,6 +24,7 @@ import reactor.util.Loggers; import org.springframework.cloud.appbroker.deployer.BackingAppDeploymentService; import org.springframework.cloud.appbroker.deployer.BrokeredServices; import org.springframework.cloud.appbroker.extensions.parameters.ParametersTransformationService; +import org.springframework.cloud.appbroker.extensions.targets.TargetService; import org.springframework.cloud.appbroker.service.UpdateServiceInstanceWorkflow; import org.springframework.cloud.servicebroker.model.instance.UpdateServiceInstanceRequest; import org.springframework.cloud.servicebroker.model.instance.UpdateServiceInstanceResponse.UpdateServiceInstanceResponseBuilder; @@ -38,17 +39,21 @@ public class AppDeploymentUpdateServiceInstanceWorkflow private final BackingAppDeploymentService deploymentService; private final ParametersTransformationService parametersTransformationService; + private final TargetService targetService; public AppDeploymentUpdateServiceInstanceWorkflow(BrokeredServices brokeredServices, BackingAppDeploymentService deploymentService, - ParametersTransformationService parametersTransformationService) { + ParametersTransformationService parametersTransformationService, + TargetService targetService) { super(brokeredServices); this.deploymentService = deploymentService; this.parametersTransformationService = parametersTransformationService; + this.targetService = targetService; } public Flux update(UpdateServiceInstanceRequest request) { return getBackingApplicationsForService(request.getServiceDefinition(), request.getPlanId()) + .flatMap(backingApps -> targetService.add(backingApps, request.getServiceInstanceId())) .flatMap(backingApps -> parametersTransformationService.transformParameters(backingApps, request.getParameters())) .flatMapMany(deploymentService::deploy) diff --git a/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflowTest.java b/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflowTest.java index 04caccf..f5acad9 100644 --- a/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflowTest.java +++ b/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentUpdateServiceInstanceWorkflowTest.java @@ -34,6 +34,7 @@ import org.springframework.cloud.appbroker.deployer.BackingApplications; import org.springframework.cloud.appbroker.deployer.BrokeredService; import org.springframework.cloud.appbroker.deployer.BrokeredServices; import org.springframework.cloud.appbroker.extensions.parameters.ParametersTransformationService; +import org.springframework.cloud.appbroker.extensions.targets.TargetService; import org.springframework.cloud.servicebroker.model.catalog.Plan; import org.springframework.cloud.servicebroker.model.catalog.ServiceDefinition; import org.springframework.cloud.servicebroker.model.instance.UpdateServiceInstanceRequest; @@ -52,6 +53,9 @@ class AppDeploymentUpdateServiceInstanceWorkflowTest { @Mock private ParametersTransformationService parametersTransformationService; + @Mock + private TargetService targetService; + private BackingApplications backingApps; private AppDeploymentUpdateServiceInstanceWorkflow updateServiceInstanceWorkflow; @@ -78,10 +82,11 @@ class AppDeploymentUpdateServiceInstanceWorkflowTest { .build()) .build(); - updateServiceInstanceWorkflow = new AppDeploymentUpdateServiceInstanceWorkflow( - brokeredServices, + updateServiceInstanceWorkflow = new AppDeploymentUpdateServiceInstanceWorkflow(brokeredServices, backingAppDeploymentService, - parametersTransformationService); + parametersTransformationService, + targetService) + ; } @Test @@ -91,6 +96,8 @@ class AppDeploymentUpdateServiceInstanceWorkflowTest { given(this.backingAppDeploymentService.deploy(eq(backingApps))) .willReturn(Flux.just("app1", "app2")); + given(this.targetService.add(eq(backingApps), eq("service-instance-id"))) + .willReturn(Mono.just(backingApps)); given(this.parametersTransformationService.transformParameters(eq(backingApps), eq(request.getParameters()))) .willReturn(Mono.just(backingApps)); @@ -102,6 +109,7 @@ class AppDeploymentUpdateServiceInstanceWorkflowTest { verifyNoMoreInteractions(this.backingAppDeploymentService); verifyNoMoreInteractions(this.parametersTransformationService); + verifyNoMoreInteractions(this.targetService); } @Test @@ -113,6 +121,8 @@ class AppDeploymentUpdateServiceInstanceWorkflowTest { .willReturn(Flux.just("app1", "app2")); given(this.parametersTransformationService.transformParameters(eq(backingApps), eq(request.getParameters()))) .willReturn(Mono.just(backingApps)); + given(this.targetService.add(eq(backingApps), eq("service-instance-id"))) + .willReturn(Mono.just(backingApps)); StepVerifier .create(updateServiceInstanceWorkflow.update(request)) @@ -122,6 +132,7 @@ class AppDeploymentUpdateServiceInstanceWorkflowTest { verifyNoMoreInteractions(this.backingAppDeploymentService); verifyNoMoreInteractions(this.parametersTransformationService); + verifyNoMoreInteractions(this.targetService); } @Test @@ -132,6 +143,7 @@ class AppDeploymentUpdateServiceInstanceWorkflowTest { verifyNoMoreInteractions(this.backingAppDeploymentService); verifyNoMoreInteractions(this.parametersTransformationService); + verifyNoMoreInteractions(this.targetService); } private UpdateServiceInstanceRequest buildRequest(String serviceName, String planName) { 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 067c484..ffd0b93 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 @@ -201,14 +201,18 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware } private Mono createSpace(String spaceName) { - return getDefaultOrganizationId() + Mono createSpacePublisher = getDefaultOrganizationId() .flatMap(orgId -> this.client.spaces() - .create(CreateSpaceRequest.builder() - .organizationId(orgId) - .name(spaceName) - .build()) - .doOnSuccess(response -> logger.info("Created space {}", spaceName)) - .then(Mono.empty())); + .create(CreateSpaceRequest.builder() + .organizationId(orgId) + .name(spaceName) + .build()) + .doOnSuccess(response -> logger.info("Created space {}", spaceName)) + .doOnError(e -> logger.warn(String.format("Error creating space %s. Exception Message %s", spaceName, e.getMessage()))) + .onErrorResume(e -> Mono.empty()) + .then(Mono.empty())); + return getSpaceIdFromName(spaceName) + .switchIfEmpty(createSpacePublisher).then(); } private Mono getDefaultOrganizationId() { @@ -252,16 +256,22 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware } private Mono deleteApplicationInSpace(String name, String spaceName) { - return createCloudFoundryOperationsForSpace(spaceName).applications() - .delete(DeleteApplicationRequest.builder() - .deleteRoutes(this.defaultDeploymentProperties.isDeleteRoutes()) - .name(name) - .build()) - .then(deleteSpace(spaceName)); + return getSpaceIdFromName(spaceName) + .doOnError(error -> logger.warn("Unable get space name: {} ", spaceName)) + .then(createCloudFoundryOperationsForSpace(spaceName) + .applications() + .delete(DeleteApplicationRequest.builder() + .deleteRoutes(this.defaultDeploymentProperties.isDeleteRoutes()) + .name(name) + .build()) + .doOnError(error -> logger.warn("Unable delete application: {} ", name)) + .then(deleteSpace(spaceName))) + .onErrorResume(e -> Mono.empty()); } private Mono deleteSpace(String spaceName) { return getSpaceIdFromName(spaceName) + .doOnError(error -> logger.warn("Unable get space name: {} ", spaceName)) .flatMap(spaceId -> this.client.spaces() .delete(DeleteSpaceRequest.builder() .spaceId(spaceId) @@ -270,10 +280,12 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware } private Mono getSpaceIdFromName(String spaceName) { - return this.operations.spaces().get(GetSpaceRequest.builder() - .name(spaceName) - .build()) - .map(SpaceDetail::getId); + return this.operations.spaces() + .get(GetSpaceRequest.builder() + .name(spaceName) + .build()) + .map(SpaceDetail::getId) + .onErrorResume(e -> Mono.empty()); } private CloudFoundryOperations createCloudFoundryOperationsForSpace(String space) {