Skip to content

feat(cli): warn of non-existent stacks in cdk destroy#27921

Merged
mergify[bot] merged 36 commits intoaws:mainfrom
go-to-k:fix/destroy-not-exits
Mar 18, 2024
Merged

feat(cli): warn of non-existent stacks in cdk destroy#27921
mergify[bot] merged 36 commits intoaws:mainfrom
go-to-k:fix/destroy-not-exits

Conversation

@go-to-k
Copy link
Contributor

@go-to-k go-to-k commented Nov 10, 2023

This PR for cli is to warn if stacks with wrong cases (=not exist) specified in cdk destroy.

Closes #27179.


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

@github-actions github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p2 labels Nov 10, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team November 10, 2023 03:34
@github-actions github-actions bot added the star-contributor [Pilot] contributed between 25-49 PRs to the CDK label Nov 10, 2023
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@aws-cdk-automation aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Nov 10, 2023
@go-to-k
Copy link
Contributor Author

go-to-k commented Nov 10, 2023

Exemption Request: this should be covered with cli-integ tests. In my environment, the behavior was confirmed as expected.

@aws-cdk-automation aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Nov 10, 2023
@vinayak-kukreja vinayak-kukreja self-assigned this Nov 17, 2023
@vinayak-kukreja
Copy link
Contributor

Hey, thank you for opening a contribution with us.
Looks like @SankyRed and @TheRealAmazonKendra were taking a look at the original PR. Will reach out to them if they want to take a look.

@go-to-k
Copy link
Contributor Author

go-to-k commented Nov 17, 2023

@vinayak-kukreja Thanks. Glad to have them see that again.

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@go-to-k
Copy link
Contributor Author

go-to-k commented Dec 4, 2023

@vinayak-kukreja @SankyRed @TheRealAmazonKendra

If this state continues, this PR will be closed again. Could you please take a look at this?

@paulhcsun
Copy link
Contributor

Hi @go-to-k, I'm very sorry for not getting back to you on this, we were in a state of waiting for another team but this was on me for not noticing your comments. I'm running the CLI integ tests through the pipeline now and will update you with the results soon. After which I will delegate this review to someone with more knowledge on the CDK CLI.

@go-to-k
Copy link
Contributor Author

go-to-k commented Mar 15, 2024

@paulhcsun Thank you! I'm looking forward to hearing from you and the team after this!

@paulhcsun
Copy link
Contributor

paulhcsun commented Mar 18, 2024

Apologies @go-to-k, there were some additional runs queued in the pipeline before this one so I couldn't check the results in time. It looks like it was skipped for some reason though, I will retry running it now.

I just did a search and it looks like the last run on Jan 24th passed all tests. It should be fine to add the cli-integ-tested label but I'll wait for the next run to complete to be sure. Full pipeline takes ~3hrs to run so I will check in soon.

@paulhcsun paulhcsun added pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested and removed pr/needs-cli-test-run This PR needs CLI tests run against it. labels Mar 18, 2024
@paulhcsun
Copy link
Contributor

@go-to-k I've added the cli-integ-tested label as the most recent run of the pipeline has passed all checks. Seems like it was just flaky integ tests the last time around. Thanks for your patience.

@aws-cdk-automation aws-cdk-automation dismissed their stale review March 18, 2024 19:17

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@paulhcsun paulhcsun removed the pr/do-not-merge This PR should not be merged at this time. label Mar 18, 2024
@aws-cdk-automation
Copy link
Collaborator

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: ab639fa
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Mar 18, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit f0d1d67 into aws:main Mar 18, 2024
@go-to-k go-to-k deleted the fix/destroy-not-exits branch March 19, 2024 10:49
scanlonp added a commit that referenced this pull request Mar 22, 2024
ahammond pushed a commit to ahammond/aws-cdk that referenced this pull request Mar 26, 2024
This PR for cli is to warn if stacks with wrong cases (=not exist) specified in `cdk destroy`.

Closes aws#27179.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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 pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested pr-linter/do-not-close The PR linter will not close this PR while this label is present pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. star-contributor [Pilot] contributed between 25-49 PRs to the CDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No exception when stack with wrong cases is destroyed

5 participants