Skip to content

feat(vm-instance): pin a static IP on secondary interfaces on both network backends - #4038

Open
lf (lfinmauritius) wants to merge 2 commits into
cozystack:mainfrom
lfinmauritius:feat/vm-instance-cozyplane-static-ip
Open

lf (lfinmauritius) wants to merge 2 commits into
cozystack:mainfrom
lfinmauritius:feat/vm-instance-cozyplane-static-ip

Conversation

@lfinmauritius

@lfinmauritius lf (lfinmauritius) commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

What this PR does

Adds an optional ipAddress to networks[] (and to the deprecated subnets[] alias) of the vm-instance chart, so a
tenant can pin a static IPv4 on a secondary (VPC) interface. The address is handed to whichever network backend the
cluster serves, discovered through .Capabilities.APIVersions.Has — never through lookup:

APIs served Rendering of ipAddress
kubeovn.io/v1 IP (with or without Cozyplane) per-network annotation <network>.<namespace>.ovn.kubernetes.io/ip_address: <ip>
sdn.cozystack.io/v1alpha1 VPC only sdn.cozystack.io/networks: '[{"ip":"…","name":"pod<sha256(network)[:11]>","vpc":"<namespace>/<network>"}]'
neither fail with a message naming the missing backend

Kube-OVN keeps precedence when both backends are served, consistent with #4013 and #4034. Without ipAddress the
rendered manifests are byte-identical to main for all four combinations (checked with helm template on every
existing test fixture), so no existing VM is affected.

The field is documented in values.yaml; values.schema.json, README.md, the Go types and the
ApplicationDefinition are regenerated with make generate (no drift).

Why

networks[] only carries name: a VM can join a VPC network but never receive a predictable address on it.
Anything that must be reachable at a known address inside a tenant VPC (gateway, resolver, database peers have in
their config) has no way to express it, and the IPAM address changes on every recreation.

This supersedes #3900, which rendered the Kube-OVN annotation only. Both backends are covered here so that the same
values file works whichever CNI a cluster runs.

Cozyplane contract (measured, not inferred)

The Cozyplane delegate (lllamnyp/cozyplane#27) selects the annotation entry whose name equals the interface name
Multus passes (CNI_IFNAME). On KubeVirt 1.8 that name is pod + the first 11 hex characters of sha256(network name), which the template computes with sha256sum $net.name | trunc 11 on the same $net.name used for the Multus
networkName. An entry without name is consumed by the primary attachment and moves the launcher's eth0 onto
the VPC, so the template always names the interface.

Tests

  • tests/network_ipaddress_test.yaml (single suite, header explains the mocks): 5 Kube-OVN cases (pin, only pinned
    interfaces get the key, coexistence with the existing lookup annotations, subnets alias, both backends ⇒ Kube-OVN),
    3 Cozyplane cases (pin, list, alias), no backend ⇒ failedTemplate, no address ⇒ nothing rendered on either.
  • 9 of the 11 cases are red on main (the two "no address" guards pass there by design).
  • make test green with the existing suites; helm lint clean.
  • Lab (Cozystack v1.6.1, Cozyplane only): networks: [{name: subnet-…, ipAddress: 172.16.10.50}] ⇒ Port
    v108.172-16-10-50 owned by the VM, address visible in the guest, kept across stop/start, a second VM asking the
    same address is refused by the CNI (ip 172.16.10.50 is already taken in VPC). Kube-OVN: unit tests + identical
    render (no Kube-OVN cluster available to me).

Notes

🤖 Generated with Claude Code

https://claude.ai/code/session_01LoggBxo8kGq53hnRQoiYfi

Summary by CodeRabbit

  • New Features

    • Added optional static IPv4 assignment for VM network interfaces.
    • Supports both primary and secondary interfaces through available cluster networking backends.
    • Empty addresses continue to use automatic IPAM allocation.
    • Invalid, unavailable, or out-of-range addresses are rejected.
  • Documentation

    • Updated VM instance configuration documentation and validation schemas with the new ipAddress settings.
  • Tests

    • Added coverage for backend selection, API capability detection, address pinning, compatibility aliases, and unsupported configurations.

Assisted by Codex

lf (lfinmauritius) and others added 2 commits September 2, 2026 20:46
Helm's `lookup` propagates the discovery error when the requested API
group is not served, so the vm-instance chart could not render at all on
clusters without the Kube-OVN CRDs. Gate the IP lookup behind
`.Capabilities.APIVersions.Has "kubeovn.io/v1/IP"`.

The regression suite mocks the IP object through kubernetesProvider and
pins both sides: with the API advertised the annotations carry the mocked
MAC and IP, without it they are absent even though the object exists.
helm-unittest's fake lookup never errors on an unserved kind, so only the
second case distinguishes the guard; it fails on main and passes here.

Assisted-By: GPT-5 <[email protected]>
Signed-off-by: Loïc Fontaine <[email protected]>
…twork backends

networks[] exposes a single field per interface, name. A tenant can attach a
VM to a VPC network but cannot choose the interface's address, and nothing
reports the allocated one either. Add an optional networks[].ipAddress and
pin it through whichever backend the cluster serves, picked from API
discovery, never from a lookup.

Kube-OVN (kubeovn.io/v1 IP served) reads a per-network annotation key,
<nad>.<namespace>.ovn.kubernetes.io/ip_address, on the launcher pod.
Cozyplane (sdn.cozystack.io/v1alpha1 VPC served, Kube-OVN absent) reads one
launcher pod annotation, sdn.cozystack.io/networks: a JSON list with an entry
per pinned NIC, selected by the interface name Multus hands to the CNI.
KubeVirt names a secondary interface "pod" plus the first eleven hex digits
of the sha256 of the network name, so each entry carries that name together
with the VPC the NetworkAttachmentDefinition names; an entry without a name
would be read as a leg of the pod itself. Both keys reuse the
<namespace>/<network> expressions the multus reference is composed from, so a
pinned network and an attached network cannot disagree; a key the backend
does not recognise is ignored without any admission error. With neither
backend served the render fails: a value that would be dropped silently is
worse than a render that says why it stopped.

Without ipAddress nothing changes: the rendering is byte-identical on every
backend, so existing VMs are untouched. The suite drives both branches from
the capability set, mocks the Kube-OVN IP object to pin that the historical
pod-network annotations keep rendering next to the new key, and is red on
the previous template for every pinning case.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: lfneosequentia <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 5c006e53-9688-47ca-a9b4-62bcddcd379b

📥 Commits

Reviewing files that changed from the base of the PR and between 3a70f91 and 4ddc400.

📒 Files selected for processing (8)
  • api/apps/v1alpha1/vminstance/types.go
  • packages/apps/vm-instance/README.md
  • packages/apps/vm-instance/templates/vm.yaml
  • packages/apps/vm-instance/tests/kubeovn_capability_test.yaml
  • packages/apps/vm-instance/tests/network_ipaddress_test.yaml
  • packages/apps/vm-instance/values.schema.json
  • packages/apps/vm-instance/values.yaml
  • packages/system/vm-instance-rd/cozyrds/vm-instance.yaml

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The VM instance gains optional static IPv4 pinning for networks and subnets. The Helm template selects Kube-OVN or Cozyplane annotations through API discovery and fails when no supported backend is available. Tests cover backend selection and compatibility behavior.

Changes

Static IP pinning

Layer / File(s) Summary
Network field and schema contracts
api/apps/v1alpha1/vminstance/types.go, packages/apps/vm-instance/values.schema.json, packages/system/vm-instance-rd/cozyrds/vm-instance.yaml, packages/apps/vm-instance/values.yaml, packages/apps/vm-instance/README.md
The Network type and both network schemas now define optional ipAddress fields. Documentation describes IPAM fallback and backend address constraints.
Backend-specific annotation rendering
packages/apps/vm-instance/templates/vm.yaml
The template gates the Kube-OVN IP lookup by API discovery. It renders pinned secondary-interface addresses through Kube-OVN or Cozyplane and fails when no supported backend is served.
Capability and pinning test coverage
packages/apps/vm-instance/tests/kubeovn_capability_test.yaml, packages/apps/vm-instance/tests/network_ipaddress_test.yaml
Helm-unittest cases cover API discovery, backend priority, pinned and unpinned interfaces, subnets, pod-network annotations, empty addresses, and unsupported backends.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant VMInstanceValues
  participant HelmTemplate
  participant ClusterAPIs
  participant NetworkBackend
  VMInstanceValues->>HelmTemplate: Provide networks[].ipAddress
  HelmTemplate->>ClusterAPIs: Check served Kube-OVN and Cozyplane APIs
  ClusterAPIs-->>HelmTemplate: Return capabilities
  HelmTemplate->>NetworkBackend: Render selected IP annotations
  NetworkBackend-->>HelmTemplate: Accept pinned address or reject unsupported rendering
Loading

Merge Risk: 🔵 Low · up to 4ddc4

The PR adds tenant-configurable static IPs for secondary VM interfaces and safely routes them to the selected network backend. Merge is reasonable with explicit owner awareness that backend-side validation, conflict handling, tenant isolation, and address cleanup must be confirmed for both supported backends.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes changes beyond issue #3900, including Cozyplane backend support and discovery-gated historical Kube-OVN primary-NIC lookup behavior. These changes are related to networking but are not… Either update or add linked issue requirements for Cozyplane support and the Kube-OVN lookup guard, or move those changes into separate pull requests. Keep this pull request limited to the issue #3900 requirements if no scope update is inte…
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR satisfies issue #3900 by adding optional secondary-interface IP pinning, preserving the deprecated subnets alias, using the Kube-OVN network reference expressions, preserving unchanged output w…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: static IP pinning on secondary interfaces for both network backends.
Full details: Out of Scope Changes check

Explanation

The PR includes changes beyond issue #3900, including Cozyplane backend support and discovery-gated historical Kube-OVN primary-NIC lookup behavior. These changes are related to networking but are not part of the linked issue's requested Kube-OVN secondary-interface scope.

Resolution

Either update or add linked issue requirements for Cozyplane support and the Kube-OVN lookup guard, or move those changes into separate pull requests. Keep this pull request limited to the issue #3900 requirements if no scope update is intended.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review kind/feature Categorizes issue or PR as related to a new feature size/L This PR changes 100-499 lines, ignoring generated files labels Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lf (@lfinmauritius) NOT LGTM: the Kube-OVN part is a reasonable feature, but half of this targets a backend main does not ship.

A fixed address for a VM inside its VPC is a fair ask, and the Kube-OVN key is right. The vpc chart sets the NAD provider to <nad>.<namespace>.ovn, so <network>.<namespace>.ovn.kubernetes.io/ip_address is the annotation Kube-OVN reads for that attachment. That part is small and I'd take it.

The Cozyplane branch should go. Nothing on main serves sdn.cozystack.io/v1alpha1 VPC. The only in-tree source is the open draft #3149, and the sdn.cozystack.io/networks format depends on an unmerged change in the Cozyplane repo. The entry name pod plus 11 hex digits of sha256 is also a KubeVirt internal. The template would carry both as a contract no test on main can check. Once Cozyplane lands, pinning for it can follow in the same place.

What I need to merge this:

  1. Only ipAddress plus the Kube-OVN annotation. Keep the fail when Kube-OVN is absent, since the value would otherwise be dropped silently.
  2. A rebase. The branch conflicts with main in packages/apps/vm-instance/README.md and the generated packages/system/vm-instance-rd/cozyrds/vm-instance.yaml, so re-run make generate after it.
  3. A run on a Kube-OVN cluster. The description says this path was only unit-tested, and it is the only path main can use. Show a VM coming up with the pinned address on the secondary NIC.
  4. Replace Co-Authored-By: Claude Fable 5.1 <[email protected]> with Assisted-by: LLM, see docs/agents/contributing.md.

Minor: ipAddress is a bare string in the schema. A typo like 10.0.0.300 passes validation and only fails when the pod is created. An IPv4 format check on the field would catch it earlier.

Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Oct 6, 2026
…4306)

