mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-06 12:03:39 +09:00
Aleksei Sviridkin
53829ea109
## What this PR does Adds `patches/vmi-informer.diff` to the `kubevirt-cloud-provider` image of the `kubernetes` package. The file is named after what it does: no upstream pull request carries this change yet (kubevirt/cloud-provider-kubevirt#382 covered the same desync and was closed as rotten), so a number would point at an unrelated upstream PR and invite someone bumping the pin to drop it. The `kubevirteps` controller derives the infra EndpointSlices of a LoadBalancer Service from the VMIs backing the tenant nodes (`getDesiredEndpoints`, and the all-VMIs fallback added by `379.diff`). But it registers informers only on infra Services, tenant EndpointSlices and infra EndpointSlices, and both factories are built with a resync period of `0`. VMI addresses are resolved with a one-off `Get` during a reconcile. Consequently a VMI appearing, disappearing or changing its address triggers no reconciliation, and no periodic relist ever catches up. The EndpointSlices keep pointing at VMIs that no longer exist and the LoadBalancer blackholes the share of the inbound connections hashed onto those dead addresses — sporadic inbound timeouts, with nothing wrong on the tenant side. Services whose tenant EndpointSlice churns along with its pods recover on their own, because that churn produces an event. Cilium Gateway API Services do not: their tenant slice is a static sentinel (`192.192.192.192`, no `NodeName`), so nothing ever forces a recompute. Seen in production on a cluster whose GPU MachineDeployment is rolled by the cluster-autoscaler: 5 of the 9 endpoints of each of the 3 gateways pointed at destroyed VMs (13 failures out of 20 requests). Worth noting for anyone debugging this: the KCCM looks dead (no log for hours) while its lease is still being renewed every second. The process is healthy, it is simply never woken up. The patch: - adds an informer on the infra `VirtualMachineInstances` that requeues the LoadBalancer Services of this cluster (those labelled `cluster.x-k8s.io/cluster-name=<clusterName>`, the ones `reconcile` does not skip) on add/delete, and on updates filtered by a fingerprint of what actually shapes an endpoint: the VMI's cluster label, `Status.NodeName`, `Status.Phase`, and the address of the `default` interface; - gives both informer factories a 5 minute resync, as a safety net if an event is ever missed; - scopes the all-VMIs fallback to the VMIs labelled `cluster.x-k8s.io/cluster-name=<clusterName>`. An infra namespace can hold several tenant clusters as well as standalone `VirtualMachines`, and their VMIs do not serve this Service's NodePort, so selecting them adds endpoints that blackhole traffic. The fallback widens to every VMI only when none of them carries the label, so an environment that does not label them does not lose all of its endpoints; once any VMI is labelled, an empty selection means this cluster has no node right now and is not widened to another cluster's VMIs. No RBAC change: the KCCM role already grants `watch` on `virtualmachineinstances`. On `main`, the Gateway scenario reaches the all-VMIs fallback without any other KCCM patch: Cilium 1.19.5 (shipped since v1.6.0) sets `kubernetes.io/service-name` on the tenant slice of a `cilium-gateway-*` Service (cilium/cilium#46242), so the existing lookup finds the sentinel slice and falls back on its missing `NodeName`. This is why #3374, which added that lookup to the KCCM, was closed. On v1.5.x and earlier, which ship Cilium 1.19.3, a Gateway Service finds no tenant slice at all, so it has no endpoints to go stale in the first place. ## Testing - `341 + 379 + vmi-informer` applies onto the pinned `a0acf33`; the Dockerfile steps (`go get`, `go mod tidy`, `go mod vendor`, `-mod=vendor` build of the manager) pass, `go vet` and `gofmt` are clean. - The patch adds 6 specs, and the suite runs 21 of 21 green: fallback scoped to the cluster, fallback preserved when no VMI is labelled, no widening when only another cluster's VMIs are present, unlabelled VMIs ignored once some are labelled, VMI add/delete requeues the Service of this cluster and not the one of another cluster, and VMI updates driven through the fake dynamic client: an annotation-only update does not requeue, while a change of phase, node, address, or cluster label does. - Mutation-tested, each one turns the suite red: always requeuing on update; dropping the phase, the node, the address or the cluster label from the fingerprint; requeuing every Service of the namespace; dropping the delete requeue. - Under `-race`, the suite reports an intermittent data race at `kubevirteps_controller_test.go:820` (a `PrependReactor` racing an informer list). It reproduces identically on `341 + 379` without this patch, and the new specs pass on every `-race` run. - Validated on a live cluster with an image built from an earlier revision of this patch (same informer and fallback, before the cluster-scoped requeue and the label in the fingerprint): endpoint added in under 2s when a VMI appears, dead address removed in 33s when it is deleted. Before the patch the dead address stayed forever. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff is one patch file of the KCCM image: it touches no values, schema, CRD, `hack/` tooling, namespace, telemetry metric or node prerequisite, so the trigger map comes out empty. ### Release note ```release-note fix(kubernetes): the KCCM now reconciles the LoadBalancer EndpointSlices when the VMIs backing the tenant nodes change, instead of keeping stale addresses that blackhole part of the inbound traffic ```