feat(instances): Add network interfaces and relays - #481
Open
jedevc wants to merge 4 commits into
Open
Conversation
The flat key=value form could only set a nested struct field by giving the outer field a text form of its own, which then had to encode every subfield in one value. Cut the key on the first dot instead and walk into the named field, allocating a nil pointer on the way. Render flattens the same way. A rendered struct is fed straight back to Parse - a shortcut flag becomes a --set string - and a nested struct rendered inline would reparse its commas as fields of the outer struct. Descent stops at a type owning the matching half of the conversion, which takes the whole value, and at anonymous fields, which are already addressed whole under their type name. The two directions check separately: sharing one test would let a marshal-only type block parsing, and vice versa. Parse and Render now share namedFields so a key they disagree on cannot exist, and walkFields is the rendering inverse of assignField. Also report a bare word as a malformed pair rather than a field named after its own value, which is what a reader of "unknown fields: [my-router-eth0]" has to work out for themselves. Signed-off-by: Justin Chadwell <justin@unikraft.com>
instances_status_add_network reports name, tap_name, autoconfig and relay, and none of them were mirrored. The interface name matters most: a relay is referenced by interface name, so without it there is no way to tell what an instance could relay through. Relay holds plain identifiers rather than a Link. It targets another instance's interface, and interfaces have no API of their own for a resource type to sit on, so Link()'s TUI drill-down would dead-end. Its DNS toggle is relay.dns, not relay.relay-dns as the API spells it - the prefix is already carried by the field it sits under. Signed-off-by: Justin Chadwell <justin@unikraft.com>
Exposes the whole interface, not just the relay: name, ip, mac, tap-name, autoconfig and relay. There is no patch counterpart because /v1/instances has no update type for interfaces at all, so this is create-only and immutable afterwards. Each --network occurrence carries one whole interface, since there is no way to address a slice element - networks.0 does not exist on a create, where the slice is still empty. Within one occurrence the relay is reached by dotted key, so relay.name, relay.uuid and relay.dns read the same in --set, --filter and the output. There is deliberately no bare relay=<name> spelling; the one that names the field it sets is enough. uuid and private-ip are reported by the API and never sent, so they are name:"-" and rejected as unknown keys rather than accepted and dropped. mac goes through AdditionalProperties: /v1/instances has accepted it since MAC-only custom interfaces landed, but the spec's network interface still omits it, same as roms at=. Note the whole array sits behind the net-manager permission, which the API reports as an unknown member rather than a permission error. Signed-off-by: Justin Chadwell <justin@unikraft.com>
Drives the whole create surface through the CLI: a router with an explicitly named interface, a client relaying through it, and the relay.dns opt-out. The router needs the explicit name because passing network_interfaces at all switches interface naming to the eth-<suffix> fallback, so the generated name cannot be predicted. Both rejections are covered too - a relay naming no existing interface, and a chain through an interface that is itself relayed. Deleting the router needs a retry. Its datapath is torn down asynchronously once the last client goes, and until that lands the delete comes back as -EBUSY, which the CLI surfaces as "Unknown error -16". Sending timeout_s=-1 as instance delete does makes it fail rather than wait, so this is not something the caller can ask to block on. Gated to staging and stable like create-env; relay landed after prod. Signed-off-by: Justin Chadwell <justin@unikraft.com>
There was a problem hiding this comment.
Pull request overview
Adds network interface and relay support to instance creation/output, backed by reusable dotted-field parsing and rendering.
Changes:
- Adds
--networkconfiguration and relay request mapping. - Supports dotted nested value paths.
- Expands unit, golden, and integration coverage.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/resource/value/parse.go |
Parses dotted nested keys. |
internal/resource/value/parse_test.go |
Tests nested parsing. |
internal/resource/value/format.go |
Renders nested dotted paths. |
internal/resource/value/format_test.go |
Tests nested rendering. |
internal/resource/value/fields.go |
Adds shared field traversal. |
internal/cmd/instances.go |
Adds network and relay CLI support. |
internal/cmd/marshal_test.go |
Tests network parsing and patches. |
internal/cmd/output_test.go |
Adds network output fixtures. |
internal/cmd/testdata/TestOutput/instances |
Updates expected instance output. |
cmd/unikraft/testdata/TestHelp/run |
Documents run network fields. |
cmd/unikraft/testdata/TestHelp/instances |
Documents instance network fields. |
cmd/unikraft/integration/instance_test.go |
Tests relay lifecycle behavior. |
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| req.Roms = append(req.Roms, reqRom) | ||
| } | ||
| case "networks": | ||
| for _, net := range field.Create.Set.([]*InstanceNetwork) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instances now report their network interfaces properly - name, tap device, autoconfig - and can configure them on create, including a relay that routes all of an instance's traffic through another instance's interface. The platform has had relays since https://github.com/unikraft-cloud/platform/pull/846 but there was no CLI surface for them at all.
Reaching the relay from the flat
key=valueform needed dotted keys ininternal/resource/value, so--network relay.name=my-router-eth0,relay.dns=falseaddresses the nested field directly instead of every nested type having to invent a text form that packs its subfields into one value.One caveat worth knowing:
network_interfacesis gated behind the net-manager permission, and the API rejects the whole field as an unknown member rather than a permission error, so--networkreads as broken rather than forbidden on an account without it.Fixes https://linear.app/unikraft/issue/TOOL-1409/relays-in-the-cli.