From 5ffcd6a2afb91b4501fa72248d1e7d4ea3162e19 Mon Sep 17 00:00:00 2001 From: Roy Clarkson Date: Mon, 25 Mar 2019 23:09:08 -0400 Subject: [PATCH] Do not fail delete operation if backing apps or services are missing If backing apps or services are missing, then the error is logged and the app name or service instance name is returned as if the service was deleted as expected. Resolves #208 --- .../appbroker/deployer/DeployerClient.java | 7 +- .../deployer/DeployerClientTest.java | 119 +++++++++++++++++- .../DeleteInstanceComponentTest.java | 2 +- ...leteInstanceWithServicesComponentTest.java | 53 ++++++++ 4 files changed, 175 insertions(+), 6 deletions(-) diff --git a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/DeployerClient.java b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/DeployerClient.java index 35fd995..3d4eecb 100644 --- a/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/DeployerClient.java +++ b/spring-cloud-app-broker-core/src/main/java/org/springframework/cloud/appbroker/deployer/DeployerClient.java @@ -83,6 +83,9 @@ public class DeployerClient { .doOnSuccess(response -> log.debug("Finished undeploying application {}", backingApplication)) .doOnError(exception -> log.error("Error undeploying application {} with error '{}'", backingApplication, exception.getMessage())) + .onErrorReturn(UndeployApplicationResponse.builder() + .name(backingApplication.getName()) + .build()) .map(UndeployApplicationResponse::getName); } @@ -129,11 +132,13 @@ public class DeployerClient { .serviceInstanceName(backingService.getServiceInstanceName()) .properties(backingService.getProperties()) .build()) - .doOnRequest(l -> log.debug("Deleting backing service {}", backingService.getName())) .doOnSuccess(response -> log.debug("Finished deleting backing service {}", backingService.getName())) .doOnError(exception -> log.error("Error deleting backing service {} with error '{}'", backingService.getName(), exception.getMessage())) + .onErrorReturn(DeleteServiceInstanceResponse.builder() + .name(backingService.getServiceInstanceName()) + .build()) .map(DeleteServiceInstanceResponse::getName); } } diff --git a/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/deployer/DeployerClientTest.java b/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/deployer/DeployerClientTest.java index 093077e..e11a824 100644 --- a/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/deployer/DeployerClientTest.java +++ b/spring-cloud-app-broker-core/src/test/java/org/springframework/cloud/appbroker/deployer/DeployerClientTest.java @@ -40,9 +40,13 @@ import static org.mockito.Mockito.when; class DeployerClientTest { private static final String APP_NAME = "helloworld"; + private static final String APP_ARCHIVE = "app.jar"; + private static final String APP_PATH = "classpath:/jars/" + APP_ARCHIVE; + private static final String SERVICE_INSTANCE_NAME = "helloservice"; + private DeployerClient deployerClient; @Mock @@ -174,9 +178,10 @@ class DeployerClientTest { } @Test - void shouldNotUndeployAppThatDoesNotExist() { + void shouldNotReturnErrorWhenUndeployingAppThatDoesNotExist() { // given - when(appDeployer.undeploy(any())).thenReturn(Mono.error(new IllegalStateException("app does not exist"))); + when(appDeployer.undeploy(any())) + .thenReturn(Mono.error(new IllegalStateException("app does not exist"))); BackingApplication application = BackingApplication.builder() .name(APP_NAME) @@ -186,12 +191,118 @@ class DeployerClientTest { // when StepVerifier.create(deployerClient.undeploy(application)) // then - .expectErrorMessage("app does not exist") - .verify(); + .expectNext(APP_NAME) + .verifyComplete(); verify(appDeployer).undeploy(argThat(request -> APP_NAME.equals(request.getName()))); } + @Test + void shouldCreateServiceInstance() { + // given + when(appDeployer.createServiceInstance(any())) + .thenReturn(Mono.just(CreateServiceInstanceResponse.builder() + .name(SERVICE_INSTANCE_NAME) + .build())); + + BackingService service = BackingService.builder() + .serviceInstanceName(SERVICE_INSTANCE_NAME) + .build(); + + // when + StepVerifier.create(deployerClient.createServiceInstance(service)) + // then + .expectNext(SERVICE_INSTANCE_NAME) + .verifyComplete(); + + verify(appDeployer).createServiceInstance(argThat(request -> + SERVICE_INSTANCE_NAME.equals(request.getServiceInstanceName()))); + } + + @Test + void shouldUpdateServiceInstance() { + // given + when(appDeployer.updateServiceInstance(any())) + .thenReturn(Mono.just(UpdateServiceInstanceResponse.builder() + .name(SERVICE_INSTANCE_NAME) + .build())); + + BackingService service = BackingService.builder() + .serviceInstanceName(SERVICE_INSTANCE_NAME) + .build(); + + // when + StepVerifier.create(deployerClient.updateServiceInstance(service)) + // then + .expectNext(SERVICE_INSTANCE_NAME) + .verifyComplete(); + + verify(appDeployer).updateServiceInstance(argThat(request -> + SERVICE_INSTANCE_NAME.equals(request.getServiceInstanceName()))); + } + + @Test + void shouldReturnErrorWhenUpdatingServiceInstanceThatDoesNotExist() { + // given + when(appDeployer.updateServiceInstance(any())) + .thenReturn(Mono.error(new IllegalStateException("service instance does not exist"))); + + BackingService service = BackingService.builder() + .serviceInstanceName(SERVICE_INSTANCE_NAME) + .build(); + + // when + StepVerifier.create(deployerClient.updateServiceInstance(service)) + // then + .expectErrorMessage("service instance does not exist") + .verify(); + + verify(appDeployer).updateServiceInstance(argThat(request -> + SERVICE_INSTANCE_NAME.equals(request.getServiceInstanceName()))); + } + + @Test + void shouldDeleteServiceInstance() { + // given + when(appDeployer.deleteServiceInstance(any())) + .thenReturn(Mono.just(DeleteServiceInstanceResponse.builder() + .name(SERVICE_INSTANCE_NAME) + .build())); + + BackingService service = BackingService.builder() + .serviceInstanceName(SERVICE_INSTANCE_NAME) + .build(); + + // when + StepVerifier.create(deployerClient.deleteServiceInstance(service)) + // then + .expectNext(SERVICE_INSTANCE_NAME) + .verifyComplete(); + + verify(appDeployer).deleteServiceInstance(argThat(request -> + SERVICE_INSTANCE_NAME.equals(request.getServiceInstanceName()))); + } + + @Test + void shouldNotReturnErrorWhenDeletingServiceInstanceThatDoesNotExist() { + // given + when(appDeployer.deleteServiceInstance(any())) + .thenReturn(Mono.error(new IllegalStateException("service instance does not exist"))); + + BackingService service = BackingService.builder() + .serviceInstanceName(SERVICE_INSTANCE_NAME) + .build(); + + // when + StepVerifier.create(deployerClient.deleteServiceInstance(service)) + // then + .expectNext(SERVICE_INSTANCE_NAME) + .verifyComplete(); + + verify(appDeployer).deleteServiceInstance(argThat(request -> + SERVICE_INSTANCE_NAME.equals(request.getServiceInstanceName()))); + } + private ArgumentMatcher matchesRequest(String appName, String appArchive, Map properties, Map environment, diff --git a/spring-cloud-app-broker-integration-tests/src/test/java/org.springframework.cloud.appbroker/integration/DeleteInstanceComponentTest.java b/spring-cloud-app-broker-integration-tests/src/test/java/org.springframework.cloud.appbroker/integration/DeleteInstanceComponentTest.java index 238220d..bc0e4d4 100644 --- a/spring-cloud-app-broker-integration-tests/src/test/java/org.springframework.cloud.appbroker/integration/DeleteInstanceComponentTest.java +++ b/spring-cloud-app-broker-integration-tests/src/test/java/org.springframework.cloud.appbroker/integration/DeleteInstanceComponentTest.java @@ -102,6 +102,6 @@ class DeleteInstanceComponentTest extends WiremockComponentTest { .body("state", is(equalTo(OperationState.IN_PROGRESS.toString()))); String state = brokerFixture.waitForAsyncOperationComplete("instance-id"); - assertThat(state).isEqualTo(OperationState.FAILED.toString()); + assertThat(state).isEqualTo(OperationState.SUCCEEDED.toString()); } } \ No newline at end of file diff --git a/spring-cloud-app-broker-integration-tests/src/test/java/org.springframework.cloud.appbroker/integration/DeleteInstanceWithServicesComponentTest.java b/spring-cloud-app-broker-integration-tests/src/test/java/org.springframework.cloud.appbroker/integration/DeleteInstanceWithServicesComponentTest.java index e4f7d5d..c2d30a5 100644 --- a/spring-cloud-app-broker-integration-tests/src/test/java/org.springframework.cloud.appbroker/integration/DeleteInstanceWithServicesComponentTest.java +++ b/spring-cloud-app-broker-integration-tests/src/test/java/org.springframework.cloud.appbroker/integration/DeleteInstanceWithServicesComponentTest.java @@ -47,6 +47,7 @@ class DeleteInstanceWithServicesComponentTest extends WiremockComponentTest { static final String APP_NAME = "app-delete-with-services"; static final String BACKING_SI_NAME = "my-db-service"; + static final String BACKING_SERVICE_NAME = "db-service"; @Autowired @@ -86,4 +87,56 @@ class DeleteInstanceWithServicesComponentTest extends WiremockComponentTest { String state = brokerFixture.waitForAsyncOperationComplete("instance-id"); assertThat(state).isEqualTo(OperationState.SUCCEEDED.toString()); } + + @Test + void deleteAppsWhenTheyExistAndServicesWhenTheyDoNotExist() { + cloudControllerFixture.stubAppExists(APP_NAME); + cloudControllerFixture.stubServiceBindingDoesNotExist(APP_NAME); + cloudControllerFixture.stubDeleteApp(APP_NAME); + + cloudControllerFixture.stubServiceInstanceDoesNotExist(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 deleteAppsAndServicesWhenTheyDoNotExist() { + cloudControllerFixture.stubAppDoesNotExist(APP_NAME); + + cloudControllerFixture.stubServiceInstanceDoesNotExist(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()); + } } \ No newline at end of file