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

Remove Owner References from Grafana ConfigMap (PROJQUAY-1788) #420

Merged
merged 1 commit into from Mar 30, 2021

Conversation

jonathankingfc
Copy link
Collaborator

Issue: https://issues.redhat.com/browse/PROJQUAY-1788

Changelog:

  • Reconcile loop ensures that no ownerReference are injected on the grafana-dashboard config map. This prevents the cross-namespace owner references that caused resource to be garbage collected after creation.

Docs:

  • N/A

Testing:

  • Install operator and create instance with monitoring enabled
  • ConfigMap -grafana-dashboard-quay should be visible in openshift-config-managed namespace

Details:

  • N/A

Copy link
Contributor

@alecmerdler alecmerdler left a comment

Choose a reason for hiding this comment

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

Add finalizer logic to replace garbage collection, and change RemoveOwnerReference to accept the metadata.name of a QuayRegistry to remove its specific ownerReference from the list.

controllers/quay/quayregistry_controller.go Outdated Show resolved Hide resolved
apis/quay/v1/quayregistry_types.go Outdated Show resolved Hide resolved
@alecmerdler alecmerdler changed the title Remove owner references from grafana config map Remove Owner References from Grafana ConfigMap (PROJQUAY-1788) Mar 30, 2021
@@ -357,6 +357,25 @@ func EnsureOwnerReference(quay *QuayRegistry, obj runtime.Object) (runtime.Objec
return obj, nil
}

// RemoveOwnerReference removes the `ownerReference` of `QuayRegistry` on the given object.
func RemoveOwnerReference(quay *QuayRegistry, obj runtime.Object) (runtime.Object, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a simple test for this?

@jonathankingfc jonathankingfc merged commit 02c850d into master Mar 30, 2021
@jonathankingfc jonathankingfc deleted the monitoring_fix branch October 27, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants