refactor podinfoproviders

Signed-off-by: Evan Baker <[email protected]>

Author: rbtr

cns/NetworkContainerContract.go

@@ -201,14 +201,14 @@ func (f PodInfoByIPProviderFunc) PodInfoByIP() (map[string]PodInfo, error) {
 	return f()
 }
 
-var GlobalPodInfoScheme podInfoScheme
+var GlobalPodInfoScheme podInfoScheme = InterfaceIDPodInfoScheme
 
 // podInfoScheme indicates which schema should be used when generating
 // the map key in the Key() function on a podInfo object.
 type podInfoScheme int
 
 const (
-	KubernetesPodInfoScheme podInfoScheme = iota
+	_ podInfoScheme = iota
 	InterfaceIDPodInfoScheme
 	InfraIDPodInfoScheme
 )
@@ -285,12 +285,8 @@ func (p *podInfo) Key() string {
 	switch p.Version {
 	case InfraIDPodInfoScheme:
 		return p.PodInfraContainerID
-	case InterfaceIDPodInfoScheme:
-		return p.PodInterfaceID
-	case KubernetesPodInfoScheme:
-		return p.PodName + ":" + p.PodNamespace
 	default:
-		return p.PodName + ":" + p.PodNamespace
+		return p.PodInterfaceID
 	}
 }
 

cns/NetworkContainerContract_test.go

