SpringCloud/spring-cloud-app-broker
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Prevent duplicate requests to delete backing services

Browse Source 
Backing services and bound services are compared and any instance with the
same service instance name and target space are considered duplicates.
This commit is contained in:
Roy Clarkson
2020-05-14 16:10:50 -04:00
committed by Roy Clarkson
parent 2f2e183aca
commit d0ea13f2e6
3 changed files with 90 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/BackingService.java
View File

@@ -120,6 +120,14 @@ public class BackingService {
this.rebindOnUpdate = rebindOnUpdate;
}
public int serviceInstanceNameAndSpaceHashCode() {
String space = null;
if (!CollectionUtils.isEmpty(properties)) {
space = properties.get(DeploymentProperties.TARGET_PROPERTY_KEY);
}
return Objects.hash(serviceInstanceName, space);
}
@Override
public boolean equals(Object o) {
if (this == o) {

59
spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflow.java
View File

@@ -16,7 +16,8 @@
package org.springframework.cloud.appbroker.workflow.instance;
import java.util.List;
import java.util.Collections;
import java.util.Map;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
@@ -27,6 +28,7 @@ import org.springframework.cloud.appbroker.deployer.BackingAppDeploymentService;
import org.springframework.cloud.appbroker.deployer.BackingService;
import org.springframework.cloud.appbroker.deployer.BackingServicesProvisionService;
import org.springframework.cloud.appbroker.deployer.BrokeredServices;
import org.springframework.cloud.appbroker.deployer.DeploymentProperties;
import org.springframework.cloud.appbroker.extensions.credentials.CredentialProviderService;
import org.springframework.cloud.appbroker.extensions.targets.TargetService;
import org.springframework.cloud.appbroker.manager.BackingAppManagementService;
@@ -35,6 +37,7 @@ import org.springframework.cloud.servicebroker.model.instance.DeleteServiceInsta
import org.springframework.cloud.servicebroker.model.instance.DeleteServiceInstanceResponse;
import org.springframework.cloud.servicebroker.model.instance.DeleteServiceInstanceResponse.DeleteServiceInstanceResponseBuilder;
import org.springframework.core.annotation.Order;
import org.springframework.util.CollectionUtils;
@Order(0)
public class AppDeploymentDeleteServiceInstanceWorkflow
@@ -75,11 +78,14 @@ public class AppDeploymentDeleteServiceInstanceWorkflow
}
private Flux<String> deleteBackingServices(DeleteServiceInstanceRequest request) {
return collectConfiguredBackingServices(request)
.mergeWith(collectBoundBackingServices(request))
.doOnEach(backingServices -> log.debug("Deleting backing services {} for {}/{}",
backingServices, request.getServiceDefinition().getName(), request.getPlan().getName()))
.flatMap(backingServicesProvisionService::deleteServiceInstance)
return collectBackingServices(request)
.collectList()
.flatMapMany(backingServices -> {
if (!CollectionUtils.isEmpty(backingServices)) {
return backingServicesProvisionService.deleteServiceInstance(backingServices);
}
return Flux.empty();
})
.doOnComplete(() -> log.debug("Finished deleting backing services for {}/{}",
request.getServiceDefinition().getName(), request.getPlan().getName()))
.doOnError(exception -> log.error(String.format("Error deleting backing services for %s/%s with error '%s'",
@@ -87,22 +93,39 @@ public class AppDeploymentDeleteServiceInstanceWorkflow
exception));
}
private Flux<List<BackingService>> collectConfiguredBackingServices(DeleteServiceInstanceRequest request) {
return getBackingServicesForService(request.getServiceDefinition(), request.getPlan())
.flatMapMany(backingServices -> getTargetForService(request.getServiceDefinition(), request.getPlan())
.flatMap(targetSpec -> targetService.addToBackingServices(backingServices, targetSpec,
request.getServiceInstanceId()))
.defaultIfEmpty(backingServices));
private Flux<BackingService> collectBackingServices(DeleteServiceInstanceRequest request) {
return collectConfiguredBackingServices(request)
.concatWith(collectBoundBackingServices(request))
.distinct(BackingService::serviceInstanceNameAndSpaceHashCode);
}
private Flux<List<BackingService>> collectBoundBackingServices(DeleteServiceInstanceRequest request) {
private Flux<BackingService> collectConfiguredBackingServices(DeleteServiceInstanceRequest request) {
return getBackingServicesForService(request.getServiceDefinition(), request.getPlan())
.flatMap(backingServices -> getTargetForService(request.getServiceDefinition(), request.getPlan())
.flatMap(targetSpec -> targetService.addToBackingServices(backingServices, targetSpec,
request.getServiceInstanceId()))
.defaultIfEmpty(backingServices))
.flatMapMany(Flux::fromIterable);
}
private Flux<BackingService> collectBoundBackingServices(DeleteServiceInstanceRequest request) {
return backingAppManagementService.getDeployedBackingApplications(request.getServiceInstanceId())
.flatMapMany(Flux::fromIterable)
.flatMap(backingApplication -> Flux.fromIterable(backingApplication.getServices())
.map(servicesSpec -> BackingService.builder()
.serviceInstanceName(servicesSpec.getServiceInstanceName())
.build())
.collectList());
.flatMap(backingApplication -> Mono.justOrEmpty(backingApplication.getServices())
.flatMapMany(Flux::fromIterable)
.flatMap(servicesSpec -> Mono.justOrEmpty(servicesSpec.getServiceInstanceName()))
.map(serviceInstanceName -> {
Map<String, String> properties = null;
if (!CollectionUtils.isEmpty(backingApplication.getProperties())) {
String target = backingApplication.getProperties()
.get(DeploymentProperties.TARGET_PROPERTY_KEY);
properties = Collections.singletonMap(DeploymentProperties.TARGET_PROPERTY_KEY, target);
}
return BackingService.builder()
.serviceInstanceName(serviceInstanceName)
.properties(properties)
.build();
}));
}
private Flux<String> undeployBackingApplications(DeleteServiceInstanceRequest request) {

54
spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/workflow/instance/AppDeploymentDeleteServiceInstanceWorkflowTest.java
View File

@@ -16,6 +16,8 @@
package org.springframework.cloud.appbroker.workflow.instance;
import java.util.Collections;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -34,6 +36,7 @@ import org.springframework.cloud.appbroker.deployer.BackingServices;
import org.springframework.cloud.appbroker.deployer.BackingServicesProvisionService;
import org.springframework.cloud.appbroker.deployer.BrokeredService;
import org.springframework.cloud.appbroker.deployer.BrokeredServices;
import org.springframework.cloud.appbroker.deployer.DeploymentProperties;
import org.springframework.cloud.appbroker.deployer.ServicesSpec;
import org.springframework.cloud.appbroker.deployer.TargetSpec;
import org.springframework.cloud.appbroker.extensions.credentials.CredentialProviderService;
@@ -74,6 +77,8 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
private BackingServices backingServices;
private BackingServices backingServices2;
private TargetSpec targetSpec;
private DeleteServiceInstanceWorkflow deleteServiceInstanceWorkflow;
@@ -104,7 +109,21 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
.build())
.build();
targetSpec = TargetSpec.builder().name("TargetSpace").build();
this.backingServices2 = BackingServices
.builder()
.backingService(BackingService
.builder()
.name("my-service2")
.plan("a-plan2")
.serviceInstanceName("my-service-instance2")
.properties(Collections.singletonMap(DeploymentProperties.TARGET_PROPERTY_KEY, "my-space2"))
.build())
.build();
targetSpec = TargetSpec.builder()
.name("TargetSpace")
.build();
BrokeredServices brokeredServices = BrokeredServices
.builder()
.service(BrokeredService
@@ -121,6 +140,13 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
.services(backingServices)
.target(targetSpec)
.build())
.service(BrokeredService.builder()
.serviceName("service3")
.planName("plan3")
.apps(backingApps)
.services(backingServices2)
.target(targetSpec)
.build())
.build();
deleteServiceInstanceWorkflow =
@@ -165,22 +191,21 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
.expectNext()
.verifyComplete();
// the same service is a configured backing service, and it is bound to two different deployed apps
verify(this.backingServicesProvisionService, Mockito.times(3)).deleteServiceInstance(any());
verify(this.backingServicesProvisionService, Mockito.times(1)).deleteServiceInstance(any());
verifyNoMoreInteractionsWithServices();
}
@Test
void deleteServiceInstanceSucceedsWhenBackingServicesDifferFromConfiguration() {
DeleteServiceInstanceRequest request = buildRequest("service1", "plan1");
DeleteServiceInstanceRequest request = buildRequest("service3", "plan3");
DeleteServiceInstanceResponse response = DeleteServiceInstanceResponse.builder().build();
given(this.backingAppDeploymentService.undeploy(eq(backingApps)))
.willReturn(Flux.just("undeployed1", "undeployed2"));
// configured backing services
given(this.targetService.addToBackingServices(eq(backingServices), eq(targetSpec), eq("service-instance-id")))
.willReturn(Mono.just(backingServices));
given(this.targetService.addToBackingServices(eq(backingServices2), eq(targetSpec), eq("service-instance-id")))
.willReturn(Mono.just(backingServices2));
// different bound services
given(this.backingAppManagementService.getDeployedBackingApplications(eq(request.getServiceInstanceId())))
@@ -191,10 +216,14 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
.willReturn(Mono.just(backingApps));
given(this.backingServicesProvisionService.deleteServiceInstance(argThat(backingServices -> {
boolean nameMatch1 = "different-service-instance".equals(backingServices.get(0).getServiceInstanceName());
boolean nameMatch2 = "my-service-instance".equals(backingServices.get(0).getServiceInstanceName());
boolean sizeMatch = backingServices.size() == 1;
return sizeMatch && (nameMatch1 || nameMatch2);
boolean nameMatch0 = "my-service-instance2".equals(backingServices.get(0).getServiceInstanceName());
boolean spaceMatch0 = "my-space2".equals(backingServices.get(0).getProperties()
.get(DeploymentProperties.TARGET_PROPERTY_KEY));
boolean nameMatch1 = "different-service-instance".equals(backingServices.get(1).getServiceInstanceName());
boolean spaceMatch1 = "TargetSpace".equals(backingServices.get(1).getProperties()
.get(DeploymentProperties.TARGET_PROPERTY_KEY));
boolean sizeMatch = backingServices.size() == 2;
return sizeMatch && (nameMatch0 && spaceMatch0 || nameMatch1 && spaceMatch1);
}))).willReturn(Flux.just("different-service-instance"));
StepVerifier.create(deleteServiceInstanceWorkflow.delete(request, response))
@@ -202,8 +231,7 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
.expectNext()
.verifyComplete();
// the same service is a configured backing service, and it is bound to two different deployed apps
verify(this.backingServicesProvisionService, Mockito.times(3)).deleteServiceInstance(any());
verify(this.backingServicesProvisionService, Mockito.times(1)).deleteServiceInstance(any());
verifyNoMoreInteractionsWithServices();
}
@@ -213,7 +241,7 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
DeleteServiceInstanceResponse response = DeleteServiceInstanceResponse.builder().build();
given(this.backingAppManagementService.getDeployedBackingApplications(eq(request.getServiceInstanceId())))
.willReturn(Mono.just(BackingApplications.builder().build()));
.willReturn(Mono.empty());
StepVerifier
.create(deleteServiceInstanceWorkflow.delete(request, response))