From 54d3a4c71465a3c83f962851b92a6ffaa7667e22 Mon Sep 17 00:00:00 2001 From: aleskandro Date: Sat, 26 Aug 2023 19:33:05 +0200 Subject: [PATCH] ClusterAPI: Allow overriding the kubernetes.io/arch label set by the scale from zero method via environment variable The architecture label in the build generic labels method of the cluster API (CAPI) provider is now populated using the GetDefaultScaleFromZeroArchitecture().Name() method. The method allows CAPI users deploying the cluster-autoscaler to define the default architecture to be used by the cluster-autoscaler for scale up from zero via the env var CAPI_SCALE_ZERO_DEFAULT_ARCH. Amd64 is kept as a fallback for historical reasons. The introduced changes will not take into account the case of nodes heterogeneous in architecture. The labels generation to infer properties like the cpu architecture from the node groups' features should be considered as a CAPI provider specific implementation. --- .../cloudprovider/clusterapi/README.md | 11 +++ .../clusterapi/clusterapi_nodegroup.go | 2 +- .../clusterapi/clusterapi_utils.go | 66 ++++++++++++++++ .../clusterapi/clusterapi_utils_test.go | 76 +++++++++++++++++++ 4 files changed, 154 insertions(+), 1 deletion(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/README.md b/cluster-autoscaler/cloudprovider/clusterapi/README.md index ba4a62660d68..2f4e247f4579 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/README.md +++ b/cluster-autoscaler/cloudprovider/clusterapi/README.md @@ -20,6 +20,7 @@ cluster. * [Scale from zero support](#scale-from-zero-support) * [RBAC changes for scaling from zero](#rbac-changes-for-scaling-from-zero) * [Pre-defined labels and taints on nodes scaled from zero](#pre-defined-labels-and-taints-on-nodes-scaled-from-zero) + * [CPU Architecture awareness for single-arch clusters](#cpu-architecture-awareness-for-single-arch-clusters) * [Specifying a Custom Resource Group](#specifying-a-custom-resource-group) * [Specifying a Custom Resource Version](#specifying-a-custom-resource-version) * [Sample manifest](#sample-manifest) @@ -276,6 +277,16 @@ metadata: capacity.cluster-autoscaler.kubernetes.io/taints: "key1=value1:NoSchedule,key2=value2:NoExecute" ``` +#### CPU Architecture awareness for single-arch clusters + +Users of single-arch non-amd64 clusters who are using scale from zero +support should also set the `CAPI_SCALE_ZERO_DEFAULT_ARCH` environment variable +to set the architecture of the nodes they want to default the node group templates to. +The autoscaler will default to `amd64` if it is not set, and the node +group templates may not match the nodes' architecture, specifically when +the workload triggering the scale-up uses a node affinity predicate checking +for the node's architecture. + ## Specifying a Custom Resource Group By default all Kubernetes resources consumed by the Cluster API provider will diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index 37a4f9ced14b..d28dc0a14f28 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -369,7 +369,7 @@ func buildGenericLabels(nodeName string) map[string]string { // TODO revisit this function and add an explanation about what these // labels are used for, or remove them if not necessary m := make(map[string]string) - m[corev1.LabelArchStable] = cloudprovider.DefaultArch + m[corev1.LabelArchStable] = GetDefaultScaleFromZeroArchitecture().Name() m[corev1.LabelOSStable] = cloudprovider.DefaultOS diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go index 27292d49e8ea..cdb0e884ecd5 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go @@ -18,8 +18,11 @@ package clusterapi import ( "fmt" + "k8s.io/klog/v2" + "os" "strconv" "strings" + "sync" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/api/resource" @@ -36,6 +39,20 @@ const ( maxPodsKey = "capacity.cluster-autoscaler.kubernetes.io/maxPods" taintsKey = "capacity.cluster-autoscaler.kubernetes.io/taints" labelsKey = "capacity.cluster-autoscaler.kubernetes.io/labels" + // UnknownArch is used if the Architecture is Unknown + UnknownArch SystemArchitecture = "" + // Amd64 is used if the Architecture is x86_64 + Amd64 SystemArchitecture = "amd64" + // Arm64 is used if the Architecture is ARM64 + Arm64 SystemArchitecture = "arm64" + // Ppc64le is used if the Architecture is ppc64le + Ppc64le SystemArchitecture = "ppc64le" + // S390x is used if the Architecture is s390x + S390x SystemArchitecture = "s390x" + // DefaultArch should be used as a fallback if not passed by the environment via the --scale-up-from-zero-default-arch + DefaultArch = Amd64 + // scaleUpFromZeroDefaultEnvVar is the name of the env var for the default architecture + scaleUpFromZeroDefaultArchEnvVar = "CAPI_SCALE_ZERO_DEFAULT_ARCH" ) var ( @@ -79,10 +96,25 @@ var ( nodeGroupMinSizeAnnotationKey = getNodeGroupMinSizeAnnotationKey() nodeGroupMaxSizeAnnotationKey = getNodeGroupMaxSizeAnnotationKey() zeroQuantity = resource.MustParse("0") + + systemArchitecture *SystemArchitecture + once sync.Once ) type normalizedProviderID string +// SystemArchitecture represents a CPU architecture (e.g., amd64, arm64, ppc64le, s390x). +// It is used to determine the default architecture to use when building the nodes templates for scaling up from zero +// by some cloud providers. This code is the same as the GCE implementation at +// https://github.com/kubernetes/autoscaler/blob/3852f352d96b8763292a9122163c1152dfedec55/cluster-autoscaler/cloudprovider/gce/templates.go#L611-L657 +// which is kept to allow for a smooth transition to this package, once the GCE team is ready to use it. +type SystemArchitecture string + +// Name returns the string value for SystemArchitecture +func (s SystemArchitecture) Name() string { + return string(s) +} + // minSize returns the minimum value encoded in the annotations keyed // by nodeGroupMinSizeAnnotationKey. Returns errMissingMinAnnotation // if the annotation doesn't exist or errInvalidMinAnnotation if the @@ -279,3 +311,37 @@ func getClusterNameLabel() string { key := fmt.Sprintf("%s/cluster-name", getCAPIGroup()) return key } + +// SystemArchitectureFromString parses a string to SystemArchitecture. Returns UnknownArch if the string doesn't represent a +// valid architecture. +func SystemArchitectureFromString(arch string) SystemArchitecture { + switch arch { + case string(Arm64): + return Arm64 + case string(Amd64): + return Amd64 + case string(Ppc64le): + return Ppc64le + case string(S390x): + return S390x + default: + return UnknownArch + } +} + +// GetDefaultScaleFromZeroArchitecture returns the SystemArchitecture from the environment variable +// CAPI_SCALE_ZERO_DEFAULT_ARCH or DefaultArch if the variable is set to an invalid value. +func GetDefaultScaleFromZeroArchitecture() SystemArchitecture { + once.Do(func() { + archStr := os.Getenv(scaleUpFromZeroDefaultArchEnvVar) + arch := SystemArchitectureFromString(archStr) + klog.V(5).Infof("the default scale from zero architecture value is set to %s (%s)", scaleUpFromZeroDefaultArchEnvVar, archStr, arch.Name()) + if arch == UnknownArch { + arch = DefaultArch + klog.Errorf("Unrecognized architecture '%s', falling back to %s", + scaleUpFromZeroDefaultArchEnvVar, DefaultArch.Name()) + } + systemArchitecture = &arch + }) + return *systemArchitecture +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go index c4d01c56e365..c6bafd50a969 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go @@ -18,8 +18,10 @@ package clusterapi import ( "fmt" + "github.com/google/go-cmp/cmp" "reflect" "strings" + "sync" "testing" "k8s.io/apimachinery/pkg/api/resource" @@ -853,3 +855,77 @@ func Test_getKeyHelpers(t *testing.T) { }) } } + +func TestSystemArchitectureFromString(t *testing.T) { + tcs := []struct { + name string + archName string + wantArch SystemArchitecture + }{ + { + name: "valid architecture is converted", + archName: "amd64", + wantArch: Amd64, + }, + { + name: "invalid architecture results in UnknownArchitecture", + archName: "some-arch", + wantArch: UnknownArch, + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + gotArch := SystemArchitectureFromString(tc.archName) + if diff := cmp.Diff(tc.wantArch, gotArch); diff != "" { + t.Errorf("ToSystemArchitecture diff (-want +got):\n%s", diff) + } + }) + } +} + +func TestGetSystemArchitectureFromEnvOrDefault(t *testing.T) { + amd64 := Amd64.Name() + arm64 := Arm64.Name() + wrongValue := "wrong" + + tcs := []struct { + name string + envValue *string + want SystemArchitecture + }{ + { + name: fmt.Sprintf("%s is set to arm64", scaleUpFromZeroDefaultArchEnvVar), + envValue: &arm64, + want: Arm64, + }, + { + name: fmt.Sprintf("%s is set to amd64", scaleUpFromZeroDefaultArchEnvVar), + envValue: &amd64, + want: Amd64, + }, + { + name: fmt.Sprintf("%s is not set", scaleUpFromZeroDefaultArchEnvVar), + envValue: nil, + want: DefaultArch, + }, + { + name: fmt.Sprintf("%s is set to a wrong value", scaleUpFromZeroDefaultArchEnvVar), + envValue: &wrongValue, + want: DefaultArch, + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + // Reset the systemArchitecture variable to nil before each test due to the lazy initialization of the variable. + systemArchitecture = nil + // Reset the once variable to its initial state before each test. + once = sync.Once{} + if tc.envValue != nil { + t.Setenv(scaleUpFromZeroDefaultArchEnvVar, *tc.envValue) + } + if got := GetDefaultScaleFromZeroArchitecture(); got != tc.want { + t.Errorf("GetDefaultScaleFromZeroArchitecture() = %v, want %v", got, tc.want) + } + }) + } +}