Skip to content

Conversation

@TGPSKI
Copy link
Contributor

@TGPSKI TGPSKI commented Dec 4, 2021

This MR fixes bug DVO-31: Deleted Namespace and Kind still show in DVO grafana / prometheus.

Reconcile watches objects and triggers validations.RunValidations(), where DVO output is shipped to DVO's /metrics endpoint. Delete metric labels operations are also handled in RunValidations().

On deletion, gvk.Kind is an empty string for all resource types besides "Pod". This string is passed into RunValidations and is used to construct the prometheus labels for the associated metric.

When a deletion event is passed to RunValidations for non-pod resources (Deployment, ReplicaSet), promLabels is missing the kind field, and as such can't associate the provided deletion event with the existing metrics. For an example nginx deployment, the prom labels do not exist / will return multiple labels.

promLabels := getPromLabels(
	request.Namespace,
	request.Name,
	kind,
)

promLabels := getPromLabels(
	'check',
	'nginx'
	'',
)

Passing gr.reconciledKind into RunValidations instead of gvk.Kind fixes this problem. Deployment and ReplicaSet kinds are passed to the validation method, and the promLabels are found and deleted.

@TGPSKI TGPSKI requested a review from rrati December 4, 2021 00:19
@openshift-ci openshift-ci bot requested review from BumbleFeng and jmelis December 4, 2021 00:19
@codecov-commenter
Copy link

Codecov Report

Merging #124 (ad51dc5) into master (83bacc7) will decrease coverage by 0.16%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #124      +/-   ##
==========================================
- Coverage   36.70%   36.54%   -0.17%     
==========================================
  Files          15       15              
  Lines         395      394       -1     
==========================================
- Hits          145      144       -1     
  Misses        230      230              
  Partials       20       20              
Impacted Files Coverage Δ
pkg/controller/generic_reconciler.go 29.41% <100.00%> (-2.02%) ⬇️

Copy link

@rrati rrati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Dec 6, 2021
@openshift-ci
Copy link

openshift-ci bot commented Dec 6, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rrati, TGPSKI

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Dec 6, 2021
@openshift-merge-robot openshift-merge-robot merged commit 19cecb4 into app-sre:master Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants