Make monitoring webhook receiver airgap-compatible: pinned image, registry resolution, node-address preference (Slice 1) - #858
Conversation
) - Add actions/monitoring config: pinned webhookReceiverImage default (traefik:v3.7.12), skipWebhookReceiver flag, ordered nodeAddressPreference (ExternalIP, InternalIP) with validation - Add ResolveImageReference: prefix hostless images with the Rancher system-default-registry setting (empty in non-airgap, image unchanged) - Add PickNodeAddress: pure resolver replacing the external-IP-only inline loop, falling back per configured preference - Thread the configurable image through createAlertWebhookReceiverDeployment instead of hardcoded traefik:latest - Load monitoringTest config in MonitoringTestSuite.SetupSuite; gate the webhook-receiver section on skipWebhookReceiver
|
MonitoringTestConfig ApplyDefaults TestSuites above were modified. TestSuites below use modified code from this PR. TestMonitoringTestConfigApplyDefaults |
| ConfigurationFileKey = "monitoringTest" | ||
|
|
||
| // DefaultWebhookReceiverImage is the pinned webhook receiver image used when no override is configured. Never a floating tag. | ||
| DefaultWebhookReceiverImage = "traefik:v3.7.12" |
There was a problem hiding this comment.
I am wondering how much of a good idea it is to pin this version, as there may be some compatibility issues in the future plus it raises the need of one more value to maintain
There was a problem hiding this comment.
@hamistao The pin is deliberate and part of the design (#848): the floating traefik:latest tag is exactly what makes the current test non-deterministic and un-mirrorable in airgap, so the PRD specifies a pinned released version for all environments, not just airgap. Maintenance-wise, it's cheap: airgap users override it via webhookReceiverImage in the config file (no code change), and a version bump is a one-line constant. I'd also argue the compatibility risk is lower than latest, which can silently break the test at any pull.
There was a problem hiding this comment.
The point is that this is a one-line constant that we will surely never again remember to update and this value will soon point to an outdated and eventually deprecated version of traefik. Then again we have so many of those at this point so what is one more. It would be nice if this lived outside the code, since it is now one more place to keep track of outdated versions (and an unusual one), but I can't think of a better suggestion than having this be the default webhookReceiverImage on our jobs' parameters, but I won't ask you to follow this unless you like the idea.
There was a problem hiding this comment.
@hamistao Keeping the default in code, mainly because the external control you're describing already exists: any job or environment can set webhookReceiverImage in the config file without touching code, so only the fallback is a constant. Moving that fallback into job parameters would give us N per-job defaults that live outside version control, outside TestMonitoringTestConfigApplyDefaults, and outside repo search, with nothing to keep them in sync. On staleness, I think the failure modes favor the pin: a stale pin is a deliberate, reviewable one-line bump, while latest breaks silently at any pull and can't be mirrored into an airgap registry at all. That asymmetry is why the design (#848) pinned it.
| ConfigurationFileKey = "monitoringTest" | ||
|
|
||
| // DefaultWebhookReceiverImage is the pinned webhook receiver image used when no override is configured. Never a floating tag. | ||
| DefaultWebhookReceiverImage = "traefik:v3.7.12" |
There was a problem hiding this comment.
I agree, we should avoid to pin versions on the tests, could this non-deterministic behavior in airgap actually be a bug?
There was a problem hiding this comment.
@lscalabrini01 Non-deterministic means that when we install only the latest, we have no control over what changes might be present in those new versions.
| } | ||
|
|
||
| func TestPickNodeAddress(t *testing.T) { | ||
| external := corev1.NodeAddress{Type: corev1.NodeExternalIP, Address: "203.0.113.10"} |
There was a problem hiding this comment.
How can we ensure the address always will be the same?
There was a problem hiding this comment.
@lscalabrini01 Deterministic by construction: PickNodeAddress (nodes.go) collects addresses only for the first preference type that yields at least one match, and every table case supplies exactly one candidate for that type. So rand.IntN(len(candidates)) is always rand.IntN(1) and the exact-equal assertion holds on every run. The random pick only applies to real multi-node clusters, where any node's address serves the NodePort equally.
3 participants
Closes #849
Summary
Slice 1 of the monitoring airgap PRD (#848): make the monitoring/alerting webhook-receiver validation airgap-capable by removing its unpullable dependencies on Docker Hub (
traefik:latest) and public node external IPs.Changes
New in
actions/monitoring:config.go—monitoringTestconfig section (modeled onneuvectorTest):webhookReceiverImage(pinned defaulttraefik:v3.7.12),skipWebhookReceiver, orderednodeAddressPreference(ExternalIP/InternalIP), withApplyDefaults/Validate/NodeAddressTypesimages.go—ResolveImageReference(defaultRegistry, image): prefixes hostless image references with the Ranchersystem-default-registrysetting value (Docker registry-host convention)nodes.go—PickNodeAddress(nodes, preference): pure resolver returning a random address for the first preference type that yields any address across nodes; errors when none matchvalidation/charts/monitoring.go:createAlertWebhookReceiverDeploymenttakes the webhook image as a parameter and resolves it againstsystem-default-registry(empty in non-airgap → image unchanged)traefik:latest→ configurable, pinned by defaultvalidation/charts/monitoring_test.go:monitoringTestconfig inSetupSuite(defaults + validation)skipWebhookReceiver: truelogs and skips only the webhook-receiver tail ofTestMonitoringChartmonitoring.PickNodeAddresshonoring the configured preferenceNon-airgap behavior
Unchanged: with no config and an empty
system-default-registry, the deployment pulls the pinnedtraefik:v3.7.12(previously floatingtraefik:latest), and node selection still prefersExternalIPwithInternalIPas fallback.Verification
cd actions && go test ./monitoring/... -v— 12/12 passcd actions && go build ./... && go test ./...— full actions module greengo vet -tags validation ./validation/charts/...— cleango build -tags validation ./...— whole repo buildsgofmtclean on all changed filestraefik:v3.7.12confirmed published on Docker Hub (multi-arch, active)TestMonitoringChartrun against a Rancher + downstream cluster not executed in the development environment (noCATTLE_TEST_CONFIG); covered by the unit-test matrix above