Add nil check when building tempalte with requirements, improve reada…

…bility
kubernetes · Feb 23, 2022 · a443605 · a443605
1 parent e365c91
commit a443605
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 29 deletions.
diff --git a/cluster-autoscaler/cloudprovider/aws/README.md b/cluster-autoscaler/cloudprovider/aws/README.md
@@ -60,7 +60,7 @@ should be updated to restrict the resources/add conditionals:
         "autoscaling:TerminateInstanceInAutoScalingGroup",
         "ec2:DescribeImages",
         "ec2:DescribeInstanceTypes",
-        "ec2:GetInstanceTypesFromInstanceRequirementsPages"
+        "ec2:GetInstanceTypesFromInstanceRequirements"
       ],
       "Resource": ["*"]
     }

diff --git a/cluster-autoscaler/cloudprovider/aws/aws_manager.go b/cluster-autoscaler/cloudprovider/aws/aws_manager.go
@@ -396,34 +396,7 @@ func (m *AwsManager) buildNodeFromTemplate(asg *asg, template *asgTemplate) (*ap
 	node.Status.Capacity[gpu.ResourceNvidiaGPU] = *resource.NewQuantity(template.InstanceType.GPU, resource.DecimalSI)
 	node.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(template.InstanceType.MemoryMb*1024*1024, resource.DecimalSI)
 
-	if asg.MixedInstancesPolicy != nil {
-		instanceRequirements, err := m.getInstanceRequirementsFromMixedInstancesPolicy(asg.MixedInstancesPolicy)
-		if err != nil {
-			klog.Error("error while building node template using instance requirements: (%s)", err)
-		}
-
-		if instanceRequirements.VCpuCount != nil {
-			if instanceRequirements.VCpuCount.Min != nil {
-				node.Status.Capacity[apiv1.ResourceCPU] = *resource.NewQuantity(*instanceRequirements.VCpuCount.Min, resource.DecimalSI)
-			}
-		}
-
-		for _, manufacturer := range instanceRequirements.AcceleratorManufacturers {
-			if *manufacturer == autoscaling.AcceleratorManufacturerNvidia {
-				for _, acceleratorType := range instanceRequirements.AcceleratorTypes {
-					if *acceleratorType == autoscaling.AcceleratorTypeGpu {
-						node.Status.Capacity[gpu.ResourceNvidiaGPU] = *resource.NewQuantity(*instanceRequirements.AcceleratorCount.Min, resource.DecimalSI)
-					}
-				}
-			}
-		}
-
-		if instanceRequirements.MemoryMiB != nil {
-			if instanceRequirements.MemoryMiB.Min != nil {
-				node.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(*instanceRequirements.MemoryMiB.Min*1024*1024, resource.DecimalSI)
-			}
-		}
-	}
+	node.Status.Capacity = *m.updateCapacityWithRequirementsOverrides(&node.Status.Capacity, asg.MixedInstancesPolicy)
 
 	resourcesFromTags := extractAllocatableResourcesFromAsg(template.Tags)
 	for resourceName, val := range resourcesFromTags {
@@ -449,6 +422,38 @@ func (m *AwsManager) buildNodeFromTemplate(asg *asg, template *asgTemplate) (*ap
 	return &node, nil
 }
 
+func (m *AwsManager) updateCapacityWithRequirementsOverrides(capacity *apiv1.ResourceList, policy *mixedInstancesPolicy) *apiv1.ResourceList {
+	if policy == nil {
+		return capacity
+	}
+
+	instanceRequirements, err := m.getInstanceRequirementsFromMixedInstancesPolicy(policy)
+	if err != nil {
+		klog.Error("error while building node template using instance requirements: (%s)", err)
+		return capacity
+	}
+
+	if instanceRequirements.VCpuCount != nil && instanceRequirements.VCpuCount.Min != nil {
+		(*capacity)[apiv1.ResourceCPU] = *resource.NewQuantity(*instanceRequirements.VCpuCount.Min, resource.DecimalSI)
+	}
+
+	if instanceRequirements.MemoryMiB != nil && instanceRequirements.MemoryMiB.Min != nil {
+		(*capacity)[apiv1.ResourceMemory] = *resource.NewQuantity(*instanceRequirements.MemoryMiB.Min*1024*1024, resource.DecimalSI)
+	}
+
+	for _, manufacturer := range instanceRequirements.AcceleratorManufacturers {
+		if *manufacturer == autoscaling.AcceleratorManufacturerNvidia {
+			for _, acceleratorType := range instanceRequirements.AcceleratorTypes {
+				if *acceleratorType == autoscaling.AcceleratorTypeGpu {
+					(*capacity)[gpu.ResourceNvidiaGPU] = *resource.NewQuantity(*instanceRequirements.AcceleratorCount.Min, resource.DecimalSI)
+				}
+			}
+		}
+	}
+
+	return capacity
+}
+
 func (m *AwsManager) getInstanceRequirementsFromMixedInstancesPolicy(policy *mixedInstancesPolicy) (*ec2.InstanceRequirements, error) {
 	instanceRequirements := &ec2.InstanceRequirements{}
 	if policy.instanceRequirementsOverrides != nil {