From 80a40c7f45350b9c71b273e45e6271e8edf29b7e Mon Sep 17 00:00:00 2001 From: Jhaanvi Golani Date: Wed, 20 Nov 2024 14:20:34 -0800 Subject: [PATCH] Add support to update both legacy and default path for kubelet-extra-args for ubuntu --- .../templates/_helpers.tpl | 2 + .../templates/daemonset.yaml | 16 +++ .../pkg/configurator/linux/linux.go | 50 ++++++--- .../pkg/configurator/linux/linux_test.go | 102 ++++++++++-------- 4 files changed, 108 insertions(+), 62 deletions(-) diff --git a/credentialproviderpackage/charts/credential-provider-package/templates/_helpers.tpl b/credentialproviderpackage/charts/credential-provider-package/templates/_helpers.tpl index bb3b18b18..166b43c31 100644 --- a/credentialproviderpackage/charts/credential-provider-package/templates/_helpers.tpl +++ b/credentialproviderpackage/charts/credential-provider-package/templates/_helpers.tpl @@ -81,6 +81,8 @@ Function to figure out os name {{- printf "bottlerocket" -}} {{- else if contains "Amazon Linux" .status.nodeInfo.osImage -}} {{- printf "default" -}} +{{- else if contains "Ubuntu" .status.nodeInfo.osImage -}} +{{- printf "ubuntu" -}} {{- else -}} {{- printf "sysconfig" -}} {{- end }} diff --git a/credentialproviderpackage/charts/credential-provider-package/templates/daemonset.yaml b/credentialproviderpackage/charts/credential-provider-package/templates/daemonset.yaml index 6c54351e8..cb7daeaaa 100644 --- a/credentialproviderpackage/charts/credential-provider-package/templates/daemonset.yaml +++ b/credentialproviderpackage/charts/credential-provider-package/templates/daemonset.yaml @@ -40,6 +40,13 @@ spec: {{- if eq $os "bottlerocket" }} - mountPath: /run/api.sock name: socket + {{- else if eq $os "ubuntu" }} + - mountPath: /node-files/kubelet-extra-args + name: kubelet-extra-args + - mountPath: /node-files/kubelet-extra-args-legacy + name: kubelet-extra-args-legacy + - name: package-mounts + mountPath: /eksa-packages {{- else}} - mountPath: /node-files/kubelet-extra-args name: kubelet-extra-args @@ -72,6 +79,15 @@ spec: hostPath: path: /etc/default/kubelet type: FileOrCreate + {{- else if eq $os "ubuntu"}} + - name: kubelet-extra-args + hostPath: + path: /etc/default/kubelet + type: FileOrCreate + - name: kubelet-extra-args-legacy + hostPath: + path: /etc/sysconfig/kubelet + type: FileOrCreate {{- else}} - name: kubelet-extra-args hostPath: diff --git a/credentialproviderpackage/pkg/configurator/linux/linux.go b/credentialproviderpackage/pkg/configurator/linux/linux.go index c283df4cb..fd9929576 100644 --- a/credentialproviderpackage/pkg/configurator/linux/linux.go +++ b/credentialproviderpackage/pkg/configurator/linux/linux.go @@ -22,11 +22,12 @@ import ( var credProviderTemplate string const ( - binPath = "/eksa-binaries/" - basePath = "/eksa-packages/" - credOutFile = "aws-creds" - mountedExtraArgs = "/node-files/kubelet-extra-args" - credProviderFile = "credential-provider-config.yaml" + binPath = "/eksa-binaries/" + basePath = "/eksa-packages/" + credOutFile = "aws-creds" + mountedExtraArgs = "/node-files/kubelet-extra-args" + ubuntuLegacyExtraArgs = "/node-files/kubelet-extra-args-legacy" + credProviderFile = "credential-provider-config.yaml" // Binaries ecrCredProviderBinary = "ecr-credential-provider" @@ -34,19 +35,21 @@ const ( ) type linuxOS struct { - profile string - extraArgsPath string - basePath string - config constants.CredentialProviderConfigOptions + profile string + extraArgsPath string + legacyExtraArgsPath string + basePath string + config constants.CredentialProviderConfigOptions } var _ configurator.Configurator = (*linuxOS)(nil) func NewLinuxConfigurator() *linuxOS { return &linuxOS{ - profile: "", - extraArgsPath: mountedExtraArgs, - basePath: basePath, + profile: "", + extraArgsPath: mountedExtraArgs, + legacyExtraArgsPath: ubuntuLegacyExtraArgs, + basePath: basePath, } } @@ -62,9 +65,8 @@ func (c *linuxOS) UpdateAWSCredentials(sourcePath, profile string) error { return err } -func (c *linuxOS) UpdateCredentialProvider(_ string) error { - // Adding to KUBELET_EXTRA_ARGS in place - file, err := ioutil.ReadFile(c.extraArgsPath) +func (c *linuxOS) updateConfigFile(configPath string) error { + file, err := os.ReadFile(configPath) if err != nil { return err } @@ -91,10 +93,26 @@ func (c *linuxOS) UpdateCredentialProvider(_ string) error { } out := strings.Join(lines, "\n") - err = ioutil.WriteFile(c.extraArgsPath, []byte(out), 0o644) + err = os.WriteFile(configPath, []byte(out), 0o644) return err } +func (c *linuxOS) UpdateCredentialProvider(_ string) error { + // Adding to KUBELET_EXTRA_ARGS in place + if err := c.updateConfigFile(mountedExtraArgs); err != nil { + return fmt.Errorf("failed to update kubelet args: %v", err) + } + + // Adding KUBELET_EXTRA_ARGS to legacy path for ubuntu + if _, err := os.Stat(ubuntuLegacyExtraArgs); err == nil { + if err := c.updateConfigFile(ubuntuLegacyExtraArgs); err != nil { + return fmt.Errorf("failed to update legacy kubelet args for ubuntu: %v", err) + } + } + + return nil +} + func (c *linuxOS) CommitChanges() error { process, err := findKubeletProcess() if err != nil { diff --git a/credentialproviderpackage/pkg/configurator/linux/linux_test.go b/credentialproviderpackage/pkg/configurator/linux/linux_test.go index 2b6656c59..562914a86 100644 --- a/credentialproviderpackage/pkg/configurator/linux/linux_test.go +++ b/credentialproviderpackage/pkg/configurator/linux/linux_test.go @@ -2,7 +2,6 @@ package linux import ( "fmt" - "io/ioutil" "os" "testing" @@ -16,10 +15,11 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) { testDir, _ := test.NewWriter(t) dir := testDir + "/" type fields struct { - profile string - extraArgsPath string - basePath string - config constants.CredentialProviderConfigOptions + profile string + extraArgsPath string + legacyExtraArgsPath string + basePath string + config constants.CredentialProviderConfigOptions } type args struct { line string @@ -36,9 +36,10 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) { { name: "test empty string", fields: fields{ - profile: "eksa-packages", - extraArgsPath: dir, - basePath: dir, + profile: "eksa-packages", + extraArgsPath: dir, + legacyExtraArgsPath: dir, + basePath: dir, config: constants.CredentialProviderConfigOptions{ ImagePatterns: []string{constants.DefaultImagePattern}, DefaultCacheDuration: constants.DefaultCacheDuration, @@ -53,9 +54,10 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) { { name: "test multiple match patterns", fields: fields{ - profile: "eksa-packages", - extraArgsPath: dir, - basePath: dir, + profile: "eksa-packages", + extraArgsPath: dir, + legacyExtraArgsPath: dir, + basePath: dir, config: constants.CredentialProviderConfigOptions{ ImagePatterns: []string{ "1234567.dkr.ecr.us-east-1.amazonaws.com", @@ -73,9 +75,10 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) { { name: "skip credential provider if already provided", fields: fields{ - profile: "eksa-packages", - extraArgsPath: dir, - basePath: dir, + profile: "eksa-packages", + extraArgsPath: dir, + legacyExtraArgsPath: dir, + basePath: dir, config: constants.CredentialProviderConfigOptions{ ImagePatterns: []string{constants.DefaultImagePattern}, DefaultCacheDuration: constants.DefaultCacheDuration, @@ -89,9 +92,10 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) { { name: "skip both cred provider and feature gate if provided", fields: fields{ - profile: "eksa-packages", - extraArgsPath: dir, - basePath: dir, + profile: "eksa-packages", + extraArgsPath: dir, + legacyExtraArgsPath: dir, + basePath: dir, config: constants.CredentialProviderConfigOptions{ ImagePatterns: []string{constants.DefaultImagePattern}, DefaultCacheDuration: constants.DefaultCacheDuration, @@ -105,9 +109,10 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) { { name: "test alpha api", fields: fields{ - profile: "eksa-packages", - extraArgsPath: dir, - basePath: dir, + profile: "eksa-packages", + extraArgsPath: dir, + legacyExtraArgsPath: dir, + basePath: dir, config: constants.CredentialProviderConfigOptions{ ImagePatterns: []string{constants.DefaultImagePattern}, DefaultCacheDuration: constants.DefaultCacheDuration, @@ -123,9 +128,10 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) { { name: "test v1 api 1.27", fields: fields{ - profile: "eksa-packages", - extraArgsPath: dir, - basePath: dir, + profile: "eksa-packages", + extraArgsPath: dir, + legacyExtraArgsPath: dir, + basePath: dir, config: constants.CredentialProviderConfigOptions{ ImagePatterns: []string{constants.DefaultImagePattern}, DefaultCacheDuration: constants.DefaultCacheDuration, @@ -135,17 +141,17 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) { outputConfigPath: dir + "/" + credProviderFile, configWantPath: "testdata/expected-config.yaml", k8sVersion: "v1.27", - want: fmt.Sprintf(" --feature-gates=KubeletCredentialProviders=true "+ - "--image-credential-provider-config=%s%s", dir, credProviderFile), + want: fmt.Sprintf(" --image-credential-provider-config=%s%s", dir, credProviderFile), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &linuxOS{ - profile: tt.fields.profile, - extraArgsPath: tt.fields.extraArgsPath, - basePath: tt.fields.basePath, - config: tt.fields.config, + profile: tt.fields.profile, + extraArgsPath: tt.fields.extraArgsPath, + legacyExtraArgsPath: tt.fields.legacyExtraArgsPath, + basePath: tt.fields.basePath, + config: tt.fields.config, } t.Setenv("K8S_VERSION", tt.k8sVersion) @@ -163,10 +169,11 @@ func Test_linuxOS_UpdateAWSCredentials(t *testing.T) { testDir, _ := test.NewWriter(t) dir := testDir + "/" type fields struct { - profile string - extraArgsPath string - basePath string - config constants.CredentialProviderConfigOptions + profile string + extraArgsPath string + legacyExtraArgsPath string + basePath string + config constants.CredentialProviderConfigOptions } type args struct { sourcePath string @@ -181,9 +188,10 @@ func Test_linuxOS_UpdateAWSCredentials(t *testing.T) { { name: "simple credential move", fields: fields{ - profile: "eksa-packages", - extraArgsPath: dir, - basePath: dir, + profile: "eksa-packages", + extraArgsPath: dir, + legacyExtraArgsPath: dir, + basePath: dir, config: constants.CredentialProviderConfigOptions{ ImagePatterns: []string{constants.DefaultImagePattern}, DefaultCacheDuration: constants.DefaultCacheDuration, @@ -200,10 +208,11 @@ func Test_linuxOS_UpdateAWSCredentials(t *testing.T) { t.Run(tt.name, func(t *testing.T) { dstFile := tt.fields.basePath + credOutFile c := &linuxOS{ - profile: tt.fields.profile, - extraArgsPath: tt.fields.extraArgsPath, - basePath: tt.fields.basePath, - config: tt.fields.config, + profile: tt.fields.profile, + extraArgsPath: tt.fields.extraArgsPath, + legacyExtraArgsPath: tt.fields.legacyExtraArgsPath, + basePath: tt.fields.basePath, + config: tt.fields.config, } if err := c.UpdateAWSCredentials(tt.args.sourcePath, tt.args.profile); (err != nil) != tt.wantErr { t.Errorf("UpdateAWSCredentials() error = %v, wantErr %v", err, tt.wantErr) @@ -219,12 +228,12 @@ func Test_linuxOS_UpdateAWSCredentials(t *testing.T) { if err != nil { t.Errorf("Failed to set file back to readable") } - expectedCreds, err := ioutil.ReadFile(tt.args.sourcePath) + expectedCreds, err := os.ReadFile(tt.args.sourcePath) if err != nil { t.Errorf("Failed to read source credential file") } - actualCreds, err := ioutil.ReadFile(dstFile) + actualCreds, err := os.ReadFile(dstFile) if err != nil { t.Errorf("Failed to read created credential file") } @@ -235,10 +244,11 @@ func Test_linuxOS_UpdateAWSCredentials(t *testing.T) { func Test_linuxOS_Initialize(t *testing.T) { type fields struct { - profile string - extraArgsPath string - basePath string - config constants.CredentialProviderConfigOptions + profile string + extraArgsPath string + legacyExtraArgsPath string + basePath string + config constants.CredentialProviderConfigOptions } type args struct { config constants.CredentialProviderConfigOptions