diff --git a/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go b/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go index 0d595a6831a1..8cd8024a1976 100644 --- a/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go +++ b/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go @@ -64,8 +64,6 @@ const ( conflictRetryInterval = 5 * time.Second // machinePriorityAnnotation is the annotation to set machine priority while deletion machinePriorityAnnotation = "machinepriority.machine.sapcloud.io" - // labelForFailureDomain is the label for failure domain used by kubernetes.io - labelForFailureDomain = "failure-domain.beta.kubernetes.io/zone" // kindAWSMachineClass is the kind for machine class used by In-tree AWS provider kindAWSMachineClass = "AWSMachineClass" // kindAzureMachineClass is the kind for machine class used by In-tree Azure provider @@ -441,10 +439,15 @@ func (m *McmManager) GetMachineDeploymentNodeTemplate(machinedeployment *Machine return nil, fmt.Errorf("Unable to fetch MachineDeployment object %s, Error: %v", machinedeployment.Name, err) } - var region, zone string - var instance instanceType - machineClass := md.Spec.Template.Spec.Class - nodeTemplateSpec := md.Spec.Template.Spec.NodeTemplateSpec + var ( + region string + zone string + instance instanceType + + machineClass = md.Spec.Template.Spec.Class + nodeTemplateSpec = md.Spec.Template.Spec.NodeTemplateSpec + ) + switch machineClass.Kind { case kindAWSMachineClass: mc, err := m.machineclient.AWSMachineClasses(m.namespace).Get(context.TODO(), machineClass.Name, metav1.GetOptions{}) @@ -459,9 +462,7 @@ func (m *McmManager) GetMachineDeploymentNodeTemplate(machinedeployment *Machine GPU: awsInstance.GPU, } region = mc.Spec.Region - if mc.Labels != nil { - zone = mc.Labels[labelForFailureDomain] - } + zone = getZoneValueFromMCLabels(mc.Labels) case kindAzureMachineClass: mc, err := m.machineclient.AzureMachineClasses(m.namespace).Get(context.TODO(), machineClass.Name, metav1.GetOptions{}) if err != nil { @@ -499,9 +500,7 @@ func (m *McmManager) GetMachineDeploymentNodeTemplate(machinedeployment *Machine GPU: awsInstance.GPU, } region = providerSpec.Region - if mc.Labels != nil { - zone = mc.Labels[labelForFailureDomain] - } + zone = getZoneValueFromMCLabels(mc.Labels) case providerAzure: var providerSpec *azureapis.AzureProviderSpec err = json.Unmarshal(mc.ProviderSpec.Raw, &providerSpec) @@ -548,6 +547,22 @@ func (m *McmManager) GetMachineDeploymentNodeTemplate(machinedeployment *Machine return nodeTmpl, nil } +func getZoneValueFromMCLabels(labels map[string]string) string { + var zone string + + if labels != nil { + if value, exists := labels[apiv1.LabelZoneFailureDomainStable]; exists { + // Prefer zone value from the new label + zone = value + } else if value, exists := labels[apiv1.LabelZoneFailureDomain]; exists { + // Fallback to zone value from deprecated label if new lable value doesn't exist + zone = value + } + } + + return zone +} + func (m *McmManager) buildNodeFromTemplate(name string, template *nodeTemplate) (*apiv1.Node, error) { node := apiv1.Node{} nodeName := fmt.Sprintf("%s-%d", name, rand.Int63()) @@ -585,12 +600,20 @@ func buildGenericLabels(template *nodeTemplate, nodeName string) map[string]stri result := make(map[string]string) // TODO: extract from MCM result[kubeletapis.LabelArch] = cloudprovider.DefaultArch + result[apiv1.LabelArchStable] = cloudprovider.DefaultArch + result[kubeletapis.LabelOS] = cloudprovider.DefaultOS + result[apiv1.LabelOSStable] = cloudprovider.DefaultOS result[apiv1.LabelInstanceType] = template.InstanceType.InstanceType + result[apiv1.LabelInstanceTypeStable] = template.InstanceType.InstanceType result[apiv1.LabelZoneRegion] = template.Region + result[apiv1.LabelZoneRegionStable] = template.Region + result[apiv1.LabelZoneFailureDomain] = template.Zone + result[apiv1.LabelZoneFailureDomainStable] = template.Zone + result[apiv1.LabelHostname] = nodeName return result } diff --git a/cluster-autoscaler/cloudprovider/mcm/mcm_manager_test.go b/cluster-autoscaler/cloudprovider/mcm/mcm_manager_test.go index 90347e6a137c..814a40b8090c 100644 --- a/cluster-autoscaler/cloudprovider/mcm/mcm_manager_test.go +++ b/cluster-autoscaler/cloudprovider/mcm/mcm_manager_test.go @@ -26,17 +26,73 @@ import ( ) func TestBuildGenericLabels(t *testing.T) { + var ( + instanceTypeC4Large = "c4.large" + regionUSEast1 = "us-east-1" + zoneUSEast1a = "us-east-1a" + testHostName = "test-hostname" + ) + labels := buildGenericLabels(&nodeTemplate{ InstanceType: &instanceType{ - InstanceType: "c4.large", + InstanceType: instanceTypeC4Large, VCPU: 2, MemoryMb: 3840, }, - Region: "us-east-1", - }, "sillyname") - assert.Equal(t, "us-east-1", labels[apiv1.LabelZoneRegion]) - assert.Equal(t, "sillyname", labels[apiv1.LabelHostname]) - assert.Equal(t, "c4.large", labels[apiv1.LabelInstanceType]) + Region: regionUSEast1, + Zone: zoneUSEast1a, + }, testHostName) + assert.Equal(t, cloudprovider.DefaultArch, labels[kubeletapis.LabelArch]) + assert.Equal(t, cloudprovider.DefaultArch, labels[apiv1.LabelArchStable]) + assert.Equal(t, cloudprovider.DefaultOS, labels[kubeletapis.LabelOS]) + assert.Equal(t, cloudprovider.DefaultOS, labels[apiv1.LabelOSStable]) + + assert.Equal(t, instanceTypeC4Large, labels[apiv1.LabelInstanceType]) + assert.Equal(t, instanceTypeC4Large, labels[apiv1.LabelInstanceTypeStable]) + + assert.Equal(t, regionUSEast1, labels[apiv1.LabelZoneRegion]) + assert.Equal(t, regionUSEast1, labels[apiv1.LabelZoneRegionStable]) + + assert.Equal(t, zoneUSEast1a, labels[apiv1.LabelZoneFailureDomain]) + assert.Equal(t, zoneUSEast1a, labels[apiv1.LabelZoneFailureDomainStable]) + + assert.Equal(t, testHostName, labels[apiv1.LabelHostname]) +} + +func TestGenerationOfCorrectZoneValueFromMCLabel(t *testing.T) { + var ( + resultingZone string + zoneA = "zone-a" + zoneB = "zone-b" + randomKey = "random-key" + randomValue = "random-value" + ) + + // Basic test to get zone value + resultingZone = getZoneValueFromMCLabels(map[string]string{ + apiv1.LabelZoneFailureDomainStable: zoneA, + }) + assert.Equal(t, resultingZone, zoneA) + + // Prefer LabelZoneFailureDomainStable label over LabelZoneFailureDomain + resultingZone = getZoneValueFromMCLabels(map[string]string{ + apiv1.LabelZoneFailureDomainStable: zoneA, + apiv1.LabelZoneFailureDomain: zoneB, + }) + assert.Equal(t, resultingZone, zoneA) + + // Fallback to LabelZoneFailureDomain when LabelZoneFailureDomainStable is not found + resultingZone = getZoneValueFromMCLabels(map[string]string{ + randomKey: randomValue, + apiv1.LabelZoneFailureDomain: zoneB, + }) + assert.Equal(t, resultingZone, zoneB) + + // When neither of the labels are found + resultingZone = getZoneValueFromMCLabels(map[string]string{ + randomKey: randomValue, + }) + assert.Equal(t, resultingZone, "") }