-
Notifications
You must be signed in to change notification settings - Fork 42
Add dependency aware stop order flag #253
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?
Conversation
e8f6150 to
1408db7
Compare
LMCROSSITXSADEPLOY-3367
1408db7 to
bb4fde3
Compare
| util.GetShortOption(taskExecutionTimeoutOpt): "Task execution timeout in seconds", | ||
| util.CombineFullAndShortParameters(startTimeoutOpt, timeoutOpt): "Start app timeout in seconds", | ||
| util.GetShortOption(shouldBackupPreviousVersionOpt): "(EXPERIMENTAL) (STRATEGY: BLUE-GREEN, INCREMENTAL-BLUE-GREEN) Backup previous version of applications, use new cli command \"rollback-mta\" to rollback to the previous version", | ||
| util.GetShortOption(dependencyAwareStopOrderOpt): "(STRATEGY: BLUE-GREEN, INCREMENTAL-BLUE-GREEN) Stop apps in a dependency-aware order during the resume phase of a blue-green deployment", |
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.
How is it guaranteed that this new flag is used only with the blue-green and incremental-blue-green strategies? Should we add validation for that and maybe a fallback for the other strategies when its wrongly used?
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.
I've tested with other strategy-specific parameters and the flag seems to be accepted and ignored for the operation that does not support it. In which case I think its ok to leave it without a strict failure for consistency.
I've moved the processBuilder call to deployment_strategy.go for better structural clarity and added relevant tests.
LMCROSSITXSADEPLOY-3367
Description:
Issue: