Skip to content

Commit

Permalink
Preserve backwards compatibility of CreateNetworkStatus
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
maiqueb committed Sep 30, 2024
1 parent 506cfda commit ed6baad
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/utils/net-attach-def.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
42 changes: 42 additions & 0 deletions pkg/utils/net-attach-def_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 := `
[{
Expand Down

0 comments on commit ed6baad

Please sign in to comment.