Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,10 @@ public class SupportedParameters {
public static final Set<String> DEPENDENCY_PARAMETERS = Set.of(BINDING_NAME, ENV_VAR_NAME, VISIBILITY, USE_LIVE_ROUTES,
SERVICE_BINDING_CONFIG, DELETE_SERVICE_KEY_AFTER_DEPLOYMENT);

public static final Set<String> MODULE_HOOK_PARAMETERS = Set.of(NAME, COMMAND, MEMORY, DISK_QUOTA, HOOK_REQUIRES);
public static final String HOOK_TARGET_APP = "hook-target-app";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this parameter used?

public static final String PHASES_CONFIG = "phases-config";

public static final Set<String> MODULE_HOOK_PARAMETERS = Set.of(NAME, COMMAND, MEMORY, DISK_QUOTA, HOOK_REQUIRES, HOOK_TARGET_APP, PHASES_CONFIG);

public static final Set<String> CONFIGURATION_REFERENCE_PARAMETERS = Set.of(PROVIDER_NID, PROVIDER_ID, TARGET, VERSION,
DEPRECATED_CONFIG_MTA_ID, DEPRECATED_CONFIG_MTA_VERSION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ public class Messages {
public static final String ERROR_PREPARING_TO_EXECUTE_TASKS_ON_APP = "Error preparing to execute tasks on application \"{0}\"";
public static final String ERROR_PREPARING_TO_RESTART_SERVICE_BROKER_SUBSCRIBERS = "Error preparing to restart service broker subscribers";
public static final String ERROR_EXECUTING_TASK_0_ON_APP_1 = "Execution of task \"{0}\" on application \"{1}\" failed.";
public static final String DUPLICATE_PHASE_IN_PHASES_CONFIG = "Duplicate phase \"{0}\" in \"phases-config\" of hook \"{1}\". Only one target-app per phase is supported.";
public static final String INVALID_PHASES_CONFIG_NOT_A_LIST = "Parameter \"phases-config\" of hook \"{0}\" must be a list.";
public static final String ERROR_DETECTING_COMPONENTS_TO_UNDEPLOY = "Error detecting components to undeploy";
public static final String ERROR_DELETING_IDLE_ROUTES = "Error deleting idle routes";
public static final String ERROR_CREATING_SERVICE_BROKERS = "Error creating service brokers";
Expand Down Expand Up @@ -213,6 +215,7 @@ public class Messages {
public static final String CANNOT_RETRIEVE_PARAMETERS_OF_BINDING_BETWEEN_APPLICATION_0_AND_SERVICE_INSTANCE_1 = "Cannot retrieve parameters of binding between application \"{0}\" and service instance \"{1}\"";
public static final String CANNOT_RETRIEVE_PARAMETERS_OF_BINDING_BETWEEN_APPLICATION_0_AND_SERVICE_INSTANCE_1_FIX = "Cannot retrieve parameters of binding between application \"{0}\" and service instance \"{1}\". Got 502.";
public static final String CANNOT_RETRIEVE_INSTANCE_OF_SERVICE = "Cannot retrieve service instance of service \"{0}\"";
public static final String HOOK_PHASE_BLUE_GREEN_BEFORE_UNMAP_ROUTES_IDLE_USED = "Hook \"{0}\" uses deprecated phase \"blue-green.application.before-unmap-routes.idle\" which is unreachable and will be removed in a future version";
public static final String COULD_NOT_DELETE_PROVIDED_DEPENDENCY = "Could not delete published provided dependency \"{0}\" from configuration registry";
public static final String COULD_NOT_DELETE_SERVICE = "Could not delete service \"{0}\", as it does not exist";
public static final String COULD_NOT_DELETE_SUBSCRIPTION = "Could not delete subscription for application \"{0}\" and resource \"{1}\"";
Expand Down Expand Up @@ -409,6 +412,7 @@ public class Messages {
public static final String WILL_NOT_REBIND_APP_TO_SERVICE_SAME_PARAMETERS = "Service instance \"{0}\" will not be rebound to application \"{1}\" because the binding parameters are not modified";
public static final String SERVICE_BROKER_DOES_NOT_EXIST = "Service broker with name \"{0}\" does not exist";
public static final String EXECUTING_HOOK_0 = "Executing hook \"{0}\"";
public static final String SKIPPING_HOOK_TASK_NO_LIVE_APP = "Skipping hook task on application \"{0}\" with target-app \"live\": no live application exists yet (initial deployment)";
public static final String WAITING_PREVIOUS_OPERATIONS_TO_FINISH = "Waiting for previous service operations to finish...";
public static final String ASYNC_OPERATION_FOR_SERVICE_BROKER_FINISHED = "Async operation for service broker \"{0}\" has finished";
public static final String STARTING_INCREMENTAL_APPLICATION_INSTANCE_UPDATE_FOR_0 = "Starting incremental application instance update for \"{0}\"...";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.cloudfoundry.multiapps.controller.persistence.services.DescriptorBackupService;
import org.cloudfoundry.multiapps.controller.process.Messages;
import org.cloudfoundry.multiapps.controller.process.security.SecretParametersCollector;
import org.cloudfoundry.multiapps.controller.process.util.HookPhasesConfigValidator;
import org.cloudfoundry.multiapps.controller.process.util.NamespaceGlobalParameters;
import org.cloudfoundry.multiapps.controller.process.util.UnsupportedParameterFinder;
import org.cloudfoundry.multiapps.controller.process.variables.Variables;
Expand All @@ -30,6 +31,8 @@
@Scope(BeanDefinition.SCOPE_PROTOTYPE)
public class MergeDescriptorsStep extends SyncFlowableStep {

private static final int HOOKS_MIN_SCHEMA_VERSION = 3;

@Inject
private DescriptorBackupService descriptorBackupService;

Expand Down Expand Up @@ -59,6 +62,9 @@ protected StepPhase executeStep(ProcessContext context) {
.toList());
context.setVariable(Variables.DEPLOYMENT_DESCRIPTOR, descriptor);

if (context.getVariable(Variables.MTA_MAJOR_SCHEMA_VERSION) >= HOOKS_MIN_SCHEMA_VERSION) {
new HookPhasesConfigValidator().validate(descriptor);
}
warnForUnsupportedParameters(descriptor);

backupDeploymentDescriptor(context, descriptor);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package org.cloudfoundry.multiapps.controller.process.steps;

import java.text.MessageFormat;
import java.util.List;
import java.util.Map;

import jakarta.inject.Named;
import org.cloudfoundry.multiapps.controller.client.lib.domain.CloudApplicationExtended;
import org.cloudfoundry.multiapps.controller.client.lib.domain.ImmutableCloudApplicationExtended;
import org.cloudfoundry.multiapps.controller.core.model.ApplicationColor;
import org.cloudfoundry.multiapps.controller.core.model.BlueGreenApplicationNameSuffix;
import org.cloudfoundry.multiapps.controller.core.model.HookPhaseProcessType;
import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters;
import org.cloudfoundry.multiapps.controller.process.Messages;
import org.cloudfoundry.multiapps.controller.process.variables.Variables;
import org.cloudfoundry.multiapps.mta.model.Hook;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.context.annotation.Scope;

@Named("resolveHookTaskTargetAppStep")
@Scope(BeanDefinition.SCOPE_PROTOTYPE)
public class ResolveHookTaskTargetAppStep extends SyncFlowableStep {

private static final String PHASE_KEY = "phase";
private static final String TARGET_APP_KEY = "target-app";

@Override
protected StepPhase executeStep(ProcessContext context) {
CloudApplicationExtended app = context.getVariable(Variables.APP_TO_PROCESS);

String resolvedAppName = resolveTargetAppName(context, app);
if (resolvedAppName == null) {
context.setVariable(Variables.TASKS_TO_EXECUTE, List.of());
return StepPhase.DONE;
}
if (!resolvedAppName.equals(app.getName())) {
context.setVariable(Variables.APP_TO_PROCESS, buildAppWithName(app, resolvedAppName));
}

return StepPhase.DONE;
}

private String resolveTargetAppName(ProcessContext context, CloudApplicationExtended app) {
Hook hook = context.getVariable(Variables.HOOK_FOR_EXECUTION);
if (hook == null) {
return app.getName();
}

List<Map<String, String>> phasesConfig = getPhasesConfig(hook);
if (phasesConfig.isEmpty()) {
return app.getName();
}

String currentPhase = buildCurrentPhaseString(context, hook);
String targetApp = phasesConfig.stream()
.filter(config -> currentPhase.equals(config.get(PHASE_KEY)))
.map(config -> config.get(TARGET_APP_KEY))
.findFirst()
.orElse(null);

if (targetApp == null) {
return app.getName();
}

return resolveAppNameForTarget(context, app, targetApp);
}

@SuppressWarnings("unchecked")
private List<Map<String, String>> getPhasesConfig(Hook hook) {
Object phasesConfigValue = hook.getParameters()
.get(SupportedParameters.PHASES_CONFIG);
if (phasesConfigValue instanceof List) {
return (List<Map<String, String>>) phasesConfigValue;
}
return List.of();
}

private String buildCurrentPhaseString(ProcessContext context, Hook hook) {
List<String> hookExecutionPhases = context.getVariable(Variables.HOOK_EXECUTION_PHASES);
return hook.getPhases()
.stream()
.filter(hookExecutionPhases::contains)
.findFirst()
.orElse("");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we reuse HookPhase.NONE?

}

private String resolveAppNameForTarget(ProcessContext context, CloudApplicationExtended app, String targetApp) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen if we using hook:
blue-green.application.before-start.idle
with target-app: live
During initial deployment when live app still does not exist?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the current implementation we hit 404. What’s the right approach here- should the process fail or just log a warning? Also, should the combination of phase: blue-green.application.before-start.idle and target-app: live even be allowed during an initial deployment?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The execution of the hook should be skipped because no live application exists

if (HookPhaseProcessType.HookProcessPhase.LIVE.getType()
.equals(targetApp)) {
if (isInitialDeploy(context)) {
getStepLogger().warn(Messages.SKIPPING_HOOK_TASK_NO_LIVE_APP, app.getName());
return null;
}
ApplicationColor liveColor = context.getVariable(Variables.LIVE_MTA_COLOR);
String suffix = liveColor != null ? liveColor.asSuffix() : BlueGreenApplicationNameSuffix.LIVE.asSuffix();
Comment on lines +90 to +95
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you extract in smaller method?

return BlueGreenApplicationNameSuffix.removeSuffix(app.getName()) + suffix;
}
if (HookPhaseProcessType.HookProcessPhase.IDLE.getType()
.equals(targetApp)) {
String baseName = BlueGreenApplicationNameSuffix.removeSuffix(app.getName());
ApplicationColor idleColor = context.getVariable(Variables.IDLE_MTA_COLOR);
if (idleColor != null) {
return baseName + idleColor.asSuffix();
}
if (isAfterRenamePhase(context)) {
return baseName;
}
return baseName + BlueGreenApplicationNameSuffix.IDLE.asSuffix();
Comment on lines +100 to +108
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you extract in smaller method?

}
return app.getName();
}

private boolean isAfterRenamePhase(ProcessContext context) {
return context.getVariable(Variables.PHASE) != null;
}

private boolean isInitialDeploy(ProcessContext context) {
return context.getVariable(Variables.DEPLOYED_MTA) == null;
}

private CloudApplicationExtended buildAppWithName(CloudApplicationExtended app, String resolvedAppName) {
return ImmutableCloudApplicationExtended.copyOf(app)
.withName(resolvedAppName);
}

@Override
protected String getStepErrorMessage(ProcessContext context) {
return MessageFormat.format(Messages.ERROR_EXECUTING_HOOK,
context.getVariable(Variables.HOOK_FOR_EXECUTION)
.getName());
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package org.cloudfoundry.multiapps.controller.process.util;

import java.text.MessageFormat;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.cloudfoundry.multiapps.common.SLException;
import org.cloudfoundry.multiapps.controller.core.model.HookPhase;
import org.cloudfoundry.multiapps.controller.core.model.SupportedParameters;
import org.cloudfoundry.multiapps.controller.process.Messages;
import org.cloudfoundry.multiapps.mta.model.DeploymentDescriptor;
import org.cloudfoundry.multiapps.mta.model.Hook;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class HookPhasesConfigValidator {

private static final Logger LOGGER = LoggerFactory.getLogger(HookPhasesConfigValidator.class);
private static final String PHASE_KEY = "phase";

public void validate(DeploymentDescriptor descriptor) {
descriptor.getModules()
.stream()
.flatMap(module -> module.getHooks().stream())
.forEach(hook -> {
warnIfDeprecatedPhaseUsed(hook);
validateHookHasNoDuplicatePhaseConfigs(hook);
});
}

private void warnIfDeprecatedPhaseUsed(Hook hook) {
boolean usesDeprecatedPhase = hook.getPhases()
.stream()
.anyMatch(phase -> HookPhase.BLUE_GREEN_APPLICATION_BEFORE_UNMAP_ROUTES_IDLE.getValue()
.equals(phase));
if (usesDeprecatedPhase) {
LOGGER.warn(Messages.HOOK_PHASE_BLUE_GREEN_BEFORE_UNMAP_ROUTES_IDLE_USED, hook.getName());
}
}

@SuppressWarnings("unchecked")
private void validateHookHasNoDuplicatePhaseConfigs(Hook hook) {
Object phasesConfigValue = hook.getParameters().get(SupportedParameters.PHASES_CONFIG);
if (phasesConfigValue == null) {
return;
}
if (!(phasesConfigValue instanceof List)) {
throw new SLException(MessageFormat.format(Messages.INVALID_PHASES_CONFIG_NOT_A_LIST, hook.getName()));
}
List<Map<String, String>> phasesConfig = (List<Map<String, String>>) phasesConfigValue;
Set<String> seenPhases = new HashSet<>();
for (Map<String, String> phaseConfig : phasesConfig) {
String phase = phaseConfig.get(PHASE_KEY);
if (phase != null && !seenPhases.add(phase)) {
throw new SLException(MessageFormat.format(Messages.DUPLICATE_PHASE_IN_PHASES_CONFIG, phase, hook.getName()));
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ private List<Hook> executeHooks(StepPhase currentStepPhase) {
moduleToDeploy.getName());
updateExecutedHooksForModule(alreadyExecutedHooksForModule, hooksWithPhases.getHookPhases(), hooksWithPhases.getHooks());
context.setVariable(Variables.HOOKS_FOR_EXECUTION, hooksWithPhases.getHooks());
context.setVariable(Variables.HOOK_EXECUTION_PHASES, hooksWithPhases.getHookPhases()
.stream()
.map(HookPhase::getValue)
.toList());
return hooksWithPhases.getHooks();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,10 @@ public interface Variables {
.type(Variable.typeReference(Hook.class))
.defaultValue(Collections.emptyList())
.build();
Variable<List<String>> HOOK_EXECUTION_PHASES = ImmutableSimpleVariable.<List<String>> builder()
.name("hookExecutionPhases")
.defaultValue(Collections.emptyList())
.build();
Variable<List<Module>> MODULES_TO_DEPLOY = ImmutableJsonBinaryListVariable.<Module> builder()
.name("modulesToDeploy")
.type(Variable.typeReference(Module.class))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
<flowable:in source="isDisposableUserProvidedServiceEnabled" target="isDisposableUserProvidedServiceEnabled"></flowable:in>
<flowable:in source="disposableUserProvidedServiceName" target="disposableUserProvidedServiceName"></flowable:in>
<flowable:in source="mtaId" target="mtaId"></flowable:in>
<flowable:in source="subprocessPhase" target="subprocessPhase"></flowable:in>
<flowable:in source="phase" target="phase"></flowable:in>
<flowable:in source="hookExecutionPhases" target="hookExecutionPhases"></flowable:in>
<flowable:in source="deployedMta" target="deployedMta"></flowable:in>
<flowable:in source="idleMtaColor" target="idleMtaColor"></flowable:in>
<flowable:in source="liveMtaColor" target="liveMtaColor"></flowable:in>
Comment on lines +29 to +32
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you double check the diagrams so they are exported from the modeler because there is some space indentation here

</extensionElements>
<multiInstanceLoopCharacteristics isSequential="false" flowable:collection="hooksForExecution" flowable:elementVariable="hookForExecution">
<extensionElements>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@
<flowable:in source="isDisposableUserProvidedServiceEnabled" target="isDisposableUserProvidedServiceEnabled"></flowable:in>
<flowable:in source="disposableUserProvidedServiceName" target="disposableUserProvidedServiceName"></flowable:in>
<flowable:in source="mtaId" target="mtaId"></flowable:in>
<flowable:in source="subprocessPhase" target="subprocessPhase"></flowable:in>
<flowable:in source="phase" target="phase"></flowable:in>
<flowable:in source="hookExecutionPhases" target="hookExecutionPhases"></flowable:in>
<flowable:in source="deployedMta" target="deployedMta"></flowable:in>
<flowable:in source="idleMtaColor" target="idleMtaColor"></flowable:in>
<flowable:in source="liveMtaColor" target="liveMtaColor"></flowable:in>
</extensionElements>
<multiInstanceLoopCharacteristics isSequential="false" flowable:collection="hooksForExecution" flowable:elementVariable="hookForExecution"></multiInstanceLoopCharacteristics>
</callActivity>
Expand Down Expand Up @@ -143,6 +149,12 @@
<flowable:in source="isDisposableUserProvidedServiceEnabled" target="isDisposableUserProvidedServiceEnabled"></flowable:in>
<flowable:in source="disposableUserProvidedServiceName" target="disposableUserProvidedServiceName"></flowable:in>
<flowable:in source="mtaId" target="mtaId"></flowable:in>
<flowable:in source="subprocessPhase" target="subprocessPhase"></flowable:in>
<flowable:in source="phase" target="phase"></flowable:in>
<flowable:in source="hookExecutionPhases" target="hookExecutionPhases"></flowable:in>
<flowable:in source="deployedMta" target="deployedMta"></flowable:in>
<flowable:in source="idleMtaColor" target="idleMtaColor"></flowable:in>
<flowable:in source="liveMtaColor" target="liveMtaColor"></flowable:in>
</extensionElements>
<multiInstanceLoopCharacteristics isSequential="false" flowable:collection="hooksForExecution" flowable:elementVariable="hookForExecution"></multiInstanceLoopCharacteristics>
</callActivity>
Expand Down
Loading
Loading