e2e tests, hotplug: test hotplug interface

Introduce tests to check:
  - hotplug a new interface
  - remove an old interface

Signed-off-by: Miguel Duarte Barroso <[email protected]>

e2e/multus_test.go

@@ -19,11 +19,14 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"strings"
 	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/format"
+	"github.com/onsi/gomega/types"
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -87,9 +90,10 @@ var _ = Describe("Multus basic operations", func() {
 				firstPodName,
 				&firstNodeName,
 				nadv1.NetworkSelectionElement{
-					Name:      networkName,
-					Namespace: namespace,
-					IPRequest: []string{firstPodIP},
+					Name:             networkName,
+					Namespace:        namespace,
+					IPRequest:        []string{firstPodIP},
+					InterfaceRequest: "net1",
 				})
 
 			secondNodeName := secondNode.GetName()
@@ -153,9 +157,108 @@ var _ = Describe("Multus basic operations", func() {
 			Expect(nonDefaultNetworkStatus).NotTo(BeNil())
 			Expect(nonDefaultNetworkStatus.IPs).To(ConsistOf(ipFromCIDR(firstPodIP)))
 		})
+
+		Context("dynamic attachments", func() {
+			const (
+				newNetworkName      = "newnet2000"
+				updateStatusTimeout = 10 * time.Second
+			)
+
+			var (
+				newNetwork *nadv1.NetworkAttachmentDefinition
+			)
+
+			BeforeEach(func() {
+				var err error
+				newNetwork, err = clientset.nadClientSet.K8sCniCncfIoV1().NetworkAttachmentDefinitions(namespace).Create(
+					context.TODO(),
+					newMacvlanNetworkAttachmentDefinitionSpec(
+						namespace, newNetworkName, lowerDevice),
+					metav1.CreateOptions{})
+				Expect(err).NotTo(HaveOccurred(), "should have been able to create the NET-ATTACH-DEF")
+			})
+
+			AfterEach(func() {
+				Expect(clientset.nadClientSet.K8sCniCncfIoV1().NetworkAttachmentDefinitions(namespace).Delete(
+					context.TODO(), newNetwork.GetName(), metav1.DeleteOptions{})).NotTo(HaveOccurred(), "should have been able to delete the NET-ATTACH-DEF")
+			})
+
+			It("add a network to the pod", func() {
+				firstPod, err := clientset.k8sClientSet.CoreV1().Pods(namespace).Get(context.TODO(), firstPod.GetName(), metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred(), "should have been able to retrieve pod")
+
+				const (
+					newIfaceName = "pluggediface1"
+					newNetworkIP = "10.10.10.2/24"
+				)
+				Expect(addNetwork(firstPod, nadv1.NetworkSelectionElement{
+					Name:             newNetworkName,
+					Namespace:        namespace,
+					IPRequest:        []string{newNetworkIP},
+					InterfaceRequest: newIfaceName,
+				})).To(Succeed())
+				Expect(clientset.k8sClientSet.CoreV1().Pods(namespace).Update(context.TODO(), firstPod, metav1.UpdateOptions{})).NotTo(BeNil())
+				Eventually(func() ([]nadv1.NetworkStatus, error) {
+					pod, err := clientset.k8sClientSet.CoreV1().Pods(namespace).Get(context.TODO(), firstPod.GetName(), metav1.GetOptions{})
+					if err != nil {
+						return nil, err
+					}
+					statuses, err := podNetworkStatus(pod)
+					if err != nil {
+						return nil, err
+					}
+					return statuses, nil
+				}, updateStatusTimeout).Should(
+					ContainElements(
+						NetworkStatus(namespace, newNetworkName, newIfaceName, ipFromCIDR(newNetworkIP))))
+			})
+
+			It("remove a network from the pod", func() {
+				firstPod, err := clientset.k8sClientSet.CoreV1().Pods(namespace).Get(context.TODO(), firstPod.GetName(), metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred(), "should have been able to retrieve pod")
+
+				Expect(removeNetwork(firstPod, nadv1.NetworkSelectionElement{
+					Name:             networkName,
+					Namespace:        namespace,
+					InterfaceRequest: "net1",
+				})).To(Succeed())
+				Expect(clientset.k8sClientSet.CoreV1().Pods(namespace).Update(context.TODO(), firstPod, metav1.UpdateOptions{})).NotTo(BeNil())
+
+				Eventually(func() ([]nadv1.NetworkStatus, error) {
+					pod, err := clientset.k8sClientSet.CoreV1().Pods(namespace).Get(context.TODO(), firstPod.GetName(), metav1.GetOptions{})
+					if err != nil {
+						return nil, err
+					}
+					statuses, err := podNetworkStatus(pod)
+					if err != nil {
+						return nil, err
+					}
+					return statuses, nil
+				}, updateStatusTimeout).ShouldNot(
+					ContainElements(
+						NetworkStatus(namespace, newNetworkName, "net1", ipFromCIDR(firstPodIP))))
+			})
+		})
 	})
 })
 
