Skip to content

Commit 2858a3a

Browse files
IvanBorislavovDimitrovIvanBorislavovDimitrov
committed
Revert "Fix service keys not renewed when existing service is used (#1724)"
This reverts commit bfd5d4c.
1 parent 546f944 commit 2858a3a

File tree

5 files changed

+52
-148
lines changed

5 files changed

+52
-148
lines changed

‎multiapps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/cf/clients/CustomServiceKeysClient.java‎

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@
33
import java.util.Collections;
44
import java.util.List;
55
import java.util.Map;
6-
import java.util.Objects;
7-
import java.util.UUID;
6+
import java.util.function.Function;
87
import java.util.stream.Collectors;
9-
import java.util.stream.Stream;
108

119
import org.cloudfoundry.client.v3.serviceinstances.ServiceInstanceType;
1210
import org.cloudfoundry.multiapps.controller.client.facade.CloudCredentials;
@@ -32,8 +30,7 @@ public CustomServiceKeysClient(ApplicationConfiguration configuration, WebClient
3230
}
3331

3432
public List<DeployedMtaServiceKey> getServiceKeysByMetadataAndGuids(String spaceGuid, String mtaId, String mtaNamespace,
35-
List<DeployedMtaService> services,
36-
List<String> existingServiceGuids) {
33+
List<DeployedMtaService> services) {
3734
String labelSelector = MtaMetadataCriteriaBuilder.builder()
3835
.label(MtaMetadataLabels.SPACE_GUID)
3936
.hasValue(spaceGuid)
@@ -46,40 +43,31 @@ public List<DeployedMtaServiceKey> getServiceKeysByMetadataAndGuids(String space
4643
.build()
4744
.get();
4845

49-
List<String> managedGuids = extractManagedServiceGuids(services);
50-
51-
List<String> allServiceGuids = Stream.concat(managedGuids.stream(), existingServiceGuids.stream())
52-
.filter(Objects::nonNull)
53-
.toList();
54-
55-
if (allServiceGuids.isEmpty()) {
56-
return List.of();
57-
}
5846
return new CustomControllerClientErrorHandler().handleErrorsOrReturnResult(
59-
() -> getServiceKeysByMetadataInternal(labelSelector, allServiceGuids));
47+
() -> getServiceKeysByMetadataInternal(labelSelector, services));
6048
}
6149

