Skip to content

remove idle suffix from bg deploy #1697

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.

Sign up for GitHub

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

Open
ikasarov wants to merge 5 commits into
base: master
Choose a base branch
from
Open

remove idle suffix from bg deploy #1697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
697eea2
remove idle suffix from bg deploy
ikasarov Jul 2, 2025
a6ae41b
remove no longer needed RemoveNewApplicationsSuffixStep
ikasarov Sep 12, 2025
06091ce
fixing comments
ikasarov Sep 29, 2025
10c2f79
removed the handleApplicationName method signature from StepFlowHandler
ikasarov Oct 10, 2025
2fe0804
additional review changes
ikasarov Jan 9, 2026
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
2 changes: 1 addition & 1 deletion ...ps-controller-core/src/main/java/org/cloudfoundry/multiapps/controller/core/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public final class Messages {
public static final String THREAD_MONITOR_CACHE_TIMEOUT = "Flowable thread monitor cache timeout: {0} seconds";
public static final String SPACE_DEVELOPERS_CACHE_TIME_IN_SECONDS = "Cache for list of space developers per SpaceGUID: {0} seconds";
public static final String APP_SHUTDOWN_REQUEST = "Application with id:\"{0}\", instance id:\"{1}\", instance index:\"{2}\", is requested to shutdown. Timeout to wait before shutdown of Flowable job executor:\"{3}\" seconds.";
public static final String APP_SHUTDOWNED = "Application with id:\"{0}\", instance id:\"{1}\", instance index:\"{2}\", is shutdowned. Timeout to wait before shutdown of Flowable job executor:\"{3}\" seconds.";
public static final String APP_SUCCESSFULLY_SHUTDOWN = "Application with id:\"{0}\", instance id:\"{1}\", instance index:\"{2}\", is shutdown. Timeout to wait before shutdown of Flowable job executor:\"{3}\" seconds.";
public static final String APP_SHUTDOWN_STATUS_MONITOR = "Monitor shutdown status of application with id:\"{0}\", instance id:\"{1}\", instance index:\"{2}\". Status:\"{3}\".";
public static final String CONTROLLER_CLIENT_SSL_HANDSHAKE_TIMEOUT_IN_SECONDS = "Controller client SSL handshake timeout in seconds: {0}";
public static final String CONTROLLER_CLIENT_CONNECT_TIMEOUT_IN_SECONDS = "Controller client connect timeout in seconds: {0}";
Expand Down
2 changes: 1 addition & 1 deletion ...c/main/java/org/cloudfoundry/multiapps/controller/core/cf/detect/AppSuffixDeterminer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public AppSuffixDeterminer(boolean keepOriginalNamesAfterDeploy, boolean isAfter
this.isAfterResumePhase = isAfterResumePhase;
}

public boolean shouldAppendApplicationSuffix() {
public boolean shouldAppendIdleSuffix() {
return keepOriginalNamesAfterDeploy && isAfterResumePhase;
}
}
2 changes: 1 addition & 1 deletion ...n/java/org/cloudfoundry/multiapps/controller/core/cf/v2/ApplicationCloudModelBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ private DeployedMtaApplication findDeployedApplication(String moduleName, Deploy

private String getApplicationName(Module module) {
String applicationName = NameUtil.getApplicationName(module);
if (appSuffixDeterminer.shouldAppendApplicationSuffix()) {
if (appSuffixDeterminer.shouldAppendIdleSuffix()) {
applicationName += BlueGreenApplicationNameSuffix.IDLE.asSuffix();
}
return applicationName;
Expand Down
8 changes: 3 additions & 5 deletions ...st/java/org/cloudfoundry/multiapps/controller/core/cf/detect/AppSuffixDeterminerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
package org.cloudfoundry.multiapps.controller.core.cf.detect;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.stream.Stream;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import static org.junit.jupiter.api.Assertions.assertEquals;

class AppSuffixDeterminerTest {

static Stream<Arguments> testAppSuffixDeterminer() {
return Stream.of(
//@formatter:off
// (1) Keep original app names is not set and the process is not after resume phase
Arguments.of(false, false, false),
// (2) Keep original app names is set but the process is not after resume phase
Expand All @@ -21,14 +20,13 @@ static Stream<Arguments> testAppSuffixDeterminer() {
Arguments.of(false, true, false),
// (4) Keep original app names is set and the process is after resume phase
Arguments.of(true, true, true)
//@formatter:on
);
}

@ParameterizedTest
@MethodSource
void testAppSuffixDeterminer(boolean keepOriginalNamesAfterDeploy, boolean isAfterResumePhase, boolean shouldAppendApplicationSuffix) {
AppSuffixDeterminer appSuffixDeterminer = new AppSuffixDeterminer(keepOriginalNamesAfterDeploy, isAfterResumePhase);
assertEquals(shouldAppendApplicationSuffix, appSuffixDeterminer.shouldAppendApplicationSuffix());
assertEquals(shouldAppendApplicationSuffix, appSuffixDeterminer.shouldAppendIdleSuffix());
}
}
1 change: 0 additions & 1 deletion ...troller-process/src/main/java/org/cloudfoundry/multiapps/controller/process/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ public class Messages {
public static final String ERROR_MONITORING_OPERATIONS_OVER_SERVICES = "Error monitoring operations over services";
public static final String ERROR_DELETING_REMAINING_FILE_PARTS = "Error deleting remaining file parts";
public static final String ERROR_DETECTING_APPLICATIONS_TO_RENAME = "Error detecting applications to rename";
public static final String ERROR_RENAMING_NEW_APPLICATIONS = "Error renaming new applications";
public static final String ERROR_MONITORING_CREATION_OR_UPDATE_OF_SERVICES = "Error monitoring creation or update of services";
public static final String ERROR_MONITORING_DELETION_OF_SERVICES = "Error monitoring deletion of services";
public static final String SERVICE_IS_ALREADY_DELETED = "Service \"{0}\" is already deleted";
Expand Down
75 changes: 73 additions & 2 deletions .../main/java/org/cloudfoundry/multiapps/controller/process/steps/CreateOrUpdateAppStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.cloudfoundry.multiapps.controller.client.facade.CloudControllerClient;
import org.cloudfoundry.multiapps.controller.client.facade.CloudCredentials;
import org.cloudfoundry.multiapps.controller.client.facade.domain.CloudApplication;
import org.cloudfoundry.multiapps.controller.client.facade.domain.ImmutableCloudApplication;
import org.cloudfoundry.multiapps.controller.client.facade.domain.ImmutableStaging;
import org.cloudfoundry.multiapps.controller.client.facade.domain.Staging;
import org.cloudfoundry.multiapps.controller.client.facade.dto.ApplicationToCreateDto;
Expand All @@ -28,8 +29,11 @@
import org.cloudfoundry.multiapps.controller.core.cf.clients.AppBoundServiceInstanceNamesGetter;
import org.cloudfoundry.multiapps.controller.core.cf.clients.WebClientFactory;
import org.cloudfoundry.multiapps.controller.core.helpers.ApplicationFileDigestDetector;
import org.cloudfoundry.multiapps.controller.core.model.BlueGreenApplicationNameSuffix;
import org.cloudfoundry.multiapps.controller.core.security.token.TokenService;
import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration;
import org.cloudfoundry.multiapps.controller.persistence.model.ConfigurationSubscription;
import org.cloudfoundry.multiapps.controller.persistence.services.ConfigurationSubscriptionService;
import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException;
import org.cloudfoundry.multiapps.controller.process.Messages;
import org.cloudfoundry.multiapps.controller.process.util.ApplicationAttributeUpdater;
Expand Down Expand Up @@ -59,6 +63,8 @@ public class CreateOrUpdateAppStep extends SyncFlowableStep {
private WebClientFactory webClientFactory;
@Inject
private ApplicationConfiguration configuration;
@Inject
private ConfigurationSubscriptionService subscriptionService;

@Override
protected StepPhase executeStep(ProcessContext context) throws FileStorageException {
Expand Down Expand Up @@ -94,7 +100,9 @@ protected AppBoundServiceInstanceNamesGetter getAppBoundServiceInstanceNamesGett
return new AppBoundServiceInstanceNamesGetter(configuration, webClientFactory, credentials, correlationId);
}

private StepFlowHandler createStepFlowHandler(ProcessContext context, CloudControllerClient client, CloudApplicationExtended app,
private StepFlowHandler createStepFlowHandler(ProcessContext context,
CloudControllerClient client,
CloudApplicationExtended app,
CloudApplication existingApp) {
if (existingApp == null) {
return new CreateAppFlowHandler(context, client, app);
Expand Down Expand Up @@ -212,7 +220,9 @@ private class UpdateAppFlowHandler extends StepFlowHandler {

final CloudApplication existingApp;

public UpdateAppFlowHandler(ProcessContext context, CloudControllerClient client, CloudApplicationExtended app,
public UpdateAppFlowHandler(ProcessContext context,
CloudControllerClient client,
CloudApplicationExtended app,
CloudApplication existingApp) {
super(context, client, app);
this.existingApp = existingApp;
Expand All @@ -230,7 +240,9 @@ public void handleApplicationAttributes() {

reportApplicationUpdateStatus(app, arePropertiesChanged);
context.setVariable(Variables.VCAP_APP_PROPERTIES_CHANGED, arePropertiesChanged);

updateApplicationEnvironment();
updateApplicationName();
}

private void updateApplicationEnvironment() {
Expand Down Expand Up @@ -278,6 +290,31 @@ private UpdateStrategy getEnvUpdateStrategy() {
.shouldKeepExistingEnv() ? UpdateStrategy.MERGE : UpdateStrategy.REPLACE;
}

public void updateApplicationName() {
boolean processIsBlueGreenWithIdleSuffix = StepsUtil.getAppSuffixDeterminer(context)
Copy link
Contributor

@IvanBorislavovDimitrov IvanBorislavovDimitrov Oct 2, 2025

Choose a reason for hiding this comment

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

maybe rename to isBlueGreenProcessAfterResumePhase or something?

Copy link
Contributor Author

@ikasarov ikasarov Oct 15, 2025

Choose a reason for hiding this comment

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

the current name seems more clear since this is only for blueGreen with idle, not with colours

Copy link
Contributor

@s-yonkov-yonkov s-yonkov-yonkov Jan 5, 2026

Choose a reason for hiding this comment

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

Is this resolved?

.shouldAppendIdleSuffix();
if (!processIsBlueGreenWithIdleSuffix) {
return;
}

String oldName = existingApp.getName();
String newName = BlueGreenApplicationNameSuffix.removeSuffix(oldName);
if (oldName.equals(newName)) {
Copy link
Contributor

@s-yonkov-yonkov s-yonkov-yonkov Sep 16, 2025

Choose a reason for hiding this comment

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

when this check will return true?

Copy link
Contributor Author

@ikasarov ikasarov Sep 29, 2025

Choose a reason for hiding this comment

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

will double-check - in general if the new app happens to have the same name (not blue-green deploy?)

Copy link
Contributor

@IvanBorislavovDimitrov IvanBorislavovDimitrov Oct 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

if it is not the not blue green, will it even reach this condition? Maybe during retry?

Copy link
Contributor Author

@ikasarov ikasarov Jan 9, 2026

Choose a reason for hiding this comment

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

CreateOrUpdateAppStep is a very generic step in the deploy app subprocess, so I think this condition will be reached if we're just updating an app's attributes/environment variables.

getStepLogger().info(Messages.THE_DETECTED_APPLICATION_HAS_THE_SAME_NAME_AS_THE_NEW_ONE);
return;
Comment on lines +303 to +304
Copy link
Contributor

@IvanBorislavovDimitrov IvanBorislavovDimitrov Oct 2, 2025

Choose a reason for hiding this comment

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

Can a rename here make this step non-idempotent? What will happen if the step fails during the update of the configuration entries? Or for example instance get shutdown and it gets re-executed by flowable?

Copy link
Contributor

@s-yonkov-yonkov s-yonkov-yonkov Jan 5, 2026

Choose a reason for hiding this comment

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

Is this resolved/answered?

Copy link
Contributor Author

@ikasarov ikasarov Jan 9, 2026

Choose a reason for hiding this comment

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

haven't tested a failure at the point of updating the subscribers but I assume the behaviour will be unchanged - the rename + update of subscribers were handled in the same step (not this one) before my changes as well

}

getStepLogger().info(Messages.RENAMING_APPLICATION_0_TO_1, oldName, newName);
client.rename(oldName, newName);
Copy link
Contributor

@IvanBorislavovDimitrov IvanBorislavovDimitrov Oct 2, 2025

Choose a reason for hiding this comment

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

What will happen if the MTA operation fail in the resume phase? I mean if the restart of the rename application (now without suffix fails) and the customer executes a new deployment, will the new process detect the "-live" applications as actually live? Is it possible for the new operation to detect the wrong application as live?

Copy link
Contributor

@IvanBorislavovDimitrov IvanBorislavovDimitrov Oct 2, 2025

Choose a reason for hiding this comment

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

I'm writing this comment because now the renaming is before the restart/restage, which is a potential point of failure.

Copy link
Contributor Author

@ikasarov ikasarov Jan 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

thsi was tested with a crash at the end of the CreateOrUpdateStep - the retries handled the resume correctly


context.setVariable(Variables.EXISTING_APP, ImmutableCloudApplication.copyOf(existingApp)
.withName(newName));
context.setVariable(Variables.APP_TO_PROCESS, ImmutableCloudApplicationExtended.copyOf(app)
.withName(newName));

updateConfigurationSubscribers(oldName, newName);
}

@Override
public void handleApplicationServices() {
if (context.getVariable(Variables.SHOULD_SKIP_SERVICE_REBINDING)) {
Expand All @@ -297,6 +334,40 @@ public void printStepEndMessage() {
getStepLogger().debug(Messages.APP_UPDATED, app.getName());
}

private void updateConfigurationSubscribers(String oldAppName, String newAppName) {
String mtaId = context.getVariable(Variables.MTA_ID);
String spaceGuid = context.getVariable(Variables.SPACE_GUID);

List<ConfigurationSubscription> subscriptions = subscriptionService.createQuery()
.mtaId(mtaId)
.spaceId(spaceGuid)
.list();
for (ConfigurationSubscription subscription : subscriptions) {
if (oldAppName.equals(subscription.getAppName())) {
getStepLogger().debug(Messages.UPDATING_CONFIGURATION_SUBSCRIPTION_0_WITH_NAME_1, subscription.getAppName(),
newAppName);
updateConfigurationSubscription(subscription, newAppName);
}
}
}

private void updateConfigurationSubscription(ConfigurationSubscription subscription, String newAppName) {
ConfigurationSubscription newSubscription = createNewSubscription(subscription, newAppName);
subscriptionService.update(subscription, newSubscription);
}

private ConfigurationSubscription createNewSubscription(ConfigurationSubscription subscription, String newAppName) {
return new ConfigurationSubscription(subscription.getId(),
subscription.getMtaId(),
subscription.getSpaceId(),
newAppName,
subscription.getFilter(),
subscription.getModuleDto(),
subscription.getResourceDto(),
subscription.getModuleId(),
subscription.getResourceId());
}

private List<String> getMtaAndExistingServices() {
var serviceNamesGetter = getAppBoundServiceInstanceNamesGetter(context);
return Stream.of(app.getServices(), serviceNamesGetter.getServiceInstanceNamesBoundToApp(existingApp.getGuid()))
Expand Down
75 changes: 0 additions & 75 deletions .../org/cloudfoundry/multiapps/controller/process/steps/RemoveNewApplicationsSuffixStep.java
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion ...-process/src/main/java/org/cloudfoundry/multiapps/controller/process/steps/StepsUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ static ApplicationCloudModelBuilder getApplicationCloudModelBuilder(ProcessConte
shouldApplyIncrementalInstancesUpdate(context));
}

static AppSuffixDeterminer getAppSuffixDeterminer(ProcessContext context) {
public static AppSuffixDeterminer getAppSuffixDeterminer(ProcessContext context) {
boolean keepOriginalNamesAfterDeploy = context.getVariable(Variables.KEEP_ORIGINAL_APP_NAMES_AFTER_DEPLOY);
boolean isAfterResumePhase = context.getVariable(Variables.PHASE) == Phase.AFTER_RESUME;
return new AppSuffixDeterminer(keepOriginalNamesAfterDeploy, isAfterResumePhase);
Expand Down
Loading
Loading