+func namespacedNetworkName(namespace, networkName string) string {
+	return fmt.Sprintf("%s/%s", namespace, networkName)
+}
+
+func podNetworkStatus(pod *v1.Pod) ([]nadv1.NetworkStatus, error) {
+	var podNetworks []nadv1.NetworkStatus
+
+	networkList, wasFound := pod.Annotations[nadv1.NetworkStatusAnnot]
+	if !wasFound {
+		return podNetworks, fmt.Errorf("the pod is missing the status annotation")
+	}
+	if err := json.Unmarshal([]byte(networkList), &podNetworks); err != nil {
+		return nil, err
+	}
+	return podNetworks, nil
+}
+
 func newMacvlanNetworkAttachmentDefinitionSpec(namespace string, networkName string, lowerDevice string) *nadv1.NetworkAttachmentDefinition {
 	config := fmt.Sprintf(`{
             "cniVersion": "0.3.1",
@@ -237,3 +340,95 @@ func filterNetworkStatus(networkStatuses []nadv1.NetworkStatus, predicate func(n
 func ipFromCIDR(cidr string) string {
 	return strings.Split(cidr, "/")[0]
 }
+
+func addNetwork(pod *v1.Pod, networks ...nadv1.NetworkSelectionElement) error {
+	networkList, wasFound := pod.Annotations[nadv1.NetworkAttachmentAnnot]
+	if !wasFound {
+		return nil
+	}
+
+	var podNetworks []nadv1.NetworkSelectionElement
+	if err := json.Unmarshal([]byte(networkList), &podNetworks); err != nil {
+		return err
+	}
+
+	for _, net := range networks {
+		podNetworks = append(podNetworks, net)
+	}
+	updatedNetworksAnnotation, err := json.Marshal(podNetworks)
+	if err != nil {
+		return err
+	}
+
+	pod.Annotations[nadv1.NetworkAttachmentAnnot] = string(updatedNetworksAnnotation)
+	return nil
+}
+
+func removeNetwork(pod *v1.Pod, networks ...nadv1.NetworkSelectionElement) error {
+	networkList, wasFound := pod.Annotations[nadv1.NetworkAttachmentAnnot]
+	if !wasFound {
+		return nil
+	}
+
+	var currentPodNetworks, updatedPodNetworks []nadv1.NetworkSelectionElement
+	if err := json.Unmarshal([]byte(networkList), &currentPodNetworks); err != nil {
+		return err
+	}
+
+	updatedPodNetworks = []nadv1.NetworkSelectionElement{}
+	for _, net := range networks {
+		for _, existingNet := range currentPodNetworks {
+			if net.Name == existingNet.Name && net.Namespace == existingNet.Namespace {
+				continue
+			}
+			updatedPodNetworks = append(updatedPodNetworks, existingNet)
+		}
+	}
+	updatedNetworksAnnotation, err := json.Marshal(updatedPodNetworks)
+	if err != nil {
+		return err
+	}
+
+	pod.Annotations[nadv1.NetworkAttachmentAnnot] = string(updatedNetworksAnnotation)
+	return nil
+}
+
+// NetworkStatus uses reflect.DeepEqual to compare actual with expected.  Equal is strict about
+//types when performing comparisons.
+//It is an error for both actual and expected to be nil.  Use BeNil() instead.
+func NetworkStatus(namespace string, networkName string, ifaceName string, ip string) types.GomegaMatcher {
+	return &NetworkStatusMatcher{
+		Expected: &nadv1.NetworkStatus{
+			Name:      namespacedNetworkName(namespace, networkName),
+			Interface: ifaceName,
+			IPs:       []string{ip},
+			Default:   false,
+		},
+	}
+}
+
+type NetworkStatusMatcher struct {
+	Expected *nadv1.NetworkStatus
+}
+
+func (matcher *NetworkStatusMatcher) Match(actual interface{}) (success bool, err error) {
+	if actual == nil && matcher.Expected == nil {
+		return false, fmt.Errorf("refusing to compare <nil> to <nil>.\nBe explicit and use BeNil() instead.  This is to avoid mistakes where both sides of an assertion are erroneously uninitialized")
+	}
+
+	actualNetStatus, didTypeCastSucceed := actual.(nadv1.NetworkStatus)
+	if !didTypeCastSucceed {
+		return false, fmt.Errorf("incorrect type passed")
+	}
+	return actualNetStatus.Name == matcher.Expected.Name &&
+		reflect.DeepEqual(actualNetStatus.IPs, matcher.Expected.IPs) &&
+		actualNetStatus.Interface == matcher.Expected.Interface, nil
+}
+
+func (matcher *NetworkStatusMatcher) FailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "Name, IPs, and InterfaceName to equal", matcher.Expected)
+}
+
+func (matcher *NetworkStatusMatcher) NegatedFailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "Name, IPs, and InterfaceName *not* to equal", matcher.Expected)
+}

pkg/controller/pod.go

@@ -1,8 +1,10 @@
 package controller
 
 import (
+	"encoding/json"
 	"fmt"
 	"github.com/containernetworking/cni/libcni"
+	nadv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	"os"
 	"reflect"
 	"strings"
@@ -31,6 +33,7 @@ import (
 const allNamespaces = ""
 const controllerName = "multus-cni-pod-networks-controller"
 const podNetworksAnnot = "k8s.v1.cni.cncf.io/networks"
+const podNetworkStatus = "k8s.v1.cni.cncf.io/network-status"
 
 // PodNetworksController handles the cncf networks annotations update, and
 // triggers adding / removing networks from a running pod.
@@ -197,45 +200,34 @@ func (pnc *PodNetworksController) addNetworks(netsToAdd []types.NetworkSelection
 }
 
 func (pnc *PodNetworksController) removeNetworks(netsToRemove []types.NetworkSelectionElement, pod *corev1.Pod) error {
+	var (
+		multusNetconf *types.NetConf
+		podNetStatus  []nadv1.NetworkStatus
+	)
+	k8sClient, err := pnc.k8sClient()
+	if err != nil {
+		return err
+	}
+
 	for _, netToRemove := range netsToRemove {
 		cniParams, err := pnc.getCNIParams(pod, netToRemove)
 		if err != nil {
 			return logging.Errorf("failed to extract CNI params to remove existing interface from pod %s: %v", pod.GetName(), err)
 		}
 		logging.Verbosef("CNI params for pod %s: %+v", pod.GetName(), cniParams)
 
-		k8sClient, err := pnc.k8sClient()
-		if err != nil {
-			return err
-		}
-
 		delegateConf, _, err := k8sclient.GetKubernetesDelegate(k8sClient, &netToRemove, pnc.confDir, pod, nil)
 		if err != nil {
 			return fmt.Errorf("error retrieving the delegate info: %w", err)
 		}
 		k8sArgs, _ := k8sclient.GetK8sArgs(&cniParams.CniCmdArgs)
 
-		//rtConf := &libcni.RuntimeConf{
-		//	ContainerID: cniParams.CniCmdArgs.ContainerID,
-		//	IfName:      cniParams.CniCmdArgs.IfName,
-		//	NetNS:       cniParams.CniCmdArgs.Netns,
-		//	Args: [][2]string{
-		//		{"IgnoreUnknown", "true"},
-		//		{"K8S_POD_NAMESPACE", cniParams.Namespace},
-		//		{"K8S_POD_NAME", cniParams.PodName},
-		//		{"K8S_POD_INFRA_CONTAINER_ID", cniParams.CniCmdArgs.ContainerID},
-		//		{"K8S_POD_NETWORK", cniParams.NetworkName},
-		//	},
-		//}
-
-		multusNetconf, err := pnc.multusConf()
-
-		rtConf := types.DelegateRuntimeConfig(cniParams.CniCmdArgs.ContainerID, delegateConf, multusNetconf.RuntimeConfig, cniParams.CniCmdArgs.IfName)
-
+		multusNetconf, err = pnc.multusConf()
 		if err != nil {
 			return logging.Errorf("failed to retrieve the multus network: %v", err)
 		}
 
+		rtConf := types.DelegateRuntimeConfig(cniParams.CniCmdArgs.ContainerID, delegateConf, multusNetconf.RuntimeConfig, cniParams.CniCmdArgs.IfName)
 		logging.Verbosef("the multus config: %+v", *multusNetconf)
 		if strings.HasPrefix(multusNetconf.BinDir, "/host") {
 			strings.ReplaceAll(multusNetconf.BinDir, "/host", "")
@@ -246,7 +238,31 @@ func (pnc *PodNetworksController) removeNetworks(netsToRemove []types.NetworkSel
 			return logging.Errorf("failed to remove network. error: %v", err)
 		}
 		logging.Verbosef("removed network %s from pod %s with interface name: %s", netToRemove.Name, pod.GetName(), cniParams.CniCmdArgs.IfName)
+
+		oldNetStatus, err := networkStatus(pod.Annotations)
+		if err != nil {
+			return fmt.Errorf("failed to extract the pod's network status: %+v", err)
+		}
+
+		for _, netStatus := range oldNetStatus {
+			if fmt.Sprintf("%s/%s", netToRemove.Namespace, netToRemove.Name) != netStatus.Name {
+				podNetStatus = append(podNetStatus, netStatus)
+			}
+		}
 	}
+
+	if multusNetconf == nil {
+		multusNetconf, err = pnc.multusConf()
+		if err != nil {
+			logging.Verbosef("error accessing the multus configuration: %+v", err)
+		}
+	}
+
+	if err := k8sclient.SetPodNetworkStatusAnnotation(k8sClient, pod.GetName(), pod.GetNamespace(), string(pod.GetUID()), podNetStatus, multusNetconf); err != nil {
+		// error happen but continue to delete
+		logging.Errorf("Multus: error unsetting the networks status: %v", err)
+	}
+
 	return nil
 }
 
@@ -277,6 +293,19 @@ func networkSelectionElements(podAnnotations map[string]string, podNamespace str
 	return podNetworkSelectionElements, nil
 }
 
+func networkStatus(podAnnotations map[string]string) ([]nadv1.NetworkStatus, error) {
+	podNetworkstatus, ok := podAnnotations[nadv1.NetworkStatusAnnot]
+	if !ok {
+		return nil, fmt.Errorf("the pod is missing the \"%s\" annotation on its annotations: %+v", podNetworksAnnot, podAnnotations)
+	}
+	var netStatus []nadv1.NetworkStatus
+	if err := json.Unmarshal([]byte(podNetworkstatus), &netStatus); err != nil {
+		return nil, err
+	}
+
+	return netStatus, nil
+}
+
 func exclusiveNetworks(needles []*types.NetworkSelectionElement, haystack []*types.NetworkSelectionElement) []types.NetworkSelectionElement {
 	setOfNeedles := indexNetworkSelectionElements(needles)
 	haystackSet := indexNetworkSelectionElements(haystack)