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

Fix: GrafanaNotificationPolicy reconcile loop ignoring namespace boundaries #1815

Merged

Conversation

Baarsgaard
Copy link
Contributor

@Baarsgaard Baarsgaard commented Jan 10, 2025

I was done updating the reconciler when I realised that there was no cross namespace check to delete and I remembered #1762 existed.
I decided to open this regardless as it looked like it had stagnated.
Feel free to close this if progress will resume.

  • fix: Warnings when returning ctrl.Result and an error and logging some errors twice.
  • chore: Folded r.resetInstance under finalize to align with other finalize functions as it was only invoked from finalize.
  • feat: status.lastResync is now updated and registered as a printed column (for GrafanaFolder and GrafanaNotificationTemplate as well)
    It was not clear to me if Age should be added as well

Labels matching, but instance is in a different namespace

kubectl get grafanas -n default grafana-testdata -o yaml

apiVersion: grafana.integreatly.org/v1beta1
kind: Grafana
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"grafana.integreatly.org/v1beta1","kind":"Grafana","metadata":{"annotations":{},"labels":{"test":"testdata"},"name":"grafana-testdata","namespace":"default"},"spec":{"config":{"auth":{"disable_login_form":"false"},"log":{"mode":"console"},"security":{"admin_password":"secret","admin_user":"root"}}}}
  creationTimestamp: "2025-01-10T01:50:51Z"
  generation: 2
  labels:
    test: testdata
  name: grafana-testdata
  namespace: default
  resourceVersion: "874"
  uid: 63d529df-c1c4-474e-a084-ec3c15f07a08
spec:
  config:
    auth:
      disable_login_form: "false"
    log:
      mode: console
    security:
      admin_password: secret
      admin_user: root
  version: 11.3.0
status:
  adminUrl: http://grafana-testdata-service.default.svc:3000
  dashboards:
  - default/testdata/testdata-dash
  datasources:
  - default/testdata/testdata-datasource
  folders:
  - default/testdata/testdata
  stage: complete
  stageStatus: success
  version: 11.3.0

kubectl get grafananotificationpolicies -n new testdata -o yaml

apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaNotificationPolicy
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"grafana.integreatly.org/v1beta1","kind":"GrafanaNotificationPolicy","metadata":{"annotations":{},"labels":{"test":"testdata"},"name":"testdata","namespace":"new"},"spec":{"instanceSelector":{"matchLabels":{"test":"testdata"}},"route":{"group_by":["grafana_folder","alertname"],"receiver":"testdata"}}}
  creationTimestamp: "2025-01-10T01:52:07Z"
  generation: 1
  labels:
    test: testdata
  name: testdata
  namespace: new
  resourceVersion: "1559"
  uid: 8ffe67b1-2748-4c02-9c79-083a621721d2
spec:
  allowCrossNamespaceImport: false
  instanceSelector:
    matchLabels:
      test: testdata
  resyncPeriod: 10m0s
  route:
    group_by:
    - grafana_folder
    - alertname
    receiver: testdata
status:
  conditions:
  - lastTransitionTime: "2025-01-10T01:52:07Z"
    message: Instances could not be fetched, reconciliation will be retried
    observedGeneration: 1
    reason: EmptyAPIReply
    status: "True"
    type: NoMatchingInstance
  lastResync: "2025-01-10T01:55:13Z"

@Baarsgaard Baarsgaard marked this pull request as ready for review January 10, 2025 01:58
@Baarsgaard Baarsgaard force-pushed the update_notificationpolicy_reconcile_loop branch from 7ae2a74 to 81eafd6 Compare January 13, 2025 18:57
@theSuess theSuess added this pull request to the merge queue Jan 14, 2025
Merged via the queue into grafana:master with commit 5d76d43 Jan 14, 2025
14 checks passed
@Baarsgaard Baarsgaard deleted the update_notificationpolicy_reconcile_loop branch January 14, 2025 17:27
@theSuess theSuess added the bugfix this PR fixes a bug label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix this PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants