feat: CNS RequestIPAddress branching for MT/V2 (#2300)

* fix overlay IPAM not reporting version

* revert file and var naming, add correct path to makefile

* proposal design for multitenant IPAM flow

* change podipinfo + linter issue

* pointer issues for printf

* update IPAM branching

* remove comments

* pod client placeholder

* address lint issue for httpservicefake

* getting pod info in validator

* linter issue

* update network container contract

* renaming

* mtpnc changes

* rebase

* revert file and var naming, add correct path to makefile

* add default route

* add unit tests

* update unit tests for ipam

* go get to fix linter

* go mod tidy

* update routes

* update routes

* remove stale comments + remove redundant method

* add contexts + change address type

* addressed review

* embedded client to mock + enum for address type

* fix error

* change addressType to NICType

* change isDefaultRoute to SkipDefaultRoutes

* address comments

* refractor: make changes according to cni/cns contract

* refractor: make adding route its own func + move swift v2 ipam branching to after normal ipam flow

* refractor: change vars naming

* refractor: more var naming

* test: add test for podv6cidr

* refractor: make the returning podIpInfo init cleaner in swiftv2.go

* refractor + tests: add contexts to ipconfigs req validators + set route tests

* refractor: change labels for swift v2 pods

* fix: fix swift v2 UT

* refractor: add v4/v6 distinction for service cidr

* rebase

* revert file and var naming, add correct path to makefile

* rebase

* revert file and var naming, add correct path to makefile

* change podipinfo + linter issue

* update IPAM branching

* pod client placeholder

* getting pod info in validator

* linter issue

* rebase

* revert file and var naming, add correct path to makefile

* refractor: fix conflicts

* refractor: revert podwatcher code changes

* docs: change comment

* refractor: change CIDR to CDIRs

* refractor: parse CIDRs as semicolons separated string from env in SetRoutes

* docs: add minor comment

* refractor: change separator for parsing CIDRs

* feat: add rbac roles

* fix: gofumpt

* fix: update clusterrole

* fix: add namespace to clusterrolebinding

* fix: UT

* fix: add labels toswift v2 clusterrole

* fix: release default ipconfig early if getting swiftv2 ipconfig failed

* test: add more UT

* fix: parsing MTPNC as CIDR instead

* fix: toggle skipDefaultRoutes for infraNic to true

* fix: add route for node cidr in ipv4 podipconfig

* feat: add node cidrs route

* fix: linter

* address comments

* fix: minor logs formatting

* feat: move cns yaml for swiftv2 scenario to a diff file + more logging for swiftv2middleware

* fix: log debugf to printf

* fix: add testmain to avoid nil pointer error for loggers

* Update azure-cns.yaml

Signed-off-by: Quang Nguyen <[email protected]>

* fix: move parseCIDRs to a common package, use net/netip instead of net

* fix: exhaustive all switch case for nic type

* fix: exhaustive all switch case for nic type

* refractor: change fmt.Errorf to errors.Wrapf

* fix: add mtpnc status check in validator + use netip package

* fix: minor

* revert: azure-cns.yaml

---------

Signed-off-by: Quang Nguyen <[email protected]>
Signed-off-by: Quang Nguyen <[email protected]>

cni/plugin.go

@@ -24,8 +24,10 @@ import (
 	"go.uber.org/zap"
 )
 
-var logger = log.CNILogger.With(zap.String("component", "cni-plugin"))
-var storeLogger = log.CNILogger.With(zap.String("component", "cni-store"))
+var (
+	logger      = log.CNILogger.With(zap.String("component", "cni-plugin"))
+	storeLogger = log.CNILogger.With(zap.String("component", "cni-store"))
+)
 
 var errEmptyContent = errors.New("read content is zero bytes")
 

cns/NetworkContainerContract.go

@@ -187,6 +187,8 @@ type PodInfo interface {
 	Equals(PodInfo) bool
 	// String implements string for logging PodInfos
 	String() string
+	// SecondaryInterfacesExist returns true if there exist a secondary interface for this pod
+	SecondaryInterfacesExist() bool
 }
 
 type KubernetesPodInfo struct {
@@ -199,9 +201,10 @@ var _ PodInfo = (*podInfo)(nil)
 // podInfo implements PodInfo for multiple schemas of Key
 type podInfo struct {
 	KubernetesPodInfo
-	PodInfraContainerID string
-	PodInterfaceID      string
-	Version             podInfoScheme
+	PodInfraContainerID   string
+	PodInterfaceID        string
+	Version               podInfoScheme
+	SecondaryInterfaceSet bool
 }
 
 func (p podInfo) String() string {
@@ -255,6 +258,10 @@ func (p *podInfo) OrchestratorContext() (json.RawMessage, error) {
 	return jsonContext, nil
 }
 
+func (p *podInfo) SecondaryInterfacesExist() bool {
+	return p.SecondaryInterfaceSet
+}
+
 // NewPodInfo returns an implementation of PodInfo that returns the passed
 // configuration for their namesake functions.
 func NewPodInfo(infraContainerID, interfaceID, name, namespace string) PodInfo {
@@ -292,6 +299,7 @@ func NewPodInfoFromIPConfigsRequest(req IPConfigsRequest) (PodInfo, error) {
 	}
 	p.(*podInfo).PodInfraContainerID = req.InfraContainerID
 	p.(*podInfo).PodInterfaceID = req.PodInterfaceID
+	p.(*podInfo).SecondaryInterfaceSet = req.SecondaryInterfacesExist
 	return p, nil
 }
 
@@ -453,11 +461,12 @@ type IPConfigRequest struct {
 
 // Same as IPConfigRequest except that DesiredIPAddresses is passed in as a slice
 type IPConfigsRequest struct {
-	DesiredIPAddresses  []string        `json:"desiredIPAddresses"`
-	PodInterfaceID      string          `json:"podInterfaceID"`
-	InfraContainerID    string          `json:"infraContainerID"`
-	OrchestratorContext json.RawMessage `json:"orchestratorContext"`
-	Ifname              string          `json:"ifname"` // Used by delegated IPAM
+	DesiredIPAddresses       []string        `json:"desiredIPAddresses"`
+	PodInterfaceID           string          `json:"podInterfaceID"`
+	InfraContainerID         string          `json:"infraContainerID"`
+	OrchestratorContext      json.RawMessage `json:"orchestratorContext"`
+	Ifname                   string          `json:"ifname"`                   // Used by delegated IPAM
+	SecondaryInterfacesExist bool            `json:"secondaryInterfacesExist"` // will be set by SWIFT v2 validator func
 }
 
 // IPConfigResponse is used in CNS IPAM mode as a response to CNI ADD

cns/api.go

@@ -48,8 +48,18 @@ type HTTPService interface {
 	GetPendingReleaseIPConfigs() []IPConfigurationStatus
 	GetPodIPConfigState() map[string]IPConfigurationStatus
 	MarkIPAsPendingRelease(numberToMark int) (map[string]IPConfigurationStatus, error)
+	AttachSWIFTv2Middleware(middleware SWIFTv2Middleware)
 }
 
+// Middleware interface for testing later on
+type SWIFTv2Middleware interface {
+	ValidateIPConfigsRequest(context.Context, *IPConfigsRequest) (types.ResponseCode, string)
+	GetIPConfig(context.Context, PodInfo) (PodIpInfo, error)
+	SetRoutes(*PodIpInfo) error
+}
+
+type IPConfigsRequestValidator func(context.Context, *IPConfigsRequest) (types.ResponseCode, string)
+
 // This is used for KubernetesCRD orchestrator Type where NC has multiple ips.
 // This struct captures the state for SecondaryIPs associated to a given NC
 type IPConfigurationStatus struct {

cns/azure-cns.yaml

@@ -189,4 +189,4 @@ data:
       "ManageEndpointState": false,
       "ProgramSNATIPTables" : false
     }
-# Toggle ManageEndpointState and ProgramSNATIPTables to true for delegated IPAM use case.
+# Toggle ManageEndpointState and ProgramSNATIPTables to true for delegated IPAM use case.

cns/configuration/env.go

@@ -11,13 +11,27 @@ const (
 	EnvNodeName = "NODENAME"
 	// EnvNodeIP is the IP of the node running this CNS binary
 	EnvNodeIP = "NODE_IP"
-	// LabelSwiftV2 is the Node label for Swift V2
-	LabelSwiftV2 = "kubernetes.azure.com/podnetwork-multi-tenancy"
+	// LabelNodeSwiftV2 is the Node label for Swift V2
+	LabelNodeSwiftV2 = "kubernetes.azure.com/podnetwork-multi-tenancy-enabled"
+	// LabelPodSwiftV2 is the Pod label for Swift V2
+	LabelPodSwiftV2 = "kubernetes.azure.com/pod-network"
+	EnvPodCIDRs     = "POD_CIDRs"
+	EnvServiceCIDRs = "SERVICE_CIDRs"
+	EnvNodeCIDRs    = "NODE_CIDRs"
 )
 
 // ErrNodeNameUnset indicates the the $EnvNodeName variable is unset in the environment.
 var ErrNodeNameUnset = errors.Errorf("must declare %s environment variable", EnvNodeName)
 
+// ErrPodCIDRsUnset indicates the the $EnvPodCIDRs variable is unset in the environment.
+var ErrPodCIDRsUnset = errors.Errorf("must declare %s environment variable", EnvPodCIDRs)
+
+// ErrServiceCIDRsUnset indicates the the $EnvServiceCIDRs variable is unset in the environment.
+var ErrServiceCIDRsUnset = errors.Errorf("must declare %s environment variable", EnvServiceCIDRs)
+
+// ErrNodeCIDRsUnset indicates the the $EnvNodeCIDRs variable is unset in the environment.
+var ErrNodeCIDRsUnset = errors.Errorf("must declare %s environment variable", EnvNodeCIDRs)
+
 // NodeName checks the environment variables for the NODENAME and returns it or an error if unset.
 func NodeName() (string, error) {
 	nodeName := os.Getenv(EnvNodeName)
@@ -31,3 +45,27 @@ func NodeName() (string, error) {
 func NodeIP() string {
 	return os.Getenv(EnvNodeIP)
 }
+
+func PodCIDRs() (string, error) {
+	podCIDRs := os.Getenv(EnvPodCIDRs)
+	if podCIDRs == "" {
+		return "", ErrPodCIDRsUnset
+	}
+	return podCIDRs, nil
+}
+
+func ServiceCIDRs() (string, error) {
+	serviceCIDRs := os.Getenv(EnvServiceCIDRs)
+	if serviceCIDRs == "" {
+		return "", ErrServiceCIDRsUnset
+	}
+	return serviceCIDRs, nil
+}
+
+func NodeCIDRs() (string, error) {
+	nodeCIDRs := os.Getenv(EnvNodeCIDRs)
+	if nodeCIDRs == "" {
+		return "", ErrNodeCIDRsUnset
+	}
+	return nodeCIDRs, nil
+}

cns/configuration/env_test.go

@@ -17,3 +17,23 @@ func TestNodeName(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, "test", name)
 }
+
+func TestPodCIDRs(t *testing.T) {
+	_, err := PodCIDRs()
+	require.Error(t, err)
+	require.ErrorIs(t, err, ErrPodCIDRsUnset)
+	os.Setenv(EnvPodCIDRs, "test")
+	cidr, err := PodCIDRs()
+	assert.NoError(t, err)
+	assert.Equal(t, "test", cidr)
+}
+
+func TestServiceCIDRs(t *testing.T) {
+	_, err := ServiceCIDRs()
+	require.Error(t, err)
+	require.ErrorIs(t, err, ErrServiceCIDRsUnset)
+	os.Setenv(EnvServiceCIDRs, "test")
+	cidr, err := ServiceCIDRs()
+	assert.NoError(t, err)
+	assert.Equal(t, "test", cidr)
+}

cns/fakes/cnsfake.go

@@ -276,3 +276,5 @@ func (fake *HTTPServiceFake) Init(*common.ServiceConfig) error {
 }
 
 func (fake *HTTPServiceFake) Stop() {}
+
+func (fake *HTTPServiceFake) AttachSWIFTv2Middleware(cns.SWIFTv2Middleware) {}

cns/middlewares/mock/mockClient.go

@@ -0,0 +1,76 @@
+package mock
+
+import (
+	"context"
+
+	"github.com/Azure/azure-container-networking/cns/configuration"
+	"github.com/Azure/azure-container-networking/crd/multitenancy/api/v1alpha1"
+	"github.com/pkg/errors"
+	v1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+var (
+	ErrPodNotFound   = errors.New("pod not found")
+	ErrMTPNCNotFound = errors.New("mtpnc not found")
+)
+
+// Client implements the client.Client interface for testing. We only care about Get, the rest is nil ops.
+type Client struct {
+	client.Client
+	mtPodCache map[string]*v1.Pod
+	mtpncCache map[string]*v1alpha1.MultitenantPodNetworkConfig
+}
+
+// NewClient returns a new MockClient.
+func NewClient() *Client {
+	const podNetwork = "azure"
+
+	testPod1 := v1.Pod{}
+	testPod1.Labels = make(map[string]string)
+	testPod1.Labels[configuration.LabelPodSwiftV2] = podNetwork
+
+	testPod3 := v1.Pod{}
+	testPod3.Labels = make(map[string]string)
+	testPod3.Labels[configuration.LabelPodSwiftV2] = podNetwork
+
+	testPod4 := v1.Pod{}
+	testPod4.Labels = make(map[string]string)
+	testPod4.Labels[configuration.LabelPodSwiftV2] = podNetwork
+
+	testMTPNC1 := v1alpha1.MultitenantPodNetworkConfig{}
+
+	testMTPNC1.Status.PrimaryIP = "192.168.0.1/32"
+	testMTPNC1.Status.MacAddress = "00:00:00:00:00:00"
+	testMTPNC1.Status.GatewayIP = "10.0.0.1"
+	testMTPNC1.Status.NCID = "testncid"
+
+	testMTPNC4 := v1alpha1.MultitenantPodNetworkConfig{}
+
+	return &Client{
+		mtPodCache: map[string]*v1.Pod{"testpod1namespace/testpod1": &testPod1, "testpod3namespace/testpod3": &testPod3, "testpod4namespace/testpod4": &testPod4},
+		mtpncCache: map[string]*v1alpha1.MultitenantPodNetworkConfig{
+			"testpod1namespace/testpod1": &testMTPNC1,
+			"testpod4namespace/testpod4": &testMTPNC4,
+		},
+	}
+}
+
+// Get implements client.Client.Get.
+func (c *Client) Get(_ context.Context, key client.ObjectKey, obj client.Object, _ ...client.GetOption) error {
+	switch o := obj.(type) {
+	case *v1.Pod:
+		if pod, ok := c.mtPodCache[key.String()]; ok {
+			*o = *pod
+		} else {
+			return ErrPodNotFound
+		}
+	case *v1alpha1.MultitenantPodNetworkConfig:
+		if mtpnc, ok := c.mtpncCache[key.String()]; ok {
+			*o = *mtpnc
+		} else {
+			return ErrMTPNCNotFound
+		}
+	}
+	return nil
+}