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

feat(aws-codedeploy): add auto rollback configuration to server Deployment Group #925

Merged
merged 1 commit into from
Oct 18, 2018

Conversation

skinny85
Copy link
Contributor

Continuing on covering the entire CodeDeploy service area in the L2.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

@skinny85 skinny85 requested a review from eladb October 13, 2018 00:01
/**
* The configuration for automatically rolling back deployments in a given Deployment Group.
*/
export interface IAutoRollbackConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

This interface shouldn't have a an I prefix

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.

*
* @default true
*/
deploymentInAlarm?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

I am assuming this requires that alarms are associated with the deployment group. Is there any meaning for this without alarms? Should we validate?

Choose a reason for hiding this comment

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

Good point on validation - by default can we set this to false if customers create a deployment group without any cloudWatch alarm and set this true when they create/update deployment with CloudWatch alarms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea. I had to change the order the PRs have been submitted in to merge the alarms one (#926) before this one, but thankfully that one was approved already.

I've rebased this PR on top of the alarms one, added validation, and changed the default to depend on whether any Alarms were provided.

: undefined;
}

private optionalBoolDefaultTrue(value?: boolean): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

Extract as a free-floating function instead of a method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method actually got completely removed in the latest iteration.

@@ -48,6 +48,14 @@
"Arn"
]
},
"AutoRollbackConfiguration": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you specify false to all three? Why are they all enabled?

Also, add a unit test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you specify false to all three? Why are they all enabled?

Those are actually 2 different tests :) I've specified false for all 3 in the integ.deployment-group, while this is integ.pipeline-code-deploy.

Also, add a unit test

Done.

// auto-rollback configuration
autoRollback: {
failedDeployment: true, // default: true,
stoppedDeployment: true, // default: true,

Choose a reason for hiding this comment

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

The default value for stoppedDeployment should be "false" probably (I'm open on this but feel it's more nature).

stop deployment request is issued by customers, and they will have a way to specify whether they want to rollback as part of the stopDeployment request, so I feel it might be good enough to just by default rollback for failedDeployment and deployment stopped due to CloudWatchAlarm on fire.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a problem with making stoppedDeployment false by default.

Changed.

*
* @default true
*/
deploymentInAlarm?: boolean;

Choose a reason for hiding this comment

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

Good point on validation - by default can we set this to false if customers create a deployment group without any cloudWatch alarm and set this true when they create/update deployment with CloudWatch alarms?

@skinny85
Copy link
Contributor Author

Updated with Elad's and Bangxi's feedback.

@skinny85 skinny85 merged commit 7ee91cf into aws:master Oct 18, 2018
@skinny85 skinny85 deleted the feature/codedeploy-rollback branch October 18, 2018 00:22
eladb pushed a commit that referenced this pull request Oct 19, 2018
### Highlights

 - __A new construct library for AWS Step Functions__
   ([docs](https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-stepfunctions/README.md)).
   The library provides rich APIs for modeling state machines by exposing a
   programmatic interface for [Amazon State
   Language](https://docs.aws.amazon.com/step-functions/latest/dg/concepts-amazon-states-language.html).
 - __A new construct library for Amazon S3 bucket deployments__
   ([docs](https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-s3-deployment/README.md)).
   You can use now automatically populate an S3 Bucket from a .zip file or a
   local directory. This is a building block for end-to-end support for static
   websites in the AWS CDK.

### Bug Fixes

* **aws-apigateway:** make LambdaRestApi proxy by default ([#963](#963)) ([a5f5e2c](a5f5e2c)), closes [#959](#959)
* **aws-cdk:** Allow use of assumed roles behind a proxy ([#898](#898)) ([f2b1048](f2b1048))
* **aws-cdk:** Auto-delete stacks that failed creating before new attempt ([#917](#917)) ([2af8309](2af8309))
* **aws-cloudfront:** expose distributionId ([#938](#938)) ([f58d98c](f58d98c))
* **aws-dynamodb:** don't emit empty array properties ([#909](#909)) ([841975a](841975a))
* **docs:** use ..code to display file structure in "writing constructs" ([#935](#935)) ([b743362](b743362))

### Features

* **assets:** isZipArchive indicates if this is a zip asset ([#944](#944)) ([65190f9](65190f9))
* **aws-cdk:** deploy supports CloudFormation Role ([#940](#940)) ([393be6f](393be6f)), closes [#735](#735)
* **aws-cloudformation:** allow specifying custom resource type ([#943](#943)) ([9de3a84](9de3a84))
* **aws-cloudformation:** correctly handle the templateConfiguration property in the CreateUpdateStack Pipeline Action. ([#923](#923)) ([d251a46](d251a46))
* **aws-cloudfront:** add support for "webAclId" ([#969](#969)) ([3ec9d76](3ec9d76))
* **aws-codedeploy:** add auto rollback configuration to server Deployment Group. ([#925](#925)) ([7ee91cf](7ee91cf))
* **aws-codedeploy:** add instance tag filter support for server Deployment Groups. ([#824](#824)) ([e6e8c51](e6e8c51))
* **aws-codedeploy:** add support for setting CloudWatch alarms on a server Deployment Group. ([#926](#926)) ([27b26b1](27b26b1))
* add support for Step Functions ([#827](#827)) ([81b533c](81b533c))
* **aws-lambda:** add grantInvoke() method ([#962](#962)) ([1ee8135](1ee8135)), closes [#961](#961)
* **aws-lambda:** improvements to the code and runtime APIs ([#945](#945)) ([36f29b6](36f29b6)), closes [#902](#902) [#188](#188) [#947](#947) [#947](#947) [#664](#664)
* **aws-logs:** extractMetric() returns Metric object ([#939](#939)) ([5558fff](5558fff)), closes [#850](#850)
* **aws-s3:** initial support for website hosting ([#946](#946)) ([2d3661c](2d3661c))
* **aws-s3-deployment:** bucket deployments ([#971](#971)) ([84d6876](84d6876)), closes [#952](#952) [#953](#953) [#954](#954)
* **docs:** added link to CloudFormation concepts ([#934](#934)) ([666bbba](666bbba))

### BREAKING CHANGES

* **aws-apigateway:** specifying a path no longer works. If you used to
provide a '/', remove it. Otherwise, you will have to supply `proxy: false`
and construct more complex resource paths yourself.
* **aws-lambda:** The construct `lambda.InlineJavaScriptLambda` is no longer supported. Use `lambda.Code.inline` instead; `lambda.Runtime.NodeJS43Edge` runtime is removed. CloudFront docs [stipulate](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/lambda-requirements-limits.html#lambda-requirements-lambda-function-configuration) that you should use node6.10 or node8.10. It is always possible to use any value by instantiating a `lambda.Runtime` object.
@eladb eladb mentioned this pull request Oct 19, 2018
eladb pushed a commit that referenced this pull request Oct 19, 2018
### Highlights

 - __A new construct library for AWS Step Functions__
   ([docs](https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-stepfunctions/README.md)).
   The library provides rich APIs for modeling state machines by exposing a
   programmatic interface for [Amazon State
   Language](https://docs.aws.amazon.com/step-functions/latest/dg/concepts-amazon-states-language.html).
 - __A new construct library for Amazon S3 bucket deployments__
   ([docs](https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-s3-deployment/README.md)).
   You can use now automatically populate an S3 Bucket from a .zip file or a
   local directory. This is a building block for end-to-end support for static
   websites in the AWS CDK.

### Bug Fixes

* **aws-apigateway:** make LambdaRestApi proxy by default ([#963](#963)) ([a5f5e2c](a5f5e2c)), closes [#959](#959)
* **aws-cdk:** Allow use of assumed roles behind a proxy ([#898](#898)) ([f2b1048](f2b1048))
* **aws-cdk:** Auto-delete stacks that failed creating before new attempt ([#917](#917)) ([2af8309](2af8309))
* **aws-cloudfront:** expose distributionId ([#938](#938)) ([f58d98c](f58d98c))
* **aws-dynamodb:** don't emit empty array properties ([#909](#909)) ([841975a](841975a))
* **docs:** use ..code to display file structure in "writing constructs" ([#935](#935)) ([b743362](b743362))

### Features

* **assets:** isZipArchive indicates if this is a zip asset ([#944](#944)) ([65190f9](65190f9))
* **aws-cdk:** deploy supports CloudFormation Role ([#940](#940)) ([393be6f](393be6f)), closes [#735](#735)
* **aws-cloudformation:** allow specifying custom resource type ([#943](#943)) ([9de3a84](9de3a84))
* **aws-cloudformation:** correctly handle the templateConfiguration property in the CreateUpdateStack Pipeline Action. ([#923](#923)) ([d251a46](d251a46))
* **aws-cloudfront:** add support for "webAclId" ([#969](#969)) ([3ec9d76](3ec9d76))
* **aws-codedeploy:** add auto rollback configuration to server Deployment Group. ([#925](#925)) ([7ee91cf](7ee91cf))
* **aws-codedeploy:** add instance tag filter support for server Deployment Groups. ([#824](#824)) ([e6e8c51](e6e8c51))
* **aws-codedeploy:** add support for setting CloudWatch alarms on a server Deployment Group. ([#926](#926)) ([27b26b1](27b26b1))
* add support for Step Functions ([#827](#827)) ([81b533c](81b533c))
* **aws-lambda:** add grantInvoke() method ([#962](#962)) ([1ee8135](1ee8135)), closes [#961](#961)
* **aws-lambda:** improvements to the code and runtime APIs ([#945](#945)) ([36f29b6](36f29b6)), closes [#902](#902) [#188](#188) [#947](#947) [#947](#947) [#664](#664)
* **aws-logs:** extractMetric() returns Metric object ([#939](#939)) ([5558fff](5558fff)), closes [#850](#850)
* **aws-s3:** initial support for website hosting ([#946](#946)) ([2d3661c](2d3661c))
* **aws-s3-deployment:** bucket deployments ([#971](#971)) ([84d6876](84d6876)), closes [#952](#952) [#953](#953) [#954](#954)
* **docs:** added link to CloudFormation concepts ([#934](#934)) ([666bbba](666bbba))

### BREAKING CHANGES

* **aws-apigateway:** specifying a path no longer works. If you used to
provide a '/', remove it. Otherwise, you will have to supply `proxy: false`
and construct more complex resource paths yourself.
* **aws-lambda:** The construct `lambda.InlineJavaScriptLambda` is no longer supported. Use `lambda.Code.inline` instead; `lambda.Runtime.NodeJS43Edge` runtime is removed. CloudFront docs [stipulate](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/lambda-requirements-limits.html#lambda-requirements-lambda-function-configuration) that you should use node6.10 or node8.10. It is always possible to use any value by instantiating a `lambda.Runtime` object.
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
eladb pushed a commit that referenced this pull request Nov 19, 2019
Bug Fixes
- java: handle null-able collections correctly (#986) (e88e5e2), closes #4316
- jsii: unable to depend on modules with private declarations (#995) (08c4294), closes #994
- kernel: cannot pass decorated structs to kernel as "any" (#997) (2bd3183), closes #5066

Features
- jsii-config: introducing jsii-config (#981) (2bbf576), closes #904
- rosetta: extract and compile samples into "tablets" (#925) (eec44e1)
mergify bot pushed a commit that referenced this pull request Nov 19, 2019
* feat: upgrade to jsii 0.20.7

Bug Fixes
- java: handle null-able collections correctly (#986) (e88e5e2), closes #4316
- jsii: unable to depend on modules with private declarations (#995) (08c4294), closes #994
- kernel: cannot pass decorated structs to kernel as "any" (#997) (2bd3183), closes #5066

Features
- jsii-config: introducing jsii-config (#981) (2bbf576), closes #904
- rosetta: extract and compile samples into "tablets" (#925) (eec44e1)

* fix broken code

* type a couple of more arrays

* fix another untyped array

* fix a couple more issues

* another untyped array

* more
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants