-
Notifications
You must be signed in to change notification settings - Fork 41
Add phase-configs support to Hook model for target-app resolution #1812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
b272617
8e768cd
ceef42a
eab5ad4
9165d29
aab7ff0
695aea6
26e0d65
80b634a
7deb07c
0ecd675
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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(""); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What will happen if we using hook:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
|---|---|---|
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this parameter used?