feat: crash cns when dupe ips are found during reconcile (#939)

cns/NetworkContainerContract.go

@@ -2,6 +2,7 @@ package cns
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net"
 	"strconv"
@@ -113,10 +114,13 @@ type ConfigureContainerNetworkingRequest struct {
 	NetworkContainerid string
 }
 
+// ErrDuplicateIP indicates that a duplicate IP has been detected during a reconcile.
+var ErrDuplicateIP = errors.New("duplicate IP detected in CNS initialization")
+
 // PodInfoByIPProvider to be implemented by initializers which provide a map
 // of PodInfos by IP.
 type PodInfoByIPProvider interface {
-	PodInfoByIP() map[string]PodInfo
+	PodInfoByIP() (map[string]PodInfo, error)
 }
 
 var _ PodInfoByIPProvider = (PodInfoByIPProviderFunc)(nil)
@@ -124,10 +128,10 @@ var _ PodInfoByIPProvider = (PodInfoByIPProviderFunc)(nil)
 // PodInfoByIPProviderFunc functional type which implements PodInfoByIPProvider.
 // Allows one-off functional implementations of the PodInfoByIPProvider
 // interface when a custom type definition is not necessary.
-type PodInfoByIPProviderFunc func() map[string]PodInfo
+type PodInfoByIPProviderFunc func() (map[string]PodInfo, error)
 
 // PodInfoByIP implements PodInfoByIPProvider on PodInfByIPProviderFunc.
-func (f PodInfoByIPProviderFunc) PodInfoByIP() map[string]PodInfo {
+func (f PodInfoByIPProviderFunc) PodInfoByIP() (map[string]PodInfo, error) {
 	return f()
 }
 

cns/cnireconciler/podinfoprovider.go

@@ -6,6 +6,7 @@ import (
 	"github.com/Azure/azure-container-networking/cni/api"
 	"github.com/Azure/azure-container-networking/cni/client"
 	"github.com/Azure/azure-container-networking/cns"
+	"github.com/pkg/errors"
 	"k8s.io/utils/exec"
 )
 
@@ -21,16 +22,19 @@ func newCNIPodInfoProvider(exec exec.Interface) (cns.PodInfoByIPProvider, error)
 	if err != nil {
 		return nil, fmt.Errorf("failed to invoke CNI client.GetEndpointState(): %w", err)
 	}
-	return cns.PodInfoByIPProviderFunc(func() map[string]cns.PodInfo {
+	return cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) {
 		return cniStateToPodInfoByIP(state)
 	}), nil
 }
 
 // cniStateToPodInfoByIP converts an AzureCNIState dumped from a CNI exec
 // into a PodInfo map, using the first endpoint IP as the key in the map.
-func cniStateToPodInfoByIP(state *api.AzureCNIState) map[string]cns.PodInfo {
+func cniStateToPodInfoByIP(state *api.AzureCNIState) (map[string]cns.PodInfo, error) {
 	podInfoByIP := map[string]cns.PodInfo{}
 	for _, endpoint := range state.ContainerInterfaces {
+		if _, ok := podInfoByIP[endpoint.IPAddresses[0].IP.String()]; ok {
+			return nil, errors.Wrap(cns.ErrDuplicateIP, endpoint.IPAddresses[0].IP.String())
+		}
 		podInfoByIP[endpoint.IPAddresses[0].IP.String()] = cns.NewPodInfo(
 			endpoint.ContainerID,
 			endpoint.PodEndpointId,
@@ -38,5 +42,5 @@ func cniStateToPodInfoByIP(state *api.AzureCNIState) map[string]cns.PodInfo {
 			endpoint.PodNamespace,
 		)
 	}
-	return podInfoByIP
+	return podInfoByIP, nil
 }

cns/cnireconciler/podinfoprovider_test.go

@@ -46,15 +46,16 @@ func TestNewCNIPodInfoProvider(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := newCNIPodInfoProvider(tt.exec)
 			if tt.wantErr {
 				assert.Error(t, err)
 				return
-			} else {
-				assert.NoError(t, err)
 			}
-			assert.Equal(t, tt.want, got.PodInfoByIP())
+			assert.NoError(t, err)
+			podInfoByIP, _ := got.PodInfoByIP()
+			assert.Equal(t, tt.want, podInfoByIP)
 		})
 	}
 }

cns/singletenantcontroller/kubecontroller/crdrequestcontroller.go

@@ -2,7 +2,6 @@ package kubecontroller
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"os"
 	"sync"
@@ -16,6 +15,7 @@ import (
 	"github.com/Azure/azure-container-networking/cns/singletenantcontroller"
 	"github.com/Azure/azure-container-networking/crd"
 	"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
+	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -262,25 +262,33 @@ func (rc *requestController) initCNS(ctx context.Context) error {
 			logger.Errorf("error when getting all pods when initializing cns: %v", err)
 			return err
 		}
-		podInfoByIPProvider = cns.PodInfoByIPProviderFunc(func() map[string]cns.PodInfo {
+		podInfoByIPProvider = cns.PodInfoByIPProviderFunc(func() (map[string]cns.PodInfo, error) {
 			return rc.kubePodsToPodInfoByIP(pods.Items)
 		})
 	}
 
-	// Call cnsclient init cns passing those two things
-	//nolint:wrapcheck
-	return rc.CNSClient.ReconcileNCState(&ncRequest, podInfoByIPProvider.PodInfoByIP(), nnc.Status.Scaler, nnc.Spec)
+	podInfoByIP, err := podInfoByIPProvider.PodInfoByIP()
+	if err != nil {
+		return errors.Wrap(err, "err in CNS initialization")
+	}
+
+	// errors.Wrap provides additional context, and return nil if the err input arg is nil
+	// Call cnsclient init cns passing those two things.
+	return errors.Wrap(rc.CNSClient.ReconcileNCState(&ncRequest, podInfoByIP, nnc.Status.Scaler, nnc.Spec), "err in CNS reconciliation")
 }
 
 // kubePodsToPodInfoByIP maps kubernetes pods to cns.PodInfos by IP
-func (rc *requestController) kubePodsToPodInfoByIP(pods []corev1.Pod) map[string]cns.PodInfo {
+func (rc *requestController) kubePodsToPodInfoByIP(pods []corev1.Pod) (map[string]cns.PodInfo, error) {
 	podInfoByIP := map[string]cns.PodInfo{}
 	for _, pod := range pods {
 		if !pod.Spec.HostNetwork {
+			if _, ok := podInfoByIP[pod.Status.PodIP]; ok {
+				return nil, errors.Wrap(cns.ErrDuplicateIP, pod.Status.PodIP)
+			}
 			podInfoByIP[pod.Status.PodIP] = cns.NewPodInfo("", "", pod.Name, pod.Namespace)
 		}
 	}
-	return podInfoByIP
+	return podInfoByIP, nil
 }
 
 // UpdateCRDSpec updates the CRD spec