From d724e0dcc4e831120994fe2f52eafa76848514a4 Mon Sep 17 00:00:00 2001 From: Yuxin Bai Date: Wed, 25 Mar 2020 11:56:25 -0400 Subject: [PATCH] Update backing app environment on upgrade update Closes #349 --- .../CloudFoundryAcceptanceTest.java | 2 +- .../UpgradeInstanceAcceptanceTest.java | 158 ++++++++++++++++++ .../fixtures/cf/CloudFoundryService.java | 27 ++- .../cloudfoundry/CloudFoundryAppDeployer.java | 7 +- ...undryAppDeployerUpdateApplicationTest.java | 37 +++- ...pdateInstanceWithUpgradeComponentTest.java | 1 + .../fixtures/CloudControllerStubFixture.java | 1 + 7 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 spring-cloud-app-broker-acceptance-tests/src/test/java/org/springframework/cloud/appbroker/acceptance/UpgradeInstanceAcceptanceTest.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 c8afdba..264f9a4 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 @@ -227,7 +227,7 @@ abstract class CloudFoundryAcceptanceTest { private Mono updateBroker(List appBrokerProperties) { return cloudFoundryService - .pushBrokerApp(testBrokerAppName(), getTestBrokerAppPath(), brokerClientId(), appBrokerProperties) + .updateBrokerApp(testBrokerAppName(), brokerClientId(), appBrokerProperties) .then(cloudFoundryService.updateServiceBroker(serviceBrokerName(), testBrokerAppName())); } diff --git a/spring-cloud-app-broker-acceptance-tests/src/test/java/org/springframework/cloud/appbroker/acceptance/UpgradeInstanceAcceptanceTest.java b/spring-cloud-app-broker-acceptance-tests/src/test/java/org/springframework/cloud/appbroker/acceptance/UpgradeInstanceAcceptanceTest.java new file mode 100644 index 0000000..cade779 --- /dev/null +++ b/spring-cloud-app-broker-acceptance-tests/src/test/java/org/springframework/cloud/appbroker/acceptance/UpgradeInstanceAcceptanceTest.java @@ -0,0 +1,158 @@ +/* + * 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.acceptance; + +import java.util.Collections; +import java.util.Optional; + +import com.jayway.jsonpath.DocumentContext; +import org.cloudfoundry.operations.applications.ApplicationSummary; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInfo; +import org.junit.jupiter.api.TestMethodOrder; + +import org.springframework.beans.factory.annotation.Autowired; + +import static org.assertj.core.api.Assertions.assertThat; + +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +class UpgradeInstanceAcceptanceTest extends CloudFoundryAcceptanceTest { + + private static final String APP_NAME = "app-upgrade"; + + private static final int FIRST_TEST = 1; + + private static final int SECOND_TEST = 2; + + private static final String SI_NAME = "si-upgrade"; + + private static final String SUFFIX = "upgrade"; + + private static final String APP_SERVICE_NAME = "app-service-" + SUFFIX; + + private static final String BACKING_SERVICE_NAME = "backing-service-" + SUFFIX; + + @Autowired + private HealthListener healthListener; + + @Override + protected String testSuffix() { + return SUFFIX; + } + + @Override + protected String appServiceName() { + return APP_SERVICE_NAME; + } + + @Override + protected String backingServiceName() { + return BACKING_SERVICE_NAME; + } + + @Test + @Tag("first") + @Order(FIRST_TEST) + @AppBrokerTestProperties({ + "spring.cloud.appbroker.services[0].service-name=" + APP_SERVICE_NAME, + "spring.cloud.appbroker.services[0].plan-name=" + PLAN_NAME, + "spring.cloud.appbroker.services[0].apps[0].name=" + APP_NAME, + "spring.cloud.appbroker.services[0].apps[0].path=" + BACKING_APP_PATH, + "spring.cloud.appbroker.services[0].apps[0].environment.parameter1=old-config1", + "spring.cloud.appbroker.services[0].apps[0].environment.parameter2=old-config2" + }) + void createsServiceInstance() { + // when a service instance is created + createServiceInstance(SI_NAME); + + // then a backing application is deployed + Optional backingApplication = getApplicationSummary(APP_NAME); + assertThat(backingApplication).hasValueSatisfying(app -> + assertThat(app.getRunningInstances()).isEqualTo(1)); + + // and the environment variables are applied correctly + DocumentContext json = getSpringAppJson(APP_NAME); + assertThat(json.read("$.parameter1").toString()).isEqualTo("old-config1"); + assertThat(json.read("$.parameter2").toString()).isEqualTo("old-config2"); + + String path = backingApplication.get().getUrls().get(0); + healthListener.start(path); + } + + @Test + @Order(SECOND_TEST) + @Tag("last") + @AppBrokerTestProperties({ + "spring.cloud.appbroker.services[0].service-name=" + APP_SERVICE_NAME, + "spring.cloud.appbroker.services[0].plan-name=" + PLAN_NAME, + "spring.cloud.appbroker.services[0].apps[0].name=" + APP_NAME, + "spring.cloud.appbroker.services[0].apps[0].path=" + BACKING_APP_PATH, + "spring.cloud.appbroker.services[0].apps[0].environment.parameter1=new-config1", + "spring.cloud.appbroker.services[0].apps[0].environment.parameter3=new-config3" + }) + void upgradesTheServiceInstanceWithNewBackingServiceAndEnvironmentVariables() { + // when the service instance is updated with a new service + updateServiceInstance(SI_NAME, Collections.singletonMap("upgrade", true)); + + // then the backing application was updated with zero downtime + healthListener.stop(); + assertThat(healthListener.getFailures()).isEqualTo(0); + assertThat(healthListener.getSuccesses()).isGreaterThan(0); + + // then a backing application is re-deployed + Optional updatedBackingApplication = getApplicationSummary(APP_NAME); + assertThat(updatedBackingApplication).hasValueSatisfying(app -> + assertThat(app.getRunningInstances()).isEqualTo(1)); + + // the backing application is updated with the new parameters + DocumentContext json = getSpringAppJson(APP_NAME); + assertThat(json.read("$.parameter1").toString()).isEqualTo("new-config1"); + assertThat(json.read("$.parameter3").toString()).isEqualTo("new-config3"); + assertThat(json.jsonString()).doesNotContain("parameter2"); + + // when the service instance is deleted + deleteServiceInstance(SI_NAME); + + // then the backing application is deleted + Optional backingApplicationAfterDeletion = getApplicationSummary(APP_NAME); + assertThat(backingApplicationAfterDeletion).isEmpty(); + } + + @Override + @BeforeEach + void setUp(TestInfo testInfo, BrokerProperties brokerProperties) { + if (testInfo.getTags().contains("first")) { + super.setUp(testInfo, brokerProperties); + } + else { + setUpForBrokerUpdate(brokerProperties); + } + } + + @Override + @AfterEach + public void tearDown(TestInfo testInfo) { + if (testInfo.getTags().contains("last")) { + super.tearDown(testInfo); + } + } +} 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 c01d05d..91154e3 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 @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import org.cloudfoundry.client.CloudFoundryClient; +import org.cloudfoundry.client.v2.applications.UpdateApplicationRequest; import org.cloudfoundry.client.v2.organizations.AssociateOrganizationManagerRequest; import org.cloudfoundry.client.v2.organizations.AssociateOrganizationManagerResponse; import org.cloudfoundry.client.v2.organizations.AssociateOrganizationUserRequest; @@ -43,6 +44,7 @@ import org.cloudfoundry.operations.applications.DeleteApplicationRequest; import org.cloudfoundry.operations.applications.GetApplicationEnvironmentsRequest; import org.cloudfoundry.operations.applications.GetApplicationRequest; import org.cloudfoundry.operations.applications.PushApplicationManifestRequest; +import org.cloudfoundry.operations.applications.RestartApplicationRequest; import org.cloudfoundry.operations.applications.StopApplicationRequest; import org.cloudfoundry.operations.domains.CreateDomainRequest; import org.cloudfoundry.operations.domains.Domain; @@ -147,7 +149,7 @@ public class CloudFoundryService { return cloudFoundryOperations.applications() .pushManifest(PushApplicationManifestRequest.builder() .manifest(ApplicationManifest.builder() - .putAllEnvironmentVariables(appBrokerDeployerEnvironmentVariables(brokerClientId)) + .environmentVariables(appBrokerDeployerEnvironmentVariables(brokerClientId)) .putAllEnvironmentVariables(propertiesToEnvironment(appBrokerProperties)) .name(appName) .path(appPath) @@ -158,6 +160,29 @@ public class CloudFoundryService { .doOnError(error -> LOG.error("Error pushing broker app " + appName + ": " + error)); } + public Mono updateBrokerApp(String appName, String brokerClientId, List appBrokerProperties) { + return cloudFoundryOperations.applications() + .get(GetApplicationRequest.builder().name(appName).build()) + .map(ApplicationDetail::getId) + .flatMap(applicationId -> + cloudFoundryClient + .applicationsV2() + .update(UpdateApplicationRequest + .builder() + .applicationId(applicationId) + .putAllEnvironmentJsons(appBrokerDeployerEnvironmentVariables(brokerClientId)) + .putAllEnvironmentJsons(propertiesToEnvironment(appBrokerProperties)) + .name(appName) + .memory(1024) + .build()) + .thenReturn(applicationId)) + .then(cloudFoundryOperations.applications() + .restart(RestartApplicationRequest.builder().name(appName).build())) + .doOnSuccess(item -> LOG.info("Updated broker app " + appName)) + .doOnError(error -> LOG.error("Error updating broker app " + appName + ": " + error)) + .then(); + } + public Mono deleteApp(String appName) { return cloudFoundryOperations.applications() .delete(DeleteApplicationRequest.builder() 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 07253f5..cfe79a2 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 @@ -234,6 +234,7 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware return get(GetApplicationRequest.builder().name(name).properties(request.getProperties()).build()) .flatMap(response -> bindNewServices(response, request.getServices(), request.getProperties())) .flatMap(applicationId -> associateHostName(applicationId, request.getProperties())) + .flatMap(applicationId -> updateEnvironment(request, applicationId)) .flatMap(applicationId -> Mono.zip(Mono.just(applicationId), upgradeApplication(request, applicationId))) .flatMap(tuple2 -> { @@ -464,11 +465,15 @@ public class CloudFoundryAppDeployer implements AppDeployer, ResourceLoaderAware .map(Package::getId)); } + return getPackageForApplication(applicationId); + } + + private Mono updateEnvironment(UpdateApplicationRequest request, String applicationId) { final Map environmentVariables = getApplicationEnvironment(request.getProperties(), request.getEnvironment(), request.getServiceInstanceId()); return updateApplicationEnvironment(applicationId, environmentVariables, request.getProperties()) - .flatMap(a -> getPackageForApplication(applicationId)); + .thenReturn(applicationId); } private Mono getPackageForApplication(String applicationId) { diff --git a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTest.java b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTest.java index 7c000ea..81ddfbc 100644 --- a/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTest.java +++ b/spring-cloud-app-broker-deployer-cloudfoundry/src/test/java/org/springframework/cloud/appbroker/deployer/cloudfoundry/CloudFoundryAppDeployerUpdateApplicationTest.java @@ -323,15 +323,43 @@ class CloudFoundryAppDeployerUpdateApplicationTest { assertThat(updateApplicationRequest.getMemory()).isEqualTo(1024); } + @Test + void updateAppEnvironment() { + ArgumentCaptor updateApplicationRequestCaptor = + ArgumentCaptor.forClass(org.cloudfoundry.client.v2.applications.UpdateApplicationRequest.class); + + given(applicationsV2.update(updateApplicationRequestCaptor.capture())) + .willReturn(Mono.just(UpdateApplicationResponse.builder() + .build())); + + UpdateApplicationRequest request = + UpdateApplicationRequest + .builder() + .name(APP_NAME) + .path(APP_PATH) + .environment("ENV_VAR", "test-env") + .build(); + + StepVerifier.create(appDeployer.update(request)) + .assertNext(response -> assertThat(response.getName()).isEqualTo(APP_NAME)) + .verifyComplete(); + + assertThat(updateApplicationRequestCaptor.getValue().getEnvironmentJsons().get("SPRING_APPLICATION_JSON")) + .isEqualTo("{\"ENV_VAR\":\"test-env\"}"); + } + @Test void updateAppWithUpgrade() { - Map properties = new HashMap<>(); - properties.put("upgrade", "true"); + ArgumentCaptor updateApplicationRequestCaptor = + ArgumentCaptor.forClass(org.cloudfoundry.client.v2.applications.UpdateApplicationRequest.class); + + given(applicationsV2.update(any())).willReturn(Mono.just(UpdateApplicationResponse.builder().build())); UpdateApplicationRequest request = UpdateApplicationRequest.builder() .name(APP_NAME) .path(APP_PATH) - .properties(properties) + .property("upgrade", "true") + .environment("TEST_KEY", "TEST_VALUE") .build(); StepVerifier.create(appDeployer.update(request)) @@ -339,6 +367,9 @@ class CloudFoundryAppDeployerUpdateApplicationTest { .verifyComplete(); then(applicationsV2).should().summary(any(SummaryApplicationRequest.class)); + then(applicationsV2).should().update(updateApplicationRequestCaptor.capture()); + assertThat(updateApplicationRequestCaptor.getValue().getEnvironmentJsons().get("SPRING_APPLICATION_JSON")) + .isEqualTo("{\"TEST_KEY\":\"TEST_VALUE\"}"); then(applicationsV2).shouldHaveNoMoreInteractions(); } diff --git a/spring-cloud-app-broker-integration-tests/src/test/java/org/springframework/cloud/appbroker/integration/UpdateInstanceWithUpgradeComponentTest.java b/spring-cloud-app-broker-integration-tests/src/test/java/org/springframework/cloud/appbroker/integration/UpdateInstanceWithUpgradeComponentTest.java index 7e8b099..909c5ba 100644 --- a/spring-cloud-app-broker-integration-tests/src/test/java/org/springframework/cloud/appbroker/integration/UpdateInstanceWithUpgradeComponentTest.java +++ b/spring-cloud-app-broker-integration-tests/src/test/java/org/springframework/cloud/appbroker/integration/UpdateInstanceWithUpgradeComponentTest.java @@ -38,6 +38,7 @@ import static org.springframework.cloud.appbroker.integration.UpdateInstanceWith "spring.cloud.appbroker.services[0].plan-name=standard", "spring.cloud.appbroker.services[0].apps[0].path=classpath:demo.jar", "spring.cloud.appbroker.services[0].apps[0].name=" + APP_NAME_1, + "spring.cloud.appbroker.services[0].apps[0].environment.ENV_VAR=testEnv", "spring.cloud.appbroker.services[0].apps[0].parameters-transformers[0].name=PropertyMapping", "spring.cloud.appbroker.services[0].apps[0].parameters-transformers[0].args.include=upgrade" }) diff --git a/spring-cloud-app-broker-integration-tests/src/test/java/org/springframework/cloud/appbroker/integration/fixtures/CloudControllerStubFixture.java b/spring-cloud-app-broker-integration-tests/src/test/java/org/springframework/cloud/appbroker/integration/fixtures/CloudControllerStubFixture.java index 7c8c8b3..c682cf7 100644 --- a/spring-cloud-app-broker-integration-tests/src/test/java/org/springframework/cloud/appbroker/integration/fixtures/CloudControllerStubFixture.java +++ b/spring-cloud-app-broker-integration-tests/src/test/java/org/springframework/cloud/appbroker/integration/fixtures/CloudControllerStubFixture.java @@ -236,6 +236,7 @@ public class CloudControllerStubFixture extends WiremockStubFixture { } public void stubUpdateAppWithUpgrade(final String appName) { + stubUpdateEnvironment(appName); stubCreatePackage(appName); stubCreateBuild(appName); stubCreateDeployment(appName);