From ed6baad2ba6958edf56e8437d78ad8fe9609738c Mon Sep 17 00:00:00 2001 From: Miguel Duarte Barroso Date: Mon, 30 Sep 2024 13:10:07 +0200 Subject: [PATCH] Preserve backwards compatibility of CreateNetworkStatus The CNI spec - [0] - states that the `sandbox` attribute in the CNI result should be used to represent: ``` The isolation domain reference (e.g. path to network namespace) for the interface, or empty if on the host. For interfaces created inside the container, this should be the value passed via CNI_NETNS ``` Some CNIs (like calico) do **not** set the sandbox for their interfaces (below you'll find an example of a cached CNI result created by calico CNI): ``` { ... "result": { "cniVersion": "0.3.1", "dns": {}, "interfaces": [ { "name": "cali05f4a1849c5" } ], "ips": [ { "address": "10.244.196.152/32", "version": "4" }, { "address": "fd10:244::c497/128", "version": "6" } ] } } ``` Thus, when multus started to use `CreateNetworkStatuses` (plural) we broke calico users relying on the network-status annotations (since that function relies on the sandbox attribute being defined to identify container interfaces). This commit changes the code to use the original status creation whenever `CreateNetworkStatuses` is created with a single interface, thus ensuring the original behavior will be provided to user. Unit tests are also added. [0] - https://github.com/containernetworking/cni/blob/main/SPEC.md#add-success Signed-off-by: Miguel Duarte Barroso --- pkg/utils/net-attach-def.go | 5 ++++ pkg/utils/net-attach-def_test.go | 42 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/pkg/utils/net-attach-def.go b/pkg/utils/net-attach-def.go index 06502e81..a22f5579 100644 --- a/pkg/utils/net-attach-def.go +++ b/pkg/utils/net-attach-def.go @@ -136,6 +136,11 @@ func CreateNetworkStatuses(r cnitypes.Result, networkName string, defaultNetwork return nil, fmt.Errorf("error converting the type.Result to cni100.Result: %v", err) } + if len(result.Interfaces) == 1 { + networkStatus, err := CreateNetworkStatus(r, networkName, defaultNetwork, dev) + return []*v1.NetworkStatus{networkStatus}, err + } + // Discover default routes upfront and reuse them if necessary. var useDefaultRoute []string for _, route := range result.Routes { diff --git a/pkg/utils/net-attach-def_test.go b/pkg/utils/net-attach-def_test.go index b60701ce..0d5fa956 100644 --- a/pkg/utils/net-attach-def_test.go +++ b/pkg/utils/net-attach-def_test.go @@ -337,6 +337,48 @@ var _ = Describe("Netwok Attachment Definition manipulations", func() { }) }) + Context("create network statuses for a single interface which omits the sandbox info", func() { + var cniResult *cni100.Result + + BeforeEach(func() { + cniResult = &cni100.Result{ + CNIVersion: "1.1.0", + Interfaces: []*cni100.Interface{ + { + Name: "foo", + }, + }, + IPs: []*cni100.IPConfig{ + { + Address: *EnsureCIDR("10.244.196.152/32"), + }, + { + Address: *EnsureCIDR("fd10:244::c497/128"), + }, + }, + } + }) + + It("creates network statuses with a single entry", func() { + networkStatuses, err := CreateNetworkStatuses(cniResult, "test-default-net-without-sandbox", true, nil) + Expect(err).NotTo(HaveOccurred()) + + Expect(networkStatuses).To(WithTransform( + func(status []*v1.NetworkStatus) []*v1.NetworkStatus { + for i := range status { + status[i].DNS = v1.DNS{} + } + return status + }, ConsistOf( + &v1.NetworkStatus{ + Name: "test-default-net-without-sandbox", + IPs: []string{"10.244.196.152", "fd10:244::c497"}, + Default: true, + }, + ))) + }) + }) + It("parse network selection element in pod", func() { selectionElement := ` [{