<!-- 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

A guest whose interfaces are rendered by netplan ignores two things the
platform already puts on the wire for it, and this adds an opt-in
`netplanDhcp` value that supplies the cloud-init network-data making it
ask for both.

`virt-launcher` starts a DHCP server on **every** bridge-bound
interface, secondary ones included — `newDHCPConfigurator` in
`pkg/network/setup/launcher/configurator.go` gates only on `iface.Bridge
!= nil`, and `BridgeConfigGenerator.Generate` serves the address the CNI
put on the pod-side interface along with that link's MTU. The chart
renders `bridge: {}` for every network, so a VM joined to a VPC subnet
is offered its reserved address and its MTU. cloud-init's fallback
network configuration covers a single interface, so on Debian and Ubuntu
nothing ever sends a DHCPDISCOVER on the second NIC and the interface
stays inert, while AlmaLinux (NetworkManager auto-profile) and Windows
come up. That asymmetry is the tell: with no DHCP server at all, none of
the three would work.

The same DHCP server advertises the launcher pod's search domains as
option 119, and `UseDomains=` defaults to `no` in systemd-networkd, so a
netplan-managed link keeps none of them. A cluster domain under `.local`
then reaches the mDNS exclusion in resolved and every lookup answers
`REFUSED`, which is #2085. `dhcp4-overrides: use-domains: true` is
enough on its own: the domains on the wire already carry whatever
cluster domain the cluster actually uses, so nothing needs to be
hardcoded here and #2372 does not collide with this.

### This is a regression, not a missing feature

`packages/apps/virtual-machine` carried exactly this, added in c63fb20
(*[apps] virtual-machine: add vpc support*, 2025-10-21) as
`templates/secret-network-config.yaml` plus a `networkDataSecretRef`,
gated on `.Values.systemDisk.image`. 08a5f98 (*feat(platform): remove
virtual-machine application*, 2026-02-12) deleted the application and
the template with it, and `vm-instance` never had one — `git log --all
-S networkDataSecretRef -- packages/apps/vm-instance/` is empty. The
gate could not be ported verbatim because `systemDisk.image` went away
in the same split, which is the likeliest reason nobody did. #2166 is
the other regression from that migration.

