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): add --force-remove-finalizers to platform destroy #2317

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

rohantmp
Copy link
Contributor

@rohantmp rohantmp commented Dec 5, 2024

What issue type does this pull request address? (keep at least one, remove the others)
/kind enhancement

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
resolves ENG-5245
resolves ENG-5264

Please provide a short message that should be published in the vcluster release notes

What else do we need to know?

Adds prompt for this flag, also changes logging to mention that externally deployed vclusters are not cleaned up.

Copy link

netlify bot commented Dec 5, 2024

Deploy Preview for vcluster-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit bf49270
🔍 Latest deploy log https://app.netlify.com/sites/vcluster-docs/deploys/6762f5abbfb9ca000824a739

@rmweir rmweir self-requested a review December 5, 2024 15:03
Copy link
Contributor

@rmweir rmweir left a comment

Choose a reason for hiding this comment

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

I have additional thoughts about cleanup of "external" virtual clusters that I am asking about and will finish reviewing once those are resolved.

pkg/cli/destroy/destroy.go Outdated Show resolved Hide resolved
@rmweir rmweir self-requested a review December 6, 2024 00:57
pkg/cli/destroy/destroy.go Outdated Show resolved Hide resolved
@rohantmp
Copy link
Contributor Author

rohantmp commented Dec 6, 2024

With changes:
image

image

@rohantmp rohantmp requested a review from rmweir December 6, 2024 09:41
Copy link
Contributor

@rmweir rmweir left a comment

Choose a reason for hiding this comment

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

I think the we should clarify that removing the database will render the virtual cluster in an irreparable bad state despite the deployment still existing. Otherwise, LGTM great job!

pkg/cli/destroy/destroy.go Outdated Show resolved Hide resolved
@cbron cbron requested a review from rmweir December 18, 2024 16:18
Copy link
Contributor

@rmweir rmweir left a comment

Choose a reason for hiding this comment

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

LGTM

@cbron cbron merged commit 5933283 into loft-sh:main Dec 18, 2024
61 checks passed
@loft-bot
Copy link

💚 All backports created successfully

Status Branch Result
v0.22

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

2 similar comments
@loft-bot
Copy link

💚 All backports created successfully

Status Branch Result
v0.22

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@loft-bot
Copy link

💚 All backports created successfully

Status Branch Result
v0.22

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants