Refactor observability controller

[eguzki]

What

Following up the work being done in #1164. Refactor to, hopefully, simplify the code to maintain.

• Monitoring resources loaded in memory are no longer static. They are instantiated every time. Reason being that go client might update objects on creation.
• Implemented mutator pattern with createOnly mutator, having the same functionality: resources can externally be updated and the operator will not revert the changes back.
• Added error handling. When resource creation reports error, the error is being returned and propagated up in the stack.
• istiodMonitor and envoyGatewayMonitor are not created repeatedly for each gateway. They are single instance per Istio control plane / EnvoyGateway control plane.

Verification steps

• Setup environment based on EnvoyGateway

make local-setup GATEWAYAPI_PROVIDER=envoygateway

• Apply the Kuadrant custom resource with observability enabled

kubectl -n kuadrant-system apply -f - <<EOF
---
apiVersion: kuadrant.io/v1beta1
kind: Kuadrant
metadata:
  name: kuadrant-sample
spec:
  observability:
    enable: true
EOF

• Wait for kuadrant instance to be ready

kubectl wait --timeout=300s --for=condition=Ready kuadrant kuadrant-sample -n kuadrant-system

• Verify the monitoring resources are there

k get servicemonitor,podmonitor -A

The expected result:

NAMESPACE              NAME                                                              AGE
envoy-gateway-system   servicemonitor.monitoring.coreos.com/envoy-gateway-monitor        148m
kuadrant-system        servicemonitor.monitoring.coreos.com/authorino-operator-monitor   148m
kuadrant-system        servicemonitor.monitoring.coreos.com/dns-operator-monitor         148m
kuadrant-system        servicemonitor.monitoring.coreos.com/kuadrant-operator-monitor    148m
kuadrant-system        servicemonitor.monitoring.coreos.com/limitador-operator-monitor   148m

NAMESPACE        NAME                                                   AGE
gateway-system   podmonitor.monitoring.coreos.com/envoy-stats-monitor   148m

Filtering by labels kuadrant-observability=true should give the same output.

k get servicemonitor,podmonitor -l kuadrant.io/observability=true -A

• Turn monitoring off

kubectl patch kuadrant kuadrant-sample --type=merge --patch '{"spec": {"observability": null}}' -n kuadrant-system

• Verify the monitoring resources are gone

k get servicemonitor,podmonitor -l kuadrant.io/observability=true -A

The expected result:

No resources found

[codecov]

Codecov Report

Attention: Patch coverage is 96.38989% with 10 lines in your changes missing coverage. Please review.

Project coverage is 83.91%. Comparing base (2ae754e) to head (2c43a4f).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
controllers/observability_reconciler.go	96.37%	8 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1178      +/-   ##
==========================================
+ Coverage   83.38%   83.91%   +0.53%     
==========================================
  Files          82       82              
  Lines        7120     7388     +268     
==========================================
+ Hits         5937     6200     +263     
- Misses        949      952       +3     
- Partials      234      236       +2     

Flag	Coverage Δ
bare-k8s-integration	24.66% <45.12%> (+1.24%)	⬆️
controllers-integration	71.28% <3.97%> (-3.00%)	⬇️
envoygateway-integration	42.53% <63.89%> (+0.89%)	⬆️
gatewayapi-integration	19.21% <3.97%> (-0.85%)	⬇️
istio-integration	45.82% <79.06%> (+1.80%)	⬆️
unit	18.37% <0.00%> (-0.72%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
api/v1beta1 (u)	90.78% <ø> (ø)
api/v1beta2 (u)	∅ <ø> (∅)
pkg/common (u)	∅ <ø> (∅)
pkg/istio (u)	62.06% <ø> (ø)
pkg/log (u)	93.18% <ø> (ø)
pkg/reconcilers (u)	25.16% <100.00%> (+0.49%)	⬆️
pkg/rlptools (u)	∅ <ø> (∅)
controllers (i)	87.11% <96.42%> (+0.58%)	⬆️

Files with missing lines	Coverage Δ
controllers/state_of_the_world.go	90.93% <100.00%> (ø)
controllers/observability_reconciler.go	94.75% <96.37%> (+15.65%)	⬆️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[david-martin]

The errors are deliberately only logged, not propagated up the stack, to allow other monitors to possibly be still created.

[eguzki]

Ok, I will leave as it is then. However, if there is a net issue or API server downtime issue, the operator would not retry until there is another event 🤷

[eguzki]

There is an issue with the hardcoded namespaces of istio and envoygateway. The namespace of those control planes cannot be assumed to be those hardcoded. I will address this issue in a follow up PR

[eguzki]

There is an issue with the lifecycle of the monitors when a gateway is deleted. The monitoring resource does not get deleted. I will address this issue in a follow up PR

[Boomatang]

There is an issue with the lifecycle of the monitors when a gateway is deleted. The monitoring resource does not get deleted. I will address this issue in a follow up PR

@eguzki is that an exist issue?

[Patryk-Stefanski]

@Boomatang, I created the follow-up issues below. They seem like good first issues that I could get started with, @eguzki could we sync on these when you're back?

• namespace of istio and envoy gateways should not be hardcoded in observability_reconciler.go #1189
• pod and service monitors should be deleted when the parent gateway is deleted. #1190

[eguzki]

uld we sync on these when you're

Yes, those are good first issues. Send invite and we sync.

[Patryk-Stefanski]

The verification steps were successful. Turning the observability option on and off created and deleted the monitoring resources as expected. /lgtm

[eguzki]

kindly asking for review @david-martin @Patryk-Stefanski