initial pass of a whitelist that adds labels relevant for aggregation #2719
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
AjayTripathy
changed the title
|
@cliffcolvin could we get a review on this one? |
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| func TestWhitelist(t *testing.T) { |
There was a problem hiding this comment.
Any way we could get an assertion here that could fail? Hard to call it a "test" if it can't fail. (Or perhaps I am being dumb and am missing that bit?)
There was a problem hiding this comment.
ah, this is my bad. good catch, I'll add an assertion.
| UseLabelsWhitelist bool `json:"useLabelsWhitelist,omitempty"` | ||
| LabelsWhitelist map[string]bool `json:"labelsWhiteList,omitempty"` |
There was a problem hiding this comment.
Where does this get configured? How would it be used?
There was a problem hiding this comment.
This is passed in a metricsConfigs JSON file https://github.com/kubecost/cost-analyzer-helm-chart/blob/16f11a314f43bb604a5d99bf3c92801c8a374b96/cost-analyzer/values.yaml#L3290
There was a problem hiding this comment.
Would it be helpful to be explicit in the variable name that these only apply to pod labels?
| @@ -29,6 +37,40 @@ func (kpmc KubePodLabelsCollector) Describe(ch chan<- *prometheus.Desc) { | |||
| } | |||
| } | |||
|
|
|||
| func (kpmc *KubePodLabelsCollector) UpdateControllerSelectorsCache() { | |||
There was a problem hiding this comment.
Do we need anything Deployment-specific here? Or does the ReplicaSet case cover Deployments adequately? Looking over pkg/costmodel/allocation.go the only relevant metrics I see are:
- service_selector_labels
- deployment_match_labels
- statefulSet_match_labels
There was a problem hiding this comment.
replicasets covers deployments!
What does this PR change?
Does this PR relate to any other PRs?
How will this PR impact users?
How was this PR tested?
Does this PR require changes to documentation?
Have you labeled this PR and its corresponding Issue as "next release" if it should be part of the next OpenCost release? If not, why not?