Merge master into release-1.16 branch for v1.16.4 release (#2816)

.github/PULL_REQUEST_TEMPLATE.md

@@ -12,23 +12,27 @@
 Add one of the following:
 bug
 cleanup
+dependency update
 documentation
 feature
+improvement
+release workflow
+testing
 -->
 
-**Which issue does this PR fix**:
+**Which issue does this PR fix?**:
+<!-- If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue -->
 
 
-**What does this PR do / Why do we need it**:
-
-
-**If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue**:
-
+**What does this PR do / Why do we need it?**:
 
 **Testing done on this change**:
 <!--
-output of manual testing/integration tests results and also attach logs
-showing the fix being resolved
+Please paste the output from manual and/or integration test results. Please also attach any relevant logs.
+-->
+
+<!-- 
+If adding a new integration test to any of the CNI release test suites, determine if the test can run against the latest VPC CNI image or if it is dependent on a future version release. If dependent, please call `Skip()` in the test to prevent it from running before the future version is available.
 -->
 
 **Will this PR introduce any new dependencies?**:

.github/workflows/deps.yml

@@ -26,14 +26,14 @@ jobs:
       - id: govulncheck
         uses: ./.github/actions/govulncheck
         with:
-          go-version-input: 1.21.6
+          go-version-input: 1.21.7
           go-version-file: go.mod
           cache: false
           repo-checkout: false
       - id: govulncheck-tests-agent
         uses: ./.github/actions/govulncheck
         with:
-          go-version-input: 1.21.6
+          go-version-input: 1.21.7
           go-version-file: test/agent/go.mod
           cache: false
           repo-checkout: false

.github/workflows/weekly-cron-tests.yaml

@@ -44,7 +44,6 @@ jobs:
           RUN_CNI_INTEGRATION_TESTS: false
           PERFORMANCE_TEST_S3_BUCKET_NAME: cni-performance-tests
           RUN_PERFORMANCE_TESTS: true
-          RUN_TESTER_LB_ADDONS: true
         run: |
           ./scripts/run-integration-tests.sh
       - name: Run kops tests
@@ -54,9 +53,8 @@ jobs:
           ROLE_ARN: ${{ secrets.EKS_CLUSTER_ROLE_ARN }}
           RUN_CNI_INTEGRATION_TESTS: false
           RUN_KOPS_TEST: true
-          RUN_TESTER_LB_ADDONS: true
-          K8S_VERSION: 1.29.0-alpha.2
-          KOPS_VERSION: v1.29.0-alpha.2
+          K8S_VERSION: 1.29.0-alpha.3
+          KOPS_VERSION: v1.29.0-alpha.3
         run: |
           ./scripts/run-integration-tests.sh
         if: always()
@@ -67,7 +65,6 @@ jobs:
           ROLE_ARN: ${{ secrets.EKS_CLUSTER_ROLE_ARN }}
           RUN_CNI_INTEGRATION_TESTS: false
           RUN_BOTTLEROCKET_TEST: true
-          RUN_TESTER_LB_ADDONS: true
         run: |
           ./scripts/run-integration-tests.sh
         if: always()

Makefile

@@ -22,7 +22,7 @@
 # VERSION is the source revision that executables and images are built from.
 VERSION ?= $(shell git describe --tags --always --dirty || echo "unknown")
 # GOLANG_IMAGE is the building golang container image used.
-GOLANG_IMAGE ?= public.ecr.aws/eks-distro-build-tooling/golang:1.21.6-7-gcc-al2
+GOLANG_IMAGE ?= public.ecr.aws/eks-distro-build-tooling/golang:1.21.7-8-gcc-al2
 # BASE_IMAGE_CNI is the base layer image for the primary AWS VPC CNI plugin container
 BASE_IMAGE_CNI ?= public.ecr.aws/eks-distro-build-tooling/eks-distro-minimal-base-iptables:latest.2
 # BASE_IMAGE_CNI_INIT is the base layer image for the AWS VPC CNI init container

README.md

@@ -226,7 +226,7 @@ Type: Integer as a String
 
 Default: 9001
 
-Used to configure the MTU size for attached ENIs. The valid range is from `576` to `9001`.
+Used to configure the MTU size for attached ENIs. The valid range for IPv4 is from `576` to `9001`, while the valid range for IPv6 is from `1280` to `9001`.
 
 #### `AWS_VPC_K8S_CNI_EXTERNALSNAT`
 
@@ -267,6 +267,15 @@ Default: empty
 Specify a comma-separated list of IPv4 CIDRs to exclude from SNAT. For every item in the list an `iptables` rule and off\-VPC
 IP rule will be applied. If an item is not a valid ipv4 range it will be skipped. This should be used when `AWS_VPC_K8S_CNI_EXTERNALSNAT=false`.
 
+#### `POD_MTU` (v1.16.4+)
+
+Type: Integer as a String
+
+*Note*: If unset, the default value is derived from `AWS_VPC_ENI_MTU`, which defaults to `9001`.
+Default: 9001
+
+Used to configure the MTU size for pod virtual interfaces. The valid range for IPv4 is from `576` to `9001`, while the valid range for IPv6 is from `1280` to `9001`.
+
 #### `WARM_ENI_TARGET`
 
 Type: Integer as a String
@@ -589,7 +598,7 @@ Setting `ANNOTATE_POD_IP` to `true` will allow IPAMD to add an annotation `vpc.a
 
 There is a known [issue](https://github.com/kubernetes/kubernetes/issues/39113) with kubelet taking time to update `Pod.Status.PodIP` leading to calico being blocked on programming the policy. Setting `ANNOTATE_POD_IP` to `true` will enable AWS VPC CNI plugin to add Pod IP as an annotation to the pod spec to address this race condition.
 
-To annotate the pod with pod IP, you will have to add "patch" permission for pods resource in aws-node clusterrole. You can use the below command -
+To annotate the pod with pod IP, you will have to add `patch` permission for pods resource in aws-node clusterrole. You can use the below command -
 
 ```
 cat << EOF > append.yaml
@@ -606,6 +615,8 @@ EOF
 kubectl apply -f <(cat <(kubectl get clusterrole aws-node -o yaml) append.yaml)
 ```
 
+NOTE: Adding `patch` permissions to the `aws-node` Daemonset increases the security scope for the plugin, so add this permission only after performing a proper security assessment of the tradeoffs.
+
 #### `ENABLE_IPv4` (v1.10.0+)
 
 Type: Boolean as a String

charts/aws-vpc-cni/README.md

@@ -108,25 +108,27 @@ $ helm install aws-vpc-cni --namespace kube-system eks/aws-vpc-cni --values valu
 
 ## Adopting the existing aws-node resources in an EKS cluster
 
-If you do not want to delete the existing aws-node resources in your cluster that run the aws-vpc-cni and then install this helm chart, you can adopt the resources into a release instead. Refer to the script below to import existing resources into helm. Once you have annotated and labeled all the resources this chart specifies, enable the `originalMatchLabels` flag. If you have been careful this should not diff and leave all the resources unmodified and now under management of helm.
+If you do not want to delete the existing aws-node resources in your cluster that run the aws-vpc-cni and then install this helm chart, you can adopt the resources into a release instead. Refer to the script below to import existing resources into helm. Once you have annotated and labeled all the resources this chart specifies, enable the `originalMatchLabels` flag. If you have been careful, this should not diff and leave all the resources unmodified and now under management of helm.
 
-WARNING: Substitute YOUR_HELM_RELEASE_NAME_HERE with the name of your helm release.
 ```
 #!/usr/bin/env bash
 
 set -euo pipefail
 
 for kind in daemonSet clusterRole clusterRoleBinding serviceAccount; do
   echo "setting annotations and labels on $kind/aws-node"
-  kubectl -n kube-system annotate --overwrite $kind aws-node meta.helm.sh/release-name=YOUR_HELM_RELEASE_NAME_HERE
+  kubectl -n kube-system annotate --overwrite $kind aws-node meta.helm.sh/release-name=aws-vpc-cni
   kubectl -n kube-system annotate --overwrite $kind aws-node meta.helm.sh/release-namespace=kube-system
   kubectl -n kube-system label --overwrite $kind aws-node app.kubernetes.io/managed-by=Helm
 done
 
-kubectl -n kube-system annotate --overwrite configmap amazon-vpc-cni meta.helm.sh/release-name=YOUR_HELM_RELEASE_NAME_HERE
+kubectl -n kube-system annotate --overwrite configmap amazon-vpc-cni meta.helm.sh/release-name=aws-vpc-cni
 kubectl -n kube-system annotate --overwrite configmap amazon-vpc-cni meta.helm.sh/release-namespace=kube-system
 kubectl -n kube-system label --overwrite configmap amazon-vpc-cni app.kubernetes.io/managed-by=Helm
 
+Kubernetes recommends using server-side apply for more control over the field manager. After adopting the chart resources, you can run the following command to apply the chart:
+```
+helm template aws-vpc-cni --include-crds --namespace kube-system eks/aws-vpc-cni --set originalMatchLabels=true | kubectl apply --server-side --force-conflicts --field-manager Helm -f -
 ```
 
 ## Migrate from Helm v2 to Helm v3

cmd/aws-vpc-cni/main.go

@@ -66,7 +66,9 @@ const (
 	defaultAWSconflistFile       = "/app/10-aws.conflist"
 	tmpAWSconflistFile           = "/tmp/10-aws.conflist"
 	defaultVethPrefix            = "eni"
-	defaultMTU                   = "9001"
+	defaultMTU                   = 9001
+	minMTUv4                     = 576
+	minMTUv6                     = 1280
 	defaultEnablePodEni          = false
 	defaultPodSGEnforcingMode    = "strict"
 	defaultPluginLogFile         = "/var/log/aws-routed-eni/plugin.log"
@@ -88,6 +90,7 @@ const (
 	envHostCniConfDirPath    = "HOST_CNI_CONFDIR_PATH"
 	envVethPrefix            = "AWS_VPC_K8S_CNI_VETHPREFIX"
 	envEniMTU                = "AWS_VPC_ENI_MTU"
+	envPodMTU                = "POD_MTU"
 	envEnablePodEni          = "ENABLE_POD_ENI"
 	envPodSGEnforcingMode    = "POD_SECURITY_GROUP_ENFORCING_MODE"
 	envPluginLogFile         = "AWS_VPC_K8S_PLUGIN_LOG_FILE"
@@ -278,15 +281,18 @@ func generateJSON(jsonFile string, outFile string, getPrimaryIP func(ipv4 bool)
 		}
 	}
 	vethPrefix := utils.GetEnv(envVethPrefix, defaultVethPrefix)
-	mtu := utils.GetEnv(envEniMTU, defaultMTU)
+	// Derive pod MTU from ENI MTU by default (note that values have already been validated)
+	eniMTU := utils.GetEnv(envEniMTU, strconv.Itoa(defaultMTU))
+	// If pod MTU environment variable is set, overwrite ENI MTU.
+	podMTU := utils.GetEnv(envPodMTU, eniMTU)
 	podSGEnforcingMode := utils.GetEnv(envPodSGEnforcingMode, defaultPodSGEnforcingMode)
 	pluginLogFile := utils.GetEnv(envPluginLogFile, defaultPluginLogFile)
 	pluginLogLevel := utils.GetEnv(envPluginLogLevel, defaultPluginLogLevel)
 	randomizeSNAT := utils.GetEnv(envRandomizeSNAT, defaultRandomizeSNAT)
 
 	netconf := string(byteValue)
 	netconf = strings.Replace(netconf, "__VETHPREFIX__", vethPrefix, -1)
-	netconf = strings.Replace(netconf, "__MTU__", mtu, -1)
+	netconf = strings.Replace(netconf, "__MTU__", podMTU, -1)
 	netconf = strings.Replace(netconf, "__PODSGENFORCINGMODE__", podSGEnforcingMode, -1)
 	netconf = strings.Replace(netconf, "__PLUGINLOGFILE__", pluginLogFile, -1)
 	netconf = strings.Replace(netconf, "__PLUGINLOGLEVEL__", pluginLogLevel, -1)
@@ -385,6 +391,11 @@ func validateEnvVars() bool {
 		return false
 	}
 
+	// Validate MTU value for ENIs and pods
+	if !validateMTU(envEniMTU) || !validateMTU(envPodMTU) {
+		return false
+	}
+
 	prefixDelegationEn := utils.GetBoolAsStringEnvVar(envEnPrefixDelegation, defaultEnPrefixDelegation)
 	warmIPTarget := utils.GetEnv(envWarmIPTarget, "0")
 	warmPrefixTarget := utils.GetEnv(envWarmPrefixTarget, "0")
@@ -398,6 +409,29 @@ func validateEnvVars() bool {
 	return true
 }
 
+func validateMTU(envVar string) bool {
+	// Validate MTU range based on IP address family
+	enabledIPv6 := utils.GetBoolAsStringEnvVar(envEnIPv6, defaultEnableIPv6)
+
+	mtu, err, input := utils.GetIntFromStringEnvVar(envVar, defaultMTU)
+	if err != nil {
+		log.Errorf("%s MUST be a valid integer. %s is invalid", envVar, input)
+		return false
+	}
+	if enabledIPv6 {
+		if mtu < minMTUv6 || mtu > defaultMTU {
+			log.Errorf("%s cannot be less than 1280 or greater than 9001 in IPv6. %s is invalid", envVar, input)
+			return false
+		}
+	} else {
+		if mtu < minMTUv4 || mtu > defaultMTU {
+			log.Errorf("%s cannot be less than 576 or greater than 9001 in IPv4. %s is invalid", envVar, input)
+			return false
+		}
+	}
+	return true
+}
+
 func main() {
 	os.Exit(_main())
 }

cmd/aws-vpc-cni/main_test.go

@@ -47,3 +47,40 @@ func TestGenerateJSONPlusBandwidthAndTuning(t *testing.T) {
 	err := generateJSON(awsConflist, devNull, getPrimaryIPMock)
 	assert.NoError(t, err)
 }
+
+func TestMTUValidation(t *testing.T) {
+	// By default, ENI MTU and pod MTU should be valid
+	assert.True(t, validateMTU(envEniMTU))
+	assert.True(t, validateMTU(envPodMTU))
+
+	// Non-integer values should fail
+	_ = os.Setenv(envEniMTU, "true")
+	_ = os.Setenv(envPodMTU, "abc")
+	assert.False(t, validateMTU(envEniMTU))
+	assert.False(t, validateMTU(envPodMTU))
+
+	// Integer values within IPv4 range should succeed
+	_ = os.Setenv(envEniMTU, "5000")
+	_ = os.Setenv(envPodMTU, "3000")
+	assert.True(t, validateMTU(envEniMTU))
+	assert.True(t, validateMTU(envPodMTU))
+
+	// Integer values outside IPv4 range should fail
+	_ = os.Setenv(envEniMTU, "10000")
+	_ = os.Setenv(envPodMTU, "500")
+	assert.False(t, validateMTU(envEniMTU))
+	assert.False(t, validateMTU(envPodMTU))
+
+	// Integer values within IPv6 range should succeed
+	_ = os.Setenv(envEnIPv6, "true")
+	_ = os.Setenv(envEniMTU, "5000")
+	_ = os.Setenv(envPodMTU, "3000")
+	assert.True(t, validateMTU(envEniMTU))
+	assert.True(t, validateMTU(envPodMTU))
+
+	// Integer values outside IPv6 range should fail
+	_ = os.Setenv(envEniMTU, "10000")
+	_ = os.Setenv(envPodMTU, "1200")
+	assert.False(t, validateMTU(envEniMTU))
+	assert.False(t, validateMTU(envPodMTU))
+}

cmd/cni-metrics-helper/README.md

@@ -15,24 +15,33 @@ The following diagram shows how `cni-metrics-helper` works in a cluster:
 As you can see in the diagram, the `cni-metrics-helper` connects to the API Server over https (`tcp/443`), and another connection is created from the API Server to the worker node over http (`tcp/61678`). If you deploy Amazon EKS with the recommended security groups from [Restricting cluster traffic](https://docs.aws.amazon.com/eks/latest/userguide/sec-group-reqs.html#security-group-restricting-cluster-traffic), then make sure that a security group is in place that allows the inbound connection from the API Server to the worker nodes over `tcp/61678`.
 
 Adding the CNI metrics helper will publish the following metrics to CloudWatch:
-```
-"addReqCount",
-"assignIPAddresses",
-"awsAPIErr",
-"awsAPILatency",
-"awsUtilErr",
-"delReqCount",
-"eniAllocated",
-"eniMaxAvailable",
-"ipamdActionInProgress",
-"ipamdErr",
-"maxIPAddresses",
-"podENIErr",
-"reconcileCount",
-"totalIPAddresses",
-"totalIPv4Prefixes",
-"totalAssignedIPv4sPerCidr"
-```
+
+| Metric | Description | Statistic[^1] |
+| ------ | ----------- | ------------- |
+| addReqCount | The number of CNI ADD requests that require an IP address | Sum |
+| assignIPAddresses |  The number of IP addresses assigned to pods | Sum |
+| awsAPIErr | The number of times AWS API returns an error | Sum |
+| awsAPILatency | AWS API call latency in ms | Max |
+| awsUtilErr | The number of errors not handled in awsutils library | Sum |
+| delReqCount | The number of CNI DEL requests | Sum |
+| eniAllocated | The number of ENIs allocated | Sum |
+| eniMaxAvailable | The maximum number of ENIs that can be attached to this instance, accounting for unmanaged ENIs | Sum |
+| ipamdActionInProgress | The number of ipamd actions in progress | Sum |
+| ipamdErr | The number of errors encountered in ipamd | Sum |
+| maxIPAddresses | The maximum number of IP addresses that can be allocated to the instance | Sum | 
+| podENIErr | The number of errors encountered while managing ENIs for pods | Sum |
+| reconcileCount | The number of times ipamd reconciles on ENIs and IP/Prefix addresses | Sum |
+| totalIPAddresses | The number of IPs allocated for pods | Sum |
+| totalIPv4Prefixes | The total number of IPv4 prefixes | Sum |
+| totalAssignedIPv4sPerCidr | The total number of IP addresses assigned per cidr | Sum |
+| forceRemoveENI | The number of ENIs force removed while they had assigned pods | Sum |
+| forceRemoveIPs | The number of IPs force removed while they had assigned pods | Sum |
+| ec2ApiReqCount | The number of requests made to EC2 APIs by CNI | Sum |
+| ec2ApiErrCount | The number of failed EC2 API requests | Sum |
+
+[^1]: This column indicates how the metric has been aggregated across all nodes
+  Sum: For datapoints from all nodes, this is the summation of those datapoints
+  Max: For datapoints from all nodes, this is the maximum value of those datapoints
 
 ## Using IRSA
 As per [AWS EKS Security Best Practice](https://docs.aws.amazon.com/eks/latest/userguide/best-practices-security.html), if you are using IRSA for pods then following requirements must be satisfied to succesfully publish metrics to CloudWatch

cmd/routed-eni-cni-plugin/cni.go

@@ -144,7 +144,8 @@ func add(args *skel.CmdArgs, cniTypes typeswrapper.CNITYPES, grpcClient grpcwrap
 		return errors.Wrap(err, "add cmd: failed to load k8s config from arg")
 	}
 
-	mtu := networkutils.GetEthernetMTU(conf.MTU)
+	// Derive pod MTU. Note that the value has already been validated.
+	mtu := networkutils.GetPodMTU(conf.MTU)
 	log.Debugf("MTU value set is %d:", mtu)
 
 	// Set up a connection to the ipamD server.

go.mod

@@ -12,7 +12,6 @@ require (
 	github.com/coreos/go-iptables v0.7.0
 	github.com/go-logr/logr v1.4.1
 	github.com/golang/mock v1.6.0
-	github.com/golang/protobuf v1.5.3
 	github.com/google/go-cmp v0.6.0
 	github.com/onsi/ginkgo/v2 v2.14.0
 	github.com/onsi/gomega v1.30.0
@@ -31,7 +30,7 @@ require (
 	google.golang.org/grpc v1.61.0
 	gopkg.in/natefinch/lumberjack.v2 v2.2.1
 	gopkg.in/yaml.v2 v2.4.0
-	helm.sh/helm/v3 v3.14.0
+	helm.sh/helm/v3 v3.14.2
 	k8s.io/api v0.29.0
 	k8s.io/apimachinery v0.29.0
 	k8s.io/cli-runtime v0.29.0
@@ -82,6 +81,7 @@ require (
 	github.com/gobwas/glob v0.2.3 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
+	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/btree v1.0.1 // indirect
 	github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect

go.sum

@@ -604,8 +604,8 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools/v3 v3.4.0 h1:ZazjZUfuVeZGLAmlKKuyv3IKP5orXcwtOwDQH6YVr6o=
 gotest.tools/v3 v3.4.0/go.mod h1:CtbdzLSsqVhDgMtKsx03ird5YTGB3ar27v0u/yKBW5g=
-helm.sh/helm/v3 v3.14.0 h1:TaZIH6uOchn7L27ptwnnuHJiFrT/BsD4dFdp/HLT2nM=
-helm.sh/helm/v3 v3.14.0/go.mod h1:2itvvDv2WSZXTllknfQo6j7u3VVgMAvm8POCDgYH424=
+helm.sh/helm/v3 v3.14.2 h1:V71fv+NGZv0icBlr+in1MJXuUIHCiPG1hW9gEBISTIA=
+helm.sh/helm/v3 v3.14.2/go.mod h1:2itvvDv2WSZXTllknfQo6j7u3VVgMAvm8POCDgYH424=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 k8s.io/api v0.29.0 h1:NiCdQMY1QOp1H8lfRyeEf8eOwV6+0xA6XEE44ohDX2A=

pkg/ipamd/datastore/data_store.go

@@ -978,8 +978,8 @@ func (ds *DataStore) GetENINeedsIP(maxIPperENI int, skipPrimary bool) *ENI {
 	ds.lock.Lock()
 	defer ds.lock.Unlock()
 	for _, eni := range ds.eniPool {
-		if skipPrimary && eni.IsPrimary {
-			ds.log.Debugf("Skip the primary ENI for need IP check")
+		if (skipPrimary && eni.IsPrimary) || eni.IsTrunk {
+			ds.log.Debugf("Skip needs IP check for trunk ENI of primary ENI when Custom Networking is enabled")
 			continue
 		}
 		if len(eni.AvailableIPv4Cidrs) < maxIPperENI {