-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: include reason label to kube_deployment_status_condition
#2719
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
Changes from 2 commits
0a50cab
b30335f
bc1e7f8
b582038
547afad
012b46d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,14 @@ var ( | |
descDeploymentLabelsDefaultLabels = []string{"namespace", "deployment"} | ||
) | ||
|
||
var ( | ||
allowedDeploymentReasons = map[string]struct{}{ | ||
"MinimumReplicasAvailable": {}, | ||
"NewReplicaSetAvailable": {}, | ||
"FailedCreate": {}, | ||
} | ||
) | ||
|
||
func deploymentMetricFamilies(allowAnnotationsList, allowLabelsList []string) []generator.FamilyGenerator { | ||
return []generator.FamilyGenerator{ | ||
*generator.NewFamilyGeneratorWithStability( | ||
|
@@ -174,8 +182,13 @@ func deploymentMetricFamilies(allowAnnotationsList, allowLabelsList []string) [] | |
for j, m := range conditionMetrics { | ||
metric := m | ||
|
||
metric.LabelKeys = []string{"condition", "status"} | ||
metric.LabelValues = append([]string{string(c.Type)}, metric.LabelValues...) | ||
reason := c.Reason | ||
if _, ok := allowedDeploymentReasons[reason]; !ok { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if it's empty, will it still be unknown? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah that was the case earlier, fixed it if its empty then it will remain empty and will show unknown if reason is not in the allowed list, also added the tests for both cases. |
||
reason = "unknown" | ||
} | ||
|
||
metric.LabelKeys = []string{"reason", "condition", "status"} | ||
metric.LabelValues = append([]string{reason, string(c.Type)}, metric.LabelValues...) | ||
ms[i*len(conditionStatuses)+j] = metric | ||
} | ||
} | ||
|
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.
Looking at these strings again
https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/deployment/util/deployment_util.go#L67
Can we refer to these here instead and create a complete list based on what's available in deployment_util.go?
Uh oh!
There was an error while loading. Please reload this page.
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.
@mrueg, When I'm trying to:
I'm getting this:
Is this a good idea to use internal k8s packages directly? should I directly use strings?
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.
Good point, you're right!
Can you copy all other reasons over as a string and add a comment that they are taken from deployment_utils.go?
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.
@mrueg thanks for the review, made the changes