Skip to content

Conversation

ikasarov
Copy link
Contributor

during blue-green deployment, move the idle application renaming before the restage (in CreateOrUpdateAppStep) - ensuring the new name is applied before the restart/restage
fix ArchiveEntryExtractorTest incorrect handling of newline leading to windows errors

See: LMCROSSITXSADEPLOY-2755

@s-yonkov-yonkov s-yonkov-yonkov self-requested a review September 12, 2025 08:45
public void handleApplicationName() {
if (!context.getVariable(Variables.KEEP_ORIGINAL_APP_NAMES_AFTER_DEPLOY)) {
getStepLogger().warn(
Variables.KEEP_ORIGINAL_APP_NAMES_AFTER_DEPLOY + " is set to false. The application name will not be updated.");
Copy link
Contributor

Choose a reason for hiding this comment

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

If KEEP_ORIGINAL_APP_NAMES_AFTER_DEPLOY is false, why The application name will not be updated? I presume that if the original name should not be kept, the app name will be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KEEP_ORIGINAL_APP_NAMES_AFTER_DEPLOY is a flag that determines the use of suffixes during the bg deploy - if true we use the temporary '-idle' suffix, if false we use blue and green colour suffixes and don't rename apps but rotate the names; this check needs to be extended however

public void handleApplicationName() {
if (!context.getVariable(Variables.KEEP_ORIGINAL_APP_NAMES_AFTER_DEPLOY)) {
getStepLogger().warn(
Variables.KEEP_ORIGINAL_APP_NAMES_AFTER_DEPLOY + " is set to false. The application name will not be updated.");
Copy link
Contributor

Choose a reason for hiding this comment

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

You may extract the string to follow the class style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

log was unnecessary - removed


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

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

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

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?

getStepLogger().info(Messages.THE_DETECTED_APPLICATION_HAS_THE_SAME_NAME_AS_THE_NEW_ONE);
return;
}
getStepLogger().warn("Renaming application " + oldName + " to " + newName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why two loggings? The first one is using string concat, the second one is using const but with the same value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

}

@Override
public void handleApplicationName() {
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.

Misleading name, except the renaming it is performing ConfigurationSubscription updates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the subscriptionas are downstream from the appName, feel like it's more important

int currentInstances = 1; // default instances when creating an app
if (existingApp != null) {
currentInstances = client.getApplicationProcess(client.getApplicationGuid(existingApp.getName()))
currentInstances = client.getApplicationProcess(client.getApplicationGuid(app.getName()))
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

was a bad solution to an issue with the changed app name - now the existingApp is overwritten in the context once the app name is updated

$ref: "#/definitions/Info"
security:
- oauth2: []
/api/v1/spaces/{spaceGuid}/files:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not delete this. Must be checked why always deleted after build

flow: "password"
scopes: {}
definitions:
AsyncUploadResult:
Copy link
Contributor

Choose a reason for hiding this comment

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

same

.shouldKeepExistingEnv() ? UpdateStrategy.MERGE : UpdateStrategy.REPLACE;
}

@Override
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating a new method and flow, can you create a new ApplicationAttributeUpdater. For example, ApplicationNameApplicationAttributeUpdater and update the application name in handleApplicationAttributes (Update flow)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decided on in-between solution - removing the new method but not implementing an ApplicationAttributeUpdater for a specific reason - the attribute updaters are used in a stream but are all dependant on the application name which the new updater would be changing. This makes the order of the attributeUpdaters very important and would be easy to break in the future.


@Override
public void handleApplicationName() {
boolean processIsBlueGreenWithIdleSuffix = StepsUtil.getAppSuffixDeterminer(context)
Copy link
Contributor

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

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


String oldName = existingApp.getName();
String newName = BlueGreenApplicationNameSuffix.removeSuffix(oldName);
if (oldName.equals(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.

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

}

getStepLogger().info(Messages.RENAMING_APPLICATION_0_TO_1, oldName, newName);
client.rename(oldName, newName);
Copy link
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 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

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.

Comment on lines 204 to 205
public void handleApplicationName() {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If you agree with my other comment this method will not be needed. If you do not, add some comment why there is no implementation here

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

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants