Fix an issue where configured backing services are not being deleted

When a service instance is deleted, all configured backing services will
be deleted as well as all services bound to any backing applications

Resolve #344
This commit is contained in:
Roy Clarkson
2020-05-11 14:37:42 -04:00
committed by Roy Clarkson
parent d370ae50c1
commit 6077c4e82d
5 changed files with 263 additions and 63 deletions

View File

@@ -16,6 +16,8 @@
package org.springframework.cloud.appbroker.workflow.instance;
import java.util.List;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.util.Logger;
@@ -73,14 +75,8 @@ public class AppDeploymentDeleteServiceInstanceWorkflow
}
private Flux<String> deleteBackingServices(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())
return collectConfiguredBackingServices(request)
.mergeWith(collectBoundBackingServices(request))
.doOnEach(backingServices -> log.debug("Deleting backing services {} for {}/{}",
backingServices, request.getServiceDefinition().getName(), request.getPlan().getName()))
.flatMap(backingServicesProvisionService::deleteServiceInstance)
@@ -91,6 +87,23 @@ public class AppDeploymentDeleteServiceInstanceWorkflow
exception));
}
private Flux<List<BackingService>> collectConfiguredBackingServices(DeleteServiceInstanceRequest request) {
return getBackingServicesForService(request.getServiceDefinition(), request.getPlan())
.flatMapMany(backingServices -> targetService.addToBackingServices(backingServices,
getTargetForService(request.getServiceDefinition(), request.getPlan()),
request.getServiceInstanceId()));
}
private Flux<List<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());
}
private Flux<String> undeployBackingApplications(DeleteServiceInstanceRequest request) {
return getBackingApplicationsForService(request.getServiceDefinition(), request.getPlan())
.flatMap(backingApps ->

View File

@@ -20,6 +20,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
@@ -44,9 +45,11 @@ import org.springframework.cloud.servicebroker.model.catalog.ServiceDefinition;
import org.springframework.cloud.servicebroker.model.instance.DeleteServiceInstanceRequest;
import org.springframework.cloud.servicebroker.model.instance.DeleteServiceInstanceResponse;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
@ExtendWith(MockitoExtension.class)
@@ -69,6 +72,8 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
private BackingApplications backingApps;
private BackingServices backingServices;
private TargetSpec targetSpec;
private DeleteServiceInstanceWorkflow deleteServiceInstanceWorkflow;
@@ -89,7 +94,7 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
.build())
.build();
BackingServices backingServices = BackingServices
this.backingServices = BackingServices
.builder()
.backingService(BackingService
.builder()
@@ -110,6 +115,12 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
.services(backingServices)
.target(targetSpec)
.build())
.service(BrokeredService.builder()
.serviceName("service2")
.planName("plan2")
.services(backingServices)
.target(targetSpec)
.build())
.build();
deleteServiceInstanceWorkflow =
@@ -123,18 +134,25 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
}
@Test
void deleteServiceInstanceSucceeds() {
void deleteServiceInstanceWithDeployedAppsAndBoundServicesSucceeds() {
DeleteServiceInstanceRequest request = buildRequest("service1", "plan1");
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));
// services bound to deployed apps
given(this.backingAppManagementService.getDeployedBackingApplications(eq(request.getServiceInstanceId())))
.willReturn(Mono.just(getExistingBackingAppsWithService("my-service-instance")));
given(this.credentialProviderService.deleteCredentials(eq(backingApps), eq(request.getServiceInstanceId())))
.willReturn(Mono.just(backingApps));
given(this.targetService.addToBackingApplications(eq(backingApps), eq(targetSpec), eq("service-instance-id")))
.willReturn(Mono.just(backingApps));
given(this.backingServicesProvisionService.deleteServiceInstance(argThat(backingServices -> {
boolean nameMatch = "my-service-instance".equals(backingServices.get(0).getServiceInstanceName());
boolean sizeMatch = backingServices.size() == 1;
@@ -147,6 +165,8 @@ 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());
verifyNoMoreInteractionsWithServices();
}
@@ -157,24 +177,33 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
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));
// different bound services
given(this.backingAppManagementService.getDeployedBackingApplications(eq(request.getServiceInstanceId())))
.willReturn(Mono.just(getExistingBackingAppsWithService("different-service-instance")));
given(this.credentialProviderService.deleteCredentials(eq(backingApps), eq(request.getServiceInstanceId())))
.willReturn(Mono.just(backingApps));
given(this.targetService.addToBackingApplications(eq(backingApps), eq(targetSpec), eq("service-instance-id")))
.willReturn(Mono.just(backingApps));
given(this.backingServicesProvisionService.deleteServiceInstance(argThat(backingServices -> {
boolean nameMatch = "different-service-instance".equals(backingServices.get(0).getServiceInstanceName());
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 && nameMatch;
return sizeMatch && (nameMatch1 || nameMatch2);
}))).willReturn(Flux.just("different-service-instance"));
StepVerifier
.create(deleteServiceInstanceWorkflow.delete(request, response))
StepVerifier.create(deleteServiceInstanceWorkflow.delete(request, response))
.expectNext()
.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());
verifyNoMoreInteractionsWithServices();
}
@@ -184,7 +213,7 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
DeleteServiceInstanceResponse response = DeleteServiceInstanceResponse.builder().build();
given(this.backingAppManagementService.getDeployedBackingApplications(eq(request.getServiceInstanceId())))
.willReturn(Mono.just(BackingApplications.builder().build()));
.willReturn(Mono.just(BackingApplications.builder().build()));
StepVerifier
.create(deleteServiceInstanceWorkflow.delete(request, response))
@@ -193,6 +222,36 @@ class AppDeploymentDeleteServiceInstanceWorkflowTest {
verifyNoMoreInteractionsWithServices();
}
@Test
void deleteServiceInstanceWithOnlyBoundServicesSucceeds() {
DeleteServiceInstanceRequest request = buildRequest("service2", "plan2");
DeleteServiceInstanceResponse response = DeleteServiceInstanceResponse.builder().build();
// configured backing services
given(this.targetService.addToBackingServices(eq(backingServices), eq(targetSpec), eq("service-instance-id")))
.willReturn(Mono.just(backingServices));
// no backing apps
given(this.backingAppManagementService.getDeployedBackingApplications(eq(request.getServiceInstanceId())))
.willReturn(Mono.empty());
given(this.credentialProviderService.deleteCredentials(any(), eq(request.getServiceInstanceId())))
.willReturn(Mono.empty());
given(this.backingServicesProvisionService.deleteServiceInstance(argThat(backingServices -> {
boolean nameMatch = "my-service-instance".equals(backingServices.get(0).getServiceInstanceName());
boolean sizeMatch = backingServices.size() == 1;
return sizeMatch && nameMatch;
}))).willReturn(Flux.just("my-service-instance"));
StepVerifier.create(deleteServiceInstanceWorkflow.delete(request, response))
.expectNext()
.expectNext()
.verifyComplete();
verify(this.backingServicesProvisionService, Mockito.times(1)).deleteServiceInstance(any());
verifyNoMoreInteractionsWithServices();
}
private void verifyNoMoreInteractionsWithServices() {
verifyNoMoreInteractions(this.backingServicesProvisionService);
verifyNoMoreInteractions(this.backingAppDeploymentService);

View File

@@ -0,0 +1,159 @@
/*
* Copyright 2002-2020 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
*
* https://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.integration;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.cloud.appbroker.integration.fixtures.CloudControllerStubFixture;
import org.springframework.cloud.appbroker.integration.fixtures.OpenServiceBrokerApiFixture;
import org.springframework.cloud.servicebroker.model.instance.OperationState;
import org.springframework.http.HttpStatus;
import org.springframework.test.context.TestPropertySource;
import static io.restassured.RestAssured.given;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.Matchers.either;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.APP_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.BACKING_PLAN_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.BACKING_SERVICE_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.BACKING_SI_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.PLAN_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.SERVICE_NAME;
@TestPropertySource(properties = {
"spring.cloud.appbroker.services[0].service-name=" + SERVICE_NAME,
"spring.cloud.appbroker.services[0].plan-name=" + PLAN_NAME,
"spring.cloud.appbroker.services[0].apps[0].path=classpath:demo.jar",
"spring.cloud.appbroker.services[0].apps[0].name=" + APP_NAME,
"spring.cloud.appbroker.services[0].apps[0].services[0].service-instance-name=" + BACKING_SI_NAME,
"spring.cloud.appbroker.services[0].services[0].service-instance-name=" + BACKING_SI_NAME,
"spring.cloud.appbroker.services[0].services[0].name=" + BACKING_SERVICE_NAME,
"spring.cloud.appbroker.services[0].services[0].plan=" + BACKING_PLAN_NAME
})
class DeleteInstanceWithAppsAndServicesComponentTest extends WiremockComponentTest {
protected static final String APP_NAME = "app-delete-with-services";
protected static final String SERVICE_NAME = "example";
protected static final String PLAN_NAME = "standard";
protected static final String BACKING_SI_NAME = "my-db-service";
protected static final String BACKING_SERVICE_NAME = "db-service";
protected static final String BACKING_PLAN_NAME = "backing-standard";
@Autowired
private OpenServiceBrokerApiFixture brokerFixture;
@Autowired
private CloudControllerStubFixture cloudControllerFixture;
@Test
void deleteAppsAndServicesWhenTheyExist() {
cloudControllerFixture.stubGetServiceInstanceWithNoBinding("instance-id", "instance-name",
SERVICE_NAME, PLAN_NAME);
cloudControllerFixture.stubAppExistsWithBackingService(APP_NAME, BACKING_SI_NAME,
BACKING_SERVICE_NAME, BACKING_PLAN_NAME);
cloudControllerFixture.stubServiceBindingDoesNotExist(APP_NAME);
cloudControllerFixture.stubDeleteApp(APP_NAME);
cloudControllerFixture.stubGetBackingServiceInstance(BACKING_SI_NAME, BACKING_SERVICE_NAME, BACKING_PLAN_NAME);
cloudControllerFixture.stubServiceBindingExists(APP_NAME, BACKING_SI_NAME);
cloudControllerFixture.stubDeleteServiceBinding(APP_NAME, BACKING_SI_NAME);
cloudControllerFixture.stubDeleteServiceInstance(BACKING_SI_NAME);
// when the service instance is deleted
given(brokerFixture.serviceInstanceRequest())
.when()
.delete(brokerFixture.deleteServiceInstanceUrl(), "instance-id")
.then()
.statusCode(HttpStatus.ACCEPTED.value());
// when the "last_operation" API is polled
given(brokerFixture.serviceInstanceRequest())
.when()
.get(brokerFixture.getLastInstanceOperationUrl(), "instance-id")
.then()
.statusCode(HttpStatus.OK.value())
.body("state", is(equalTo(OperationState.IN_PROGRESS.toString())));
String state = brokerFixture.waitForAsyncOperationComplete("instance-id");
assertThat(state).isEqualTo(OperationState.SUCCEEDED.toString());
}
@Test
void deleteAppsWhenTheyExistAndServicesWhenTheyDoNotExist() {
cloudControllerFixture.stubGetServiceInstanceWithNoBinding("instance-id", "instance-name",
SERVICE_NAME, PLAN_NAME);
cloudControllerFixture.stubAppExists(APP_NAME);
cloudControllerFixture.stubServiceBindingDoesNotExist(APP_NAME);
cloudControllerFixture.stubDeleteApp(APP_NAME);
// when the service instance is deleted
given(brokerFixture.serviceInstanceRequest())
.when()
.delete(brokerFixture.deleteServiceInstanceUrl(), "instance-id")
.then()
.statusCode(HttpStatus.ACCEPTED.value());
// when the "last_operation" API is polled
given(brokerFixture.serviceInstanceRequest())
.when()
.get(brokerFixture.getLastInstanceOperationUrl(), "instance-id")
.then()
.statusCode(HttpStatus.OK.value())
.body("state", is(equalTo(OperationState.IN_PROGRESS.toString())));
String state = brokerFixture.waitForAsyncOperationComplete("instance-id");
assertThat(state).isEqualTo(OperationState.SUCCEEDED.toString());
}
@Test
void deleteAppsAndServicesWhenTheyDoNotExist() {
cloudControllerFixture.stubGetServiceInstanceWithNoBinding("instance-id", "instance-name",
SERVICE_NAME, PLAN_NAME);
cloudControllerFixture.stubAppDoesNotExist(APP_NAME);
// when the service instance is deleted
given(brokerFixture.serviceInstanceRequest())
.when()
.delete(brokerFixture.deleteServiceInstanceUrl(), "instance-id")
.then()
.statusCode(HttpStatus.ACCEPTED.value());
// when the "last_operation" API is polled
given(brokerFixture.serviceInstanceRequest())
.when()
.get(brokerFixture.getLastInstanceOperationUrl(), "instance-id")
.then()
.statusCode(HttpStatus.OK.value())
.body("state",
either(equalTo(OperationState.IN_PROGRESS.toString()))
// if the error occurs immediately it will return succeeded status
.or(equalTo(OperationState.SUCCEEDED.toString())));
String state = brokerFixture.waitForAsyncOperationComplete("instance-id");
assertThat(state).isEqualTo(OperationState.SUCCEEDED.toString());
}
}

View File

@@ -30,27 +30,21 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.Matchers.either;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithServicesComponentTest.APP_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithServicesComponentTest.BACKING_PLAN_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithServicesComponentTest.BACKING_SERVICE_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithServicesComponentTest.BACKING_SI_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithServicesComponentTest.PLAN_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithServicesComponentTest.SERVICE_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.BACKING_PLAN_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.BACKING_SERVICE_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.BACKING_SI_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.PLAN_NAME;
import static org.springframework.cloud.appbroker.integration.DeleteInstanceWithAppsAndServicesComponentTest.SERVICE_NAME;
@TestPropertySource(properties = {
"spring.cloud.appbroker.services[0].service-name=" + SERVICE_NAME,
"spring.cloud.appbroker.services[0].plan-name=" + PLAN_NAME,
"spring.cloud.appbroker.services[0].apps[0].path=classpath:demo.jar",
"spring.cloud.appbroker.services[0].apps[0].name=" + APP_NAME,
"spring.cloud.appbroker.services[0].apps[0].services[0].service-instance-name=" + BACKING_SI_NAME,
"spring.cloud.appbroker.services[0].services[0].service-instance-name=" + BACKING_SI_NAME,
"spring.cloud.appbroker.services[0].services[0].name=" + BACKING_SERVICE_NAME,
"spring.cloud.appbroker.services[0].services[0].plan=" + BACKING_PLAN_NAME
})
class DeleteInstanceWithServicesComponentTest extends WiremockComponentTest {
protected static final String APP_NAME = "app-delete-with-services";
protected static final String SERVICE_NAME = "example";
protected static final String PLAN_NAME = "standard";
@@ -68,18 +62,13 @@ class DeleteInstanceWithServicesComponentTest extends WiremockComponentTest {
private CloudControllerStubFixture cloudControllerFixture;
@Test
void deleteAppsAndServicesWhenTheyExist() {
void deleteServicesWhenTheyExist() {
cloudControllerFixture.stubGetServiceInstanceWithNoBinding("instance-id", "instance-name",
SERVICE_NAME, PLAN_NAME);
cloudControllerFixture.stubAppExistsWithBackingService(APP_NAME, BACKING_SI_NAME,
BACKING_SERVICE_NAME, BACKING_PLAN_NAME);
cloudControllerFixture.stubServiceBindingDoesNotExist(APP_NAME);
cloudControllerFixture.stubDeleteApp(APP_NAME);
SERVICE_NAME, PLAN_NAME);
cloudControllerFixture.stubGetBackingServiceInstance(BACKING_SI_NAME, BACKING_SERVICE_NAME, BACKING_PLAN_NAME);
cloudControllerFixture.stubServiceBindingExists(APP_NAME, BACKING_SI_NAME);
cloudControllerFixture.stubDeleteServiceBinding(APP_NAME, BACKING_SI_NAME);
cloudControllerFixture.stubServiceBindingsDoNotExist(BACKING_SI_NAME);
cloudControllerFixture.stubDeleteServiceInstance(BACKING_SI_NAME);
// when the service instance is deleted
@@ -102,37 +91,9 @@ class DeleteInstanceWithServicesComponentTest extends WiremockComponentTest {
}
@Test
void deleteAppsWhenTheyExistAndServicesWhenTheyDoNotExist() {
void deleteServicesWhenTheyDoNotExist() {
cloudControllerFixture.stubGetServiceInstanceWithNoBinding("instance-id", "instance-name",
SERVICE_NAME, PLAN_NAME);
cloudControllerFixture.stubAppExists(APP_NAME);
cloudControllerFixture.stubServiceBindingDoesNotExist(APP_NAME);
cloudControllerFixture.stubDeleteApp(APP_NAME);
// when the service instance is deleted
given(brokerFixture.serviceInstanceRequest())
.when()
.delete(brokerFixture.deleteServiceInstanceUrl(), "instance-id")
.then()
.statusCode(HttpStatus.ACCEPTED.value());
// when the "last_operation" API is polled
given(brokerFixture.serviceInstanceRequest())
.when()
.get(brokerFixture.getLastInstanceOperationUrl(), "instance-id")
.then()
.statusCode(HttpStatus.OK.value())
.body("state", is(equalTo(OperationState.IN_PROGRESS.toString())));
String state = brokerFixture.waitForAsyncOperationComplete("instance-id");
assertThat(state).isEqualTo(OperationState.SUCCEEDED.toString());
}
@Test
void deleteAppsAndServicesWhenTheyDoNotExist() {
cloudControllerFixture.stubGetServiceInstanceWithNoBinding("instance-id", "instance-name",
SERVICE_NAME, PLAN_NAME);
cloudControllerFixture.stubAppDoesNotExist(APP_NAME);
// when the service instance is deleted
given(brokerFixture.serviceInstanceRequest())

View File

@@ -492,6 +492,14 @@ public class CloudControllerStubFixture extends WiremockStubFixture {
stubGetServiceAndGetPlan(serviceName, planName);
}
public void stubServiceBindingsDoNotExist(String serviceInstanceName) {
stubFor(get(urlPathEqualTo("/v2/service_bindings"))
.withQueryParam("q", equalTo("service_instance_guid:" + serviceInstanceGuid(serviceInstanceName)))
.withQueryParam("page", equalTo("1"))
.willReturn(ok()
.withBody(cc("empty-query-results"))));
}
public void stubGetBackingServiceInstance(String serviceInstanceName, String serviceName, String planName) {
String serviceInstanceId = serviceInstanceGuid(serviceInstanceName);
stubServiceInstanceExists(serviceInstanceId, serviceInstanceName, serviceName, planName);