From 4aee468ebaee18602c3a1ebdfff49e7338f9b16d Mon Sep 17 00:00:00 2001 From: Lionel Jouin Date: Thu, 7 Sep 2023 10:43:40 +0200 Subject: [PATCH] Controller: Set CNI args on the delegate request The CNI args has to be set in the delegate request so the cni-args in the network attachment annotation are considered. Signed-off-by: Lionel Jouin --- .github/workflows/check.yaml | 2 +- .golangci.yml | 1 - pkg/controller/pod.go | 16 ++++++-- pkg/controller/pod_test.go | 73 +++++++++++++++++++++++++++++++++--- 4 files changed, 82 insertions(+), 10 deletions(-) diff --git a/.github/workflows/check.yaml b/.github/workflows/check.yaml index 77518229..7efc0bd3 100644 --- a/.github/workflows/check.yaml +++ b/.github/workflows/check.yaml @@ -31,7 +31,7 @@ jobs: - name: Linters uses: golangci/golangci-lint-action@v3 with: - version: v1.48.0 + version: v1.54.2 args: --timeout 3m --verbose cmd/... pkg/... - name: Test diff --git a/.golangci.yml b/.golangci.yml index d9c32805..9ba13ee3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -58,7 +58,6 @@ linters: enable: - bodyclose - deadcode - - depguard - dogsled - dupl - errcheck diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go index f3fbcc2f..19fa9415 100644 --- a/pkg/controller/pod.go +++ b/pkg/controller/pod.go @@ -243,7 +243,7 @@ func (pnc *PodNetworksController) processNextWorkItem() bool { PodNetNS: netnsPath, }) if err != nil { - klog.Errorf("error removing attachments: %v") + klog.Errorf("error removing attachments: %v", err) return true } } @@ -440,11 +440,20 @@ func podContainerID(pod *corev1.Pod) string { } func addIfaceEventFormat(pod *corev1.Pod, network *nadv1.NetworkSelectionElement) string { + attributes := "" + if len(network.IPRequest) > 0 || network.MacRequest != "" || network.CNIArgs != nil { + attributes = fmt.Sprintf("(ips: %v, mac: %s, cni-args: %v)", + network.IPRequest, + network.MacRequest, + network.CNIArgs, + ) + } return fmt.Sprintf( - "pod [%s]: added interface %s to network: %s", + "pod [%s]: added interface %s to network: %s%s", annotations.NamespacedName(pod.GetNamespace(), pod.GetName()), network.InterfaceRequest, network.Name, + attributes, ) } @@ -465,10 +474,11 @@ func rejectInterfaceAddEventFormat(pod *corev1.Pod) string { } func interfaceAttributes(networkData nadv1.NetworkSelectionElement) *multusapi.DelegateInterfaceAttributes { - if len(networkData.IPRequest) > 0 || networkData.MacRequest != "" { + if len(networkData.IPRequest) > 0 || networkData.MacRequest != "" || networkData.CNIArgs != nil { return &multusapi.DelegateInterfaceAttributes{ IPRequest: networkData.IPRequest, MacRequest: networkData.MacRequest, + CNIArgs: networkData.CNIArgs, } } return nil diff --git a/pkg/controller/pod_test.go b/pkg/controller/pod_test.go index da0677a3..3ae6b8cd 100644 --- a/pkg/controller/pod_test.go +++ b/pkg/controller/pod_test.go @@ -66,11 +66,13 @@ var _ = Describe("Dynamic Attachment controller", func() { Context("with an existing running pod", func() { const ( cniVersion = "0.3.0" + ipAddr = "172.16.0.1" macAddr = "02:03:04:05:06:07" namespace = "default" networkName = "tiny-net" podName = "tiny-winy-pod" ) + cniArgs := &map[string]string{"foo": "bar"} var ( eventRecorder *record.FakeRecorder k8sClient k8sclient.Interface @@ -168,8 +170,8 @@ var _ = Describe("Dynamic Attachment controller", func() { } return status, nil }).Should(ConsistOf( - ifaceStatus(namespace, networkName, "net0", ""), - ifaceStatus(namespace, networkToAdd, "net1", macAddr))) + ifaceStatusForDefaultNamespace(networkName, "net0", ""), + ifaceStatusForDefaultNamespace(networkToAdd, "net1", macAddr))) }) }) @@ -233,7 +235,7 @@ var _ = Describe("Dynamic Attachment controller", func() { } return status, nil }).Should(ConsistOf( - ifaceStatus(namespace, networkName, "net0", ""))) + ifaceStatusForDefaultNamespace(networkName, "net0", ""))) }) It("throws an event indicating the interface add operation is rejected", func() { @@ -244,6 +246,61 @@ var _ = Describe("Dynamic Attachment controller", func() { Eventually(<-eventRecorder.Events).Should(Equal(expectedEventPayload)) }) }) + + When("an attachment is added with attributes (IPs, MAC, cni-args)", func() { + JustBeforeEach(func() { + pod = updatePodSpec(pod) + netSelectionElements := append(generateNetworkSelectionElements(namespace, networkName), + nad.NetworkSelectionElement{ + Name: networkToAdd, + Namespace: namespace, + InterfaceRequest: "net1", + IPRequest: []string{ipAddr}, + MacRequest: macAddr, + CNIArgs: &map[string]interface{}{ + "foo": "bar", + }, + }, + ) + serelizedNetSelectionElements, _ := json.Marshal(netSelectionElements) + pod.Annotations[nad.NetworkAttachmentAnnot] = string(serelizedNetSelectionElements) + _, err := k8sClient.CoreV1().Pods(namespace).UpdateStatus( + context.TODO(), + pod, + metav1.UpdateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + + It("an `AddedInterface` event is seen in the event recorded", func() { + expectedEventPayload := fmt.Sprintf( + "Normal AddedInterface pod [%s]: added interface %s to network: %s(ips: [%s], mac: %s, cni-args: %v)", + annotations.NamespacedName(namespace, podName), + "net1", + networkToAdd, + ipAddr, + macAddr, + cniArgs, + ) + Eventually(<-eventRecorder.Events).Should(Equal(expectedEventPayload)) + }) + + It("the pod network-status is updated with the new network attachment", func() { + Eventually(func() ([]nad.NetworkStatus, error) { + updatedPod, err := k8sClient.CoreV1().Pods(namespace).Get(context.TODO(), podName, metav1.GetOptions{}) + if err != nil { + return nil, err + } + status, err := annotations.PodDynamicNetworkStatus(updatedPod) + if err != nil { + return nil, err + } + return status, nil + }).Should(ConsistOf( + ifaceStatusForDefaultNamespace(networkName, "net0", ""), + ifaceStatusForDefaultNamespace(networkToAdd, "net1", macAddr))) + }) + }) + }) }) }) @@ -425,7 +482,7 @@ func podNetworkConfig(networkNames ...string) map[string]string { } } -func generateNetworkSelectionAnnotation(namespace string, networkNames ...string) string { +func generateNetworkSelectionElements(namespace string, networkNames ...string) []nad.NetworkSelectionElement { var netSelectionElements []nad.NetworkSelectionElement for i, networkName := range networkNames { netSelectionElements = append( @@ -439,6 +496,11 @@ func generateNetworkSelectionAnnotation(namespace string, networkNames ...string if netSelectionElements == nil { netSelectionElements = make([]nad.NetworkSelectionElement, 0) } + return netSelectionElements +} + +func generateNetworkSelectionAnnotation(namespace string, networkNames ...string) string { + netSelectionElements := generateNetworkSelectionElements(namespace, networkNames...) serelizedNetSelectionElements, err := json.Marshal(netSelectionElements) if err != nil { return "" @@ -479,7 +541,8 @@ func dummyMultusConfig() string { }` } -func ifaceStatus(namespace, networkName, ifaceName, macAddress string) nad.NetworkStatus { +func ifaceStatusForDefaultNamespace(networkName, ifaceName, macAddress string) nad.NetworkStatus { + const namespace = "default" return nad.NetworkStatus{ Name: fmt.Sprintf("%s/%s", namespace, networkName), Interface: ifaceName,