Skip to content
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

fix(cli): no longer disable rollback by default for hotswap deployments #17317

Merged
merged 2 commits into from Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions packages/aws-cdk/bin/cdk.ts
Expand Up @@ -105,10 +105,13 @@ async function parseCommandLineArguments() {
.option('outputs-file', { type: 'string', alias: 'O', desc: 'Path to file where stack outputs will be written as JSON', requiresArg: true })
.option('previous-parameters', { type: 'boolean', default: true, desc: 'Use previous values for existing parameters (you must specify all parameters on every deployment if this is disabled)' })
.option('progress', { type: 'string', choices: [StackActivityProgress.BAR, StackActivityProgress.EVENTS], desc: 'Display mode for stack activity events' })
.option('rollback', { type: 'boolean', desc: 'Rollback stack to stable state on failure (iterate more rapidly with --no-rollback or -R)' })
.option('rollback', {
type: 'boolean',
desc: "Rollback stack to stable state on failure. Defaults to 'true', iterate more rapidly with --no-rollback or -R. " +
'Note: do **not** disable this flag for deployments with resource replacements, as that will always fail',
})
// Hack to get '-R' as an alias for '--no-rollback', suggested by: https://github.com/yargs/yargs/issues/1729
.option('R', { type: 'boolean', hidden: true })
.middleware(yargsNegativeAlias('R', 'rollback'), true)
.option('R', { type: 'boolean', hidden: true }).middleware(yargsNegativeAlias('R', 'rollback'), true)
.option('hotswap', {
type: 'boolean',
desc: "Attempts to perform a 'hotswap' deployment, " +
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/api/deploy-stack.ts
Expand Up @@ -317,7 +317,7 @@ async function prepareAndExecuteChangeSet(
if (execute) {
debug('Initiating execution of changeset %s on stack %s', changeSet.Id, deployName);

const shouldDisableRollback = (options.rollback === undefined && options.hotswap === true) || options.rollback === false;
const shouldDisableRollback = options.rollback === false;
// Do a bit of contortions to only pass the `DisableRollback` flag if it's true. That way,
// CloudFormation won't balk at the unrecognized option in regions where the feature is not available yet.
const disableRollback = shouldDisableRollback ? { DisableRollback: true } : undefined;
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Expand Up @@ -650,6 +650,7 @@ export interface DeployOptions {
* @default true
*/
readonly rollback?: boolean;

/*
* Whether to perform a 'hotswap' deployment.
* A 'hotswap' deployment will attempt to short-circuit CloudFormation
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/test/api/deploy-stack.test.ts
Expand Up @@ -98,7 +98,7 @@ test("does not call tryHotswapDeployment() if 'hotswap' is false", async () => {
expect(tryHotswapDeployment).not.toHaveBeenCalled();
});

test("rollback defaults to disabled if 'hotswap' is true", async () => {
test("rollback still defaults to enabled even if 'hotswap' is enabled", async () => {
// WHEN
await deployStack({
...standardDeployStackArguments(),
Expand All @@ -107,7 +107,7 @@ test("rollback defaults to disabled if 'hotswap' is true", async () => {
});

// THEN
expect(cfnMocks.executeChangeSet).toHaveBeenCalledWith(expect.objectContaining({
expect(cfnMocks.executeChangeSet).not.toHaveBeenCalledWith(expect.objectContaining({
DisableRollback: true,
}));
});
Expand Down