Skip to content

Commit

Permalink
Merge pull request kubernetes#4982 from towca/jtuznik/arch-fix
Browse files Browse the repository at this point in the history 
CA: GCE: Return UnknownArch from ToSystemArchitecture for invalid architectures
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jun 21, 2022
2 parents 298beab + 902a764 commit 8037b0f
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 21 deletions.
28 changes: 16 additions & 12 deletions cluster-autoscaler/cloudprovider/gce/templates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,10 @@ func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, template *gce.Instan
if osDistribution == OperatingSystemDistributionUnknown {
return nil, fmt.Errorf("could not obtain os-distribution from kube-env from template metadata")
}
arch := extractSystemArchitectureFromKubeEnv(kubeEnvValue)
if arch == UnknownArch {
return nil, fmt.Errorf("could not obtain arch from kube-env from template metadata")
arch, err := extractSystemArchitectureFromKubeEnv(kubeEnvValue)
if err != nil {
arch = DefaultArch
klog.Errorf("Couldn't extract architecture from kube-env for MIG %q, falling back to %q. Error: %v", mig.Id(), arch, err)
}

var ephemeralStorage int64 = -1
Expand Down Expand Up @@ -578,28 +579,31 @@ func (s SystemArchitecture) Name() string {
return string(s)
}

func extractSystemArchitectureFromKubeEnv(kubeEnv string) SystemArchitecture {
arch, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "arch")
func extractSystemArchitectureFromKubeEnv(kubeEnv string) (SystemArchitecture, error) {
archName, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "arch")
if err != nil {
klog.Errorf("error while obtaining arch from AUTOSCALER_ENV_VARS; using default %v", err)
return UnknownArch
return UnknownArch, fmt.Errorf("error while obtaining arch from AUTOSCALER_ENV_VARS: %v", err)
}
if !found {
klog.V(4).Infof("no arch defined in AUTOSCALER_ENV_VARS; using default %v", err)
return DefaultArch
return UnknownArch, fmt.Errorf("no arch defined in AUTOSCALER_ENV_VARS")
}
return ToSystemArchitecture(arch)
arch := ToSystemArchitecture(archName)
if arch == UnknownArch {
return UnknownArch, fmt.Errorf("unknown arch %q defined in AUTOSCALER_ENV_VARS", archName)
}
return arch, nil
}

// ToSystemArchitecture parses a string to SystemArchitecture
// ToSystemArchitecture parses a string to SystemArchitecture. Returns UnknownArch if the string doesn't represent a
// valid architecture.
func ToSystemArchitecture(arch string) SystemArchitecture {
switch arch {
case string(Arm64):
return Arm64
case string(Amd64):
return Amd64
default:
return DefaultArch
return UnknownArch
}
}

Expand Down
116 changes: 107 additions & 9 deletions cluster-autoscaler/cloudprovider/gce/templates_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
gpuUtils "k8s.io/autoscaler/cluster-autoscaler/utils/gpu"
"k8s.io/autoscaler/cluster-autoscaler/utils/units"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/stretchr/testify/assert"
gce "google.golang.org/api/compute/v1"
apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -170,15 +172,6 @@ func TestBuildNodeFromTemplateSetsResources(t *testing.T) {
attachedLocalSSDCount: 4,
expectedErr: false,
},
{
scenario: "handle empty arch gracefully",
kubeEnv: "AUTOSCALER_ENV_VARS: os_distribution=cos;arch=;os=linux;ephemeral_storage_local_ssd_count=2\n",
physicalCpu: 8,
physicalMemory: 200 * units.MiB,
ephemeralStorageLocalSSDCount: 2,
attachedLocalSSDCount: 4,
expectedErr: false,
},
{
scenario: "ephemeral storage on local SSDs with kube-reserved",
kubeEnv: "AUTOSCALER_ENV_VARS: kube_reserved=cpu=0,memory=0,ephemeral-storage=10Gi;os_distribution=cos;os=linux;ephemeral_storage_local_ssd_count=2\n",
Expand Down Expand Up @@ -1172,6 +1165,111 @@ func TestParseKubeReserved(t *testing.T) {
}
}

func TestToSystemArchitecture(t *testing.T) {
for tn, tc := range map[string]struct {
archName string
wantArch SystemArchitecture
}{
"valid architecture is converted": {
archName: "amd64",
wantArch: Amd64,
},
"invalid architecture results in UnknownArchitecture": {
archName: "some-arch",
wantArch: UnknownArch,
},
} {
t.Run(tn, func(t *testing.T) {
gotArch := ToSystemArchitecture(tc.archName)
if diff := cmp.Diff(tc.wantArch, gotArch); diff != "" {
t.Errorf("ToSystemArchitecture diff (-want +got):\n%s", diff)
}
})
}
}

func TestExtractSystemArchitectureFromKubeEnv(t *testing.T) {
for tn, tc := range map[string]struct {
kubeEnv string
wantArch SystemArchitecture
wantErr error
}{
"valid arch defined in AUTOSCALER_ENV_VARS": {
kubeEnv: "AUTOSCALER_ENV_VARS: os_distribution=cos;arch=arm64;os=linux\n",
wantArch: Arm64,
},
"invalid arch defined in AUTOSCALER_ENV_VARS": {
kubeEnv: "AUTOSCALER_ENV_VARS: os_distribution=cos;arch=blah;os=linux\n",
wantArch: UnknownArch,
wantErr: cmpopts.AnyError,
},
"empty arch defined in AUTOSCALER_ENV_VARS": {
kubeEnv: "AUTOSCALER_ENV_VARS: os_distribution=cos;arch=;os=linux\n",
wantArch: UnknownArch,
wantErr: cmpopts.AnyError,
},

"no arch defined in AUTOSCALER_ENV_VARS": {
kubeEnv: "AUTOSCALER_ENV_VARS: os_distribution=cos;os=linux\n",
wantArch: UnknownArch,
wantErr: cmpopts.AnyError,
},
"KUBE_ENV parsing error": {
kubeEnv: "some-invalid-string",
wantArch: UnknownArch,
wantErr: cmpopts.AnyError,
},
} {
t.Run(tn, func(t *testing.T) {
gotArch, gotErr := extractSystemArchitectureFromKubeEnv(tc.kubeEnv)
if diff := cmp.Diff(tc.wantArch, gotArch); diff != "" {
t.Errorf("extractSystemArchitectureFromKubeEnv diff (-want +got):\n%s", diff)
}
if diff := cmp.Diff(tc.wantErr, gotErr, cmpopts.EquateErrors()); diff != "" {
t.Errorf("extractSystemArchitectureFromKubeEnv error diff (-want +got):\n%s", diff)
}
})
}
}

func TestBuildNodeFromTemplateArch(t *testing.T) {
for tn, tc := range map[string]struct {
kubeEnv string
wantArch SystemArchitecture
}{
"valid arch defined in KUBE_ENV is passed through": {
kubeEnv: "AUTOSCALER_ENV_VARS: os_distribution=cos;arch=arm64;os=linux\n",
wantArch: Arm64,
},
"invalid arch defined in KUBE_ENV is defaulted to the default arch": {
kubeEnv: "AUTOSCALER_ENV_VARS: os_distribution=cos;arch=;os=linux\n",
wantArch: DefaultArch,
},
} {
t.Run(tn, func(t *testing.T) {
mig := &gceMig{gceRef: GceRef{Name: "some-name", Project: "some-proj", Zone: "us-central1-b"}}
template := &gce.InstanceTemplate{
Name: "node-name",
Properties: &gce.InstanceProperties{
Metadata: &gce.Metadata{
Items: []*gce.MetadataItems{{Key: "kube-env", Value: &tc.kubeEnv}},
},
Disks: []*gce.AttachedDisk{},
},
}
tb := &GceTemplateBuilder{}
gotNode, gotErr := tb.BuildNodeFromTemplate(mig, template, 16, 128, nil, &GceReserved{})
if gotErr != nil {
t.Fatalf("BuildNodeFromTemplate unexpected error: %v", gotErr)
}
gotArch := gotNode.Labels[apiv1.LabelArchStable]
if diff := cmp.Diff(tc.wantArch.Name(), gotArch); diff != "" {
t.Errorf("BuildNodeFromTemplate arch label diff (-want +got):\n%s", diff)
}
})
}
}

func makeTaintSet(taints []apiv1.Taint) map[apiv1.Taint]bool {
set := make(map[apiv1.Taint]bool)
for _, taint := range taints {
Expand Down

0 comments on commit 8037b0f

Please sign in to comment.