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: improve migration from label to annotation tracking #7899
Conversation
374591f
to
8b2c75d
Compare
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.
Reviewed code, looking good. Please fix the compile issue in repository_test.go.
util/argo/resource_tracking.go
Outdated
err = argokube.SetAppInstanceLabel(config, labelKey, label) | ||
if err != nil { | ||
return err | ||
labels := live.GetLabels() |
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.
I think it will be better move this part to argokube module
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.
Done.
Thank you for helping solve an issue with the tracking method. I just added a minor comment. |
745f9f2
to
bb82e43
Compare
Thank you for review @mayzhang2000 and @pasha-codefresh ! PTAL |
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
bb82e43
to
b9dda1c
Compare
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.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #7899 +/- ##
==========================================
+ Coverage 41.44% 41.47% +0.02%
==========================================
Files 169 169
Lines 22310 22328 +18
==========================================
+ Hits 9247 9260 +13
- Misses 11741 11744 +3
- Partials 1322 1324 +2
Continue to review full report at Codecov.
|
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com
PR resolves two issues related to migration from label to annotation tracking method:
Normalize
method should not touch "target" resource since it is re-used during the syncing process.