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

Enhance fallback to noop logic for apps to account for multiple namespaces #1394

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

100mik
Copy link
Contributor

@100mik 100mik commented Nov 8, 2023

What this PR does / why we need it:

It ensures that Apps and PackageInstalls do not block namespace deletion when the app can safely fallback to noopDelete

Which issue(s) this PR fixes:

Fixes #1392

Review Checklist:
  • Follows the developer guidelines
  • Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

@100mik 100mik force-pushed the fallback-to-noop-multi-ns branch 2 times, most recently from 7ecc8a9 to 0efe841 Compare November 9, 2023 00:03
Copy link
Member

@praveenrewar praveenrewar left a comment

Choose a reason for hiding this comment

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

🚀

pkg/app/app.go Outdated Show resolved Hide resolved
pkg/app/app.go Outdated Show resolved Hide resolved
pkg/app/app.go Outdated Show resolved Hide resolved
@100mik 100mik force-pushed the fallback-to-noop-multi-ns branch 2 times, most recently from 0b643f8 to 49f8af4 Compare November 9, 2023 22:34
…paces

Signed-off-by: Soumik Majumder <soumikm@vmware.com>
Signed-off-by: Soumik Majumder <soumikm@vmware.com>
@100mik 100mik marked this pull request as ready for review November 9, 2023 23:03
Copy link
Member

@praveenrewar praveenrewar left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

LGTM

@praveenrewar praveenrewar merged commit 934d244 into carvel-dev:develop Nov 15, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Unblock app deletion by falling back to noop when safe
3 participants