-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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: handle apiGroup updates in resource-tracking #11012
Conversation
6e20a06
to
b95c221
Compare
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
b95c221
to
d72707b
Compare
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Codecov ReportBase: 45.60% // Head: 45.61% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #11012 +/- ##
==========================================
+ Coverage 45.60% 45.61% +0.01%
==========================================
Files 237 237
Lines 28914 28920 +6
==========================================
+ Hits 13185 13191 +6
Misses 13913 13913
Partials 1816 1816
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@leoluz how far back should this be cherry-picked? |
@crenshaw-dev I would say 2.4 branch |
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.
Code looks good! Feel free to take or leave the comment suggestions. 😄
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
* fix: handle apiGroup updates in resource-tracking Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * Fix test Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * change the fix approach by inspecting tracking id from the config Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * add unit-test to validate the scenario Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * fix test lint Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * review fixes Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * Reword godocs for clarity Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
* fix: handle apiGroup updates in resource-tracking Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * Fix test Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * change the fix approach by inspecting tracking id from the config Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * add unit-test to validate the scenario Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * fix test lint Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * review fixes Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * Reword godocs for clarity Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Cherry-picked onto release-2.5 for 2.5.2 and release-2.4 for 2.4.17. |
* fix: handle apiGroup updates in resource-tracking Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * Fix test Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * change the fix approach by inspecting tracking id from the config Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * add unit-test to validate the scenario Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * fix test lint Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * review fixes Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> * Reword godocs for clarity Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
We noticed a strange behaviour in Argo CD 2.4.11 when users upgrade to a newer APIGroup (e.g. going from
extensions/Ingress
tonetworking.k8s.io/Ingress
). In this case the resource sync status displays asunknown
and Argo CD is unable to manage the resource anymore even if it is present in the source (git).The issue is caused because the trackingId annotation (
argocd.argoproj.io/tracking-id
) changes like in the example below:ingress-app:extensions/Ingress:default/some-ingress
ingress-app:networking.k8s.io/Ingress:default/some-ingress
This is related to the code introduced in #9791 and #10198 while addressing the issue #8683. In this case Argo CD displays the Ingress resource but stops managing it, skipping all future change in the desired state (git).
Maybe #8683 needs to be re-assessed to find a different solution not relying on the APIGroup for comparison.
Another possible fix is stop verifying the APIGroup in
isSelfReferencedObj
method which would make it simpler and work even when SSA is enabled. However I'd like to get @jannfis or @jessesuen opinion first before going in this direction to make sure #9791 is still covered in this case.Signed-off-by: Leonardo Luz Almeida leonardo_almeida@intuit.com
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: