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: Split resource deletion #1336

Merged
merged 29 commits into from
Mar 11, 2024
Merged

Conversation

ruanxin
Copy link
Contributor

@ruanxin ruanxin commented Feb 21, 2024

related issue: #1281

This PR address above issue by split diff resource during deletion, it trying to delete resources managed by module operator first, after those managed resources are deleted, then LM start to delete operator-related resources, e.g: ServiceAccount, Role, ClusterRole, RoleBinding, ClusterRoleBinding. to ensure the module operator not being affected during deletion.

@ruanxin ruanxin requested a review from a team as a code owner February 21, 2024 13:57
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 21, 2024
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch from 1bfb5c5 to 631dfc5 Compare February 21, 2024 13:58
@ruanxin ruanxin marked this pull request as draft February 21, 2024 14:22
@kyma-bot kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 21, 2024
@ruanxin ruanxin marked this pull request as ready for review February 27, 2024 08:14
@kyma-bot kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 27, 2024
@kyma-bot kyma-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 4, 2024
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch from 3b33d9f to 1ba56fe Compare March 4, 2024 17:37
@lindnerby lindnerby self-requested a review March 5, 2024 08:40
@lindnerby lindnerby self-assigned this Mar 5, 2024
Copy link
Member

@lindnerby lindnerby left a comment

Choose a reason for hiding this comment

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

This is a good opportunity to increase unit test coverage.
There is no internal/pkg/resources/cleanup_test.go. Let's add these and also lock this pkg in unit-test-coverage.yaml!
May be a bit of work and refactoring needed, and also concurrency involved internally, so not super trivial, but we agreed to increase the coverage.

pkg/testutils/manifest.go Outdated Show resolved Hide resolved
tests/e2e/kyma_metrics_test.go Show resolved Hide resolved
tests/e2e/kyma_metrics_test.go Show resolved Hide resolved
internal/pkg/resources/cleanup.go Outdated Show resolved Hide resolved
internal/pkg/resources/cleanup.go Show resolved Hide resolved
internal/pkg/resources/cleanup.go Outdated Show resolved Hide resolved
internal/pkg/resources/cleanup.go Show resolved Hide resolved
internal/pkg/resources/cleanup.go Outdated Show resolved Hide resolved
pkg/testutils/manifest.go Outdated Show resolved Hide resolved
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch 2 times, most recently from 9f908f3 to 458bbef Compare March 7, 2024 20:04
enhance e2e to test module deletion when managed CR get blocked
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch from 458bbef to 60aff34 Compare March 7, 2024 20:35
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch from 310ad2f to 01de87c Compare March 7, 2024 22:21
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch from 084ea36 to 6f572d3 Compare March 8, 2024 09:50
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch from 520c8a8 to fe9502a Compare March 8, 2024 12:19
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch from 1f19806 to 34ef28c Compare March 8, 2024 13:29
@ruanxin ruanxin force-pushed the blocking-deletion-mode branch from 34ef28c to c754ee9 Compare March 8, 2024 13:34
@kyma-bot kyma-bot added the lgtm Looks good to me! label Mar 11, 2024
@kyma-bot kyma-bot merged commit a0c4943 into kyma-project:main Mar 11, 2024
36 of 38 checks passed
@jeremyharisch jeremyharisch assigned ruanxin and unassigned lindnerby Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. lgtm Looks good to me! size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants