-
Notifications
You must be signed in to change notification settings - Fork 867
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
feat(analysis): Adds rollout Spec.Selector.MatchLabels to AnalysisRun. Fixes #2888 #2903
Conversation
Signed-off-by: Gerald Nunn <[email protected]>
…llouts into analysisrun-label
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2903 +/- ##
=======================================
Coverage 81.74% 81.74%
=======================================
Files 134 134
Lines 20367 20370 +3
=======================================
+ Hits 16649 16652 +3
Misses 2861 2861
Partials 857 857
☔ View full report in Codecov by Sentry. |
@@ -460,6 +460,11 @@ func (c *rolloutContext) newAnalysisRunFromRollout(rolloutAnalysis *v1alpha1.Rol | |||
for k, v := range rolloutAnalysis.AnalysisRunMetadata.Labels { | |||
run.Labels[k] = v | |||
} | |||
|
|||
for k, v := range c.rollout.Spec.Selector.MatchLabels { |
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 taking the labels on the rollout object vs the MatchLabels probably make more sense and follows what we do with ReplicaSets, with RS we take the pod template labels and put them on the RS as labels.
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.
Thanks @zachaller, I'll have a look at the RS code and follow the same practice.
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.
Sorry for the delay, as requested updated to use template labels.
Signed-off-by: zachaller <[email protected]>
Signed-off-by: zachaller <[email protected]>
Signed-off-by: zachaller <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
fixes: #2888
Checklist:
"fix(controller): Updates such and such. Fixes #1234"
.This is a small change to add the rollout matchLabels to the AnalysisRun when created as per the linked issue.
Note that I am not a golang developer, even though it's a tiny chnage that I've tested locally it's possible I've missed things so feel free to give me feedback and I'm happy to modify as needed. I'm also not sure how/were a unit test for this would need to be created as I'm not seeing existing tests for labels in analysis_test.go that I could use as a reference, happy to take feedback on that as well if one is needed.