diff --git a/api/v1alpha1/vpcattachment_types.go b/api/v1alpha1/vpcattachment_types.go index 8115c26..13cbe33 100644 --- a/api/v1alpha1/vpcattachment_types.go +++ b/api/v1alpha1/vpcattachment_types.go @@ -70,7 +70,7 @@ type IPAddress string // VPCAttachmentInterfaceMode is how the workload consumes the interface. It // describes the guest, not the data plane, so a change of implementation on the // data plane side does not move this API. -// +kubebuilder:validation:Enum=Netns;Hypervisor +// +kubebuilder:validation:Enum=Netns;Hypervisor;HypervisorDeclared type VPCAttachmentInterfaceMode string const ( @@ -81,6 +81,15 @@ const ( // VPCAttachmentInterfaceModeHypervisor hands the interface to a hypervisor as // a device, which is what a virtual machine guest consumes. VPCAttachmentInterfaceModeHypervisor VPCAttachmentInterfaceMode = "Hypervisor" + + // VPCAttachmentInterfaceModeHypervisorDeclared also hands the interface to a + // hypervisor as a device. It differs from Hypervisor in who tells the + // hypervisor that the device exists. Under Hypervisor the hypervisor finds + // the device from what the node publishes. Under HypervisorDeclared the data + // plane states the device, its addresses, and its MTU to the hypervisor + // directly, which is what a guest whose hypervisor reads no node state + // needs. + VPCAttachmentInterfaceModeHypervisorDeclared VPCAttachmentInterfaceMode = "HypervisorDeclared" ) // VPCAttachmentInterface defines the network interface details. diff --git a/cmd/main.go b/cmd/main.go index 6e55155..f499299 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -65,7 +65,7 @@ func main() { flag.BoolVar(&enableLeaderElection, "leader-elect", true, "Enable leader election. A single writer is what makes identifier allocation safe.") flag.StringVar(&rawAttachmentMode, "attachment-mode", "", - "Required. How guests in this cell consume an interface: Netns or Hypervisor.") + "Required. How guests in this cell consume an interface, for any interface that states no mode of its own: Netns, Hypervisor or HypervisorDeclared.") flag.IntVar(&webhookPort, "webhook-port", 9443, "Port the admission webhook server binds to.") flag.StringVar(&webhookCertDir, "webhook-cert-dir", "/tmp/k8s-webhook-server/serving-certs", "Directory holding the webhook server's tls.crt and tls.key.") @@ -160,9 +160,11 @@ func parseAttachmentMode(value string) (cloudv1alpha1.VPCAttachmentInterfaceMode return cloudv1alpha1.VPCAttachmentInterfaceModeNetns, nil case cloudv1alpha1.VPCAttachmentInterfaceModeHypervisor: return cloudv1alpha1.VPCAttachmentInterfaceModeHypervisor, nil + case cloudv1alpha1.VPCAttachmentInterfaceModeHypervisorDeclared: + return cloudv1alpha1.VPCAttachmentInterfaceModeHypervisorDeclared, nil case "": return "", errors.New("--attachment-mode is required: set Netns for container cells or Hypervisor for microVM cells") default: - return "", fmt.Errorf("--attachment-mode %q is not one of Netns, Hypervisor", value) + return "", fmt.Errorf("--attachment-mode %q is not one of Netns, Hypervisor, HypervisorDeclared", value) } } diff --git a/config/crd/cloud.datumapis.com_vpcattachments.yaml b/config/crd/cloud.datumapis.com_vpcattachments.yaml index b587ed3..d49ec0b 100644 --- a/config/crd/cloud.datumapis.com_vpcattachments.yaml +++ b/config/crd/cloud.datumapis.com_vpcattachments.yaml @@ -61,6 +61,7 @@ spec: enum: - Netns - Hypervisor + - HypervisorDeclared type: string name: default: eth0 diff --git a/docs/api/vpc.md b/docs/api/vpc.md index d5a70bd..537f5ee 100644 --- a/docs/api/vpc.md +++ b/docs/api/vpc.md @@ -186,7 +186,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `name` _string_ | Name of the interface (e.g., eth0). | | | -| `mode` _[VPCAttachmentInterfaceMode](#vpcattachmentinterfacemode)_ | Mode is how the workload consumes the interface, resolved and written by
the attachment controller rather than by whoever runs the workload. | Netns | Enum: [Netns Hypervisor]
| +| `mode` _[VPCAttachmentInterfaceMode](#vpcattachmentinterfacemode)_ | Mode is how the workload consumes the interface, resolved and written by
the attachment controller rather than by whoever runs the workload. | Netns | Enum: [Netns Hypervisor HypervisorDeclared]
| | `addresses` _[IPAddress](#ipaddress) array_ | A list of IPv4 or IPv6 addresses associated with the interface. Empty when
the guest manages its own addressing. | | MaxItems: 16
MaxLength: 64
| @@ -199,7 +199,7 @@ describes the guest, not the data plane, so a change of implementation on the data plane side does not move this API. _Validation:_ -- Enum: [Netns Hypervisor] +- Enum: [Netns Hypervisor HypervisorDeclared] _Appears in:_ - [VPCAttachmentInterface](#vpcattachmentinterface) @@ -208,6 +208,7 @@ _Appears in:_ | --- | --- | | `Netns` | VPCAttachmentInterfaceModeNetns moves the interface into the workload's
network namespace, which is what a container consumes.
| | `Hypervisor` | VPCAttachmentInterfaceModeHypervisor hands the interface to a hypervisor as
a device, which is what a virtual machine guest consumes.
| +| `HypervisorDeclared` | VPCAttachmentInterfaceModeHypervisorDeclared also hands the interface to a
hypervisor as a device. It differs from Hypervisor in who tells the
hypervisor that the device exists. Under Hypervisor the hypervisor finds
the device from what the node publishes. Under HypervisorDeclared the data
plane states the device, its addresses, and its MTU to the hypervisor
directly, which is what a guest whose hypervisor reads no node state
needs.
| #### VPCAttachmentSpec diff --git a/go.mod b/go.mod index c4857a9..bc6038a 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,12 @@ require ( github.com/kenshaw/baseconv v0.1.1 go.datum.net/compute v0.8.0-dev.7.0.20260821003916-1a0e4d6443f0 go.datum.net/network v0.0.0-20260819160013-45d0ff9deaee - go.datum.net/network-services-operator v0.26.1-0.20260820201844-f366b960529b + // UNMERGED: pinned to the head of network-services-operator#450, which + // adds the HypervisorDeclared attachment mode this repo now acts on. The + // commit sits on top of main and carries nothing else. That branch may be + // force-pushed or deleted, which would break `go mod download` here, so + // re-pin to a main commit or a tag as soon as it merges. + go.datum.net/network-services-operator v0.26.5-0.20260911224752-0ac10aee98bb go.miloapis.com/ipam v0.3.2-0.20260813184449-4fac0aa96194 k8s.io/api v0.36.3 k8s.io/apimachinery v0.36.3 diff --git a/go.sum b/go.sum index 50ca567..ea3f882 100644 --- a/go.sum +++ b/go.sum @@ -125,8 +125,8 @@ go.datum.net/compute v0.8.0-dev.7.0.20260821003916-1a0e4d6443f0 h1:EDzArN7AXyfWz go.datum.net/compute v0.8.0-dev.7.0.20260821003916-1a0e4d6443f0/go.mod h1:HEyoohOD3mQxkUsnlDIgRrh9L9P8j7nVtiFB1+utIAA= go.datum.net/network v0.0.0-20260819160013-45d0ff9deaee h1:7tA+0C1pb/fu/wrgB3Vu+P3nOJK4aNnKaUeB5wzrxVY= go.datum.net/network v0.0.0-20260819160013-45d0ff9deaee/go.mod h1:dqzM8WZczbiZ9bCvsxjkoI10GJqQ24NVWnc9boXgOkE= -go.datum.net/network-services-operator v0.26.1-0.20260820201844-f366b960529b h1:Qkh/+0XW+JXVXx89H/vyG1KBl+I0jLnKNiQSB8UgUQA= -go.datum.net/network-services-operator v0.26.1-0.20260820201844-f366b960529b/go.mod h1:A7JNOuc+e6j/KkUVCcZ7Z2odvf6JFMQlX0Zo1Awj2TY= +go.datum.net/network-services-operator v0.26.5-0.20260911224752-0ac10aee98bb h1:vNXRwZC6cozozlH+DvMjm9Ck5TdvM0fjwgDJ2m1VPrY= +go.datum.net/network-services-operator v0.26.5-0.20260911224752-0ac10aee98bb/go.mod h1:9nuuBWdrkdnIBMaWJsWM3j4CcbKJIF1GDuCmAjdpIHo= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= diff --git a/internal/controller/networkinterface_controller.go b/internal/controller/networkinterface_controller.go index 4a6db7b..2a08829 100644 --- a/internal/controller/networkinterface_controller.go +++ b/internal/controller/networkinterface_controller.go @@ -75,7 +75,8 @@ type NetworkInterfaceReconciler struct { // AttachmentMode is how guests in this cell consume an interface. It is // required configuration standing in for a capability class that does not - // exist yet, so a cell states what it is rather than defaulting. + // exist yet, so a cell states what it is rather than defaulting. It covers + // every interface that states no mode of its own. AttachmentMode cloudv1alpha1.VPCAttachmentInterfaceMode } @@ -158,7 +159,7 @@ func (r *NetworkInterfaceReconciler) reconcileAttachment( attachment.Spec.VPC = cloudv1alpha1.VPCRef{Name: vpc.Name} attachment.Spec.InterfaceRef = &cloudv1alpha1.NetworkInterfaceRef{Name: networkInterface.Name} attachment.Spec.Interface.Name = networkInterface.Spec.InterfaceName - attachment.Spec.Interface.Mode = r.AttachmentMode + attachment.Spec.Interface.Mode = r.attachmentMode(networkInterface) attachment.Spec.Interface.Addresses = interfaceAddresses(networkInterface) return controllerutil.SetControllerReference(networkInterface, attachment, r.Scheme) }); err != nil { @@ -200,7 +201,8 @@ func (r *NetworkInterfaceReconciler) reconcileNAD( }) } config, err := galactic.ConflistJSON(attachment.Name, masterPlugin(attachment.Spec.Interface.Mode), - vpc.Status.VPC, attachmentID, networkInterface.Spec.MTU, addresses) + vpc.Status.VPC, attachmentID, networkInterface.Spec.MTU, addresses, + declaresDevice(attachment.Spec.Interface.Mode)) if err != nil { return nil, err } @@ -219,13 +221,41 @@ func (r *NetworkInterfaceReconciler) reconcileNAD( return nad, nil } +// attachmentMode resolves how one guest consumes its interface. An interface +// that states a mode carries the workload's own requirement, so it wins. The +// cell-wide mode covers everything else, which is every interface written +// before a workload could state one. +func (r *NetworkInterfaceReconciler) attachmentMode( + networkInterface *networkingv1alpha.NetworkInterface, +) cloudv1alpha1.VPCAttachmentInterfaceMode { + switch networkInterface.Spec.AttachmentMode { + case networkingv1alpha.NetworkInterfaceAttachmentModeNetns: + return cloudv1alpha1.VPCAttachmentInterfaceModeNetns + case networkingv1alpha.NetworkInterfaceAttachmentModeHypervisor: + return cloudv1alpha1.VPCAttachmentInterfaceModeHypervisor + case networkingv1alpha.NetworkInterfaceAttachmentModeHypervisorDeclared: + return cloudv1alpha1.VPCAttachmentInterfaceModeHypervisorDeclared + default: + return r.AttachmentMode + } +} + // masterPlugin translates how a guest consumes an interface into the galactic // binary that realizes it. This is the only place the two vocabularies meet. func masterPlugin(mode cloudv1alpha1.VPCAttachmentInterfaceMode) string { - if mode == cloudv1alpha1.VPCAttachmentInterfaceModeHypervisor { + switch mode { + case cloudv1alpha1.VPCAttachmentInterfaceModeHypervisor, + cloudv1alpha1.VPCAttachmentInterfaceModeHypervisorDeclared: return galactic.PluginTap + default: + return galactic.PluginVeth } - return galactic.PluginVeth +} + +// declaresDevice reports whether the tap plugin describes the device to the +// hypervisor instead of leaving the hypervisor to discover it. +func declaresDevice(mode cloudv1alpha1.VPCAttachmentInterfaceMode) bool { + return mode == cloudv1alpha1.VPCAttachmentInterfaceModeHypervisorDeclared } // interfaceAddresses copies the addresses NSO allocated onto the attachment, so diff --git a/internal/controller/networkinterface_controller_test.go b/internal/controller/networkinterface_controller_test.go index a2d7f89..a5f57e1 100644 --- a/internal/controller/networkinterface_controller_test.go +++ b/internal/controller/networkinterface_controller_test.go @@ -34,6 +34,7 @@ func TestMasterPlugin(t *testing.T) { }{ {cloudv1alpha1.VPCAttachmentInterfaceModeNetns, galactic.PluginVeth}, {cloudv1alpha1.VPCAttachmentInterfaceModeHypervisor, galactic.PluginTap}, + {cloudv1alpha1.VPCAttachmentInterfaceModeHypervisorDeclared, galactic.PluginTap}, } for _, test := range tests { t.Run(string(test.mode), func(t *testing.T) { @@ -83,3 +84,46 @@ func TestClaimFulfilled(t *testing.T) { }) } } + +// The cell-wide mode is the only signal every attachment written until now +// carries, so it has to keep deciding for all of them. An interface that +// states its own mode is the exception. +func TestAttachmentModeFallsBackToTheCell(t *testing.T) { + r := &NetworkInterfaceReconciler{ + AttachmentMode: cloudv1alpha1.VPCAttachmentInterfaceModeNetns, + } + + tests := []struct { + name string + iface networkingv1alpha.NetworkInterfaceAttachmentMode + want cloudv1alpha1.VPCAttachmentInterfaceMode + }{ + {"unset", "", cloudv1alpha1.VPCAttachmentInterfaceModeNetns}, + {"netns", networkingv1alpha.NetworkInterfaceAttachmentModeNetns, + cloudv1alpha1.VPCAttachmentInterfaceModeNetns}, + {"hypervisor", networkingv1alpha.NetworkInterfaceAttachmentModeHypervisor, + cloudv1alpha1.VPCAttachmentInterfaceModeHypervisor}, + {"declared", networkingv1alpha.NetworkInterfaceAttachmentModeHypervisorDeclared, + cloudv1alpha1.VPCAttachmentInterfaceModeHypervisorDeclared}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + networkInterface := &networkingv1alpha.NetworkInterface{ + Spec: networkingv1alpha.NetworkInterfaceSpec{AttachmentMode: test.iface}, + } + if got := r.attachmentMode(networkInterface); got != test.want { + t.Errorf("got %q, want %q", got, test.want) + } + }) + } +} + +// Only the declared mode asks the tap plugin to describe the device. +func TestDeclaresDevice(t *testing.T) { + if declaresDevice(cloudv1alpha1.VPCAttachmentInterfaceModeHypervisor) { + t.Error("a discovered hypervisor attachment must not ask for a description") + } + if !declaresDevice(cloudv1alpha1.VPCAttachmentInterfaceModeHypervisorDeclared) { + t.Error("a declared hypervisor attachment must ask for a description") + } +} diff --git a/internal/galactic/galactic.go b/internal/galactic/galactic.go index d49da5c..4e82cf9 100644 --- a/internal/galactic/galactic.go +++ b/internal/galactic/galactic.go @@ -83,7 +83,12 @@ type MasterPlugin struct { VPCAttachment string `json:"vpcattachment"` Namespace string `json:"namespace"` MTU int32 `json:"mtu,omitempty"` - IPAM *IPAM `json:"ipam,omitempty"` + // DAN asks the tap plugin to describe the device to the hypervisor in a + // file the hypervisor reads, rather than leaving it to be discovered from + // the node. Only the tap plugin reads it, and it is omitted when false so + // every conflist rendered until now is byte-identical. + DAN bool `json:"dan,omitempty"` + IPAM *IPAM `json:"ipam,omitempty"` } // BGPPlugin is the galactic-bgp stanza. It is never optional: the master plugin @@ -112,14 +117,16 @@ type Address struct { // Conflist renders the conflist for one attachment. Addresses are the addresses // NSO already allocated; an empty list means the guest addresses itself and no -// IPAM block is emitted. -func Conflist(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Address) NetConfList { +// IPAM block is emitted. Set dan for a guest whose hypervisor is handed the +// device rather than discovering it. +func Conflist(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Address, dan bool) NetConfList { master := MasterPlugin{ Type: plugin, VPC: vpc, VPCAttachment: vpcAttachment, Namespace: SystemNamespace, MTU: mtu, + DAN: dan, } if len(addresses) > 0 { master.IPAM = &IPAM{Type: PluginIPAM, Addresses: addresses} @@ -135,8 +142,8 @@ func Conflist(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Ad } // ConflistJSON renders the conflist as the string a NAD's spec.config holds. -func ConflistJSON(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Address) (string, error) { - raw, err := json.Marshal(Conflist(name, plugin, vpc, vpcAttachment, mtu, addresses)) +func ConflistJSON(name, plugin, vpc, vpcAttachment string, mtu int32, addresses []Address, dan bool) (string, error) { + raw, err := json.Marshal(Conflist(name, plugin, vpc, vpcAttachment, mtu, addresses, dan)) if err != nil { return "", fmt.Errorf("marshal CNI conflist: %w", err) } diff --git a/internal/galactic/galactic_test.go b/internal/galactic/galactic_test.go index e800fcc..d1ac56a 100644 --- a/internal/galactic/galactic_test.go +++ b/internal/galactic/galactic_test.go @@ -27,7 +27,7 @@ func TestConflistChainIsComplete(t *testing.T) { []Address{ {Address: "fd00:10:ff01:0:1::1/96", Gateway: "fd00:10:ff01::1"}, {Address: "172.20.1.7/32", Gateway: "172.20.1.1"}, - }) + }, false) if conflist.CNIVersion != "1.0.0" { t.Errorf("cniVersion: got %q, want %q", conflist.CNIVersion, "1.0.0") @@ -64,7 +64,7 @@ func TestConflistChainIsComplete(t *testing.T) { } func TestConflistOmitsIPAMForSelfAddressingGuest(t *testing.T) { - raw, err := ConflistJSON("web-eth0", PluginTap, "0000000jU", "01a", 0, nil) + raw, err := ConflistJSON("web-eth0", PluginTap, "0000000jU", "01a", 0, nil, false) if err != nil { t.Fatalf("ConflistJSON: %v", err) } @@ -126,3 +126,35 @@ func TestSplitAdvertisementName(t *testing.T) { }) } } + +// The tap plugin reads this one field to decide whether it describes the device +// to the hypervisor. Every attachment rendered until now leaves it out, so its +// absence has to stay the default. +func TestConflistCarriesTheDeclaredDeviceRequest(t *testing.T) { + declared, err := ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 1400, nil, true) + if err != nil { + t.Fatalf("ConflistJSON: %v", err) + } + discovered, err := ConflistJSON("vm-eth0", PluginTap, "0000000jU", "01a", 1400, nil, false) + if err != nil { + t.Fatalf("ConflistJSON: %v", err) + } + + if got := masterStanza(t, declared)["dan"]; got != true { + t.Errorf("declared attachment: got %v, want true", got) + } + if _, present := masterStanza(t, discovered)["dan"]; present { + t.Errorf("discovered attachment carries the field: %s", discovered) + } +} + +func masterStanza(t *testing.T, raw string) map[string]any { + t.Helper() + var decoded struct { + Plugins []map[string]any `json:"plugins"` + } + if err := json.Unmarshal([]byte(raw), &decoded); err != nil { + t.Fatalf("unmarshal conflist: %v", err) + } + return decoded.Plugins[0] +}