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

Running "cdk diff" returns error code 1 #5885

Closed
nicklaw5 opened this issue Jan 20, 2020 · 20 comments
Closed

Running "cdk diff" returns error code 1 #5885

nicklaw5 opened this issue Jan 20, 2020 · 20 comments
Assignees
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p2 package/tools Related to AWS CDK Tools or CLI

Comments

@nicklaw5
Copy link
Contributor

Despite the fix introduced in #4650. I'm still experiencing cdk diff exiting with status code 1.

Reproduction Steps

Run the following shell script:

#!/usr/bin/env bash

set -uo pipefail

cdk diff
EXIT_STATUS=$?
echo "Exit: $EXIT_STATUS"

Environment

  • CLI Version : 1.21.1 (build 842cc5f)
  • Framework Version: 1.21.1 (build 842cc5f)
  • OS : macOS Catalina 10.15.2
  • Language : Typescript

Other

Relates to #2111


This is 🐛 Bug Report

@nicklaw5 nicklaw5 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 20, 2020
@shivlaks
Copy link
Contributor

@nicklaw5 - can you provide some additional information?

what version of the CDK was your project created with? I'm trying to reproduce your error but can't seem to get there.

Steps I've tried:

  1. cdk init --language typescript
  2. cdk diff - this has a diff (Conditions) -> echo $? returns 0
  3. cdk deploy
  4. cdk diff - No differences -> echo $? returns 0
  5. cdk diff --fail -> echo $? returns 1

what does your cdk.context.json contain in it? does it have the "aws-cdk:enableDiffNoFail": "true" key?

@shivlaks shivlaks added needs-reproduction This issue needs reproduction. package/tools Related to AWS CDK Tools or CLI response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jan 21, 2020
@nicklaw5
Copy link
Contributor Author

@shivlaks Thanks for taking a look. I will put together a small repro tomorrow.

@SomayaB SomayaB removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jan 21, 2020
@nicklaw5
Copy link
Contributor Author

Doesn't appear I can reproduce using the sample app:

npm install -g aws-cdk
cdk version # 1.21.1 (build 842cc5f)
cdk init sample-app --language=typescript
cdk diff # exits with 0

This may having something to do with how we extend and wrap cdk.Stack with our own opinions. I'll see if I can figure it out. Feel free to close this in the mean time.

@nicklaw5
Copy link
Contributor Author

I've managed to create a repro: https://github.com/nicklaw5/aws-cdk-5885-repro. Please follow the steps in the readme to reproduce yourself.

@shivlaks shivlaks added p2 and removed needs-reproduction This issue needs reproduction. labels Jan 22, 2020
@shivlaks shivlaks added the effort/small Small work item – less than a day of effort label Feb 5, 2020
@udondan
Copy link
Contributor

udondan commented Apr 1, 2020

Confirming this, I'm experiencing the same problem.

$ cdk diff MyApp
Stack MyApp
Resources
[~] AWS::AutoScaling::LaunchConfiguration ASG-MyApp/LaunchConfig ASGMyAppLaunchConfig298A8983 replace
 └─ [~] ImageId (requires replacement)
     ├─ [-] ami-075138d2c6349b882
     └─ [+] ami-0516c16a5a46a3b17

$ echo $?
1

CDK Version: 1.31.0 (build 8f3ac79)

what does your cdk.context.json contain in it? does it have the "aws-cdk:enableDiffNoFail": "true" key?

No, my cdk.context.json does not contain this setting at all.

@norbinsh
Copy link

norbinsh commented Apr 4, 2020

I can also reproduce this from our CI environment (codebuild) running 1.22.0 (diff --fail supported since 1.19).

$? will return 1 instead of 0 in case of any diff, without the --fail flag.

@angusfz
Copy link

angusfz commented Apr 17, 2020

Same issue as @udondan
cdk version: 1.32.2 (build e19e206)

Resolved this issue by adding "aws-cdk:enableDiffNoFail": "true" in cdk.context.json.
But, according to the changelog , cdk diff should always exit with 0 ?

@xycodex
Copy link

xycodex commented Apr 17, 2020

+1 this is an issue for me as well, running cdk version 1.32.2, python 3.8.2, as well as python 3.8

@xycodex
Copy link

xycodex commented Apr 17, 2020

workaround @angusfz mentioned works for me locally, but not amenable for a cicd system since cdk.context.json isn't supposed to be checked into a vcs

@shivlaks
Copy link
Contributor

shivlaks commented Apr 17, 2020

@angusfz - as mentioned in the changelog, it's under a feature flag and not natively as it represents a change in behaviour that we introduced. It does seem like our CHANGELOG is missing the breaking change entry from the commit

@xycodex - runtime context can be set in a few locations. Your alternatives to committing it would include setting the key in cdk.json or providing it as an option to your command. cdk diff -c aws-cdk:enableDiffNoFail=true

@udondan
Copy link
Contributor

udondan commented Apr 17, 2020

OK, so you HAVE to set that manually in the context. I missed that, I assumed it is the default. Thanks for clarification @shivlaks

@shivlaks
Copy link
Contributor

@udondan Yes, we flipped it to being the default for projects initialized through cdk init from that version onwards. But since it represents a change in behaviour, projects prior to it or created outside of the CLI's cdk init would need to opt into it.

I also amended the changelog entry to include the details from the initial commit (#7401)

@xycodex
Copy link

xycodex commented Apr 17, 2020

ah, that all makes sense now. I did try adding the flag to cdk.json, but was wrong about the format.
https://docs.aws.amazon.com/cdk/latest/guide/featureflags.html helped me there, and now the flag works. =)

However, my cdk.json gets overwritten to just
{
"app": "python3 app.py"
}
every time I run something like cdk diff, for some reason, and the flag is removed.

@udondan
Copy link
Contributor

udondan commented Apr 17, 2020

Yeah I noticed this as well. I first added it to cdk.json. But when I run cdk diff, it automatically removes that key from the cdk.json and writes it into cdk.context.json. I guess that's alright in CI. Just will confuse you if you run it locally.

@shivlaks
Copy link
Contributor

shivlaks commented Apr 17, 2020

@xycodex @udondan - yep that's a bug (fix is being tracked in #7399)

@norbinsh
Copy link

norbinsh commented Apr 17, 2020

thanks for the update! added this to the cdk.json files and it works as expected through codebuild.

@eikeon
Copy link

eikeon commented Apr 23, 2020

Shouldn't the --fail option be removed if it doesn't actually work? Seems like it needs to be spelled cdk diff -c aws-cdk:enableDiffNoFail=true?

@metametadata
Copy link

+1. --fail is still mentioned in cdk help diff and doesn't seem to work. v1.60.0.

@shivlaks
Copy link
Contributor

@metametadata - does the context have aws-cdk:enableDiffNoFail set to true? the --fail flag only works when that context key is set (in ~/.cdk.json, cdk.json in your project or with -c aws-cdk:enableDiffNoFail=true)

The reason the option is not removed is because the context can be established in multiple ways. Perhaps the wording can still clarify that fail requires the context key aws-cdk:enableDiffNoFail

@metametadata
Copy link

Thank you for the explanation. I set no context in my cases. I agree that the wording should be improved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

No branches or pull requests

9 participants