Skip to content

Commit 4129816

Browse files
fletcherwfletcherw
authored
chore: cleanup unneeded logic for EOL versions (#2382)
1 parent 69480ac commit 4129816

File tree

19 files changed

+51
-303
lines changed

19 files changed

+51
-303
lines changed

‎nodeadm/internal/kubelet/config.go‎

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -208,54 +208,28 @@ func (ksc *kubeletConfig) withNodeIp(cfg *api.NodeConfig, flags map[string]strin
208208
return nil
209209
}
210210

211-
func (ksc *kubeletConfig) withVersionToggles(cfg *api.NodeConfig, flags map[string]string) {
212-
// TODO: remove when 1.26 is EOL
213-
if semver.Compare(cfg.Status.KubeletVersion, "v1.27.0") < 0 {
214-
// --container-runtime flag is gone in 1.27+
215-
flags["container-runtime"] = "remote"
216-
// --container-runtime-endpoint moved to kubelet config start from 1.27
217-
// https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.27.md?plain=1#L1800-L1801
218-
flags["container-runtime-endpoint"] = ksc.ContainerRuntimeEndpoint
219-
}
220-
221-
// TODO: Remove this during 1.27 EOL
222-
// Enable Feature Gate for KubeletCredentialProviders in versions less than 1.28 since this feature flag was removed in 1.28.
223-
if semver.Compare(cfg.Status.KubeletVersion, "v1.28.0") < 0 {
224-
ksc.FeatureGates["KubeletCredentialProviders"] = true
225-
}
226-
227-
// for K8s versions that suport API Priority & Fairness, increase our API server QPS
228-
// in 1.27, the default is already increased to 50/100, so use the higher defaults
229-
if semver.Compare(cfg.Status.KubeletVersion, "v1.27.0") < 0 {
230-
ksc.KubeAPIQPS = ptr.Int(10)
231-
ksc.KubeAPIBurst = ptr.Int(20)
232-
}
233-
211+
func (ksc *kubeletConfig) withVersionToggles(cfg *api.NodeConfig) {
234212
// EKS enables DRA on 1.33+
235213
if semver.Compare(cfg.Status.KubeletVersion, "v1.33.0") >= 0 {
236214
ksc.FeatureGates["DynamicResourceAllocation"] = true
237215
}
238216
}
239217

240218
func (ksc *kubeletConfig) withCloudProvider(cfg *api.NodeConfig, flags map[string]string) {
241-
if semver.Compare(cfg.Status.KubeletVersion, "v1.26.0") >= 0 {
242-
// ref: https://github.com/kubernetes/kubernetes/pull/121367
243-
flags["cloud-provider"] = "external"
244-
// provider ID needs to be specified when the cloud provider is external
245-
ksc.ProviderID = ptr.String(getProviderId(cfg.Status.Instance.AvailabilityZone, cfg.Status.Instance.ID))
246-
var nodeName string
247-
if api.IsFeatureEnabled(api.InstanceIdNodeName, cfg.Spec.FeatureGates) {
248-
zap.L().Info("Opt-in Instance Id naming strategy")
249-
nodeName = cfg.Status.Instance.ID
250-
} else {
251-
// the name of the Node object default to EC2 PrivateDnsName
252-
// see: https://github.com/awslabs/amazon-eks-ami/pull/1264
253-
nodeName = cfg.Status.Instance.PrivateDNSName
254-
}
255-
flags["hostname-override"] = nodeName
219+
// ref: https://github.com/kubernetes/kubernetes/pull/121367
220+
flags["cloud-provider"] = "external"
221+
// provider ID needs to be specified when the cloud provider is external
222+
ksc.ProviderID = ptr.String(getProviderId(cfg.Status.Instance.AvailabilityZone, cfg.Status.Instance.ID))
223+
var nodeName string
224+
if api.IsFeatureEnabled(api.InstanceIdNodeName, cfg.Spec.FeatureGates) {
225+
zap.L().Info("Opt-in Instance Id naming strategy")
226+
nodeName = cfg.Status.Instance.ID
256227
} else {
257-
flags["cloud-provider"] = "aws"
228+
// the name of the Node object default to EC2 PrivateDnsName
229+
// see: https://github.com/awslabs/amazon-eks-ami/pull/1264
230+
nodeName = cfg.Status.Instance.PrivateDNSName
258231
}
232+
flags["hostname-override"] = nodeName
259233
}
260234

261235
// When the DefaultReservedResources flag is enabled, override the kubelet
@@ -314,7 +288,7 @@ func (k *kubelet) GenerateKubeletConfig(cfg *api.NodeConfig) (*kubeletConfig, er
314288
return nil, err
315289
}
316290

317-
kubeletConfig.withVersionToggles(cfg, k.flags)
291+
kubeletConfig.withVersionToggles(cfg)
318292
kubeletConfig.withCloudProvider(cfg, k.flags)
319293
kubeletConfig.withDefaultReservedResources(cfg, k.resources)
320294
kubeletConfig.withImageServiceEndpoint(cfg, k.resources)

‎nodeadm/internal/kubelet/config_test.go‎

Lines changed: 16 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package kubelet
33
import (
44
"testing"
55

6-
"github.com/aws/smithy-go/ptr"
76
"github.com/awslabs/amazon-eks-ami/nodeadm/internal/api"
87
"github.com/awslabs/amazon-eks-ami/nodeadm/internal/containerd"
98
"github.com/stretchr/testify/assert"
@@ -14,24 +13,18 @@ func TestKubeletCredentialProvidersFeatureFlag(t *testing.T) {
1413
kubeletVersion string
1514
expectedValue *bool
1615
}{
17-
{kubeletVersion: "v1.27.0", expectedValue: ptr.Bool(true)},
18-
{kubeletVersion: "v1.28.0", expectedValue: nil},
16+
{kubeletVersion: "v1.28.0"},
1917
}
2018

2119
for _, test := range tests {
22-
kubetConfig := defaultKubeletSubConfig()
20+
kubeletConfig := defaultKubeletSubConfig()
2321
nodeConfig := api.NodeConfig{
2422
Status: api.NodeConfigStatus{
2523
KubeletVersion: test.kubeletVersion,
2624
},
2725
}
28-
kubetConfig.withVersionToggles(&nodeConfig, make(map[string]string))
29-
kubeletCredentialProviders, present := kubetConfig.FeatureGates["KubeletCredentialProviders"]
30-
if test.expectedValue == nil && present {
31-
t.Errorf("KubeletCredentialProviders shouldn't be set for versions %s", test.kubeletVersion)
32-
} else if test.expectedValue != nil && *test.expectedValue != kubeletCredentialProviders {
33-
t.Errorf("expected %v but got %v for KubeletCredentialProviders feature gate", *test.expectedValue, kubeletCredentialProviders)
34-
}
26+
kubeletConfig.withVersionToggles(&nodeConfig)
27+
assert.NotContainsf(t, kubeletConfig.FeatureGates, "KubeletCredentialProviders", "KubeletCredentialProviders shouldn't be set for versions %s", test.kubeletVersion)
3528
}
3629
}
3730

@@ -40,58 +33,19 @@ func TestContainerRuntime(t *testing.T) {
4033
kubeletVersion string
4134
expectedContainerRuntime *string
4235
}{
43-
{kubeletVersion: "v1.26.0", expectedContainerRuntime: ptr.String("remote")},
44-
{kubeletVersion: "v1.27.0", expectedContainerRuntime: nil},
45-
{kubeletVersion: "v1.28.0", expectedContainerRuntime: nil},
36+
{kubeletVersion: "v1.28.0"},
4637
}
4738

4839
for _, test := range tests {
49-
kubeletAruments := make(map[string]string)
50-
kubetConfig := defaultKubeletSubConfig()
40+
kubeletConfig := defaultKubeletSubConfig()
5141
nodeConfig := api.NodeConfig{
5242
Status: api.NodeConfigStatus{
5343
KubeletVersion: test.kubeletVersion,
5444
},
5545
}
56-
kubetConfig.withVersionToggles(&nodeConfig, kubeletAruments)
57-
containerRuntime, present := kubeletAruments["container-runtime"]
58-
if test.expectedContainerRuntime == nil {
59-
if present {
60-
t.Errorf("container-runtime shouldn't be set for versions %s", test.kubeletVersion)
61-
} else {
62-
assert.Equal(t, containerd.ContainerRuntimeEndpoint, kubetConfig.ContainerRuntimeEndpoint)
63-
}
64-
} else if test.expectedContainerRuntime != nil {
65-
if *test.expectedContainerRuntime != containerRuntime {
66-
t.Errorf("expected %v but got %s for container-runtime", *test.expectedContainerRuntime, containerRuntime)
67-
} else {
68-
assert.Equal(t, containerd.ContainerRuntimeEndpoint, kubeletAruments["container-runtime-endpoint"])
69-
}
70-
}
71-
}
72-
}
46+
kubeletConfig.withVersionToggles(&nodeConfig)
7347

74-
func TestKubeAPILimits(t *testing.T) {
75-
var tests = []struct {
76-
kubeletVersion string
77-
expectedKubeAPIQS *int
78-
expectedKubeAPIBurst *int
79-
}{
80-
{kubeletVersion: "v1.26.0", expectedKubeAPIQS: ptr.Int(10), expectedKubeAPIBurst: ptr.Int(20)},
81-
{kubeletVersion: "v1.27.0", expectedKubeAPIQS: nil, expectedKubeAPIBurst: nil},
82-
{kubeletVersion: "v1.28.0", expectedKubeAPIQS: nil, expectedKubeAPIBurst: nil},
83-
}
84-
85-
for _, test := range tests {
86-
kubetConfig := defaultKubeletSubConfig()
87-
nodeConfig := api.NodeConfig{
88-
Status: api.NodeConfigStatus{
89-
KubeletVersion: test.kubeletVersion,
90-
},
91-
}
92-
kubetConfig.withVersionToggles(&nodeConfig, make(map[string]string))
93-
assert.Equal(t, test.expectedKubeAPIQS, kubetConfig.KubeAPIQPS)
94-
assert.Equal(t, test.expectedKubeAPIBurst, kubetConfig.KubeAPIBurst)
48+
assert.Equal(t, containerd.ContainerRuntimeEndpoint, kubeletConfig.ContainerRuntimeEndpoint)
9549
}
9650
}
9751

@@ -100,9 +54,8 @@ func TestProviderID(t *testing.T) {
10054
kubeletVersion string
10155
expectedCloudProvider string
10256
}{
103-
{kubeletVersion: "v1.25.0", expectedCloudProvider: "aws"},
104-
{kubeletVersion: "v1.26.0", expectedCloudProvider: "external"},
105-
{kubeletVersion: "v1.27.0", expectedCloudProvider: "external"},
57+
{kubeletVersion: "v1.28.0"},
58+
{kubeletVersion: "v1.33.0"},
10659
}
10760

10861
nodeConfig := api.NodeConfig{
@@ -116,14 +69,12 @@ func TestProviderID(t *testing.T) {
11669
providerId := getProviderId(nodeConfig.Status.Instance.AvailabilityZone, nodeConfig.Status.Instance.ID)
11770

11871
for _, test := range tests {
119-
kubeletAruments := make(map[string]string)
120-
kubetConfig := defaultKubeletSubConfig()
72+
kubeletArguments := make(map[string]string)
73+
kubeletConfig := defaultKubeletSubConfig()
12174
nodeConfig.Status.KubeletVersion = test.kubeletVersion
122-
kubetConfig.withCloudProvider(&nodeConfig, kubeletAruments)
123-
assert.Equal(t, test.expectedCloudProvider, kubeletAruments["cloud-provider"])
124-
if kubeletAruments["cloud-provider"] == "external" {
125-
assert.Equal(t, *kubetConfig.ProviderID, providerId)
126-
// TODO assert that the --hostname-override == PrivateDnsName
127-
}
75+
kubeletConfig.withCloudProvider(&nodeConfig, kubeletArguments)
76+
assert.Equal(t, "external", kubeletArguments["cloud-provider"])
77+
assert.Equal(t, providerId, *kubeletConfig.ProviderID)
78+
// TODO assert that the --hostname-override == PrivateDnsName
12879
}
12980
}

‎nodeadm/internal/kubelet/daemon.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func (k *kubelet) Configure(cfg *api.NodeConfig) error {
3535
if err := k.writeKubeconfig(cfg); err != nil {
3636
return err
3737
}
38-
if err := k.writeImageCredentialProviderConfig(cfg); err != nil {
38+
if err := k.writeImageCredentialProviderConfig(); err != nil {
3939
return err
4040
}
4141
if err := writeClusterCaCert(cfg.Spec.Cluster.CertificateAuthority); err != nil {

‎nodeadm/internal/kubelet/image-credential-provider.go‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ import (
99
"path/filepath"
1010
"text/template"
1111

12-
"github.com/awslabs/amazon-eks-ami/nodeadm/internal/api"
1312
"github.com/awslabs/amazon-eks-ami/nodeadm/internal/util"
1413
"go.uber.org/zap"
15-
"golang.org/x/mod/semver"
1614
)
1715

1816
const (
@@ -32,7 +30,7 @@ var (
3230
imageCredentialProviderConfigPath = path.Join(imageCredentialProviderRoot, imageCredentialProviderConfig)
3331
)
3432

35-
func (k *kubelet) writeImageCredentialProviderConfig(cfg *api.NodeConfig) error {
33+
func (k *kubelet) writeImageCredentialProviderConfig() error {
3634
// fallback default for image credential provider binary if not overridden
3735
ecrCredentialProviderBinPath := path.Join(imageCredentialProviderRoot, "ecr-credential-provider")
3836
if binPath, set := os.LookupEnv(ecrCredentialProviderBinPathEnvironmentName); set {
@@ -43,7 +41,7 @@ func (k *kubelet) writeImageCredentialProviderConfig(cfg *api.NodeConfig) error
4341
return err
4442
}
4543

46-
config, err := generateImageCredentialProviderConfig(cfg, ecrCredentialProviderBinPath)
44+
config, err := generateImageCredentialProviderConfig(ecrCredentialProviderBinPath)
4745
if err != nil {
4846
return err
4947
}
@@ -60,17 +58,13 @@ type imageCredentialProviderTemplateVars struct {
6058
EcrProviderName string
6159
}
6260

63-
func generateImageCredentialProviderConfig(cfg *api.NodeConfig, ecrCredentialProviderBinPath string) ([]byte, error) {
61+
func generateImageCredentialProviderConfig(ecrCredentialProviderBinPath string) ([]byte, error) {
6462
templateVars := imageCredentialProviderTemplateVars{
65-
EcrProviderName: filepath.Base(ecrCredentialProviderBinPath),
66-
}
67-
if semver.Compare(cfg.Status.KubeletVersion, "v1.27.0") < 0 {
68-
templateVars.ConfigApiVersion = "kubelet.config.k8s.io/v1alpha1"
69-
templateVars.ProviderApiVersion = "credentialprovider.kubelet.k8s.io/v1alpha1"
70-
} else {
71-
templateVars.ConfigApiVersion = "kubelet.config.k8s.io/v1"
72-
templateVars.ProviderApiVersion = "credentialprovider.kubelet.k8s.io/v1"
63+
EcrProviderName: filepath.Base(ecrCredentialProviderBinPath),
64+
ConfigApiVersion: "kubelet.config.k8s.io/v1",
65+
ProviderApiVersion: "credentialprovider.kubelet.k8s.io/v1",
7366
}
67+
7468
var buf bytes.Buffer
7569
if err := imageCredentialProviderTemplate.Execute(&buf, templateVars); err != nil {
7670
return nil, err

‎nodeadm/test/e2e/cases/image-credential-provider/expected-image-credential-provider-config-126.json‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎nodeadm/test/e2e/cases/image-credential-provider/run.sh‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,6 @@ source /helpers.sh
99
mock::aws
1010
wait::dbus-ready
1111

12-
mock::kubelet 1.26.0
13-
14-
nodeadm init --skip run --config-source file://config.yaml
15-
16-
assert::json-files-equal /etc/eks/image-credential-provider/config.json expected-image-credential-provider-config-126.json
17-
1812
mock::kubelet 1.27.0
1913

2014
nodeadm init --skip run --config-source file://config.yaml

‎nodeadm/test/e2e/cases/kubelet-config-new-instance-type/expected-kubelet-config.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
"clusterDomain": "cluster.local",
2727
"containerRuntimeEndpoint": "unix:///run/containerd/containerd.sock",
2828
"featureGates": {
29-
"KubeletCredentialProviders": true,
3029
"RotateKubeletServerCertificate": true
3130
},
3231
"hairpinMode": "hairpin-veth",

‎nodeadm/test/e2e/cases/kubelet-config-new-instance-type/run.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ source /helpers.sh
99
config_path=/tmp/aemm-default-config.json
1010
cat /etc/aemm-default-config.json | jq '.metadata.values."instance-type" = "mock-type.large" | .dynamic.values."instance-identity-document".instanceType = "mock-type.large"' | tee ${config_path}
1111
mock::aws ${config_path}
12-
mock::kubelet 1.27.0
12+
mock::kubelet 1.28.0
1313
wait::dbus-ready
1414

1515
for config in config.*; do

‎nodeadm/test/e2e/cases/kubelet-config-set-empty/expected-kubelet-config.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
"clusterDomain": "cluster.local",
2727
"containerRuntimeEndpoint": "unix:///run/containerd/containerd.sock",
2828
"featureGates": {
29-
"KubeletCredentialProviders": true,
3029
"RotateKubeletServerCertificate": true
3130
},
3231
"hairpinMode": "hairpin-veth",

‎nodeadm/test/e2e/cases/kubelet-config-set-empty/run.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ set -o pipefail
77
source /helpers.sh
88

99
mock::aws
10-
mock::kubelet 1.27.0
10+
mock::kubelet 1.28.0
1111
wait::dbus-ready
1212

1313
# this test covers cases where the user wants to utilize `reservedSystemCPUs`,

0 commit comments

Comments
 (0)