@@ -58,7 +58,6 @@ func TestUnmarshalPodInfo(t *testing.T) {
 
 func TestNewPodInfoFromIPConfigsRequest(t *testing.T) {
 	GlobalPodInfoScheme = InterfaceIDPodInfoScheme
-	defer func() { GlobalPodInfoScheme = KubernetesPodInfoScheme }()
 	tests := []struct {
 		name    string
 		req     IPConfigsRequest

cns/nodesubnet/initialization_test.go

@@ -6,10 +6,10 @@ import (
 	"testing"
 
 	"github.com/Azure/azure-container-networking/cns"
-	"github.com/Azure/azure-container-networking/cns/cnireconciler"
 	"github.com/Azure/azure-container-networking/cns/logger"
 	"github.com/Azure/azure-container-networking/cns/nodesubnet"
 	"github.com/Azure/azure-container-networking/cns/restserver"
+	podprovider "github.com/Azure/azure-container-networking/cns/stateprovider/cns"
 	"github.com/Azure/azure-container-networking/cns/types"
 	"github.com/Azure/azure-container-networking/store"
 )
@@ -88,7 +88,7 @@ func TestNewCNSPodInfoProvider(t *testing.T) {
 			ctx, cancel := testContext(t)
 			defer cancel()
 
-			podInfoByIPProvider, err := cnireconciler.NewCNSPodInfoProvider(tt.store)
+			podInfoByIPProvider, err := podprovider.New(tt.store)
 			checkErr(t, err, false)
 
 			got, err := nodesubnet.ReconcileInitialCNSState(ctx, tt.reconciler, podInfoByIPProvider)

cns/restserver/internalapi_test.go

@@ -602,12 +602,12 @@ func TestPodIPsIndexedByInterface(t *testing.T) {
 				"fe80::7":  cns.NewPodInfo("some-guid-2", "def-eth0", "reconcilePod2", "PodNS2"),
 			},
 			expectedOutput: map[string]podIPs{
-				"reconcilePod1:PodNS1": {
+				"abc-eth0": {
 					PodInfo: cns.NewPodInfo("some-guid-1", "abc-eth0", "reconcilePod1", "PodNS1"),
 					v4IP:    net.IPv4(10, 0, 0, 6),
 					v6IP:    []byte{0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x06},
 				},
-				"reconcilePod2:PodNS2": {
+				"def-eth0": {
 					PodInfo: cns.NewPodInfo("some-guid-2", "def-eth0", "reconcilePod2", "PodNS2"),
 					v4IP:    net.IPv4(10, 0, 0, 7),
 					v6IP:    []byte{0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x07},
@@ -677,7 +677,6 @@ func TestReconcileNCWithExistingStateFromInterfaceID(t *testing.T) {
 	setEnv(t)
 	setOrchestratorTypeInternal(cns.KubernetesCRD)
 	cns.GlobalPodInfoScheme = cns.InterfaceIDPodInfoScheme
-	defer func() { cns.GlobalPodInfoScheme = cns.KubernetesPodInfoScheme }()
 
 	secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig)
 
@@ -716,52 +715,6 @@ func TestReconcileNCWithExistingStateFromInterfaceID(t *testing.T) {
 	validateNCStateAfterReconcile(t, req, expectedNcCount+1, expectedAssignedPods, nil)
 }
 
-func TestReconcileCNSIPAMWithKubePodInfoProvider(t *testing.T) {
-	restartService()
-	setEnv(t)
-	setOrchestratorTypeInternal(cns.KubernetesCRD)
-
-	secondaryIPConfigs := make(map[string]cns.SecondaryIPConfig)
-
-	startingIndex := 6
-	for i := 0; i < 4; i++ {
-		ipaddress := "10.0.0." + strconv.Itoa(startingIndex)
-		secIpConfig := newSecondaryIPConfig(ipaddress, -1)
-		ipId := uuid.New()
-		secondaryIPConfigs[ipId.String()] = secIpConfig
-		startingIndex++
-	}
-	req := generateNetworkContainerRequest(secondaryIPConfigs, uuid.New().String(), "-1")
-
-	// the following pod info constructors leave container id and interface id blank on purpose.
-	// this is to simulate the information we get from the kube info provider
-	expectedAssignedPods := make(map[string]cns.PodInfo)
-	expectedAssignedPods["10.0.0.6"] = cns.NewPodInfo("", "", "customerpod1", "PodNS1")
-
-	// allocate non-vnet IP for pod in host network
-	expectedAssignedPods["192.168.0.1"] = cns.NewPodInfo("", "", "systempod", "kube-system")
-
-	expectedNcCount := len(svc.state.ContainerStatus)
-	returnCode := svc.ReconcileIPAMStateForSwift([]*cns.CreateNetworkContainerRequest{req}, expectedAssignedPods, &v1alpha.NodeNetworkConfig{
-		Status: v1alpha.NodeNetworkConfigStatus{
-			Scaler: v1alpha.Scaler{
-				BatchSize:               batchSize,
-				ReleaseThresholdPercent: releasePercent,
-				RequestThresholdPercent: requestPercent,
-			},
-		},
-		Spec: v1alpha.NodeNetworkConfigSpec{
-			RequestedIPCount: initPoolSize,
-		},
-	})
-	if returnCode != types.Success {
-		t.Errorf("Unexpected failure on reconcile with no state %d", returnCode)
-	}
-
-	delete(expectedAssignedPods, "192.168.0.1")
-	validateNCStateAfterReconcile(t, req, expectedNcCount, expectedAssignedPods, nil)
-}
-
 func setOrchestratorTypeInternal(orchestratorType string) {
 	fmt.Println("setOrchestratorTypeInternal")
 	svc.state.OrchestratorType = orchestratorType

cns/restserver/nodesubnet_test.go

@@ -5,8 +5,8 @@ import (
 	"net"
 	"testing"
 
-	"github.com/Azure/azure-container-networking/cns/cnireconciler"
 	"github.com/Azure/azure-container-networking/cns/logger"
+	"github.com/Azure/azure-container-networking/cns/stateprovider/cns"
 	"github.com/Azure/azure-container-networking/cns/restserver"
 	"github.com/Azure/azure-container-networking/cns/types"
 	"github.com/Azure/azure-container-networking/store"
@@ -71,7 +71,7 @@ func (m *MockCNIConflistGenerator) Close() error {
 // TestNodeSubnet tests initialization of NodeSubnet with endpoint info, and verfies that
 // the conflist is generated after fetching secondary IPs
 func TestNodeSubnet(t *testing.T) {
-	podInfoByIPProvider, err := cnireconciler.NewCNSPodInfoProvider(getMockStore())
+	podInfoByIPProvider, err := cns.New(getMockStore())
 	if err != nil {
 		t.Fatalf("NewCNSPodInfoProvider returned an error: %v", err)
 	}

cns/service/main.go

@@ -23,7 +23,6 @@ import (
 	cnsclient "github.com/Azure/azure-container-networking/cns/client"
 	cnscli "github.com/Azure/azure-container-networking/cns/cmd/cli"
 	"github.com/Azure/azure-container-networking/cns/cniconflist"
-	"github.com/Azure/azure-container-networking/cns/cnireconciler"
 	"github.com/Azure/azure-container-networking/cns/common"
 	"github.com/Azure/azure-container-networking/cns/configuration"
 	"github.com/Azure/azure-container-networking/cns/deviceplugin"
@@ -47,6 +46,8 @@ import (
 	"github.com/Azure/azure-container-networking/cns/multitenantcontroller/multitenantoperator"
 	"github.com/Azure/azure-container-networking/cns/restserver"
 	restserverv2 "github.com/Azure/azure-container-networking/cns/restserver/v2"
+	cnipodprovider "github.com/Azure/azure-container-networking/cns/stateprovider/cni"
+	cnspodprovider "github.com/Azure/azure-container-networking/cns/stateprovider/cns"
 	cnstypes "github.com/Azure/azure-container-networking/cns/types"
 	"github.com/Azure/azure-container-networking/cns/wireserver"
 	acn "github.com/Azure/azure-container-networking/common"
@@ -833,7 +834,7 @@ func main() {
 
 		// Check the CNI statefile mount, and if the file is empty
 		// stub an empty JSON object
-		if err := cnireconciler.WriteObjectToCNIStatefile(); err != nil {
+		if err := cnipodprovider.WriteObjectToCNIStatefile(); err != nil { //nolint:govet //shadow okay
 			logger.Errorf("Failed to write empty object to CNI state: %v", err)
 			return
 		}
@@ -1654,7 +1655,7 @@ func getPodInfoByIPProvider(
 	switch {
 	case cnsconfig.ManageEndpointState:
 		logger.Printf("Initializing from self managed endpoint store")
-		podInfoByIPProvider, err = cnireconciler.NewCNSPodInfoProvider(httpRestServiceImplementation.EndpointStateStore) // get reference to endpoint state store from rest server
+		podInfoByIPProvider, err = cnspodprovider.New(httpRestServiceImplementation.EndpointStateStore) // get reference to endpoint state store from rest server
 		if err != nil {
 			if errors.Is(err, store.ErrKeyNotFound) {
 				logger.Printf("[Azure CNS] No endpoint state found, skipping initializing CNS state")
@@ -1664,7 +1665,7 @@ func getPodInfoByIPProvider(
 		}
 	default:
 		logger.Printf("Initializing from CNI")
-		podInfoByIPProvider, err = cnireconciler.NewCNIPodInfoProvider()
+		podInfoByIPProvider, err = cnipodprovider.New()
 		if err != nil {
 			return podInfoByIPProvider, errors.Wrap(err, "failed to create CNI PodInfoProvider")
 		}
@@ -1713,7 +1714,7 @@ func createOrUpdateNodeInfoCRD(ctx context.Context, restConfig *rest.Config, nod
 // PopulateCNSEndpointState initilizes CNS Endpoint State by Migrating the CNI state.
 func PopulateCNSEndpointState(endpointStateStore store.KeyValueStore) error {
 	logger.Printf("State Migration is enabled")
-	endpointState, err := cnireconciler.MigrateCNISate()
+	endpointState, err := cnspodprovider.MigrateCNISate()
 	if err != nil {
 		return errors.Wrap(err, "failed to create CNS Endpoint state from CNI")
 	}

cns/stateprovider/cni/podinfoprovider.go

@@ -0,0 +1,49 @@
+package cni
+
+import (
+	"fmt"
+
+	"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"
+	kexec "k8s.io/utils/exec"
+)
+
+// New returns an implementation of cns.PodInfoByIPProvider
+// that execs out to the CNI and uses the response to build the PodInfo map.
+func New() (cns.PodInfoByIPProvider, error) {
+	return podInfoProvider(kexec.New())
+}
+
+func podInfoProvider(exec kexec.Interface) (cns.PodInfoByIPProvider, error) {
+	cli := client.New(exec)
+	state, err := cli.GetEndpointState()
+	if err != nil {
+		return nil, fmt.Errorf("failed to invoke CNI client.GetEndpointState(): %w", err)
+	}
+	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 endpoint IPs as keys in the map.
+// for pods with multiple IPs (such as in dualstack cases), this means multiple keys in the map
+// will point to the same pod information.
+func cniStateToPodInfoByIP(state *api.AzureCNIState) (map[string]cns.PodInfo, error) {
+	podInfoByIP := map[string]cns.PodInfo{}
+	for _, endpoint := range state.ContainerInterfaces {
+		for _, epIP := range endpoint.IPAddresses {
+			podInfo := cns.NewPodInfo(endpoint.ContainerID, endpoint.PodEndpointId, endpoint.PodName, endpoint.PodNamespace)
+
+			ipKey := epIP.IP.String()
+			if prevPodInfo, ok := podInfoByIP[ipKey]; ok {
+				return nil, errors.Wrapf(cns.ErrDuplicateIP, "duplicate ip %s found for different pods: pod: %+v, pod: %+v", ipKey, podInfo, prevPodInfo)
+			}
+
+			podInfoByIP[ipKey] = podInfo
+		}
+	}
+	return podInfoByIP, nil
+}

cns/cnireconciler/podinfoprovider_test.go renamed to cns/stateprovider/cni/podinfoprovider_test.go

@@ -1,12 +1,9 @@
-package cnireconciler
+package cni
 
 import (
-	"net"
 	"testing"
 
 	"github.com/Azure/azure-container-networking/cns"
-	"github.com/Azure/azure-container-networking/cns/restserver"
-	"github.com/Azure/azure-container-networking/store"
 	testutils "github.com/Azure/azure-container-networking/test/utils"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/utils/exec"
@@ -76,54 +73,7 @@ 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
-			}
-			assert.NoError(t, err)
-			podInfoByIP, _ := got.PodInfoByIP()
-			assert.Equal(t, tt.want, podInfoByIP)
-		})
-	}
-}
-
-func TestNewCNSPodInfoProvider(t *testing.T) {
-	goodStore := store.NewMockStore("")
-	goodEndpointState := make(map[string]*restserver.EndpointInfo)
-	endpointInfo := &restserver.EndpointInfo{PodName: "goldpinger-deploy-bbbf9fd7c-z8v4l", PodNamespace: "default", IfnameToIPMap: make(map[string]*restserver.IPInfo)}
-	endpointInfo.IfnameToIPMap["eth0"] = &restserver.IPInfo{IPv4: []net.IPNet{{IP: net.IPv4(10, 241, 0, 65), Mask: net.IPv4Mask(255, 255, 255, 0)}}}
-
-	goodEndpointState["0a4917617e15d24dc495e407d8eb5c88e4406e58fa209e4eb75a2c2fb7045eea"] = endpointInfo
-	err := goodStore.Write(restserver.EndpointStoreKey, goodEndpointState)
-	if err != nil {
-		t.Fatalf("Error writing to store: %v", err)
-	}
-	tests := []struct {
-		name    string
-		store   store.KeyValueStore
-		want    map[string]cns.PodInfo
-		wantErr bool
-	}{
-		{
-			name:  "good",
-			store: goodStore,
-			want: map[string]cns.PodInfo{"10.241.0.65": cns.NewPodInfo("0a4917617e15d24dc495e407d8eb5c88e4406e58fa209e4eb75a2c2fb7045eea",
-				"0a4917617e15d24dc495e407d8eb5c88e4406e58fa209e4eb75a2c2fb7045eea", "goldpinger-deploy-bbbf9fd7c-z8v4l", "default")},
-			wantErr: false,
-		},
-		{
-			name:    "empty store",
-			store:   store.NewMockStore(""),
-			want:    map[string]cns.PodInfo{},
-			wantErr: true,
-		},
-	}
-
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := newCNSPodInfoProvider(tt.store)
+			got, err := podInfoProvider(tt.exec)
 			if tt.wantErr {
 				assert.Error(t, err)
 				return

cns/cnireconciler/statefile.go renamed to cns/stateprovider/cni/statefile.go

@@ -1,4 +1,4 @@
-package cnireconciler
+package cni
 
 import (
 	"encoding/json"

cns/cnireconciler/statefile_test.go renamed to cns/stateprovider/cni/statefile_test.go

@@ -1,4 +1,4 @@
-package cnireconciler
+package cni
 
 import (
 	"os"
@@ -12,8 +12,9 @@ import (
 
 func TestWriteObjectToFile(t *testing.T) {
 	name := "testdata/test"
-	err := os.MkdirAll(path.Dir(name), 0o666)
+	err := os.MkdirAll(path.Dir(name), 0o755)
 	require.NoError(t, err)
+	defer os.RemoveAll("testdata")
 
 	_, err = os.Stat(name)
 	require.ErrorIs(t, err, os.ErrNotExist)