Skip to content

Commit

Permalink
Merge pull request #357 from orange-cloudfoundry/fix-issue-356-during…
Browse files Browse the repository at this point in the history
…-update

Fix issue #356
  • Loading branch information
gberche-orange authored Jan 27, 2021
2 parents afe0f3b + e9f5fa3 commit 58ecef8
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.springframework.cloud.servicebroker.exception.ServiceInstanceDoesNotExistException;
import org.springframework.cloud.servicebroker.model.binding.CreateServiceInstanceBindingRequest;
import org.springframework.cloud.servicebroker.model.binding.CreateServiceInstanceBindingResponse;
import org.springframework.cloud.servicebroker.model.binding.DeleteServiceInstanceBindingRequest;
Expand Down Expand Up @@ -150,7 +151,15 @@ public void preUpdate(Context ctx) {

CoabVarsFileDto coabVarsFileDto = wrapUpdateOsbIntoVarsDto(updateRequest);

//Check pre-requisites and generate paas-template structure
//Check secret pre-requisites (i.e. that a service instance exists)
if (! this.secretsGenerator.isEnableDeploymentFileIsPresent(secretsWorkDir, serviceInstanceId)) {
logger.warn("Enable-deployment is missing while receiving a request to update instance {} Suspecting " +
"inconsistency from OSB client and COA deployment. Please check if manual cleanup was performed on " +
"coa deployments without going through CF (OSB client)", serviceInstanceId);
throw new ServiceInstanceDoesNotExistException(serviceInstanceId);
}

//Check model pre-requisites and generate paas-template structure
this.templatesGenerator.checkPrerequisites(templatesWorkDir);
this.templatesGenerator.generateCoabVarsFile(templatesWorkDir, serviceInstanceId, coabVarsFileDto);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,20 @@ protected void generateEnableDeploymentFile(Path workDir, String serviceInstance
String fileName = DeploymentConstants.ENABLE_DEPLOYMENT_FILENAME;
StructureGeneratorHelper.generateFile(workDir, targetPathElements, fileName, fileName, null);
}
protected boolean isEnableDeploymentFileIsPresent(Path workDir, String serviceInstanceId){
String[] targetPathElements = new String[] {this.rootDeployment, this.computeDeploymentName(serviceInstanceId)};
String fileName = DeploymentConstants.ENABLE_DEPLOYMENT_FILENAME;
//Compute target path
Path targetDir = StructureGeneratorHelper.generatePath(workDir, targetPathElements);
Path targetFile = StructureGeneratorHelper.generatePath(targetDir, fileName);


boolean isMarkerPresent = !StructureGeneratorHelper.isMissingResource(targetFile);
if (!isMarkerPresent) {
logger.warn("enable-deployment.yml is missing at " + targetFile);
}
return isMarkerPresent;
}

protected void removeEnableDeploymentFile(Path workDir, String serviceInstanceId) {
String[] pathElements = new String[] {this.rootDeployment, this.computeDeploymentName(serviceInstanceId)};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import org.springframework.cloud.servicebroker.exception.ServiceInstanceDoesNotExistException;
import org.springframework.cloud.servicebroker.model.CloudFoundryContext;
import org.springframework.cloud.servicebroker.model.binding.CreateServiceInstanceBindingRequest;
import org.springframework.cloud.servicebroker.model.binding.CreateServiceInstanceBindingResponse;
Expand All @@ -36,6 +37,7 @@
import static com.orange.oss.ondemandbroker.ProcessorChainServiceInstanceService.OSB_PROFILE_ORGANIZATION_GUID;
import static com.orange.oss.ondemandbroker.ProcessorChainServiceInstanceService.OSB_PROFILE_SPACE_GUID;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.eq;
Expand Down Expand Up @@ -135,6 +137,7 @@ public void updates_coab_vars_and_returns_async_response() {
//Given a mock behaviour
TemplatesGenerator templatesGenerator = mock(TemplatesGenerator.class);
SecretsGenerator secretsGenerator = mock(SecretsGenerator.class);
when(secretsGenerator.isEnableDeploymentFileIsPresent(any(), any())).thenReturn(true);

//given a configured timeout
PipelineCompletionTracker tracker = aCompletionTracker();
Expand Down Expand Up @@ -173,6 +176,42 @@ public void updates_coab_vars_and_returns_async_response() {
assertThat(customSecretsMessage).isNotNull();
}

@Test
public void updates_fails_when_deployment_is_missing() {
//Given an update request
UpdateServiceInstanceRequest request = UpdateServiceInstanceRequest.builder()
.serviceDefinitionId("service_definition_id")
.planId("plan_id")
.serviceInstanceId(SERVICE_INSTANCE_ID)
.context(CloudFoundryContext.builder()
.organizationGuid("org_id1")
.spaceGuid("space_id1")
.build()
)
.build();

//Given a populated context
Context context = new Context();
context.contextKeys.put(ProcessorChainServiceInstanceService.UPDATE_SERVICE_INSTANCE_REQUEST, request);
context.contextKeys.put(TEMPLATES_REPOSITORY_ALIAS_NAME + GitProcessorContext.workDir.toString(), aGitRepoWorkDir());
context.contextKeys.put(SECRETS_REPOSITORY_ALIAS_NAME + GitProcessorContext.workDir.toString(), aGitRepoWorkDir());

//Given a mock behaviour
TemplatesGenerator templatesGenerator = mock(TemplatesGenerator.class);
SecretsGenerator secretsGenerator = mock(SecretsGenerator.class);
when(secretsGenerator.isEnableDeploymentFileIsPresent(any(), any())).thenReturn(false);

//given a configured timeout
PipelineCompletionTracker tracker = aCompletionTracker();

BoshProcessor boshProcessor = new BoshProcessor(TEMPLATES_REPOSITORY_ALIAS_NAME, SECRETS_REPOSITORY_ALIAS_NAME, templatesGenerator, secretsGenerator, tracker, "Cassandra", "c", "_","https://static-dashboard.com"
);

//When
assertThatThrownBy(() -> boshProcessor.preUpdate(context))
.isInstanceOf(ServiceInstanceDoesNotExistException.class);
}

@Test
public void constructs_a_dto_from_a_provisionning_request() {
//Given mocked dependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,38 @@ public void check_that_enable_deployment_file_is_generated() {
assertThat("Enable deployment file doesn't exist:" + targetEnableDeploymentFile, Files.exists(targetEnableDeploymentFile));
}

@Test
public void check_that_enable_deployment_file_is_present() {
//Given
Structure deploymentStructure = new Structure.StructureBuilder(this.workDir)
.withDirectoryHierarchy(this.deploymentProperties.getRootDeployment(), this.secretsGenerator.computeDeploymentName(SERVICE_INSTANCE_ID))
.build();
this.secretsGenerator.generateEnableDeploymentFile(this.workDir, SERVICE_INSTANCE_ID);

//When
boolean enableDeploymentFileIsPresent = this.secretsGenerator
.isEnableDeploymentFileIsPresent(this.workDir, SERVICE_INSTANCE_ID);

//Then
assertThat("Enable-deployment should be reported as present", enableDeploymentFileIsPresent);
}

@Test
public void check_that_enable_deployment_file_is_missing() {
//Given
Structure deploymentStructure = new Structure.StructureBuilder(this.workDir)
.withDirectoryHierarchy(this.deploymentProperties.getRootDeployment(), this.secretsGenerator.computeDeploymentName(SERVICE_INSTANCE_ID))
.build();

//When
boolean enableDeploymentFileIsPresent = this.secretsGenerator
.isEnableDeploymentFileIsPresent(this.workDir, SERVICE_INSTANCE_ID);

//Then
assertThat("Enable-deployment should be reported as missing", ! enableDeploymentFileIsPresent);
}


@Test
public void check_that_enable_deployment_file_is_removed() {
//Given
Expand Down

0 comments on commit 58ecef8

Please sign in to comment.