diff --git a/cluster-autoscaler/cloudprovider/gce/cache.go b/cluster-autoscaler/cloudprovider/gce/cache.go index 0b9df2500268..28e37848b0ee 100644 --- a/cluster-autoscaler/cloudprovider/gce/cache.go +++ b/cluster-autoscaler/cloudprovider/gce/cache.go @@ -68,6 +68,7 @@ type GceCache struct { migBaseNameCache map[GceRef]string instanceTemplateNameCache map[GceRef]string instanceTemplatesCache map[GceRef]*gce.InstanceTemplate + kubeEnvCache map[GceRef]KubeEnv } // NewGceCache creates empty GceCache. @@ -84,6 +85,7 @@ func NewGceCache() *GceCache { migBaseNameCache: map[GceRef]string{}, instanceTemplateNameCache: map[GceRef]string{}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, + kubeEnvCache: map[GceRef]KubeEnv{}, } } @@ -409,6 +411,46 @@ func (gc *GceCache) InvalidateAllMigInstanceTemplates() { gc.instanceTemplatesCache = map[GceRef]*gce.InstanceTemplate{} } +// GetMigKubeEnv returns the cached KubeEnv for a mig GceRef +func (gc *GceCache) GetMigKubeEnv(ref GceRef) (KubeEnv, bool) { + gc.cacheMutex.Lock() + defer gc.cacheMutex.Unlock() + + kubeEnv, found := gc.kubeEnvCache[ref] + if found { + klog.V(5).Infof("Kube-env cache hit for %s", ref) + } + return kubeEnv, found +} + +// SetMigKubeEnv sets KubeEnv for a mig GceRef +func (gc *GceCache) SetMigKubeEnv(ref GceRef, kubeEnv KubeEnv) { + gc.cacheMutex.Lock() + defer gc.cacheMutex.Unlock() + + gc.kubeEnvCache[ref] = kubeEnv +} + +// InvalidateMigKubeEnv clears the kube-env cache for a mig GceRef +func (gc *GceCache) InvalidateMigKubeEnv(ref GceRef) { + gc.cacheMutex.Lock() + defer gc.cacheMutex.Unlock() + + if _, found := gc.kubeEnvCache[ref]; found { + klog.V(5).Infof("Kube-env cache invalidated for %s", ref) + delete(gc.kubeEnvCache, ref) + } +} + +// InvalidateAllMigKubeEnvs clears the kube-env cache +func (gc *GceCache) InvalidateAllMigKubeEnvs() { + gc.cacheMutex.Lock() + defer gc.cacheMutex.Unlock() + + klog.V(5).Infof("Kube-env cache invalidated") + gc.kubeEnvCache = map[GceRef]KubeEnv{} +} + // GetMachine retrieves machine type from cache under lock. func (gc *GceCache) GetMachine(machineTypeName string, zone string) (MachineType, bool) { gc.cacheMutex.Lock() diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager.go b/cluster-autoscaler/cloudprovider/gce/gce_manager.go index db1329037e27..aff80ff1e4c4 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager.go @@ -345,12 +345,12 @@ func (m *gceManagerImpl) refreshAutoscalingOptions() { klog.Warningf("Failed to extract autoscaling options from %q metadata: instance template is incomplete", template.Name) continue } - kubeEnvValue, err := getKubeEnvValueFromTemplateMetadata(template) + kubeEnv, err := m.migInfoProvider.GetMigKubeEnv(mig.GceRef()) if err != nil { klog.Warningf("Failed to extract autoscaling options from %q instance template's metadata: can't get KubeEnv: %v", template.Name, err) continue } - options, err := extractAutoscalingOptionsFromKubeEnv(kubeEnvValue) + options, err := extractAutoscalingOptionsFromKubeEnv(kubeEnv) if err != nil { klog.Warningf("Failed to extract autoscaling options from %q instance template's metadata: %v", template.Name, err) continue @@ -591,15 +591,19 @@ func (m *gceManagerImpl) GetMigTemplateNode(mig Mig) (*apiv1.Node, error) { if err != nil { return nil, err } + kubeEnv, err := m.migInfoProvider.GetMigKubeEnv(mig.GceRef()) + if err != nil { + return nil, err + } machineType, err := m.migInfoProvider.GetMigMachineType(mig.GceRef()) if err != nil { return nil, err } - migOsInfo, err := m.templates.MigOsInfo(mig.Id(), template) + migOsInfo, err := m.templates.MigOsInfo(mig.Id(), kubeEnv) if err != nil { return nil, err } - return m.templates.BuildNodeFromTemplate(mig, migOsInfo, template, machineType.CPU, machineType.Memory, nil, m.reserved) + return m.templates.BuildNodeFromTemplate(mig, migOsInfo, template, kubeEnv, machineType.CPU, machineType.Memory, nil, m.reserved) } // parseMIGAutoDiscoverySpecs returns any provided NodeGroupAutoDiscoverySpecs diff --git a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go index 953c9789bf28..287d5bba8721 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_manager_test.go @@ -346,6 +346,7 @@ func newTestGceManager(t *testing.T, testServerURL string, regional bool) *gceMa migTargetSizeCache: map[GceRef]int64{}, instanceTemplateNameCache: map[GceRef]string{}, instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{}, + kubeEnvCache: map[GceRef]KubeEnv{}, migBaseNameCache: map[GceRef]string{}, } migLister := NewMigLister(cache) diff --git a/cluster-autoscaler/cloudprovider/gce/kube_env.go b/cluster-autoscaler/cloudprovider/gce/kube_env.go new file mode 100644 index 000000000000..a772948eed32 --- /dev/null +++ b/cluster-autoscaler/cloudprovider/gce/kube_env.go @@ -0,0 +1,73 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package gce + +import ( + "errors" + "fmt" + + gce "google.golang.org/api/compute/v1" + "sigs.k8s.io/yaml" +) + +const ( + kubeEnvKey = "kube-env" +) + +// KubeEnv stores kube-env information from InstanceTemplate +type KubeEnv struct { + templateName string + env map[string]string +} + +// ExtractKubeEnv extracts kube-env from InstanceTemplate +func ExtractKubeEnv(template *gce.InstanceTemplate) (KubeEnv, error) { + if template == nil { + return KubeEnv{}, errors.New("instance template is nil") + } + if template.Properties == nil || template.Properties.Metadata == nil { + return KubeEnv{}, fmt.Errorf("instance template %s has no metadata", template.Name) + } + for _, item := range template.Properties.Metadata.Items { + if item.Key == kubeEnvKey { + if item.Value == nil { + return KubeEnv{}, fmt.Errorf("no kube-env content in metadata") + } + return ParseKubeEnv(template.Name, *item.Value) + } + } + return KubeEnv{templateName: template.Name}, nil +} + +// ParseKubeEnv parses kube-env from its string representation +func ParseKubeEnv(templateName, kubeEnvValue string) (KubeEnv, error) { + env := make(map[string]string) + err := yaml.Unmarshal([]byte(kubeEnvValue), &env) + if err != nil { + return KubeEnv{}, fmt.Errorf("error unmarshalling kubeEnv: %v", err) + } + return KubeEnv{templateName: templateName, env: env}, nil +} + +// Var extracts variable from KubeEnv +func (ke KubeEnv) Var(name string) (string, bool) { + if ke.env == nil { + return "", false + } + val, found := ke.env[name] + return val, found +} diff --git a/cluster-autoscaler/cloudprovider/gce/kube_env_test.go b/cluster-autoscaler/cloudprovider/gce/kube_env_test.go new file mode 100644 index 000000000000..8176c0b539dc --- /dev/null +++ b/cluster-autoscaler/cloudprovider/gce/kube_env_test.go @@ -0,0 +1,226 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package gce + +import ( + "testing" + + "github.com/stretchr/testify/assert" + gce "google.golang.org/api/compute/v1" +) + +func TestExtractKubeEnv(t *testing.T) { + templateName := "instance-template" + correctKubeEnv := "VAR1: VALUE1\nVAR2: VALUE2" + someValue := "Lorem ipsum dolor sit amet" + + testCases := []struct { + name string + template *gce.InstanceTemplate + wantKubeEnv KubeEnv + wantErr bool + }{ + { + name: "template is nil", + template: nil, + wantErr: true, + }, + { + name: "template without instance properties", + template: &gce.InstanceTemplate{}, + wantErr: true, + }, + { + name: "template without instance properties metadata", + template: &gce.InstanceTemplate{ + Properties: &gce.InstanceProperties{}, + }, + wantErr: true, + }, + { + name: "template without kube-env", + template: &gce.InstanceTemplate{ + Name: templateName, + Properties: &gce.InstanceProperties{ + Metadata: &gce.Metadata{ + Items: []*gce.MetadataItems{ + {Key: "key-1", Value: &someValue}, + {Key: "key-2", Value: &someValue}, + }, + }, + }, + }, + wantKubeEnv: KubeEnv{templateName: templateName}, + }, + { + name: "template with nil kube-env", + template: &gce.InstanceTemplate{ + Name: templateName, + Properties: &gce.InstanceProperties{ + Metadata: &gce.Metadata{ + Items: []*gce.MetadataItems{ + {Key: "key-1", Value: &someValue}, + {Key: "key-2", Value: &someValue}, + {Key: "kube-env", Value: nil}, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "template with incorrect kube-env", + template: &gce.InstanceTemplate{ + Properties: &gce.InstanceProperties{ + Metadata: &gce.Metadata{ + Items: []*gce.MetadataItems{ + {Key: "key-1", Value: &someValue}, + {Key: "key-2", Value: &someValue}, + {Key: "kube-env", Value: &someValue}, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "template with correct kube-env", + template: &gce.InstanceTemplate{ + Name: templateName, + Properties: &gce.InstanceProperties{ + Metadata: &gce.Metadata{ + Items: []*gce.MetadataItems{ + {Key: "key-1", Value: &someValue}, + {Key: "key-2", Value: &someValue}, + {Key: "kube-env", Value: &correctKubeEnv}, + }, + }, + }, + }, + wantKubeEnv: KubeEnv{ + templateName: templateName, + env: map[string]string{ + "VAR1": "VALUE1", + "VAR2": "VALUE2", + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + kubeEnv, err := ExtractKubeEnv(tc.template) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.wantKubeEnv, kubeEnv) + } + }) + } +} + +func TestParseKubeEnv(t *testing.T) { + templateName := "instance-template" + testCases := []struct { + name string + kubeEnvValue string + wantKubeEnv KubeEnv + wantErr bool + }{ + { + name: "kube-env value is empty", + kubeEnvValue: "", + wantKubeEnv: KubeEnv{ + templateName: templateName, + env: map[string]string{}, + }, + }, + { + name: "kube-env value is incorrect", + kubeEnvValue: "Lorem ipsum dolor sit amet", + wantErr: true, + }, + { + name: "kube-env value is correct", + kubeEnvValue: "VAR1: VALUE1\nVAR2: VALUE2", + wantKubeEnv: KubeEnv{ + templateName: templateName, + env: map[string]string{ + "VAR1": "VALUE1", + "VAR2": "VALUE2", + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + kubeEnv, err := ParseKubeEnv(templateName, tc.kubeEnvValue) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.wantKubeEnv, kubeEnv) + } + }) + } +} + +func TestKubeEnvVar(t *testing.T) { + testCases := []struct { + name string + kubeEnv KubeEnv + variable string + wantValue string + wantFound bool + }{ + { + name: "kube-env is nil", + variable: "VAR1", + wantFound: false, + }, + { + name: "kube-env does not have this variable", + kubeEnv: KubeEnv{ + env: map[string]string{ + "VAR1": "VALUE1", + "VAR2": "VALUE2", + }, + }, + variable: "VAR3", + wantFound: false, + }, + { + name: "kube-env has this variable", + kubeEnv: KubeEnv{ + env: map[string]string{ + "VAR1": "VALUE1", + "VAR2": "VALUE2", + }, + }, + variable: "VAR2", + wantValue: "VALUE2", + wantFound: true, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + value, found := tc.kubeEnv.Var(tc.variable) + assert.Equal(t, tc.wantValue, value) + assert.Equal(t, tc.wantFound, found) + }) + } +} diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go index b96a477bf1af..ffdd4e18a6b6 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider.go @@ -46,6 +46,8 @@ type MigInfoProvider interface { GetMigInstanceTemplateName(migRef GceRef) (string, error) // GetMigInstanceTemplate returns instance template for given MIG ref GetMigInstanceTemplate(migRef GceRef) (*gce.InstanceTemplate, error) + // GetMigKubeEnv returns kube-env for given MIG ref + GetMigKubeEnv(migRef GceRef) (KubeEnv, error) // GetMigMachineType returns machine type used by a MIG. // For custom machines cpu and memory information is based on parsing // machine name. For standard types it's retrieved from GCE API. @@ -284,6 +286,29 @@ func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.Ins return template, nil } +func (c *cachingMigInfoProvider) GetMigKubeEnv(migRef GceRef) (KubeEnv, error) { + templateName, err := c.GetMigInstanceTemplateName(migRef) + if err != nil { + return KubeEnv{}, err + } + + kubeEnv, kubeEnvFound := c.cache.GetMigKubeEnv(migRef) + if kubeEnvFound && kubeEnv.templateName == templateName { + return kubeEnv, nil + } + + template, err := c.GetMigInstanceTemplate(migRef) + if err != nil { + return KubeEnv{}, err + } + kubeEnv, err = ExtractKubeEnv(template) + if err != nil { + return KubeEnv{}, err + } + c.cache.SetMigKubeEnv(migRef, kubeEnv) + return kubeEnv, nil +} + // filMigInfoCache needs to be called with migInfoMutex locked func (c *cachingMigInfoProvider) fillMigInfoCache() error { var zones []string diff --git a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go index 6c2ea6dc75e6..b0bd51bdf50a 100644 --- a/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go +++ b/cluster-autoscaler/cloudprovider/gce/mig_info_provider_test.go @@ -951,6 +951,163 @@ func TestGetMigInstanceTemplate(t *testing.T) { } } +func TestGetMigInstanceKubeEnv(t *testing.T) { + templateName := "template-name" + kubeEnvValue := "VAR1: VALUE1\nVAR2: VALUE2" + kubeEnv, err := ParseKubeEnv(templateName, kubeEnvValue) + assert.NoError(t, err) + template := &gce.InstanceTemplate{ + Name: templateName, + Description: "instance template", + Properties: &gce.InstanceProperties{ + Metadata: &gce.Metadata{ + Items: []*gce.MetadataItems{ + {Key: "kube-env", Value: &kubeEnvValue}, + }, + }, + }, + } + + oldTemplateName := "old-template-name" + oldKubeEnvValue := "VAR3: VALUE3\nVAR4: VALUE4" + oldKubeEnv, err := ParseKubeEnv(oldTemplateName, oldKubeEnvValue) + assert.NoError(t, err) + oldTemplate := &gce.InstanceTemplate{ + Name: oldTemplateName, + Description: "old instance template", + Properties: &gce.InstanceProperties{ + Metadata: &gce.Metadata{ + Items: []*gce.MetadataItems{ + {Key: "kube-env", Value: &oldKubeEnvValue}, + }, + }, + }, + } + + testCases := []struct { + name string + cache *GceCache + fetchMigs func(string) ([]*gce.InstanceGroupManager, error) + fetchMigTemplateName func(GceRef) (string, error) + fetchMigTemplate func(GceRef, string) (*gce.InstanceTemplate, error) + expectedKubeEnv KubeEnv + expectedCachedKubeEnv KubeEnv + expectedErr error + }{ + { + name: "kube-env in cache", + cache: &GceCache{ + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + kubeEnvCache: map[GceRef]KubeEnv{mig.GceRef(): kubeEnv}, + }, + expectedKubeEnv: kubeEnv, + expectedCachedKubeEnv: kubeEnv, + }, + { + name: "cache without kube-env, template in cache", + cache: &GceCache{ + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): template}, + kubeEnvCache: make(map[GceRef]KubeEnv), + }, + expectedKubeEnv: kubeEnv, + expectedCachedKubeEnv: kubeEnv, + }, + { + name: "cache without kube-env, fetch success", + cache: &GceCache{ + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), + kubeEnvCache: make(map[GceRef]KubeEnv), + }, + fetchMigTemplate: fetchMigTemplateConst(template), + expectedKubeEnv: kubeEnv, + expectedCachedKubeEnv: kubeEnv, + }, + { + name: "cache with old kube-env, new template cached", + cache: &GceCache{ + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): template}, + kubeEnvCache: map[GceRef]KubeEnv{mig.GceRef(): oldKubeEnv}, + }, + expectedKubeEnv: kubeEnv, + expectedCachedKubeEnv: kubeEnv, + }, + { + name: "cache with old kube-env, fetch success", + cache: &GceCache{ + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): oldTemplate}, + kubeEnvCache: map[GceRef]KubeEnv{mig.GceRef(): oldKubeEnv}, + }, + fetchMigTemplate: fetchMigTemplateConst(template), + expectedKubeEnv: kubeEnv, + expectedCachedKubeEnv: kubeEnv, + }, + { + name: "cache without kube-env, fetch failure", + cache: &GceCache{ + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplatesCache: make(map[GceRef]*gce.InstanceTemplate), + kubeEnvCache: make(map[GceRef]KubeEnv), + }, + fetchMigTemplate: fetchMigTemplateFail, + expectedErr: errFetchMigTemplate, + }, + { + name: "cache with old kube-env, fetch failure", + cache: &GceCache{ + migs: map[GceRef]Mig{mig.GceRef(): mig}, + instanceTemplateNameCache: map[GceRef]string{mig.GceRef(): templateName}, + instanceTemplatesCache: map[GceRef]*gce.InstanceTemplate{mig.GceRef(): oldTemplate}, + kubeEnvCache: map[GceRef]KubeEnv{mig.GceRef(): oldKubeEnv}, + }, + fetchMigTemplate: fetchMigTemplateFail, + expectedCachedKubeEnv: oldKubeEnv, + expectedErr: errFetchMigTemplate, + }, + { + name: "template name fetch failure", + cache: emptyCache(), + fetchMigs: fetchMigsFail, + fetchMigTemplateName: fetchMigTemplateNameFail, + expectedErr: errFetchMigTemplateName, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + client := &mockAutoscalingGceClient{ + fetchMigs: tc.fetchMigs, + fetchMigTemplateName: tc.fetchMigTemplateName, + fetchMigTemplate: tc.fetchMigTemplate, + } + migLister := NewMigLister(tc.cache) + provider := NewCachingMigInfoProvider(tc.cache, migLister, client, mig.GceRef().Project, 1, 0*time.Second) + + kubeEnv, err := provider.GetMigKubeEnv(mig.GceRef()) + cachedKubeEnv, found := tc.cache.GetMigKubeEnv(mig.GceRef()) + + assert.Equal(t, tc.expectedErr, err) + if tc.expectedErr == nil { + assert.Equal(t, tc.expectedKubeEnv, kubeEnv) + } + + assert.Equal(t, tc.expectedCachedKubeEnv.env != nil, found) + if tc.expectedCachedKubeEnv.env != nil { + assert.Equal(t, tc.expectedCachedKubeEnv, cachedKubeEnv) + } + }) + } +} + func TestGetMigMachineType(t *testing.T) { knownZone := "us-cache1-a" unknownZone := "us-nocache42-c" diff --git a/cluster-autoscaler/cloudprovider/gce/templates.go b/cluster-autoscaler/cloudprovider/gce/templates.go index 868917d796af..e258a59b4ae3 100644 --- a/cluster-autoscaler/cloudprovider/gce/templates.go +++ b/cluster-autoscaler/cloudprovider/gce/templates.go @@ -29,12 +29,10 @@ import ( apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/klog/v2" - "sigs.k8s.io/yaml" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" "k8s.io/autoscaler/cluster-autoscaler/utils/gpu" "k8s.io/autoscaler/cluster-autoscaler/utils/units" + "k8s.io/klog/v2" ) // GceTemplateBuilder builds templates for GCE nodes. @@ -107,13 +105,7 @@ func (t *GceTemplateBuilder) BuildCapacity(m MigOsInfo, cpu int64, mem int64, ac // BuildAllocatableFromKubeEnv builds node allocatable based on capacity of the node and // value of kubeEnv. -// KubeEnv is a multi-line string containing entries in the form of -// :. One of the resources it contains is a list of -// kubelet arguments from which we can extract the resources reserved by -// the kubelet for its operation. Allocated resources are capacity minus reserved. -// If we fail to extract the reserved resources from kubeEnv (e.g it is in a -// wrong format or does not contain kubelet arguments), we return an error. -func (t *GceTemplateBuilder) BuildAllocatableFromKubeEnv(capacity apiv1.ResourceList, kubeEnv string, evictionHard *EvictionHard) (apiv1.ResourceList, error) { +func (t *GceTemplateBuilder) BuildAllocatableFromKubeEnv(capacity apiv1.ResourceList, kubeEnv KubeEnv, evictionHard *EvictionHard) (apiv1.ResourceList, error) { kubeReserved, err := extractKubeReservedFromKubeEnv(kubeEnv) if err != nil { return nil, err @@ -145,39 +137,20 @@ func (t *GceTemplateBuilder) CalculateAllocatable(capacity apiv1.ResourceList, k return allocatable } -func getKubeEnvValueFromTemplateMetadata(template *gce.InstanceTemplate) (string, error) { - if template.Properties.Metadata == nil { - return "", fmt.Errorf("instance template %s has no metadata", template.Name) - } - for _, item := range template.Properties.Metadata.Items { - if item.Key == "kube-env" { - if item.Value == nil { - return "", fmt.Errorf("no kube-env content in metadata") - } - return *item.Value, nil - } - } - return "", nil -} - // MigOsInfo return os detailes information that stored in template. -func (t *GceTemplateBuilder) MigOsInfo(migId string, template *gce.InstanceTemplate) (MigOsInfo, error) { - kubeEnvValue, err := getKubeEnvValueFromTemplateMetadata(template) - if err != nil { - return nil, fmt.Errorf("could not obtain kube-env from template metadata; %v", err) - } - os := extractOperatingSystemFromKubeEnv(kubeEnvValue) +func (t *GceTemplateBuilder) MigOsInfo(migId string, kubeEnv KubeEnv) (MigOsInfo, error) { + os := extractOperatingSystemFromKubeEnv(kubeEnv) if os == OperatingSystemUnknown { return nil, fmt.Errorf("could not obtain os from kube-env from template metadata") } - osDistribution := extractOperatingSystemDistributionFromKubeEnv(kubeEnvValue) + osDistribution := extractOperatingSystemDistributionFromKubeEnv(kubeEnv) if osDistribution == OperatingSystemDistributionUnknown { osDistribution = OperatingSystemDistributionDefault klog.Errorf("could not obtain os-distribution from kube-env from template metadata, falling back to %q", osDistribution) } - arch, err := extractSystemArchitectureFromKubeEnv(kubeEnvValue) + arch, err := extractSystemArchitectureFromKubeEnv(kubeEnv) if err != nil { arch = DefaultArch klog.Errorf("Couldn't extract architecture from kube-env for MIG %q, falling back to %q. Error: %v", migId, arch, err) @@ -186,7 +159,7 @@ func (t *GceTemplateBuilder) MigOsInfo(migId string, template *gce.InstanceTempl } // BuildNodeFromTemplate builds node from provided GCE template. -func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo, template *gce.InstanceTemplate, cpu int64, mem int64, pods *int64, reserved OsReservedCalculator) (*apiv1.Node, error) { +func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo, template *gce.InstanceTemplate, kubeEnv KubeEnv, cpu int64, mem int64, pods *int64, reserved OsReservedCalculator) (*apiv1.Node, error) { if template.Properties == nil { return nil, fmt.Errorf("instance template %s has no properties", template.Name) @@ -195,11 +168,6 @@ func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo, node := apiv1.Node{} nodeName := fmt.Sprintf("%s-template-%d", template.Name, rand.Int63()) - kubeEnvValue, err := getKubeEnvValueFromTemplateMetadata(template) - if err != nil { - return nil, fmt.Errorf("could not obtain kube-env from template metadata; %v", err) - } - node.ObjectMeta = metav1.ObjectMeta{ Name: nodeName, SelfLink: fmt.Sprintf("/api/v1/nodes/%s", nodeName), @@ -208,7 +176,8 @@ func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo, addBootDiskAnnotations(&node, template.Properties) var ephemeralStorage int64 = -1 - if !isBootDiskEphemeralStorageWithInstanceTemplateDisabled(kubeEnvValue) { + var err error + if !isBootDiskEphemeralStorageWithInstanceTemplateDisabled(kubeEnv) { // ephemeral storage is backed up by boot disk ephemeralStorage, err = getBootDiskEphemeralStorageFromInstanceTemplateProperties(template.Properties) } else { @@ -220,7 +189,7 @@ func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo, if localSsdCount > 0 { addAnnotation(&node, LocalSsdCountAnnotation, strconv.FormatInt(localSsdCount, 10)) } - ephemeralStorageLocalSsdCount := ephemeralStorageLocalSSDCount(kubeEnvValue) + ephemeralStorageLocalSsdCount := ephemeralStorageLocalSSDCount(kubeEnv) if err == nil && ephemeralStorageLocalSsdCount > 0 { ephemeralStorage, err = getEphemeralStorageOnLocalSsd(localSsdCount, ephemeralStorageLocalSsdCount) } @@ -228,7 +197,7 @@ func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo, return nil, fmt.Errorf("could not fetch ephemeral storage from instance template: %v", err) } - extendedResources, err := extractExtendedResourcesFromKubeEnv(kubeEnvValue) + extendedResources, err := extractExtendedResourcesFromKubeEnv(kubeEnv) if err != nil { // External Resources are optional and should not break the template creation klog.Errorf("could not fetch extended resources from instance template: %v", err) @@ -244,29 +213,29 @@ func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo, } var nodeAllocatable apiv1.ResourceList - if kubeEnvValue != "" { + if kubeEnv.env != nil { // Extract labels - kubeEnvLabels, err := extractLabelsFromKubeEnv(kubeEnvValue) + kubeEnvLabels, err := extractLabelsFromKubeEnv(kubeEnv) if err != nil { return nil, err } node.Labels = cloudprovider.JoinStringMaps(node.Labels, kubeEnvLabels) // Extract taints - kubeEnvTaints, err := extractTaintsFromKubeEnv(kubeEnvValue) + kubeEnvTaints, err := extractTaintsFromKubeEnv(kubeEnv) if err != nil { return nil, err } node.Spec.Taints = append(node.Spec.Taints, kubeEnvTaints...) // Extract Eviction Hard - evictionHardFromKubeEnv, err := extractEvictionHardFromKubeEnv(kubeEnvValue) + evictionHardFromKubeEnv, err := extractEvictionHardFromKubeEnv(kubeEnv) if err != nil || len(evictionHardFromKubeEnv) == 0 { klog.Warning("unable to get evictionHardFromKubeEnv values, continuing without it.") } evictionHard := ParseEvictionHardOrGetDefault(evictionHardFromKubeEnv) - if allocatable, err := t.BuildAllocatableFromKubeEnv(node.Status.Capacity, kubeEnvValue, evictionHard); err == nil { + if allocatable, err := t.BuildAllocatableFromKubeEnv(node.Status.Capacity, kubeEnv, evictionHard); err == nil { nodeAllocatable = allocatable } } @@ -289,8 +258,8 @@ func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo, return &node, nil } -func ephemeralStorageLocalSSDCount(kubeEnvValue string) int64 { - v, found, err := extractAutoscalerVarFromKubeEnv(kubeEnvValue, "ephemeral_storage_local_ssd_count") +func ephemeralStorageLocalSSDCount(kubeEnv KubeEnv) int64 { + v, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "ephemeral_storage_local_ssd_count") if err != nil { klog.Warningf("cannot extract ephemeral_storage_local_ssd_count from kube-env, default to 0: %v", err) return 0 @@ -334,8 +303,8 @@ func getEphemeralStorageOnLocalSsd(localSsdCount, ephemeralStorageLocalSsdCount // isBootDiskEphemeralStorageWithInstanceTemplateDisabled will allow bypassing Disk Size of Boot Disk from being // picked up from Instance Template and used as Ephemeral Storage, in case other type of storage are used // as ephemeral storage -func isBootDiskEphemeralStorageWithInstanceTemplateDisabled(kubeEnvValue string) bool { - v, found, err := extractAutoscalerVarFromKubeEnv(kubeEnvValue, "BLOCK_EPH_STORAGE_BOOT_DISK") +func isBootDiskEphemeralStorageWithInstanceTemplateDisabled(kubeEnv KubeEnv) bool { + v, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "BLOCK_EPH_STORAGE_BOOT_DISK") if err == nil && found && v == "true" { return true } @@ -402,16 +371,12 @@ func parseKubeReserved(kubeReserved string) (apiv1.ResourceList, error) { return reservedResources, nil } -// GetLabelsFromTemplate returns labels from instance template -func GetLabelsFromTemplate(template *gce.InstanceTemplate) (map[string]string, error) { - kubeEnv, err := getKubeEnvValueFromTemplateMetadata(template) - if err != nil { - return nil, err - } +// GetLabelsFromKubeEnv returns labels from kube-env +func GetLabelsFromKubeEnv(kubeEnv KubeEnv) (map[string]string, error) { return extractLabelsFromKubeEnv(kubeEnv) } -func extractLabelsFromKubeEnv(kubeEnv string) (map[string]string, error) { +func extractLabelsFromKubeEnv(kubeEnv KubeEnv) (map[string]string, error) { // In v1.10+, labels are only exposed for the autoscaler via AUTOSCALER_ENV_VARS // see kubernetes/kubernetes#61119. We try AUTOSCALER_ENV_VARS first, then // fall back to the old way. @@ -420,24 +385,17 @@ func extractLabelsFromKubeEnv(kubeEnv string) (map[string]string, error) { klog.Errorf("error while trying to extract node_labels from AUTOSCALER_ENV_VARS: %v", err) } if !found { - labels, err = extractFromKubeEnv(kubeEnv, "NODE_LABELS") - if err != nil { - return nil, err - } + labels, _ = kubeEnv.Var("NODE_LABELS") } return parseKeyValueListToMap(labels) } -// GetTaintsFromTemplate returns labels from instance template -func GetTaintsFromTemplate(template *gce.InstanceTemplate) ([]apiv1.Taint, error) { - kubeEnv, err := getKubeEnvValueFromTemplateMetadata(template) - if err != nil { - return nil, err - } +// GetTaintsFromKubeEnv returns labels from kube-env +func GetTaintsFromKubeEnv(kubeEnv KubeEnv) ([]apiv1.Taint, error) { return extractTaintsFromKubeEnv(kubeEnv) } -func extractTaintsFromKubeEnv(kubeEnv string) ([]apiv1.Taint, error) { +func extractTaintsFromKubeEnv(kubeEnv KubeEnv) ([]apiv1.Taint, error) { // In v1.10+, taints are only exposed for the autoscaler via AUTOSCALER_ENV_VARS // see kubernetes/kubernetes#61119. We try AUTOSCALER_ENV_VARS first, then // fall back to the old way. @@ -446,10 +404,7 @@ func extractTaintsFromKubeEnv(kubeEnv string) ([]apiv1.Taint, error) { klog.Errorf("error while trying to extract node_taints from AUTOSCALER_ENV_VARS: %v", err) } if !found { - taints, err = extractFromKubeEnv(kubeEnv, "NODE_TAINTS") - if err != nil { - return nil, err - } + taints, _ = kubeEnv.Var("NODE_TAINTS") } taintMap, err := parseKeyValueListToMap(taints) if err != nil { @@ -458,7 +413,7 @@ func extractTaintsFromKubeEnv(kubeEnv string) ([]apiv1.Taint, error) { return buildTaints(taintMap) } -func extractKubeReservedFromKubeEnv(kubeEnv string) (string, error) { +func extractKubeReservedFromKubeEnv(kubeEnv KubeEnv) (string, error) { // In v1.10+, kube-reserved is only exposed for the autoscaler via AUTOSCALER_ENV_VARS // see kubernetes/kubernetes#61119. We try AUTOSCALER_ENV_VARS first, then // fall back to the old way. @@ -467,10 +422,7 @@ func extractKubeReservedFromKubeEnv(kubeEnv string) (string, error) { klog.Errorf("error while trying to extract kube_reserved from AUTOSCALER_ENV_VARS: %v", err) } if !found { - kubeletArgs, err := extractFromKubeEnv(kubeEnv, "KUBELET_TEST_ARGS") - if err != nil { - return "", err - } + kubeletArgs, _ := kubeEnv.Var("KUBELET_TEST_ARGS") resourcesRegexp := regexp.MustCompile(`--kube-reserved=([^ ]+)`) matches := resourcesRegexp.FindStringSubmatch(kubeletArgs) @@ -482,8 +434,8 @@ func extractKubeReservedFromKubeEnv(kubeEnv string) (string, error) { return kubeReserved, nil } -func extractExtendedResourcesFromKubeEnv(kubeEnvValue string) (apiv1.ResourceList, error) { - extendedResourcesAsString, found, err := extractAutoscalerVarFromKubeEnv(kubeEnvValue, "extended_resources") +func extractExtendedResourcesFromKubeEnv(kubeEnv KubeEnv) (apiv1.ResourceList, error) { + extendedResourcesAsString, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "extended_resources") if err != nil { klog.Warningf("error while obtaining extended_resources from AUTOSCALER_ENV_VARS; %v", err) return nil, err @@ -524,7 +476,7 @@ const ( OperatingSystemDefault = OperatingSystemLinux ) -func extractOperatingSystemFromKubeEnv(kubeEnv string) OperatingSystem { +func extractOperatingSystemFromKubeEnv(kubeEnv KubeEnv) OperatingSystem { osValue, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "os") if err != nil { klog.Errorf("error while obtaining os from AUTOSCALER_ENV_VARS; %v", err) @@ -627,7 +579,7 @@ func (s SystemArchitecture) Name() string { return string(s) } -func extractSystemArchitectureFromKubeEnv(kubeEnv string) (SystemArchitecture, error) { +func extractSystemArchitectureFromKubeEnv(kubeEnv KubeEnv) (SystemArchitecture, error) { archName, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "arch") if err != nil { return UnknownArch, fmt.Errorf("error while obtaining arch from AUTOSCALER_ENV_VARS: %v", err) @@ -655,7 +607,7 @@ func ToSystemArchitecture(arch string) SystemArchitecture { } } -func extractOperatingSystemDistributionFromKubeEnv(kubeEnv string) OperatingSystemDistribution { +func extractOperatingSystemDistributionFromKubeEnv(kubeEnv KubeEnv) OperatingSystemDistribution { osDistributionValue, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "os_distribution") if err != nil { klog.Errorf("error while obtaining os from AUTOSCALER_ENV_VARS; %v", err) @@ -721,8 +673,8 @@ func getDurationOption(options map[string]string, templateName, name string) (ti return option, true } -func extractAutoscalingOptionsFromKubeEnv(kubeEnvValue string) (map[string]string, error) { - optionsAsString, found, err := extractAutoscalerVarFromKubeEnv(kubeEnvValue, "autoscaling_options") +func extractAutoscalingOptionsFromKubeEnv(kubeEnv KubeEnv) (map[string]string, error) { + optionsAsString, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "autoscaling_options") if err != nil { klog.Warningf("error while obtaining autoscaling_options from AUTOSCALER_ENV_VARS: %v", err) return nil, err @@ -736,8 +688,8 @@ func extractAutoscalingOptionsFromKubeEnv(kubeEnvValue string) (map[string]strin return parseKeyValueListToMap(optionsAsString) } -func extractEvictionHardFromKubeEnv(kubeEnvValue string) (map[string]string, error) { - evictionHardAsString, found, err := extractAutoscalerVarFromKubeEnv(kubeEnvValue, "evictionHard") +func extractEvictionHardFromKubeEnv(kubeEnv KubeEnv) (map[string]string, error) { + evictionHardAsString, found, err := extractAutoscalerVarFromKubeEnv(kubeEnv, "evictionHard") if err != nil { klog.Warningf("error while obtaining eviction-hard from AUTOSCALER_ENV_VARS; %v", err) return nil, err @@ -751,11 +703,11 @@ func extractEvictionHardFromKubeEnv(kubeEnvValue string) (map[string]string, err return parseKeyValueListToMap(evictionHardAsString) } -func extractAutoscalerVarFromKubeEnv(kubeEnv, name string) (value string, found bool, err error) { +func extractAutoscalerVarFromKubeEnv(kubeEnv KubeEnv, name string) (value string, found bool, err error) { const autoscalerVars = "AUTOSCALER_ENV_VARS" - autoscalerVals, err := extractFromKubeEnv(kubeEnv, autoscalerVars) - if err != nil { - return "", false, err + autoscalerVals, found := kubeEnv.Var(autoscalerVars) + if !found { + return "", false, nil } if strings.Trim(autoscalerVals, " ") == "" { @@ -777,15 +729,6 @@ func extractAutoscalerVarFromKubeEnv(kubeEnv, name string) (value string, found return "", false, nil } -func extractFromKubeEnv(kubeEnv, resource string) (string, error) { - kubeEnvMap := make(map[string]string) - err := yaml.Unmarshal([]byte(kubeEnv), &kubeEnvMap) - if err != nil { - return "", fmt.Errorf("error unmarshalling kubeEnv: %v", err) - } - return kubeEnvMap[resource], nil -} - func parseKeyValueListToMap(kvList string) (map[string]string, error) { result := make(map[string]string) if len(kvList) == 0 { diff --git a/cluster-autoscaler/cloudprovider/gce/templates_test.go b/cluster-autoscaler/cloudprovider/gce/templates_test.go index 5026542478f0..831887b21c79 100644 --- a/cluster-autoscaler/cloudprovider/gce/templates_test.go +++ b/cluster-autoscaler/cloudprovider/gce/templates_test.go @@ -262,12 +262,16 @@ func TestBuildNodeFromTemplateSetsResources(t *testing.T) { if tc.kubeEnv != "" { template.Properties.Metadata.Items = []*gce.MetadataItems{{Key: "kube-env", Value: &tc.kubeEnv}} } - migOsInfo, err := tb.MigOsInfo(mig.Id(), template) + var migOsInfo MigOsInfo + kubeEnv, err := ExtractKubeEnv(template) + if err == nil { + migOsInfo, err = tb.MigOsInfo(mig.Id(), kubeEnv) + } if tc.expectedMigInfoErr { assert.Error(t, err) return } - node, err := tb.BuildNodeFromTemplate(mig, migOsInfo, template, tc.physicalCpu, tc.physicalMemory, tc.pods, &GceReserved{}) + node, err := tb.BuildNodeFromTemplate(mig, migOsInfo, template, kubeEnv, tc.physicalCpu, tc.physicalMemory, tc.pods, &GceReserved{}) if tc.expectedNodeTemplateErr { assert.Error(t, err) } else { @@ -439,7 +443,7 @@ func TestCalculateAllocatable(t *testing.T) { func TestBuildAllocatableFromKubeEnv(t *testing.T) { type testCase struct { - kubeEnv string + kubeEnvValue string capacityCpu string capacityMemory string capacityEphemeralStorage string @@ -450,7 +454,7 @@ func TestBuildAllocatableFromKubeEnv(t *testing.T) { expectedErr bool } testCases := []testCase{{ - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "KUBELET_TEST_ARGS: --experimental-allocatable-ignore-eviction --kube-reserved=cpu=1000m,memory=300000Mi,ephemeral-storage=30Gi\n" + @@ -464,7 +468,7 @@ func TestBuildAllocatableFromKubeEnv(t *testing.T) { gpuCount: 10, expectedErr: false, }, { - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "NODE_TAINTS: 'dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c'\n", @@ -476,7 +480,11 @@ func TestBuildAllocatableFromKubeEnv(t *testing.T) { capacity, err := makeResourceList(tc.capacityCpu, tc.capacityMemory, tc.gpuCount, tc.capacityEphemeralStorage) assert.NoError(t, err) tb := GceTemplateBuilder{} - allocatable, err := tb.BuildAllocatableFromKubeEnv(capacity, tc.kubeEnv, ParseEvictionHardOrGetDefault(nil)) + var allocatable apiv1.ResourceList + kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue) + if err == nil { + allocatable, err = tb.BuildAllocatableFromKubeEnv(capacity, kubeEnv, ParseEvictionHardOrGetDefault(nil)) + } if tc.expectedErr { assert.Error(t, err) } else { @@ -618,39 +626,39 @@ func TestBuildCapacityMemory(t *testing.T) { func TestExtractAutoscalingOptionsFromKubeEnv(t *testing.T) { cases := []struct { desc string - env string + kubeEnvValue string expectedValue map[string]string expectedErr bool }{ { desc: "autoscaling_options not specified", - env: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d;node_taints=a=b:c,d=e:f\n", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d;node_taints=a=b:c,d=e:f\n", expectedValue: map[string]string{}, expectedErr: false, }, { desc: "empty KubeEnv", - env: "", + kubeEnvValue: "", expectedValue: map[string]string{}, expectedErr: false, }, { desc: "unparsable KubeEnv", - env: "AUTOSCALER_ENV_VARS", + kubeEnvValue: "AUTOSCALER_ENV_VARS", expectedValue: nil, expectedErr: true, }, { - desc: "partial option set", - env: "AUTOSCALER_ENV_VARS: node_labels=a=b;autoscaling_options=scaledownunreadytime=1h", + desc: "partial option set", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b;autoscaling_options=scaledownunreadytime=1h", expectedValue: map[string]string{ config.DefaultScaleDownUnreadyTimeKey: "1h", }, expectedErr: false, }, { - desc: "full option set", - env: "AUTOSCALER_ENV_VARS: node_labels=a,b;autoscaling_options=scaledownutilizationthreshold=0.4,scaledowngpuutilizationthreshold=0.5,scaledownunneededtime=30m,scaledownunreadytime=1h", + desc: "full option set", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a,b;autoscaling_options=scaledownutilizationthreshold=0.4,scaledowngpuutilizationthreshold=0.5,scaledownunneededtime=30m,scaledownunreadytime=1h", expectedValue: map[string]string{ config.DefaultScaleDownUtilizationThresholdKey: "0.4", config.DefaultScaleDownGpuUtilizationThresholdKey: "0.5", @@ -662,7 +670,11 @@ func TestExtractAutoscalingOptionsFromKubeEnv(t *testing.T) { } for _, c := range cases { t.Run(c.desc, func(t *testing.T) { - value, err := extractAutoscalingOptionsFromKubeEnv(c.env) + var value map[string]string + kubeEnv, err := ParseKubeEnv("test", c.kubeEnvValue) + if err == nil { + value, err = extractAutoscalingOptionsFromKubeEnv(kubeEnv) + } assert.Equal(t, c.expectedValue, value) if c.expectedErr { assert.Error(t, err) @@ -677,7 +689,7 @@ func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) { cases := []struct { desc string name string - env string + kubeEnvValue string expectedValue string expectedFound bool expectedErr error @@ -685,7 +697,7 @@ func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) { { desc: "node_labels", name: "node_labels", - env: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d;node_taints=a=b:c,d=e:f\n", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d;node_taints=a=b:c,d=e:f\n", expectedValue: "a=b,c=d", expectedFound: true, expectedErr: nil, @@ -693,7 +705,7 @@ func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) { { desc: "node_labels not found", name: "node_labels", - env: "AUTOSCALER_ENV_VARS: node_taints=a=b:c,d=e:f\n", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_taints=a=b:c,d=e:f\n", expectedValue: "", expectedFound: false, expectedErr: nil, @@ -701,7 +713,7 @@ func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) { { desc: "node_labels empty", name: "node_labels", - env: "AUTOSCALER_ENV_VARS: node_labels=;node_taints=a=b:c,d=e:f\n", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=;node_taints=a=b:c,d=e:f\n", expectedValue: "", expectedFound: true, expectedErr: nil, @@ -709,7 +721,7 @@ func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) { { desc: "node_taints", name: "node_taints", - env: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d;node_taints=a=b:c,d=e:f\n", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d;node_taints=a=b:c,d=e:f\n", expectedValue: "a=b:c,d=e:f", expectedFound: true, expectedErr: nil, @@ -717,7 +729,7 @@ func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) { { desc: "malformed node_labels", name: "node_labels", - env: "AUTOSCALER_ENV_VARS: node_labels;node_taints=a=b:c,d=e:f\n", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels;node_taints=a=b:c,d=e:f\n", expectedValue: "", expectedFound: false, expectedErr: fmt.Errorf("malformed autoscaler var: node_labels"), @@ -725,7 +737,12 @@ func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) { } for _, c := range cases { t.Run(c.desc, func(t *testing.T) { - value, found, err := extractAutoscalerVarFromKubeEnv(c.env, c.name) + var value string + var found bool + kubeEnv, err := ParseKubeEnv("test", c.kubeEnvValue) + if err == nil { + value, found, err = extractAutoscalerVarFromKubeEnv(kubeEnv, c.name) + } assert.Equal(t, c.expectedValue, value) assert.Equal(t, c.expectedFound, found) assert.Equal(t, c.expectedErr, err) @@ -743,14 +760,14 @@ func TestExtractLabelsFromKubeEnv(t *testing.T) { preemptibleLabel: "true", } cases := []struct { - desc string - env string - expect map[string]string - err error + desc string + kubeEnvValue string + expect map[string]string + err error }{ { desc: "from NODE_LABELS", - env: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n", expect: expectedLabels, @@ -758,7 +775,7 @@ func TestExtractLabelsFromKubeEnv(t *testing.T) { }, { desc: "from AUTOSCALER_ENV_VARS.node_labels", - env: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + @@ -768,15 +785,19 @@ func TestExtractLabelsFromKubeEnv(t *testing.T) { err: nil, }, { - desc: "malformed key-value in AUTOSCALER_ENV_VARS.node_labels", - env: "AUTOSCALER_ENV_VARS: node_labels=ab,c=d\n", - err: fmt.Errorf("error while parsing key-value list, val: ab"), + desc: "malformed key-value in AUTOSCALER_ENV_VARS.node_labels", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=ab,c=d\n", + err: fmt.Errorf("error while parsing key-value list, val: ab"), }, } for _, c := range cases { t.Run(c.desc, func(t *testing.T) { - labels, err := extractLabelsFromKubeEnv(c.env) + var labels map[string]string + kubeEnv, err := ParseKubeEnv("test", c.kubeEnvValue) + if err == nil { + labels, err = extractLabelsFromKubeEnv(kubeEnv) + } assert.Equal(t, c.err, err) if c.err != nil { return @@ -789,14 +810,17 @@ func TestExtractLabelsFromKubeEnv(t *testing.T) { Items: []*gce.MetadataItems{ { Key: "kube-env", - Value: &c.env, + Value: &c.kubeEnvValue, }, }, }, }, } - labels, err = GetLabelsFromTemplate(template) + kubeEnv, err = ExtractKubeEnv(template) + if err == nil { + labels, err = GetLabelsFromKubeEnv(kubeEnv) + } assert.Equal(t, c.err, err) if c.err != nil { return @@ -826,14 +850,14 @@ func TestExtractTaintsFromKubeEnv(t *testing.T) { }) cases := []struct { - desc string - env string - expect map[apiv1.Taint]bool - err error + desc string + kubeEnvValue string + expect map[apiv1.Taint]bool + err error }{ { desc: "from NODE_TAINTS", - env: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "NODE_TAINTS: 'dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c'\n", @@ -841,7 +865,7 @@ func TestExtractTaintsFromKubeEnv(t *testing.T) { }, { desc: "from AUTOSCALER_ENV_VARS.node_taints", - env: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -851,22 +875,26 @@ func TestExtractTaintsFromKubeEnv(t *testing.T) { }, { desc: "from empty AUTOSCALER_ENV_VARS.node_taints", - env: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints=\n", expect: makeTaintSet([]apiv1.Taint{}), }, { - desc: "malformed key-value in AUTOSCALER_ENV_VARS.node_taints", - env: "AUTOSCALER_ENV_VARS: node_taints='dedicatedml:NoSchedule,test=dev:PreferNoSchedule,a=b:c'\n", - err: fmt.Errorf("error while parsing key-value list, val: dedicatedml:NoSchedule"), + desc: "malformed key-value in AUTOSCALER_ENV_VARS.node_taints", + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_taints='dedicatedml:NoSchedule,test=dev:PreferNoSchedule,a=b:c'\n", + err: fmt.Errorf("error while parsing key-value list, val: dedicatedml:NoSchedule"), }, } for _, c := range cases { t.Run(c.desc, func(t *testing.T) { - taints, err := extractTaintsFromKubeEnv(c.env) + var taints []apiv1.Taint + kubeEnv, err := ParseKubeEnv("test", c.kubeEnvValue) + if err == nil { + taints, err = extractTaintsFromKubeEnv(kubeEnv) + } assert.Equal(t, c.err, err) if c.err != nil { return @@ -879,14 +907,17 @@ func TestExtractTaintsFromKubeEnv(t *testing.T) { Items: []*gce.MetadataItems{ { Key: "kube-env", - Value: &c.env, + Value: &c.kubeEnvValue, }, }, }, }, } - taints, err = GetTaintsFromTemplate(template) + kubeEnv, err = ExtractKubeEnv(template) + if err == nil { + taints, err = GetTaintsFromKubeEnv(kubeEnv) + } assert.Equal(t, c.err, err) if c.err != nil { return @@ -899,14 +930,14 @@ func TestExtractTaintsFromKubeEnv(t *testing.T) { func TestExtractKubeReservedFromKubeEnv(t *testing.T) { type testCase struct { - kubeEnv string + kubeEnvValue string expectedReserved string expectedErr bool } testCases := []testCase{ { - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "KUBELET_TEST_ARGS: --experimental-allocatable-ignore-eviction --kube-reserved=cpu=1000m,memory=300000Mi\n" + @@ -915,7 +946,7 @@ func TestExtractKubeReservedFromKubeEnv(t *testing.T) { expectedErr: false, }, { - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -927,7 +958,7 @@ func TestExtractKubeReservedFromKubeEnv(t *testing.T) { }, { // Multi-line KUBELET_ARGS - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -938,7 +969,7 @@ func TestExtractKubeReservedFromKubeEnv(t *testing.T) { expectedErr: false, }, { - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "KUBELET_TEST_ARGS: --experimental-allocatable-ignore-eviction\n" + @@ -947,7 +978,7 @@ func TestExtractKubeReservedFromKubeEnv(t *testing.T) { expectedErr: true, }, { - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "NODE_TAINTS: 'dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c'\n", @@ -957,7 +988,11 @@ func TestExtractKubeReservedFromKubeEnv(t *testing.T) { } for _, tc := range testCases { - reserved, err := extractKubeReservedFromKubeEnv(tc.kubeEnv) + var reserved string + kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue) + if err == nil { + reserved, err = extractKubeReservedFromKubeEnv(kubeEnv) + } assert.Equal(t, tc.expectedReserved, reserved) if tc.expectedErr { assert.Error(t, err) @@ -970,14 +1005,14 @@ func TestExtractKubeReservedFromKubeEnv(t *testing.T) { func TestExtractOperatingSystemFromKubeEnv(t *testing.T) { type testCase struct { name string - kubeEnv string + kubeEnvValue string expectedOperatingSystem OperatingSystem } testCases := []testCase{ { name: "linux", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -988,7 +1023,7 @@ func TestExtractOperatingSystemFromKubeEnv(t *testing.T) { }, { name: "windows", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -999,7 +1034,7 @@ func TestExtractOperatingSystemFromKubeEnv(t *testing.T) { }, { name: "no AUTOSCALER_ENV_VARS", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "KUBELET_TEST_ARGS: --experimental-allocatable-ignore-eviction --kube-reserved=cpu=1000m,memory=300000Mi\n" + @@ -1008,7 +1043,7 @@ func TestExtractOperatingSystemFromKubeEnv(t *testing.T) { }, { name: "no os defined", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1018,7 +1053,7 @@ func TestExtractOperatingSystemFromKubeEnv(t *testing.T) { }, { name: "os is empty", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1029,7 +1064,7 @@ func TestExtractOperatingSystemFromKubeEnv(t *testing.T) { }, { name: "unknown (macos)", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1042,7 +1077,9 @@ func TestExtractOperatingSystemFromKubeEnv(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actualOperatingSystem := extractOperatingSystemFromKubeEnv(tc.kubeEnv) + kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue) + assert.NoError(t, err) + actualOperatingSystem := extractOperatingSystemFromKubeEnv(kubeEnv) assert.Equal(t, tc.expectedOperatingSystem, actualOperatingSystem) }) } @@ -1051,14 +1088,14 @@ func TestExtractOperatingSystemFromKubeEnv(t *testing.T) { func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { type testCase struct { name string - kubeEnv string + kubeEnvValue string expectedOperatingSystemDistribution OperatingSystemDistribution } testCases := []testCase{ { name: "cos", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1069,7 +1106,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "cos containerd", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1080,7 +1117,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "ubuntu containerd", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1091,7 +1128,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "ubuntu", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1102,7 +1139,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "windows ltsc", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1113,7 +1150,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "windows sac", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1124,7 +1161,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "no AUTOSCALER_ENV_VARS", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "NODE_LABELS: a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "KUBELET_TEST_ARGS: --experimental-allocatable-ignore-eviction --kube-reserved=cpu=1000m,memory=300000Mi\n" + @@ -1133,7 +1170,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "no os distribution defined", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1143,7 +1180,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "os distribution is empty", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1154,7 +1191,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { }, { name: "unknown (macos)", - kubeEnv: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + + kubeEnvValue: "ENABLE_NODE_PROBLEM_DETECTOR: 'daemonset'\n" + "DNS_SERVER_IP: '10.0.0.10'\n" + "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + @@ -1167,7 +1204,9 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actualOperatingSystem := extractOperatingSystemDistributionFromKubeEnv(tc.kubeEnv) + kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue) + assert.NoError(t, err) + actualOperatingSystem := extractOperatingSystemDistributionFromKubeEnv(kubeEnv) assert.Equal(t, tc.expectedOperatingSystemDistribution, actualOperatingSystem) }) } @@ -1176,7 +1215,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) { func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { type testCase struct { name string - kubeEnv string + kubeEnvValue string expectedExtendedResources apiv1.ResourceList expectedErr bool } @@ -1184,7 +1223,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { testCases := []testCase{ { name: "numeric value", - kubeEnv: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + "extended_resources=foo=10", @@ -1195,7 +1234,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { }, { name: "numeric value with quantity suffix", - kubeEnv: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + "extended_resources=foo=10G", @@ -1206,7 +1245,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { }, { name: "multiple extended_resources definition", - kubeEnv: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + "extended_resources=foo=10G,bar=230", @@ -1218,7 +1257,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { }, { name: "invalid value", - kubeEnv: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + "extended_resources=foo=bar", @@ -1227,7 +1266,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { }, { name: "both valid and invalid values", - kubeEnv: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + "extended_resources=foo=bar,baz=10G", @@ -1238,7 +1277,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { }, { name: "invalid quantity suffix", - kubeEnv: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + "extended_resources=foo=10Wi", @@ -1247,7 +1286,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { }, { name: "malformed extended_resources map", - kubeEnv: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + "extended_resources=foo", @@ -1256,7 +1295,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { }, { name: "malformed extended_resources definition", - kubeEnv: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + + kubeEnvValue: "AUTOSCALER_ENV_VARS: node_labels=a=b,c=d,cloud.google.com/gke-nodepool=pool-3,cloud.google.com/gke-preemptible=true;" + "node_taints='dedicated=ml:NoSchedule,test=dev:PreferNoSchedule,a=b:c';" + "kube_reserved=cpu=1000m,memory=300000Mi;" + "extended_resources/", @@ -1267,7 +1306,11 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - extendedResources, err := extractExtendedResourcesFromKubeEnv(tc.kubeEnv) + var extendedResources apiv1.ResourceList + kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue) + if err == nil { + extendedResources, err = extractExtendedResourcesFromKubeEnv(kubeEnv) + } assertEqualResourceLists(t, "Resources", tc.expectedExtendedResources, extendedResources) if tc.expectedErr { assert.Error(t, err) @@ -1342,38 +1385,42 @@ func TestToSystemArchitecture(t *testing.T) { func TestExtractSystemArchitectureFromKubeEnv(t *testing.T) { for tn, tc := range map[string]struct { - kubeEnv string - wantArch SystemArchitecture - wantErr error + kubeEnvValue 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, + kubeEnvValue: "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, + kubeEnvValue: "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, + kubeEnvValue: "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, + kubeEnvValue: "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, + kubeEnvValue: "some-invalid-string", + wantArch: UnknownArch, + wantErr: cmpopts.AnyError, }, } { t.Run(tn, func(t *testing.T) { - gotArch, gotErr := extractSystemArchitectureFromKubeEnv(tc.kubeEnv) + var gotArch SystemArchitecture + kubeEnv, gotErr := ParseKubeEnv("test", tc.kubeEnvValue) + if gotErr == nil { + gotArch, gotErr = extractSystemArchitectureFromKubeEnv(kubeEnv) + } if diff := cmp.Diff(tc.wantArch, gotArch); diff != "" { t.Errorf("extractSystemArchitectureFromKubeEnv diff (-want +got):\n%s", diff) } @@ -1410,11 +1457,15 @@ func TestBuildNodeFromTemplateArch(t *testing.T) { }, } tb := &GceTemplateBuilder{} - migOsInfo, gotErr := tb.MigOsInfo(mig.Id(), template) + kubeEnv, gotErr := ExtractKubeEnv(template) + if gotErr != nil { + t.Fatalf("ExtractKubeEnv unexpected error: %v", gotErr) + } + migOsInfo, gotErr := tb.MigOsInfo(mig.Id(), kubeEnv) if gotErr != nil { t.Fatalf("MigOsInfo unexpected error: %v", gotErr) } - gotNode, gotErr := tb.BuildNodeFromTemplate(mig, migOsInfo, template, 16, 128, nil, &GceReserved{}) + gotNode, gotErr := tb.BuildNodeFromTemplate(mig, migOsInfo, template, kubeEnv, 16, 128, nil, &GceReserved{}) if gotErr != nil { t.Fatalf("BuildNodeFromTemplate unexpected error: %v", gotErr) }