From 2f8bcaa731335c4a0ceef40b957dd8d2a51d99f6 Mon Sep 17 00:00:00 2001
From: Nick Baker <nbakerd@amazon.com>
Date: Wed, 31 Jan 2024 23:34:05 +0000
Subject: [PATCH] remove pod infra flag on 1.29+ since its a noop

---
 nodeadm/internal/kubelet/config.go            | 52 ++++++++++---------
 .../test/e2e/cases/pod-infra-container/run.sh |  6 ++-
 2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/nodeadm/internal/kubelet/config.go b/nodeadm/internal/kubelet/config.go
index 3f5ec3c91..9bce97e0f 100644
--- a/nodeadm/internal/kubelet/config.go
+++ b/nodeadm/internal/kubelet/config.go
@@ -240,30 +240,34 @@ func (ksc *kubeletConfig) withDefaultReservedResources() {
 	ksc.KubeReservedCgroup = ptr.String("/runtime")
 }
 
-// The '--pod-infra-container-image' flags is added so that the sandbox image is
-// not garbage collected. There are several way in which we could remove this:
-//   - wait until a minimum supported version of kubernetes which implements the
-//     image pinning CRI support: https://github.com/kubernetes/kubernetes/pull/118544
-//   - update to containerd 2.0, which reworks the abstraction and no longer
-//     requires sandbox image
-func (ksc *kubeletConfig) withPodInfraContainerImage(cfg *api.NodeConfig, flags map[string]string) error {
-	awsDomain, err := util.GetAwsDomain(context.TODO(), imds.New(imds.Options{}))
-	if err != nil {
-		return err
-	}
-	ecrUri, err := util.GetEcrUri(util.GetEcrUriRequest{
-		Region:    cfg.Status.Instance.Region,
-		Domain:    awsDomain,
-		AllowFips: true,
-	})
-	if err != nil {
-		return err
-	}
-	pauseContainerImage, err := util.GetPauseContainer(ecrUri)
-	if err != nil {
-		return err
+// withPodInfraContainerImage determines whether to add the
+// '--pod-infra-container-image' flag, which is used to ensure the sandbox image
+// is not garbage collected.
+//
+// TODO: revisit once the minimum supportted version catches up or the container
+// runtime is moved to containerd 2.0
+func (ksc *kubeletConfig) withPodInfraContainerImage(cfg *api.NodeConfig, kubeletVersion string, flags map[string]string) error {
+	// the flag is a noop on 1.29+, since the behavior was changed to use the
+	// CRI image pinning behavior and not longer considers the flag value.
+	if semver.Compare(kubeletVersion, "v1.29.0") < 0 {
+		awsDomain, err := util.GetAwsDomain(context.TODO(), imds.New(imds.Options{}))
+		if err != nil {
+			return err
+		}
+		ecrUri, err := util.GetEcrUri(util.GetEcrUriRequest{
+			Region:    cfg.Status.Instance.Region,
+			Domain:    awsDomain,
+			AllowFips: true,
+		})
+		if err != nil {
+			return err
+		}
+		pauseContainerImage, err := util.GetPauseContainer(ecrUri)
+		if err != nil {
+			return err
+		}
+		flags["pod-infra-container-image"] = pauseContainerImage
 	}
-	flags["pod-infra-container-image"] = pauseContainerImage
 	return nil
 }
 
@@ -286,7 +290,7 @@ func (k *kubelet) GenerateKubeletConfig(cfg *api.NodeConfig) (*kubeletConfig, er
 	if err := kubeletConfig.withNodeIp(cfg, k.flags); err != nil {
 		return nil, err
 	}
-	if err := kubeletConfig.withPodInfraContainerImage(cfg, k.flags); err != nil {
+	if err := kubeletConfig.withPodInfraContainerImage(cfg, kubeletVersion, k.flags); err != nil {
 		return nil, err
 	}
 
diff --git a/nodeadm/test/e2e/cases/pod-infra-container/run.sh b/nodeadm/test/e2e/cases/pod-infra-container/run.sh
index 2324455cf..fbe65ad91 100755
--- a/nodeadm/test/e2e/cases/pod-infra-container/run.sh
+++ b/nodeadm/test/e2e/cases/pod-infra-container/run.sh
@@ -9,6 +9,10 @@ source /helpers.sh
 mock::imds
 wait::dbus-ready
 
-mock::kubelet 1.27.0
+mock::kubelet 1.28.0
 nodeadm init --skip run --config-source file://config.yaml
 assert::file-contains /etc/eks/kubelet/environment '--pod-infra-container-image=602401143452.dkr.ecr.us-west-2.amazonaws.com/eks/pause:3.5'
+
+mock::kubelet 1.29.0
+nodeadm init --skip run --config-source file://config.yaml
+assert::file-not-contains /etc/eks/kubelet/environment 'pod-infra-container-image'