62-
private List<String> extractManagedServiceGuids(List<DeployedMtaService> services) {
63-
return getManagedServices(services).stream()
64-
.map(DeployedMtaService::getGuid)
65-
.map(UUID::toString)
66-
.toList();
67-
}
68-
69-
private List<DeployedMtaServiceKey> getServiceKeysByMetadataInternal(String labelSelector, List<String> guids) {
70-
71-
String uriSuffix = INCLUDE_SERVICE_INSTANCE_RESOURCES_PARAM
72-
+ "&service_instance_guids=" + String.join(",", guids);
73-
74-
return getListOfResources(new ServiceKeysResponseMapper(),
75-
SERVICE_KEYS_BY_METADATA_SELECTOR_URI + uriSuffix,
50+
private List<DeployedMtaServiceKey> getServiceKeysByMetadataInternal(String labelSelector, List<DeployedMtaService> services) {
51+
String uriSuffix = INCLUDE_SERVICE_INSTANCE_RESOURCES_PARAM;
52+
List<DeployedMtaService> managedServices = getManagedServices(services);
53+
if (managedServices != null) {
54+
uriSuffix += "&service_instance_guids=" + managedServices.stream()
55+
.map(service -> service.getGuid()
56+
.toString())
57+
.collect(Collectors.joining(","));
58+
}
59+
return getListOfResources(new ServiceKeysResponseMapper(managedServices), SERVICE_KEYS_BY_METADATA_SELECTOR_URI + uriSuffix,
7660
labelSelector);
7761
}
7862

7963
private List<DeployedMtaService> getManagedServices(List<DeployedMtaService> services) {
80-
return services.stream()
81-
.filter(this::serviceIsNotUserProvided)
82-
.toList();
64+
if (services == null) {
65+
return null;
66+
}
67+
List<DeployedMtaService> managedServices = services.stream()
68+
.filter(this::serviceIsNotUserProvided)
69+
.collect(Collectors.toList());
70+
return managedServices.isEmpty() ? null : managedServices;
8371
}
8472

8573
private boolean serviceIsNotUserProvided(DeployedMtaService service) {
@@ -88,12 +76,23 @@ private boolean serviceIsNotUserProvided(DeployedMtaService service) {
8876
}
8977

9078
protected class ServiceKeysResponseMapper extends ResourcesResponseMapper<DeployedMtaServiceKey> {
91-
public ServiceKeysResponseMapper() {
79+
80+
List<DeployedMtaService> mtaServices;
81+
82+
public ServiceKeysResponseMapper(List<DeployedMtaService> mtaServices) {
83+
this.mtaServices = mtaServices;
9284
}
9385

9486
@Override
9587
public List<DeployedMtaServiceKey> getMappedResources() {
96-
Map<String, CloudServiceInstance> serviceMapping = getIncludedServiceInstancesMapping();
88+
Map<String, CloudServiceInstance> serviceMapping;
89+
if (mtaServices != null) {
90+
serviceMapping = mtaServices.stream()
91+
.collect(Collectors.toMap(service -> service.getGuid()
92+
.toString(), Function.identity()));
93+
} else {
94+
serviceMapping = getIncludedServiceInstancesMapping();
95+
}
9796
return getQueriedResources().stream()
9897
.map(resource -> resourceMapper.mapServiceKeyResource(resource, serviceMapping))
9998
.collect(Collectors.toList());

‎multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -789,8 +789,6 @@ public class Messages {
789789
public static final String GETTING_FEATURES_FOR_APPLICATION_0 = "Getting features for application \"{0}\"";
790790

791791
public static final String TOTAL_SIZE_OF_ALL_RESOLVED_CONTENT_0 = "Total size for all resolved content {0}";
792-
public static final String IGNORING_NOT_FOUND_OPTIONAL_SERVICE = "Service {0} not found but is optional";
793-
public static final String IGNORING_NOT_FOUND_INACTIVE_SERVICE = "Service {0} not found but is inactive";
794792

795793
// Not log messages
796794
public static final String SERVICE_TYPE = "{0}/{1}";

‎multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/BuildCloudUndeployModelStep.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,17 +65,9 @@ protected StepPhase executeStep(ProcessContext context) {
6565
getStepLogger().debug(Messages.BUILDING_CLOUD_UNDEPLOY_MODEL);
6666
DeployedMta deployedMta = context.getVariable(Variables.DEPLOYED_MTA);
6767

68-
List<DeployedMtaServiceKey> serviceKeysToDelete = computeServiceKeysToDelete(context);
69-
getStepLogger().debug(Messages.SERVICE_KEYS_FOR_DELETION, serviceKeysToDelete);
70-
7168
if (deployedMta == null) {
7269
setComponentsToUndeploy(context, Collections.emptyList(), Collections.emptyList(), Collections.emptyList(),
7370
Collections.emptyList(), Collections.emptyList());
74-
75-
if (!serviceKeysToDelete.isEmpty()) {
76-
context.setVariable(Variables.SERVICE_KEYS_TO_DELETE,
77-
getServiceKeysToDelete(context, serviceKeysToDelete));
78-
}
7971
return StepPhase.DONE;
8072
}
8173

@@ -106,6 +98,9 @@ protected StepPhase executeStep(ProcessContext context) {
10698
servicesForApplications, serviceNames);
10799
getStepLogger().debug(Messages.SERVICES_TO_DELETE, servicesToDelete);
108100

101+
List<DeployedMtaServiceKey> serviceKeysToDelete = computeServiceKeysToDelete(context);
102+
getStepLogger().debug(Messages.SERVICE_KEYS_FOR_DELETION, serviceKeysToDelete);
103+
109104
List<CloudApplication> appsToUndeploy = computeAppsToUndeploy(deployedAppsToUndeploy, context.getControllerClient());
110105

111106
DeployedMta backupMta = context.getVariable(Variables.BACKUP_MTA);

‎multiapps-controller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStep.java‎

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -9,27 +9,19 @@
99
import org.apache.commons.lang3.StringUtils;
1010
import org.cloudfoundry.multiapps.controller.client.facade.CloudControllerClient;
1111
import org.cloudfoundry.multiapps.controller.client.facade.CloudCredentials;
12-
import org.cloudfoundry.multiapps.controller.client.facade.CloudOperationException;
13-
import org.cloudfoundry.multiapps.controller.client.facade.domain.CloudServiceInstance;
1412
import org.cloudfoundry.multiapps.controller.core.cf.clients.CustomServiceKeysClient;
1513
import org.cloudfoundry.multiapps.controller.core.cf.clients.WebClientFactory;
1614
import org.cloudfoundry.multiapps.controller.core.cf.detect.DeployedMtaDetector;
1715
import org.cloudfoundry.multiapps.controller.core.cf.metadata.MtaMetadata;
18-
import org.cloudfoundry.multiapps.controller.core.cf.v2.ResourceType;
1916
import org.cloudfoundry.multiapps.controller.core.model.DeployedMta;
2017
import org.cloudfoundry.multiapps.controller.core.model.DeployedMtaService;
2118
import org.cloudfoundry.multiapps.controller.core.model.DeployedMtaServiceKey;
2219
import org.cloudfoundry.multiapps.controller.core.security.serialization.SecureSerialization;
2320
import org.cloudfoundry.multiapps.controller.core.security.token.TokenService;
24-
import org.cloudfoundry.multiapps.controller.core.util.CloudModelBuilderUtil;
2521
import org.cloudfoundry.multiapps.controller.core.util.NameUtil;
2622
import org.cloudfoundry.multiapps.controller.process.Constants;
2723
import org.cloudfoundry.multiapps.controller.process.Messages;
2824
import org.cloudfoundry.multiapps.controller.process.variables.Variables;
29-
import org.cloudfoundry.multiapps.mta.model.DeploymentDescriptor;
30-
import org.cloudfoundry.multiapps.mta.model.Resource;
31-
import org.slf4j.Logger;
32-
import org.slf4j.LoggerFactory;
3325
import org.springframework.beans.factory.annotation.Qualifier;
3426
import org.springframework.beans.factory.config.BeanDefinition;
3527
import org.springframework.context.annotation.Scope;
@@ -46,8 +38,6 @@ public class DetectDeployedMtaStep extends SyncFlowableStep {
4638
@Inject
4739
private WebClientFactory webClientFactory;
4840

49-
private static final Logger LOGGER = LoggerFactory.getLogger(DetectDeployedMtaStep.class);
50-
5141
@Override
5242
protected StepPhase executeStep(ProcessContext context) {
5343
getStepLogger().debug(Messages.DETECTING_DEPLOYED_MTA);
@@ -104,67 +94,14 @@ private void detectBackupMta(String mtaId, String mtaNamespace, CloudControllerC
10494

10595
private List<DeployedMtaServiceKey> detectDeployedServiceKeys(String mtaId, String mtaNamespace, DeployedMta deployedMta,
10696
ProcessContext context) {
107-
List<DeployedMtaService> deployedManagedMtaServices = Optional.ofNullable(deployedMta)
108-
.map(DeployedMta::getServices)
109-
.orElse(List.of());
97+
List<DeployedMtaService> deployedMtaServices = deployedMta == null ? null : deployedMta.getServices();
11098
String spaceGuid = context.getVariable(Variables.SPACE_GUID);
11199
String userGuid = context.getVariable(Variables.USER_GUID);
112100
var token = tokenService.getToken(userGuid);
113101
var creds = new CloudCredentials(token, true);
114102

115103
CustomServiceKeysClient serviceKeysClient = getCustomServiceKeysClient(creds, context.getVariable(Variables.CORRELATION_ID));
116-
117-
List<String> existingInstanceGuids = getExistingServiceGuids(context);
118-
119-
return serviceKeysClient.getServiceKeysByMetadataAndGuids(
120-
spaceGuid, mtaId, mtaNamespace, deployedManagedMtaServices, existingInstanceGuids
121-
);
122-
}
123-
124-
private List<String> getExistingServiceGuids(ProcessContext context) {
125-
CloudControllerClient client = context.getControllerClient();
126-
List<Resource> resources = getExistingServiceResourcesFromDescriptor(context);
127-
128-
return resources.stream()
129-
.map(resource -> resolveServiceGuid(client, resource))
130-
.flatMap(Optional::stream)
131-
.toList();
132-
}
133-
134-
private Optional<String> resolveServiceGuid(CloudControllerClient client, Resource resource) {
135-
try {
136-
CloudServiceInstance instance = client.getServiceInstance(resource.getName());
137-
return Optional.of(instance.getGuid()
138-
.toString());
139-
} catch (CloudOperationException e) {
140-
if (resource.isOptional()) {
141-
logIgnoredService(Messages.IGNORING_NOT_FOUND_OPTIONAL_SERVICE, resource.getName(), e);
142-
return Optional.empty();
143-
}
144-
if (!resource.isActive()) {
145-
logIgnoredService(Messages.IGNORING_NOT_FOUND_INACTIVE_SERVICE, resource.getName(), e);
146-
return Optional.empty();
147-
}
148-
throw e;
149-
}
150-
}
151-
152-
private void logIgnoredService(String message, String serviceName, Exception e) {
153-
String formattedMessage = MessageFormat.format(message, serviceName);
154-
getStepLogger().debug(formattedMessage);
155-
LOGGER.error(formattedMessage, e);
156-
}
157-
158-
private List<Resource> getExistingServiceResourcesFromDescriptor(ProcessContext context) {
159-
DeploymentDescriptor descriptor = context.getVariable(Variables.DEPLOYMENT_DESCRIPTOR);
160-
161-
if (descriptor == null) {
162-
return List.of();
163-
}
164-
return descriptor.getResources()
165-
.stream()
166-
.filter(resource -> CloudModelBuilderUtil.getResourceType(resource) == ResourceType.EXISTING_SERVICE)
167-
.toList();
104+
return serviceKeysClient.getServiceKeysByMetadataAndGuids(spaceGuid, mtaId, mtaNamespace, deployedMtaServices);
168105
}
169106

170107
private void logNoMtaDeployedDetected(String mtaId, String mtaNamespace) {

‎multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/steps/DetectDeployedMtaStepTest.java‎

Lines changed: 14 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import org.cloudfoundry.client.v3.Metadata;
88
import org.cloudfoundry.multiapps.common.test.TestUtil;
9-
import org.cloudfoundry.multiapps.common.test.Tester;
9+
import org.cloudfoundry.multiapps.common.test.Tester.Expectation;
1010
import org.cloudfoundry.multiapps.common.util.JsonUtil;
1111
import org.cloudfoundry.multiapps.controller.client.facade.CloudControllerClient;
1212
import org.cloudfoundry.multiapps.controller.client.facade.CloudCredentials;
@@ -31,12 +31,6 @@
3131

3232
import static org.junit.jupiter.api.Assertions.assertEquals;
3333
import static org.junit.jupiter.api.Assertions.assertNull;
34-
import static org.mockito.ArgumentMatchers.any;
35-
import static org.mockito.ArgumentMatchers.anyList;
36-
import static org.mockito.ArgumentMatchers.anyString;
37-
import static org.mockito.ArgumentMatchers.argThat;
38-
import static org.mockito.ArgumentMatchers.eq;
39-
import static org.mockito.ArgumentMatchers.isNull;
4034
import static org.mockito.Mockito.when;
4135

4236
class DetectDeployedMtaStepTest extends SyncFlowableStepTest<DetectDeployedMtaStep> {
@@ -68,47 +62,28 @@ void testExecuteWithDeployedMta() {
6862
.getKeys();
6963
List<DeployedMta> deployedComponents = List.of(deployedMta);
7064

71-
when(deployedMtaDetector.detectDeployedMtas(any(CloudControllerClient.class)))
72-
.thenReturn(deployedComponents);
73-
when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(eq(MTA_ID), eq(null),
74-
any(CloudControllerClient.class)))
75-
.thenReturn(Optional.of(deployedMta));
76-
77-
when(customClientMock.getServiceKeysByMetadataAndGuids(
78-
eq(SPACE_GUID), eq(MTA_ID), isNull(),
79-
eq(deployedMta.getServices()),
80-
argThat(List::isEmpty)))
81-
.thenReturn(deployedKeys);
82-
83-
when(customClientMock.getServiceKeysByMetadataAndGuids(
84-
eq(SPACE_GUID), eq(MTA_ID), isNull(),
85-
anyList(),
86-
argThat(list -> list != null && !list.isEmpty())))
87-
.thenReturn(Collections.emptyList());
65+
when(deployedMtaDetector.detectDeployedMtas(Mockito.any(CloudControllerClient.class))).thenReturn(deployedComponents);
66+
when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(Mockito.eq(MTA_ID), Mockito.eq(null),
67+
Mockito.any(
68+
CloudControllerClient.class))).thenReturn(
69+
Optional.of(deployedMta));
70+
when(customClientMock.getServiceKeysByMetadataAndGuids(Mockito.eq(SPACE_GUID), Mockito.eq(MTA_ID), Mockito.isNull(),
71+
Mockito.eq(deployedMta.getServices()))).thenReturn(deployedKeys);
8872

8973
step.execute(execution);
9074

9175
assertStepFinishedSuccessfully();
9276

93-
tester.test(() -> context.getVariable(Variables.DEPLOYED_MTA),
94-
new Tester.Expectation(Tester.Expectation.Type.JSON, DEPLOYED_MTA_LOCATION));
77+
tester.test(() -> context.getVariable(Variables.DEPLOYED_MTA), new Expectation(Expectation.Type.JSON, DEPLOYED_MTA_LOCATION));
9578
assertEquals(deployedKeys, context.getVariable(Variables.DEPLOYED_MTA_SERVICE_KEYS));
9679
}
9780

9881
@Test
9982
void testExecuteWithoutDeployedMta() {
10083
when(deployedMtaDetector.detectDeployedMtas(client)).thenReturn(Collections.emptyList());
10184
when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(MTA_ID, null, client)).thenReturn(Optional.empty());
102-
when(customClientMock.getServiceKeysByMetadataAndGuids(
103-
eq(SPACE_GUID), eq(MTA_ID), isNull(),
104-
eq(Collections.emptyList()),
105-
eq(Collections.emptyList())))
106-
.thenReturn(Collections.emptyList());
107-
108-
when(customClientMock.getServiceKeysByMetadataAndGuids(
109-
anyString(), anyString(), isNull(),
110-
anyList(), anyList()))
111-
.thenReturn(Collections.emptyList());
85+
when(customClientMock.getServiceKeysByMetadataAndGuids(SPACE_GUID, MTA_ID, null,
86+
Collections.emptyList())).thenReturn(Collections.emptyList());
11287

11388
step.execute(execution);
11489

@@ -155,10 +130,10 @@ void testExecuteWithBackupdMta() {
155130
.build())
156131
.build();
157132

158-
when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(eq(MTA_ID), eq(null),
133+
when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(Mockito.eq(MTA_ID), Mockito.eq(null),
159134
Mockito.any())).thenReturn(Optional.of(deployedMta));
160-
when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(eq(MTA_ID),
161-
eq(NameUtil.computeUserNamespaceWithSystemNamespace(
135+
when(deployedMtaDetector.detectDeployedMtaByNameAndNamespace(Mockito.eq(MTA_ID),
136+
Mockito.eq(NameUtil.computeUserNamespaceWithSystemNamespace(
162137
Constants.MTA_BACKUP_NAMESPACE,
163138
null)),
164139
Mockito.any())).thenReturn(Optional.of(backupMta));

0 commit comments

Comments
 (0)