From 71f12d951ec8963bf9bcf0cea0af56b00b47183c Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Thu, 10 Jan 2019 14:30:09 -0600 Subject: [PATCH] Fix rebinding of backing services on update --- ...ateInstanceWithServicesAcceptanceTest.java | 2 -- .../cloudfoundry/CloudFoundryAppDeployer.java | 34 +++++++++++++++---- .../CloudFoundryAppDeployerTest.java | 15 +++++++- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceWithServicesAcceptanceTest.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceWithServicesAcceptanceTest.java index 2d18c99..85dee3d 100644 --- a/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceWithServicesAcceptanceTest.java +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org.springframework.cloud.appbroker.acceptance/UpdateInstanceWithServicesAcceptanceTest.java @@ -18,7 +18,6 @@ package org.springframework.cloud.appbroker.acceptance; import org.cloudfoundry.operations.applications.ApplicationSummary; import org.cloudfoundry.operations.services.ServiceInstance; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import java.util.HashMap; @@ -40,7 +39,6 @@ class UpdateInstanceWithServicesAcceptanceTest extends CloudFoundryAcceptanceTes private HealthListener healthListener; @Test - @Disabled @AppBrokerTestProperties({ "spring.cloud.appbroker.services[0].service-name=" + APP_SERVICE_NAME, "spring.cloud.appbroker.services[0].plan-name=" + PLAN_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 2d08c2c..ddce744 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 @@ -72,6 +72,7 @@ import org.cloudfoundry.operations.applications.PushApplicationManifestRequest; import org.cloudfoundry.operations.applications.Route; import org.cloudfoundry.operations.organizations.OrganizationDetail; import org.cloudfoundry.operations.organizations.OrganizationInfoRequest; +import org.cloudfoundry.operations.services.BindServiceInstanceRequest; import org.cloudfoundry.operations.services.GetServiceInstanceRequest; import org.cloudfoundry.operations.services.ServiceInstance; import org.cloudfoundry.operations.services.UnbindServiceInstanceRequest; @@ -174,7 +175,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .get(GetApplicationRequest.builder().name(name).build()) .map(ApplicationDetail::getId) .flatMap(applicationId -> - updateApplicationEnvironment(environmentVariables, applicationId) + updateApplicationEnvironment(applicationId, environmentVariables) .thenReturn(applicationId) ) .flatMap(applicationId -> Mono.zip(Mono.just(applicationId), @@ -298,7 +299,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware } private Mono updateApplicationEnvironment( - Map environmentVariables, String applicationId) { + String applicationId, Map environmentVariables) { return this.client.applicationsV2() .update(org.cloudfoundry.client.v2.applications.UpdateApplicationRequest .builder() @@ -699,7 +700,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware @Override public Mono updateServiceInstance(UpdateServiceInstanceRequest request) { CloudFoundryOperations cloudFoundryOperations = getOperations(request.getProperties()); - return unbindServiceInstanceIfNecessary(request, cloudFoundryOperations) + return rebindServiceInstanceIfNecessary(request, cloudFoundryOperations) .then(updateServiceInstanceIfNecessary(request, cloudFoundryOperations)); } @@ -725,7 +726,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware } private Mono unbindServiceInstance(String serviceInstanceName, - CloudFoundryOperations cloudFoundryOperations) { + CloudFoundryOperations cloudFoundryOperations) { return cloudFoundryOperations.services().getInstance(GetServiceInstanceRequest.builder() .name(serviceInstanceName) .build()) @@ -740,10 +741,31 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .then(Mono.empty())); } - private Mono unbindServiceInstanceIfNecessary(UpdateServiceInstanceRequest request, + private Mono rebindServiceInstance(String serviceInstanceName, + CloudFoundryOperations cloudFoundryOperations) { + return cloudFoundryOperations.services().getInstance(GetServiceInstanceRequest.builder() + .name(serviceInstanceName) + .build()) + .map(ServiceInstance::getApplications) + .flatMap(applications -> Flux.fromIterable(applications) + .flatMap(application -> cloudFoundryOperations.services().unbind( + UnbindServiceInstanceRequest.builder() + .applicationName(application) + .serviceInstanceName(serviceInstanceName) + .build()) + .then(cloudFoundryOperations.services().bind( + BindServiceInstanceRequest.builder() + .applicationName(application) + .serviceInstanceName(serviceInstanceName) + .build())) + ) + .then(Mono.empty())); + } + + private Mono rebindServiceInstanceIfNecessary(UpdateServiceInstanceRequest request, CloudFoundryOperations cloudFoundryOperations) { if (request.isRebindOnUpdate()) { - return unbindServiceInstance(request.getServiceInstanceName(), cloudFoundryOperations); + return rebindServiceInstance(request.getServiceInstanceName(), cloudFoundryOperations); } return Mono.empty(); } 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 1c809aa..f78ef46 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 @@ -28,6 +28,7 @@ import org.cloudfoundry.operations.applications.ApplicationHealthCheck; import org.cloudfoundry.operations.applications.ApplicationManifest; import org.cloudfoundry.operations.applications.Applications; import org.cloudfoundry.operations.applications.PushApplicationManifestRequest; +import org.cloudfoundry.operations.services.BindServiceInstanceRequest; import org.cloudfoundry.operations.services.GetServiceInstanceRequest; import org.cloudfoundry.operations.services.ServiceInstance; import org.cloudfoundry.operations.services.ServiceInstanceType; @@ -392,7 +393,7 @@ class CloudFoundryAppDeployerTest { } @Test - void updateServiceInstanceUnbindsWhenRequired() { + void updateServiceInstanceRebindsWhenRequired() { when(operationsServices.updateInstance( org.cloudfoundry.operations.services.UpdateServiceInstanceRequest.builder() .serviceInstanceName("service-instance-name") @@ -422,6 +423,18 @@ class CloudFoundryAppDeployerTest { .build())) .thenReturn(Mono.empty()); + when(operationsServices.bind(BindServiceInstanceRequest.builder() + .applicationName("app1") + .serviceInstanceName("service-instance-name") + .build())) + .thenReturn(Mono.empty()); + + when(operationsServices.bind(BindServiceInstanceRequest.builder() + .applicationName("app2") + .serviceInstanceName("service-instance-name") + .build())) + .thenReturn(Mono.empty()); + UpdateServiceInstanceRequest request = UpdateServiceInstanceRequest.builder() .serviceInstanceName("service-instance-name")