mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
Aleksei Sviridkin
1f35c26ece
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `kind/backport` if it's a bugfix that needs to be
backported to a previous version.
-->
## What this PR does
The virt-launcher pods of Kubernetes worker nodes are never marked as
managed by the lineage webhook, so anything that relies on the webhook
having resolved them misses them. The visible symptom: a SecurityGroup
attached to a `Kubernetes` application labels the control-plane pods but
never the worker nodes, because the securitygroup-controller caches only
pods carrying `internal.cozystack.io/managed-by-cozystack=true`.
**Root cause.** The webhook is not skipped for these pods (they do not
carry the managed-by label, so the `objectSelector` lets them through);
it runs and fails to resolve their owner chain. The walk goes Pod →
VirtualMachineInstance → VirtualMachine and stops there: the Cluster API
KubeVirt provider creates the VirtualMachine with no ownerReference and
links it to its KubevirtMachine through the
`capk.cluster.x-k8s.io/kubevirt-machine-{name,namespace}` labels only.
With no ownerReference, no HelmRelease kind and no
`helm.toolkit.fluxcd.io/name` label on the VM, `WalkOwnershipGraph`
returns no ancestor, `computeLabels` returns nothing and the pod is
admitted unmanaged. The
`apps.cozystack.io/application.{group,kind,name}` labels the pods do
carry come from the `KubevirtMachineTemplate` in
`packages/apps/kubernetes-nodes/templates/nodegroup.yaml`, not from the
webhook.
**Measured on a v1.6.5 cluster.** Every Kubernetes cluster on it showed
the same shape (three clusters across two tenants). For one worker node:
the pod carries the three lineage labels but not `managed-by-cozystack`;
the pod is owned by its VMI and the VMI by its VM; the VM has no
ownerReferences, and its labels include
`capk.cluster.x-k8s.io/kubevirt-machine-name` and
`capk.cluster.x-k8s.io/kubevirt-machine-namespace` pointing at a
KubevirtMachine in the same namespace. From there ownerReferences run
KubevirtMachine → Machine → MachineSet → MachineDeployment → Cluster,
and the CAPI Cluster carries `helm.toolkit.fluxcd.io/name:
kubernetes-<name>`. The bridge is one hop. Setting
`managed-by-cozystack=true` by hand on a worker's virt-launcher pod made
the securitygroup-controller stamp the membership label within ten
seconds.
**Fix.** `WalkOwnershipGraph` now follows a VirtualMachine without
ownerReferences to the KubevirtMachine named by its CAPK labels, and
keeps walking from there. Node pods then resolve to the parent
`Kubernetes` application, which is the attribution the machine template
already hard-codes, and they get `managed-by-cozystack` like every other
managed pod. The bridge is only taken when the label names the VM's own
namespace: CAPK can target an external infrastructure cluster, and the
walk must never attribute an object across namespaces. The webhook's
ClusterRole already grants `get` on every resource, so no RBAC change is
needed.
**Why at the source rather than in the consumer.** Relaxing the
securitygroup-controller's pod cache to
`apps.cozystack.io/application.kind` Exists would also have fixed the
symptom, but it would leave every other consumer of the managed-by label
blind to node pods, and it would rest membership on labels the webhook
never vouched for. Stamping `managed-by-cozystack` from the machine
template was ruled out too: the `objectSelector` would then skip these
pods entirely, and they would lose the scheduling-class injection the
webhook performs on every pod.
**Blast radius.** Only objects below a CAPK VM change: the VMI, the
virt-launcher pods and the PVCs of the VM's `dataVolumeTemplates` now
get the lineage labels of their Kubernetes application. None of them
match a resource selector of the `Kubernetes` ApplicationDefinition, so
they get `tenantresource=false`. Nothing selects PVCs on
`application.kind=Kubernetes` (the Velero strategy selects `VMDisk`).
The `-userdata` cloud-init Secret CAPK creates is not affected either
way: at the pinned provider commit, `reconcileKubevirtBootstrapSecret`
creates it with the bootstrap Secret's labels and no ownerReferences, so
the walk reaches neither the VM nor the KubevirtMachine from it, before
or after this change. Existing node pods pick the labels up on their
next admission (recreation, live migration, or any update).
**Tests.** `pkg/lineage/capk_test.go` builds the observed chain in a
fake dynamic client and checks that a worker's virt-launcher pod
resolves to `Kubernetes/cluster-a`, that a VM pointing at a
KubevirtMachine in another namespace is not followed, that a VM without
CAPK labels still has no ancestor, and, in a table of negative cases,
that the CAPK labels are ignored on any object other than a
`kubevirt.io/v1` VirtualMachine (another kind of the `kubevirt.io`
group, a VirtualMachineInstance, a VirtualMachine of another group or
version). Weakening the kind and API version guard to match on either
field alone fails all four cases. With the bridge disabled, the first
test fails. No chainsaw case: the only suites that bring up worker VMs
are `kubernetes-latest` and `kubernetes-previous`, whose shared runner
script is already over its time budget, and the unit test covers the
walk exactly as it runs in the webhook.
### Screenshots
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body. Do not push a speculative PR into another
repository just to fill a line here.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
- [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:
### Release note
<!-- Write a release note:
- Explain what has changed internally and for users.
- Start with the same `type(scope):` prefix as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->
```release-note
fix(lineage, kubernetes): the virt-launcher pods of Kubernetes worker nodes are now resolved to their Kubernetes application and marked as managed by Cozystack, so a SecurityGroup attached to a Kubernetes cluster now applies to its worker nodes, not only to its control plane.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **New Features**
* Lineage results now connect supported virtual machines to their
associated machine and Kubernetes application when the required
association labels are present.
* **Bug Fixes**
* Association tracing is limited to supported virtual machine resources
and machines in the same namespace. Unlabelled standalone virtual
machines and machines in another namespace do not gain ancestors through
this association.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->