pipecd icon indicating copy to clipboard operation
pipecd copied to clipboard

Remove checking the resource key with the annotation because their generation logic are different

Open ffjlabo opened this issue 1 year ago • 3 comments

What this PR does / why we need it:

context: https://github.com/pipe-cd/pipecd/issues/4269#issuecomment-2031079364

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: https://github.com/pipe-cd/pipecd/pull/4858#issuecomment-2048982687

So I decided to remove the checking.

Which issue(s) this PR fixes:

Part of https://github.com/pipe-cd/pipecd/issues/4269 Previous PR https://github.com/pipe-cd/pipecd/pull/4858

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

ffjlabo avatar Apr 22 '24 09:04 ffjlabo

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 29.25%. Comparing base (42371a0) to head (92164f2).

Files Patch % Lines
...g/app/piped/platformprovider/kubernetes/applier.go 0.00% 1 Missing :warning:
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           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 22 '24 10:04 codecov[bot]

I checked the prune behavior like below.

cluster scoped resource

  • ✅ namespace exists in app.pipecd.yaml 
  • ✅ namespace doesn't exist in app.pipecd.yaml

namespace scoped resource

  • ✅ namespace exists in app.pipecd.yaml 
  • ✅ namespace doesn't exist in app.pipecd.yaml

ffjlabo avatar Apr 22 '24 11:04 ffjlabo

There is a problem about the resource key of cluster scoped resources. I'll consider it later to modify the effect scope.

ffjlabo avatar Apr 22 '24 11:04 ffjlabo

close this PR because I decided another solution for it. context: https://github.com/pipe-cd/pipecd/issues/4269#issuecomment-2104055928 follow the PR → https://github.com/pipe-cd/pipecd/pull/4916

ffjlabo avatar May 22 '24 04:05 ffjlabo