From d9bad91368b9209a1f52b42cdbeee6d81ee0e8c5 Mon Sep 17 00:00:00 2001 From: Miguel Duarte Barroso Date: Wed, 26 Oct 2022 13:09:30 +0200 Subject: [PATCH] controller: update status via the net-attach-def-client lib Signed-off-by: Miguel Duarte Barroso --- pkg/annotations/dynamic-network-status.go | 25 ++---- .../dynamic-network-status_test.go | 90 ++++++++++++++++--- pkg/controller/pod.go | 15 +--- 3 files changed, 87 insertions(+), 43 deletions(-) diff --git a/pkg/annotations/dynamic-network-status.go b/pkg/annotations/dynamic-network-status.go index 85da7dad..a79a3501 100644 --- a/pkg/annotations/dynamic-network-status.go +++ b/pkg/annotations/dynamic-network-status.go @@ -11,10 +11,10 @@ import ( multusapi "gopkg.in/k8snetworkplumbingwg/multus-cni.v3/pkg/server/api" ) -func AddDynamicIfaceToStatus(currentPod *corev1.Pod, networkSelectionElement *nettypes.NetworkSelectionElement, response *multusapi.Response) (string, error) { +func AddDynamicIfaceToStatus(currentPod *corev1.Pod, networkSelectionElement *nettypes.NetworkSelectionElement, response *multusapi.Response) ([]nettypes.NetworkStatus, error) { currentIfaceStatus, err := podDynamicNetworkStatus(currentPod) if err != nil { - return "", err + return nil, err } if response != nil && response.Result != nil { @@ -25,22 +25,18 @@ func AddDynamicIfaceToStatus(currentPod *corev1.Pod, networkSelectionElement *ne nil, ) if err != nil { - return "", fmt.Errorf("failed to create NetworkStatus from the response: %v", err) + return nil, fmt.Errorf("failed to create NetworkStatus from the response: %v", err) } - newIfaceString, err := json.Marshal(append(currentIfaceStatus, *newIfaceStatus)) - if err != nil { - return "", fmt.Errorf("failed to marshall the dynamic networks status after interface creation") - } - return string(newIfaceString), nil + return append(currentIfaceStatus, *newIfaceStatus), nil } - return "", fmt.Errorf("got an empty response from multus: %+v", response) + return nil, fmt.Errorf("got an empty response from multus: %+v", response) } -func DeleteDynamicIfaceFromStatus(currentPod *corev1.Pod, networkSelectionElement *nettypes.NetworkSelectionElement) (string, error) { +func DeleteDynamicIfaceFromStatus(currentPod *corev1.Pod, networkSelectionElement *nettypes.NetworkSelectionElement) ([]nettypes.NetworkStatus, error) { currentIfaceStatus, err := podDynamicNetworkStatus(currentPod) if err != nil { - return "", err + return nil, err } netName := NamespacedName(networkSelectionElement.Namespace, networkSelectionElement.Name) @@ -52,12 +48,7 @@ func DeleteDynamicIfaceFromStatus(currentPod *corev1.Pod, networkSelectionElemen } newIfaceStatus = append(newIfaceStatus, currentIfaceStatus[i]) } - - newIfaceString, err := json.Marshal(newIfaceStatus) - if err != nil { - return "", fmt.Errorf("failed to marshall the dynamic networks status after deleting interface") - } - return string(newIfaceString), nil + return newIfaceStatus, nil } func podDynamicNetworkStatus(currentPod *corev1.Pod) ([]nettypes.NetworkStatus, error) { diff --git a/pkg/annotations/dynamic-network-status_test.go b/pkg/annotations/dynamic-network-status_test.go index 22ea5e24..0b3e2df9 100644 --- a/pkg/annotations/dynamic-network-status_test.go +++ b/pkg/annotations/dynamic-network-status_test.go @@ -19,16 +19,14 @@ import ( var _ = Describe("NetworkStatusFromResponse", func() { const ( ifaceName = "ens32" + ifaceToAdd = "newiface" + macAddr = "02:03:04:05:06:07" namespace = "ns1" networkName = "tenantnetwork" podName = "tpod" ) - DescribeTable("add dynamic interface to network status", func(initialNetStatus []nadv1.NetworkStatus, resultIPs []string, expectedNetworkStatus string) { - const ( - ifaceToAdd = "newiface" - macAddr = "02:03:04:05:06:07" - ) + DescribeTable("add dynamic interface to network status", func(initialNetStatus []nadv1.NetworkStatus, resultIPs []string, expectedNetworkStatus []nadv1.NetworkStatus) { Expect( AddDynamicIfaceToStatus( newPod(podName, namespace, initialNetStatus...), @@ -37,7 +35,18 @@ var _ = Describe("NetworkStatusFromResponse", func() { ), ).To(Equal(expectedNetworkStatus)) }, - Entry("initial empty pod", []nadv1.NetworkStatus{}, nil, `[{"name":"ns1/tenantnetwork","interface":"newiface","mac":"02:03:04:05:06:07","dns":{}}]`), + Entry("initial empty pod", []nadv1.NetworkStatus{}, nil, []nadv1.NetworkStatus{ + { + Name: NamespacedName(namespace, networkName), + Interface: ifaceToAdd, + Mac: macAddr, + DNS: nadv1.DNS{ + Nameservers: []string{}, + Domain: "", + Search: []string{}, + Options: []string{}, + }, + }}), Entry("pod with a network present in the network status", []nadv1.NetworkStatus{ { Name: "net1", @@ -45,7 +54,24 @@ var _ = Describe("NetworkStatusFromResponse", func() { Mac: "00:00:00:20:10:00", }}, nil, - `[{"name":"net1","interface":"iface1","mac":"00:00:00:20:10:00","dns":{}},{"name":"ns1/tenantnetwork","interface":"newiface","mac":"02:03:04:05:06:07","dns":{}}]`), + []nadv1.NetworkStatus{ + { + Name: "net1", + Interface: "iface1", + Mac: "00:00:00:20:10:00", + }, + { + Name: NamespacedName(namespace, networkName), + Interface: ifaceToAdd, + Mac: macAddr, + DNS: nadv1.DNS{ + Nameservers: []string{}, + Domain: "", + Search: []string{}, + Options: []string{}, + }, + }}, + ), Entry("result with IPs", []nadv1.NetworkStatus{ { Name: "net1", @@ -53,9 +79,27 @@ var _ = Describe("NetworkStatusFromResponse", func() { Mac: "00:00:00:20:10:00", }}, []string{"10.10.10.10/24"}, - `[{"name":"net1","interface":"iface1","mac":"00:00:00:20:10:00","dns":{}},{"name":"ns1/tenantnetwork","interface":"newiface","ips":["10.10.10.10"],"mac":"02:03:04:05:06:07","dns":{}}]`)) + []nadv1.NetworkStatus{ + { + Name: "net1", + Interface: "iface1", + Mac: "00:00:00:20:10:00", + }, + { + Name: NamespacedName(namespace, networkName), + Interface: ifaceToAdd, + Mac: macAddr, + IPs: []string{"10.10.10.10"}, + DNS: nadv1.DNS{ + Nameservers: []string{}, + Domain: "", + Search: []string{}, + Options: []string{}, + }, + }}, + )) - DescribeTable("remove an interface to the current network status", func(initialNetStatus []nadv1.NetworkStatus, networkName, ifaceToRemove, expectedNetworkStatus string) { + DescribeTable("remove an interface to the current network status", func(initialNetStatus []nadv1.NetworkStatus, networkName string, ifaceToRemove string, expectedNetworkStatus []nadv1.NetworkStatus) { Expect( DeleteDynamicIfaceFromStatus( newPod(podName, namespace, initialNetStatus...), @@ -63,19 +107,29 @@ var _ = Describe("NetworkStatusFromResponse", func() { ), ).To(Equal(expectedNetworkStatus)) }, - Entry("when there aren't any existing interfaces", nil, "net1", "iface1", "[]"), + Entry("when there aren't any existing interfaces", nil, "net1", "iface1", []nadv1.NetworkStatus{}), Entry("when we remove all the currently existing interfaces", []nadv1.NetworkStatus{ { Name: NamespacedName(namespace, networkName), Interface: "iface1", Mac: "00:00:00:20:10:00", - }}, networkName, "iface1", "[]"), + }}, networkName, "iface1", []nadv1.NetworkStatus{}), Entry("when there is *not* a matching interface to remove", []nadv1.NetworkStatus{ { Name: NamespacedName(namespace, networkName), Interface: "iface1", Mac: "00:00:00:20:10:00", - }}, "net2", "iface1", `[{"name":"ns1/tenantnetwork","interface":"iface1","mac":"00:00:00:20:10:00","dns":{}}]`), + }}, + "net2", + "iface1", + []nadv1.NetworkStatus{ + { + Name: NamespacedName(namespace, networkName), + Interface: "iface1", + Mac: "00:00:00:20:10:00", + }, + }, + ), Entry("when we remove one of the existing interfaces", []nadv1.NetworkStatus{ { Name: NamespacedName(namespace, networkName), @@ -87,7 +141,17 @@ var _ = Describe("NetworkStatusFromResponse", func() { Interface: "iface2", Mac: "aa:bb:cc:20:10:00", }, - }, "net2", "iface2", `[{"name":"ns1/tenantnetwork","interface":"iface1","mac":"00:00:00:20:10:00","dns":{}}]`)) + }, + "net2", + "iface2", + []nadv1.NetworkStatus{ + { + Name: NamespacedName(namespace, networkName), + Interface: "iface1", + Mac: "00:00:00:20:10:00", + }, + }, + )) }) func newPod(podName string, namespace string, netStatus ...nadv1.NetworkStatus) *corev1.Pod { diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go index 85a8368d..33c759b4 100644 --- a/pkg/controller/pod.go +++ b/pkg/controller/pod.go @@ -1,7 +1,6 @@ package controller import ( - "context" "encoding/json" "fmt" "reflect" @@ -9,7 +8,6 @@ import ( "time" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" v1coreinformerfactory "k8s.io/client-go/informers" @@ -283,7 +281,7 @@ func (pnc *PodNetworksController) addNetworks(dynamicAttachmentRequest *DynamicA return fmt.Errorf("failed to compute the updated network status: %v", err) } - if err := pnc.updatePodNetworkStatus(pod, newIfaceStatus); err != nil { + if err := nadutils.SetNetworkStatus(pnc.k8sClientSet, pod, newIfaceStatus); err != nil { return err } @@ -334,7 +332,7 @@ func (pnc *PodNetworksController) removeNetworks(dynamicAttachmentRequest *Dynam err, ) } - if err := pnc.updatePodNetworkStatus(pod, newIfaceStatus); err != nil { + if err := nadutils.SetNetworkStatus(pnc.k8sClientSet, pod, newIfaceStatus); err != nil { return err } @@ -344,15 +342,6 @@ func (pnc *PodNetworksController) removeNetworks(dynamicAttachmentRequest *Dynam return nil } -func (pnc *PodNetworksController) updatePodNetworkStatus(pod *corev1.Pod, newIfaceStatus string) error { - pod.Annotations[nadv1.NetworkStatusAnnot] = newIfaceStatus - - if _, err := pnc.k8sClientSet.CoreV1().Pods(pod.GetNamespace()).Update(context.Background(), pod, metav1.UpdateOptions{}); err != nil { - return fmt.Errorf("failed to update pod's network-status annotations for %s: %v", pod.GetName(), err) - } - return nil -} - func networkSelectionElements(podAnnotations map[string]string, podNamespace string) ([]*nadv1.NetworkSelectionElement, error) { podNetworks, ok := podAnnotations[nadv1.NetworkAttachmentAnnot] if !ok || podNetworks == "" {