-
Notifications
You must be signed in to change notification settings - Fork 135
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
Remove checking the resource key with the annotation because their generation logic are different #4891
Conversation
…neration logic are different Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4891 +/- ##
=======================================
Coverage 29.24% 29.25%
=======================================
Files 318 318
Lines 40597 40593 -4
=======================================
Hits 11874 11874
+ Misses 27784 27780 -4
Partials 939 939 ☔ View full report in Codecov by Sentry. |
I checked the prune behavior like below. cluster scoped resource
namespace scoped resource
|
There is a problem about the resource key of cluster scoped resources. I'll consider it later to modify the effect scope. |
@@ -147,21 +146,17 @@ func (a *applier) Delete(ctx context.Context, k ResourceKey) (err error) { | |||
return a.initErr | |||
} | |||
|
|||
m, err := a.kubectl.Get( | |||
// Check the resource exists or not using the resource key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Check the resource exists or not using the resource key. | |
// Check whether the resource exists or not using the resource key. |
close this PR because I decided another solution for it. |
What this PR does / why we need it:
context: #4269 (comment)
Removed checking the resource key with the annotation because their generation logic differs.
Previously, I tried to keep using the annotation
pipecd.dev/resource-key
, but found that some k8s resources inherit the parent's annotation and can't be attached correct resource key.ref: #4858 (comment)
So I decided to remove the checking.
Which issue(s) this PR fixes:
Part of #4269
Previous PR #4858
Does this PR introduce a user-facing change?: