From 76802d3338199e3af849a44f8c61f062101a3760 Mon Sep 17 00:00:00 2001 From: Nick Baker Date: Mon, 29 Jan 2024 20:46:17 +0000 Subject: [PATCH 1/3] handle pod-infra-container-image on older versions --- nodeadm/internal/kubelet/config.go | 33 +++++++++++++++++++ .../e2e/cases/pod-infra-container/config.yaml | 9 +++++ .../test/e2e/cases/pod-infra-container/run.sh | 18 ++++++++++ 3 files changed, 60 insertions(+) create mode 100644 nodeadm/test/e2e/cases/pod-infra-container/config.yaml create mode 100755 nodeadm/test/e2e/cases/pod-infra-container/run.sh diff --git a/nodeadm/internal/kubelet/config.go b/nodeadm/internal/kubelet/config.go index fa8a79ff7..e6a5d04d1 100644 --- a/nodeadm/internal/kubelet/config.go +++ b/nodeadm/internal/kubelet/config.go @@ -240,6 +240,35 @@ 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, kubeletVersion string, flags map[string]string) error { + if semver.Compare(kubeletVersion, "v1.27.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 + } + return nil +} + func (k *kubelet) GenerateKubeletConfig(cfg *api.NodeConfig) (*kubeletConfig, error) { // Get the kubelet/kubernetes version to help conditionally enable features kubeletVersion, err := GetKubeletVersion() @@ -249,6 +278,7 @@ func (k *kubelet) GenerateKubeletConfig(cfg *api.NodeConfig) (*kubeletConfig, er zap.L().Info("Detected kubelet version", zap.String("version", kubeletVersion)) kubeletConfig := defaultKubeletSubConfig() + if err := kubeletConfig.withFallbackClusterDns(&cfg.Spec.Cluster); err != nil { return nil, err } @@ -258,6 +288,9 @@ 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, kubeletVersion, k.flags); err != nil { + return nil, err + } kubeletConfig.withVersionToggles(kubeletVersion, k.flags) kubeletConfig.withCloudProvider(cfg, k.flags) diff --git a/nodeadm/test/e2e/cases/pod-infra-container/config.yaml b/nodeadm/test/e2e/cases/pod-infra-container/config.yaml new file mode 100644 index 000000000..e859ba74b --- /dev/null +++ b/nodeadm/test/e2e/cases/pod-infra-container/config.yaml @@ -0,0 +1,9 @@ +--- +apiVersion: node.eks.aws/v1alpha1 +kind: NodeConfig +spec: + cluster: + name: my-cluster + apiServerEndpoint: https://example.com + certificateAuthority: Y2VydGlmaWNhdGVBdXRob3JpdHk= + cidr: 10.100.0.0/16 diff --git a/nodeadm/test/e2e/cases/pod-infra-container/run.sh b/nodeadm/test/e2e/cases/pod-infra-container/run.sh new file mode 100755 index 000000000..2a2d2e1ad --- /dev/null +++ b/nodeadm/test/e2e/cases/pod-infra-container/run.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash + +set -o errexit +set -o nounset +set -o pipefail + +source /helpers.sh + +mock::imds +wait::dbus-ready + +mock::kubelet 1.26.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.27.0 +nodeadm init --skip run --config-source file://config.yaml +assert::file-not-contains /etc/eks/kubelet/environment 'pod-infra-container-image' From a200bdecce74c9edd27f3b55a3d8403cf6866596 Mon Sep 17 00:00:00 2001 From: Nick Baker Date: Mon, 29 Jan 2024 22:44:36 +0000 Subject: [PATCH 2/3] remove version requirement for adding flag --- nodeadm/internal/kubelet/config.go | 38 +++++++++---------- .../test/e2e/cases/pod-infra-container/run.sh | 6 +-- 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/nodeadm/internal/kubelet/config.go b/nodeadm/internal/kubelet/config.go index e6a5d04d1..3f5ec3c91 100644 --- a/nodeadm/internal/kubelet/config.go +++ b/nodeadm/internal/kubelet/config.go @@ -246,26 +246,24 @@ func (ksc *kubeletConfig) withDefaultReservedResources() { // 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, kubeletVersion string, flags map[string]string) error { - if semver.Compare(kubeletVersion, "v1.27.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 +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 } + flags["pod-infra-container-image"] = pauseContainerImage return nil } @@ -288,7 +286,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, kubeletVersion, k.flags); err != nil { + if err := kubeletConfig.withPodInfraContainerImage(cfg, 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 2a2d2e1ad..2324455cf 100755 --- a/nodeadm/test/e2e/cases/pod-infra-container/run.sh +++ b/nodeadm/test/e2e/cases/pod-infra-container/run.sh @@ -9,10 +9,6 @@ source /helpers.sh mock::imds wait::dbus-ready -mock::kubelet 1.26.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.27.0 nodeadm init --skip run --config-source file://config.yaml -assert::file-not-contains /etc/eks/kubelet/environment 'pod-infra-container-image' +assert::file-contains /etc/eks/kubelet/environment '--pod-infra-container-image=602401143452.dkr.ecr.us-west-2.amazonaws.com/eks/pause:3.5' From f9849b4a9101c983da46937d760f8d8efe5eb46c Mon Sep 17 00:00:00 2001 From: Nick Baker Date: Wed, 31 Jan 2024 23:34:05 +0000 Subject: [PATCH 3/3] remove pod infra flag on 1.29+ since its a noop --- nodeadm/internal/kubelet/config.go | 53 ++++++++++--------- .../test/e2e/cases/pod-infra-container/run.sh | 6 ++- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/nodeadm/internal/kubelet/config.go b/nodeadm/internal/kubelet/config.go index 3f5ec3c91..df8c639e3 100644 --- a/nodeadm/internal/kubelet/config.go +++ b/nodeadm/internal/kubelet/config.go @@ -240,30 +240,35 @@ 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 no longer considers the flag value. + // see: https://github.com/kubernetes/kubernetes/pull/118544 + 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 +291,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'