`docs/vm-external-vlan.md` has since documented the absence as a
property of the chart and prescribed a `write_files` + `netplan apply`
workaround, so this PR updates that page too.

### Why it is opt-in

The network-data renders only when `netplanDhcp` is `true`, and the
value defaults to `false`. Keying it on `instanceProfile` instead cannot
work: the profile defaults to `ubuntu` whatever image the disk holds,
and it is a KubeVirt preference rather than a declaration of the guest's
network stack. cloud-init passes a v2 configuration through verbatim
only for the netplan renderer; sysconfig, NetworkManager and eni turn
each `match` stanza into an interface literally named `cozy-en` or
`cozy-eth`, so a non-netplan guest handed this configuration loses DHCP
on its real NIC.

Two cases where the value must stay off, both stated in its description
and in `docs/vm-external-vlan.md`:

- A guest not rendered by netplan (AlmaLinux, Rocky, Alpine), for the
reason above.
- A guest that configures a NIC statically through netplan, as the
external-VLAN recipe does. Netplan writes one networkd file per
definition ID and systemd-networkd applies only the first `.network`
file in lexical order that matches a link, so
`10-netplan-cozy-en.network` shadows `10-netplan-enp2s0.network` and the
static address is never applied, whatever number the user's netplan file
carries. Reproduced with `netplan generate` on Ubuntu 24.04.

Matching by MAC with a v1 configuration would be renderer-independent
and would not shadow a named interface, but it needs the chart to know
the MACs at render time; `lookup` cannot supply them on a first install,
so the chart would have to pin them, which is a larger change that
overlaps #4038.

### What this does not do

A VM that leaves `netplanDhcp` unset renders exactly as before: no
network-data Secret, and no cloud-init disk where there was none.
Turning the value on for a VM with neither `cloudInit` nor `sshKeys`
adds a cloud-init disk, and because KubeVirt uses the pinned firmware
UUID as the NoCloud instance-id, a guest that has never seen that id
treats its next boot as a first boot (new SSH host keys); the value's
description says so. On a VM that already has a cloud-init disk the
instance-id does not change, and it is derived from the namespace, the
name and `cloudInitSeed`, so recreating the VM keeps it too; cloud-init
applies network configuration per instance by default, so such a guest
picks the network-data up once `cloudInitSeed` is changed. Static
addressing is still `userData`.

### Tests

`tests/network_data_test.yaml`, 6 cases: the rendered network-data
asserted in full (both properties, not just the presence of the key),
the VM carrying network-data with no user-data requested, user-data and
network-data side by side, a VM on the default profile rendering neither
the Secret nor a cloud-init disk, user-data alone when the value is off,
and the value rather than the profile deciding. With the templates gated
on the `ubuntu` profile again, the default-profile case and the
value-off case go red.

`helm unittest` green on all four suites of the package (32 tests).
`make generate` output is committed (`values.schema.json`, `README.md`,
`api/apps/v1alpha1/vminstance/types.go`,
`packages/system/vm-instance-rd/`). `helm lint` fails on this package
identically on an untouched tree (`_cluster.scheduling` nil pointer,
`cozy-lib` dependency metadata), so it is not a signal here.

### Screenshots

Not a UI change.

### 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.
-->

<!-- Trigger map walked file by file against the diff.
`docs/vm-external-vlan.md` documents the absence this PR removes, which
is the "make a documented workaround obsolete" trigger, so the website
copies of that page need the same edit and issue 702 carries it.
`netplanDhcp` is a new field in `values.schema.json`, which the provider
models by hand, and issue 62 there carries it. No package added, renamed
or removed; no hack/ or ApplicationDefinition change; the cozy-proxy
annotations in templates/service.yaml are untouched. -->

- [x] [cozystack/website](https://github.com/cozystack/website) -
follow-up: cozystack/website#702
- [x]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
cozystack/terraform-provider-cozystack#62
- [ ]
[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(vm-instance): a new `netplanDhcp` value, off by default, makes the chart supply cloud-init network-data that requests DHCP on every ethernet interface and accepts the DNS search domains offered over DHCP. A netplan guest (Ubuntu, Debian) attached to a VPC subnet then configures that interface on its own instead of leaving it unaddressed, and cluster service names under the cluster domain resolve inside the guest. Leave it off for guests not rendered by netplan and for guests that configure a NIC statically through netplan. VMs that do not set it are unchanged.
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added an opt-in setting to request DHCP on Ethernet interfaces in
supported virtual machines while retaining DHCP-provided DNS search
domains.
* The setting is off by default. Enabling it can add a cloud-init disk
and may interfere with static network settings or guests that do not use
netplan.

* **Documentation**
* Updated setup guidance with considerations for enabling DHCP and
configuring static networking on VLANs without a DHCP server.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review kind/feature Categorizes issue or PR as related to a new feature size/L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants