Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to update both legacy and default path for kubelet-extra-args for ubuntu #1177

Merged
merged 2 commits into from
Dec 4, 2024
+104 −59
Merged

Add support to update both legacy and default path for kubelet-extra-args for ubuntu #1177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
acab50b
Add support to update both legacy and default path for kubelet-extra-…
jhaanvi5 Nov 20, 2024
ef4aa8a
fix: rename filepath to keep it consistent to variable name
jhaanvi5 Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions credentialproviderpackage/charts/credential-provider-package/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
16 changes: 16 additions & 0 deletions credentialproviderpackage/charts/credential-provider-package/templates/daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/ubuntu-legacy-kubelet-extra-args
name: ubuntu-legacy-kubelet-extra-args
- name: package-mounts
mountPath: /eksa-packages
{{- else}}
- mountPath: /node-files/kubelet-extra-args
name: kubelet-extra-args
Expand Down Expand Up @@ -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: ubuntu-legacy-kubelet-extra-args
hostPath:
path: /etc/sysconfig/kubelet
type: FileOrCreate
{{- else}}
- name: kubelet-extra-args
hostPath:
Expand Down
53 changes: 35 additions & 18 deletions credentialproviderpackage/pkg/configurator/linux/linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
_ "embed"
"fmt"
"io"
"io/ioutil"
"os"
"strings"
"syscall"
Expand All @@ -22,31 +21,34 @@ 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/ubuntu-legacy-kubelet-extra-args"
credProviderFile = "credential-provider-config.yaml"

// Binaries
ecrCredProviderBinary = "ecr-credential-provider"
iamRolesSigningBinary = "aws_signing_helper"
)

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,
}
}

Expand All @@ -62,9 +64,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
}
Expand All @@ -91,10 +92,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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious why are we changing from ioutil to os right now? Are there other references to ioutil in this file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ioutil.readfile was deprecated it basically calls os.readfile, so I updated it, let me know if we should update this with this change. I noticed the same in other files, I can refactor those too.

https://pkg.go.dev/io/ioutil#ReadFile

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 {
Expand Down Expand Up @@ -208,7 +225,7 @@ func (c *linuxOS) createConfig() (string, error) {
if err != nil {
return "", nil
}
err = ioutil.WriteFile(dstPath, bytes, 0o600)
err = os.WriteFile(dstPath, bytes, 0o600)
if err != nil {
return "", err
}
Expand Down
92 changes: 51 additions & 41 deletions credentialproviderpackage/pkg/configurator/linux/linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package linux

import (
"fmt"
"io/ioutil"
"os"
"testing"

Expand All @@ -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
Expand All @@ -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,
Expand All @@ -52,9 +53,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",
Expand All @@ -71,9 +73,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,
Expand All @@ -87,9 +90,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,
Expand All @@ -104,9 +108,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,
Expand All @@ -122,10 +127,11 @@ func Test_linuxOS_updateKubeletArguments(t *testing.T) {
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)

Expand All @@ -143,10 +149,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
Expand All @@ -161,9 +168,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,
Expand All @@ -180,10 +188,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)
Expand All @@ -199,12 +208,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")
}
Expand All @@ -215,10 +224,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
Expand Down
Loading