Skip to content
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

MGDAPI-5082 - fix issue with metrics creation #675

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

adam-cattermole
Copy link
Member

@adam-cattermole adam-cattermole commented Mar 28, 2023

Overview

MGDAPI-5082

What

Discovered an issue in v1.0.0 - metrics were added during the cloudmetrics_controller initialisation here for GCP 553f8d9 (#671) but the result was that the original metrics created by AWS were registered twice. Once the Postgres instance is created, the metric is re-registered when setting for the first time here but without the help message. This results in a conflict and the pod was crash looping.

1.6799907382457628e+09 INFO Observed a panic in reconciler: a previously registered descriptor with the same fully-qualified name as Desc{fqName: "cro_postgres_current_allocated_storage", help: "", constLabels: {}, variableLabels: [clusterID resourceID namespace instanceID productName strategy]} has different label names or a different help string {"controller": "postgres", "controllerGroup": "integreatly.org", "controllerKind": "Postgres", "postgres": {"name":"rhsso-postgres-rhoam","namespace":"redhat-rhoam-operator"}, "namespace": "redhat-rhoam-operator", "name": "rhsso-postgres-rhoam", "reconcileID": "1be6bb90-c28f-4a9e-b3ab-42f5fc50ba37"}
panic: a previously registered descriptor with the same fully-qualified name as Desc{fqName: "cro_postgres_current_allocated_storage", help: "", constLabels: {}, variableLabels: [clusterID resourceID namespace instanceID productName strategy]} has different label names or a different help string [recovered]
panic: a previously registered descriptor with the same fully-qualified name as Desc{fqName: "cro_postgres_current_allocated_storage", help: "", constLabels: {}, variableLabels: [clusterID resourceID namespace instanceID productName strategy]} has different label names or a different help string

The change here is to add all initially registered metrics from the cloudmetrics controller to the overall metrics vector so that any subsequent calls to SetMetric do not try to re-register the metric.

Verification

  • Clone this branch
  • Run make cluster/prepare
  • Run make run
  • Ensure...

@codecov
Copy link

codecov bot commented Mar 28, 2023

Codecov Report

Merging #675 (21c0274) into master (2311c10) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #675   +/-   ##
=======================================
  Coverage   72.24%   72.24%           
=======================================
  Files          53       53           
  Lines        6812     6812           
=======================================
  Hits         4921     4921           
  Misses       1494     1494           
  Partials      397      397           

@adam-cattermole
Copy link
Member Author

/test unit

@carlkyrillos
Copy link
Contributor

Verified on cluster and reviewed changed files
/lgtm

@laurafitzgerald
Copy link
Collaborator

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 28, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: laurafitzgerald

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-merge-robot openshift-merge-robot merged commit 2dff57d into integr8ly:master Mar 28, 2023
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