Fix: GrafanaNotificationPolicy reconcile loop ignoring namespace boundaries

[Baarsgaard]

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"

[theSuess]

Is there a reason as to why this is no longer scoped? I'd assume the scoping to be relevant here as well

[Baarsgaard]

It's due to spec.allowCrossNamespaceImport not being immutable.
If you change it true -> false it will not finalize all instances, as it no longer matches instances outside the namespace when Scoped.
GetMatchingInstances was not scoped either.
The difference between GetMatchingInstances and GetAllMatchingInstances: https://github.com/grafana/grafana-operator/blob/master/controllers/controller_shared.go#L151-L155
Which might be incorrect, it could be argued that returning the full list with unready instances is preferred during finalize, that way the finalizer would error and a retry later?

Edit: I remembered that I left a comment about allowCrossNamespaceImport on the function.