add support for CRD CNINode updates for SGP feature (#2442)

charts/aws-vpc-cni/templates/clusterrole.yaml

@@ -33,3 +33,8 @@ rules:
     resources:
       - events
     verbs: ["create", "patch", "list"]
+  - apiGroups:
+      - vpcresources.k8s.aws
+    resources:
+      - cninodes
+    verbs: ["get", "list", "patch"]

config/master/aws-k8s-cni-cn.yaml

@@ -66,6 +66,11 @@ rules:
     resources:
       - events
     verbs: ["create", "patch", "list"]
+  - apiGroups:
+      - vpcresources.k8s.aws
+    resources:
+      - cninodes
+    verbs: ["get", "list", "patch"]
 ---
 # Source: aws-vpc-cni/templates/clusterrolebinding.yaml
 apiVersion: rbac.authorization.k8s.io/v1

config/master/aws-k8s-cni-us-gov-east-1.yaml

@@ -66,6 +66,11 @@ rules:
     resources:
       - events
     verbs: ["create", "patch", "list"]
+  - apiGroups:
+      - vpcresources.k8s.aws
+    resources:
+      - cninodes
+    verbs: ["get", "list", "patch"]
 ---
 # Source: aws-vpc-cni/templates/clusterrolebinding.yaml
 apiVersion: rbac.authorization.k8s.io/v1

config/master/aws-k8s-cni-us-gov-west-1.yaml

@@ -66,6 +66,11 @@ rules:
     resources:
       - events
     verbs: ["create", "patch", "list"]
+  - apiGroups:
+      - vpcresources.k8s.aws
+    resources:
+      - cninodes
+    verbs: ["get", "list", "patch"]
 ---
 # Source: aws-vpc-cni/templates/clusterrolebinding.yaml
 apiVersion: rbac.authorization.k8s.io/v1

config/master/aws-k8s-cni.yaml

@@ -61,11 +61,16 @@ rules:
   - apiGroups: [""]
     resources:
       - nodes
-    verbs: ["list", "watch", "get", "update"]
+    verbs: ["list", "watch", "get"]
   - apiGroups: ["", "events.k8s.io"]
     resources:
       - events
     verbs: ["create", "patch", "list"]
+  - apiGroups:
+      - vpcresources.k8s.aws
+    resources:
+      - cninodes
+    verbs: ["get", "list", "patch"]
 ---
 # Source: aws-vpc-cni/templates/clusterrolebinding.yaml
 apiVersion: rbac.authorization.k8s.io/v1

go.mod

@@ -5,7 +5,7 @@ go 1.20
 require (
 	github.com/apparentlymart/go-cidr v1.0.1
 	github.com/aws/amazon-vpc-cni-k8s/test/agent v0.0.0-20230614172555-5788cde362fa
-	github.com/aws/amazon-vpc-resource-controller-k8s v1.1.5
+	github.com/aws/amazon-vpc-resource-controller-k8s v1.1.8
 	github.com/aws/aws-sdk-go v1.43.29
 	github.com/containernetworking/cni v1.1.1
 	github.com/containernetworking/plugins v1.1.1
@@ -123,6 +123,7 @@ require (
 	github.com/rubenv/sql-migrate v1.2.0 // indirect
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
 	github.com/safchain/ethtool v0.0.0-20210803160452-9aa261dae9b1 // indirect
+	github.com/samber/lo v1.38.1
 	github.com/shopspring/decimal v1.2.0 // indirect
 	github.com/spf13/cast v1.4.1 // indirect
 	github.com/spf13/cobra v1.6.1 // indirect
@@ -135,13 +136,14 @@ require (
 	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect
 	golang.org/x/crypto v0.5.0 // indirect
+	golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect
 	golang.org/x/oauth2 v0.4.0 // indirect
 	golang.org/x/sync v0.1.0 // indirect
 	golang.org/x/term v0.6.0 // indirect
 	golang.org/x/text v0.8.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
 	golang.org/x/tools v0.6.0 // indirect
-	gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect
+	gomodules.xyz/jsonpatch/v2 v2.3.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/genproto v0.0.0-20220502173005-c8bf987b8c21 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect

go.sum

@@ -77,8 +77,8 @@ github.com/asaskevich/govalidator v0.0.0-20200428143746-21a406dcc535 h1:4daAzAu0
 github.com/asaskevich/govalidator v0.0.0-20200428143746-21a406dcc535/go.mod h1:oGkLhpf+kjZl6xBf758TQhh5XrAeiJv/7FRz/2spLIg=
 github.com/aws/amazon-vpc-cni-k8s/test/agent v0.0.0-20230614172555-5788cde362fa h1:92EaVRXJVzV5mu0D9W4MTAdulShMZ7rFZIFVVO8u2XA=
 github.com/aws/amazon-vpc-cni-k8s/test/agent v0.0.0-20230614172555-5788cde362fa/go.mod h1:nPrcILbew8ccLKKYwzyHJ8uZvE0C7ubmDmV6M4fYnp0=
-github.com/aws/amazon-vpc-resource-controller-k8s v1.1.5 h1:tyZBu4++W5SvSIxlVIh51cT84weqq0H+CmrsHtIgeys=
-github.com/aws/amazon-vpc-resource-controller-k8s v1.1.5/go.mod h1:W2c9R0NFy+HkO50f/ZHge0Be6QZpol6B+SegWwOCk3c=
+github.com/aws/amazon-vpc-resource-controller-k8s v1.1.8 h1:1YPnp2ltCTJVYeiPhNmQJaefIOVLGPtZJqG/NwyKVyE=
+github.com/aws/amazon-vpc-resource-controller-k8s v1.1.8/go.mod h1:HwoojB30a88mOfShN5qkciEIdyevfs5Y/I28ru9m1s8=
 github.com/aws/aws-sdk-go v1.43.29 h1:P6tBpMLwVLS/QwPkaBxfDIF3SmPouoacIk+/7NKnDxY=
 github.com/aws/aws-sdk-go v1.43.29/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo=
 github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
@@ -171,7 +171,6 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210217033140-668b12f5399d/go.m
 github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.mod h1:hliV/p42l8fGbc6Y9bQ70uLwIvmJyVE5k4iMKlh8wCQ=
 github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE=
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
-github.com/evanphx/json-patch v0.5.2/go.mod h1:ZWS5hhDbVDyob71nXKNL0+PWn6ToqBHMikGIFbs31qQ=
 github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U=
 github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
 github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww=
@@ -543,6 +542,8 @@ github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb
 github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4=
 github.com/safchain/ethtool v0.0.0-20210803160452-9aa261dae9b1 h1:ZFfeKAhIQiiOrQaI3/znw0gOmYpO28Tcu1YaqMa/jtQ=
 github.com/safchain/ethtool v0.0.0-20210803160452-9aa261dae9b1/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4=
+github.com/samber/lo v1.38.1 h1:j2XEAqXKb09Am4ebOg31SpvzUTTs6EN3VfgeLUhPdXM=
+github.com/samber/lo v1.38.1/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA=
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
 github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
 github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
@@ -647,6 +648,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
 golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
+golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc=
+golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
 golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
@@ -897,8 +900,8 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-gomodules.xyz/jsonpatch/v2 v2.2.0 h1:4pT439QV83L+G9FkcCriY6EkpcK6r6bK+A5FBUMI7qY=
-gomodules.xyz/jsonpatch/v2 v2.2.0/go.mod h1:WXp+iVDkoLQqPudfQ9GBlwB2eZ5DKOnjQZCYdOS8GPY=
+gomodules.xyz/jsonpatch/v2 v2.3.0 h1:8NFhfS6gzxNqjLIYnZxg319wZ5Qjnx4m/CcX+Klzazc=
+gomodules.xyz/jsonpatch/v2 v2.3.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY=
 google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE=
 google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M=
 google.golang.org/api v0.8.0/go.mod h1:o4eAsZoiT+ibD93RtjEohWalFOjRDx6CVaqeizhEnKg=

pkg/ipamd/ipamd.go

@@ -31,6 +31,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/samber/lo"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -44,6 +45,7 @@ import (
 	"github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils"
 	"github.com/aws/amazon-vpc-cni-k8s/pkg/utils/eventrecorder"
 	"github.com/aws/amazon-vpc-cni-k8s/pkg/utils/logger"
+	rcv1alpha1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1alpha1"
 )
 
 // The package ipamd is a long running daemon which manages a warm pool of available IP addresses.
@@ -135,12 +137,6 @@ const (
 	// envNodeName will be used to store Node name
 	envNodeName = "MY_NODE_NAME"
 
-	// vpcENIConfigLabel is used by the VPC resource controller to pick the right ENI config.
-	vpcENIConfigLabel = "vpc.amazonaws.com/eniConfig"
-
-	// trunkInterfaceLabel is used by CNI to check if correct label was added by the VPC resource controller
-	trunkInterfaceLabel = "vpc.amazonaws.com/has-trunk-attached"
-
 	//envEnableIpv4PrefixDelegation is used to allocate /28 prefix instead of secondary IP for an ENI.
 	envEnableIpv4PrefixDelegation = "ENABLE_PREFIX_DELEGATION"
 
@@ -575,34 +571,19 @@ func (c *IPAMContext) nodeInit() error {
 
 	eniConfigName, err := eniconfig.GetNodeSpecificENIConfigName(node)
 	if err == nil && c.useCustomNetworking && eniConfigName != "default" {
-		// Signal to VPC Resource Controller that the node is using custom networking
-		err := c.SetNodeLabel(ctx, vpcENIConfigLabel, eniConfigName)
+		// Add the feature name to CNINode of this node
+		err := c.AddFeatureToCNINode(ctx, rcv1alpha1.CustomNetworking, eniConfigName)
 		if err != nil {
-			log.Errorf("Failed to set eniConfig node label", err)
-			podENIErrInc("nodeInit")
-			return err
-		}
-	} else {
-		// Remove the custom networking label
-		err := c.SetNodeLabel(ctx, vpcENIConfigLabel, "")
-		if err != nil {
-			log.Errorf("Failed to delete eniConfig node label", err)
+			log.Errorf("Failed to add feature custom networking into CNINode", err)
 			podENIErrInc("nodeInit")
 			return err
 		}
+		log.Infof("Enabled feature %s in CNINode for node %s if not existing", rcv1alpha1.CustomNetworking, c.myNodeName)
 	}
 
-	if metadataResult.TrunkENI != "" {
-		// Signal to VPC Resource Controller that the node has a trunk already
-		err := c.SetNodeLabel(ctx, trunkInterfaceLabel, "true")
-		if err != nil {
-			log.Errorf("Failed to set node label", err)
-			podENIErrInc("nodeInit")
-			// If this fails, we probably can't talk to the API server. Let the pod restart
-			return err
-		}
-	} else {
-		// Check if we want to label node for trunk interface during node init
+	if c.enablePodENI {
+		// Check if we want to add feature into CNINode for trunk interface during node init
+		// we don't check if the node already has trunk added during initialization
 		c.askForTrunkENIIfNeeded(ctx)
 	}
 
@@ -701,7 +682,9 @@ func (c *IPAMContext) StartNodeIPPoolManager() {
 }
 
 func (c *IPAMContext) updateIPPoolIfRequired(ctx context.Context) {
-	c.askForTrunkENIIfNeeded(ctx)
+	if c.enablePodENI && c.dataStore.GetTrunkENI() == "" {
+		c.askForTrunkENIIfNeeded(ctx)
+	}
 
 	if c.isDatastorePoolTooLow() {
 		c.increaseDatastorePool(ctx)
@@ -1283,27 +1266,18 @@ func (c *IPAMContext) logPoolStats(dataStoreStats *datastore.DataStoreStats) {
 }
 
 func (c *IPAMContext) askForTrunkENIIfNeeded(ctx context.Context) {
-	if c.enablePodENI && c.dataStore.GetTrunkENI() == "" {
-		// Check that there is room for a trunk ENI to be attached:
-		if c.dataStore.GetENIs() >= (c.maxENI - c.unmanagedENI) {
-			log.Debug("No slot available for a trunk ENI to be attached. Not labeling the node")
-			return
-		}
-		// We need to signal that VPC Resource Controller needs to attach a trunk ENI
-		err := c.SetNodeLabel(ctx, trunkInterfaceLabel, "false")
-		if err != nil {
-			podENIErrInc("askForTrunkENIIfNeeded")
-			log.Errorf("Failed to set node label", err)
-		}
+	// Check that there is room for a trunk ENI to be attached:
+	if c.dataStore.GetENIs() >= (c.maxENI - c.unmanagedENI) {
+		log.Error("No slot available for a trunk ENI to be attached. Not init the node")
+		return
 	}
-}
-
-func (c *IPAMContext) getNodeLabels(ctx context.Context) (map[string]string, error) {
-	var node corev1.Node
-	if err := c.cachedK8SClient.Get(ctx, types.NamespacedName{Name: c.myNodeName}, &node); err != nil {
-		return nil, err
+	// We need to signal that VPC Resource Controller needs to attach a trunk ENI
+	err := c.AddFeatureToCNINode(ctx, rcv1alpha1.SecurityGroupsForPods, "")
+	if err != nil {
+		podENIErrInc("askForTrunkENIIfNeeded")
+		log.Errorf("Failed to add SGP feature into the CNINode", err)
 	} else {
-		return node.GetLabels(), err
+		log.Infof("Successfully added feature %s to CNINode if not existing", rcv1alpha1.SecurityGroupsForPods)
 	}
 }
 
@@ -1417,13 +1391,6 @@ func (c *IPAMContext) nodeIPPoolReconcile(ctx context.Context, interval time.Dur
 
 		if c.enablePodENI && metadataResult.TrunkENI != "" {
 			log.Debugf("Trunk interface (%s) has been added to the node already.", metadataResult.TrunkENI)
-			// Label the node that we have a trunk
-			err = c.SetNodeLabel(ctx, trunkInterfaceLabel, "true")
-			if err != nil {
-				podENIErrInc("askForTrunkENIIfNeeded")
-				log.Errorf("Failed to set node label for trunk. Aborting reconcile", err)
-				return
-			}
 		}
 		// Update trunk ENI
 		trunkENI = metadataResult.TrunkENI
@@ -1994,54 +1961,6 @@ func (c *IPAMContext) getTrunkLinkIndex() (int, error) {
 	return -1, errors.New("no trunk found")
 }
 
-// SetNodeLabel sets or deletes a node label
-func (c *IPAMContext) SetNodeLabel(ctx context.Context, key, value string) error {
-	request := types.NamespacedName{
-		Name: c.myNodeName,
-	}
-
-	err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-		node := &corev1.Node{}
-		// Find my node
-		err := c.cachedK8SClient.Get(ctx, request, node)
-		if err != nil {
-			log.Errorf("Failed to get node: %v", err)
-			return err
-		}
-		log.Debugf("Node found %q - no of labels - %d", node.Name, len(node.Labels))
-
-		if labelValue, ok := node.Labels[key]; ok && labelValue == value {
-			log.Debugf("Node label %q is already %q", key, labelValue)
-			return nil
-		}
-
-		// Make deep copy for modification
-		updateNode := node.DeepCopy()
-
-		// in case for some reason node object doesn't have label map
-		if updateNode.Labels == nil {
-			updateNode.Labels = make(map[string]string)
-		}
-
-		// Set node label
-		if value != "" {
-			updateNode.Labels[key] = value
-		} else {
-			// Empty value, delete the label
-			log.Debugf("Deleting label %q", key)
-			delete(updateNode.Labels, key)
-		}
-
-		if err = c.cachedK8SClient.Update(ctx, updateNode); err != nil {
-			log.Errorf("Failed to patch node %s with label %q: %q, error: %v", c.myNodeName, key, value, err)
-			return err
-		}
-		log.Debugf("Updated node %s with label %q: %q", c.myNodeName, key, value)
-		return nil
-	})
-	return err
-}
-
 // GetPod returns the pod matching the name and namespace
 func (c *IPAMContext) GetPod(podName, namespace string) (*corev1.Pod, error) {
 	ctx := context.TODO()
@@ -2371,3 +2290,43 @@ func (c *IPAMContext) isConfigValid() bool {
 
 	return true
 }
+
+func (c *IPAMContext) AddFeatureToCNINode(ctx context.Context, featureName rcv1alpha1.FeatureName, featureValue string) error {
+	cniNode := &rcv1alpha1.CNINode{}
+
+	if err := c.rawK8SClient.Get(ctx, types.NamespacedName{Name: c.myNodeName}, cniNode); err != nil {
+		return err
+	}
+
+	if lo.ContainsBy(cniNode.Spec.Features, func(addedFeature rcv1alpha1.Feature) bool {
+		return featureName == addedFeature.Name && featureValue == addedFeature.Value
+	}) {
+		return nil
+	}
+
+	newCNINode := cniNode.DeepCopy()
+	newFeature := rcv1alpha1.Feature{
+		Name:  featureName,
+		Value: featureValue,
+	}
+
+	if lo.ContainsBy(cniNode.Spec.Features, func(addedFeature rcv1alpha1.Feature) bool {
+		return featureName == addedFeature.Name
+	}) {
+		// this should happen when user updated eniConfig name
+		// aws-node restarted and CNINode need to be updated if node wasn't terminated
+		// we need groom old features here
+		newCNINode.Spec.Features = []rcv1alpha1.Feature{}
+		for _, oldFeature := range cniNode.Spec.Features {
+			if oldFeature.Name == featureName {
+				continue
+			} else {
+				newCNINode.Spec.Features = append(newCNINode.Spec.Features, oldFeature)
+			}
+		}
+	}
+
+	newCNINode.Spec.Features = append(newCNINode.Spec.Features, newFeature)
+
+	return c.rawK8SClient.Patch(ctx, newCNINode, client.MergeFromWithOptions(cniNode, client.MergeFromWithOptimisticLock{}))
+}