From a443605b35f934a8e2fc784bc486747bcd901dbb Mon Sep 17 00:00:00 2001 From: Austin Siu Date: Tue, 22 Feb 2022 22:02:53 -0600 Subject: [PATCH] Add nil check when building tempalte with requirements, improve readability --- .../cloudprovider/aws/README.md | 2 +- .../cloudprovider/aws/aws_manager.go | 61 ++++++++++--------- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/aws/README.md b/cluster-autoscaler/cloudprovider/aws/README.md index 0c012248318..b03377e0b41 100644 --- 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 index c9b56f5bf37..7d569b35f71 100644 --- 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 {