Skip to content

OCPBUGS-99759: fix(nodepool): add CAPI autoscaler capacity annotations for scale-from-zero #9110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
judexzhu wants to merge 1 commit into
base: main
Choose a base branch
from
+63 −18
Open

OCPBUGS-99759: fix(nodepool): add CAPI autoscaler capacity annotations for scale-from-zero #9110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions hypershift-operator/controllers/nodepool/scale_from_zero.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,20 @@ import (
)

const (
// Annotation keys for scale-from-zero workaround
// Legacy annotation keys (Machine API provider, --cloud-provider=openshift)
cpuKey = "machine.openshift.io/vCPU"
memoryKey = "machine.openshift.io/memoryMb"
gpuKey = "machine.openshift.io/GPU"
labelsKey = "capacity.cluster-autoscaler.kubernetes.io/labels"
taintsKey = "capacity.cluster-autoscaler.kubernetes.io/taints"

// CAPI autoscaler annotation keys (--cloud-provider=clusterapi)
capiCPUKey = "capacity.cluster-autoscaler.kubernetes.io/cpu"
capiMemoryKey = "capacity.cluster-autoscaler.kubernetes.io/memory"
capiGPUCountKey = "capacity.cluster-autoscaler.kubernetes.io/gpu-count"
capiGPUTypeKey = "capacity.cluster-autoscaler.kubernetes.io/gpu-type"
labelsKey = "capacity.cluster-autoscaler.kubernetes.io/labels"
taintsKey = "capacity.cluster-autoscaler.kubernetes.io/taints"

defaultGPUResourceName = "nvidia.com/gpu"

archLabelKey = "kubernetes.io/arch"
)
Expand Down Expand Up @@ -91,7 +99,7 @@ func setScaleFromZeroAnnotationsOnObject(ctx context.Context, provider instancet
// 2. Check if Status.Capacity is already provided (prefer native support)
if len(statusCapacity) > 0 {
// Clean up workaround annotations (if they exist)
annotationKeys := []string{cpuKey, memoryKey, gpuKey, labelsKey, taintsKey}
annotationKeys := []string{cpuKey, memoryKey, gpuKey, capiCPUKey, capiMemoryKey, capiGPUCountKey, capiGPUTypeKey, labelsKey, taintsKey}
for _, key := range annotationKeys {
delete(annotations, key)
}
Expand Down Expand Up @@ -120,15 +128,21 @@ func setScaleFromZeroAnnotationsOnObject(ctx context.Context, provider instancet
}

// 5. Set workaround annotations
annotations[cpuKey] = strconv.FormatInt(int64(instanceInfo.VCPU), 10)
cpuStr := strconv.FormatInt(int64(instanceInfo.VCPU), 10)
annotations[cpuKey] = cpuStr
annotations[capiCPUKey] = cpuStr
annotations[memoryKey] = strconv.FormatInt(instanceInfo.MemoryMb, 10)
annotations[capiMemoryKey] = fmt.Sprintf("%dMi", instanceInfo.MemoryMb)

// Set GPU annotation only if GPU > 0 (consistent with taints handling)
if instanceInfo.GPU > 0 {
annotations[gpuKey] = strconv.FormatInt(int64(instanceInfo.GPU), 10)
gpuStr := strconv.FormatInt(int64(instanceInfo.GPU), 10)
annotations[gpuKey] = gpuStr
annotations[capiGPUCountKey] = gpuStr
annotations[capiGPUTypeKey] = defaultGPUResourceName
} else {
// Remove GPU annotation if there are no GPUs
delete(annotations, gpuKey)
delete(annotations, capiGPUCountKey)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
delete(annotations, capiGPUTypeKey)
}

// 6. Set labels (including architecture and NodePool.Spec.NodeLabels)
Expand Down
27 changes: 24 additions & 3 deletions hypershift-operator/controllers/nodepool/scale_from_zero_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ func TestSetScaleFromZeroAnnotationsOnObject(t *testing.T) {
cpuKey: "4",
memoryKey: "16384",
gpuKey: "1",
capiCPUKey: "4",
capiMemoryKey: "16384Mi",
capiGPUCountKey: "1",
capiGPUTypeKey: "nvidia.com/gpu",
labelsKey: "kubernetes.io/arch=amd64",
taintsKey: "dedicated=gpu:NoSchedule",
"custom.io/keep": "preserved",
Expand All @@ -169,7 +173,7 @@ func TestSetScaleFromZeroAnnotationsOnObject(t *testing.T) {
expectErr: false,
validate: func(g Gomega, md *capiv1.MachineDeployment) {
a := md.GetAnnotations()
for _, k := range []string{cpuKey, memoryKey, gpuKey, labelsKey, taintsKey} {
for _, k := range []string{cpuKey, memoryKey, gpuKey, capiCPUKey, capiMemoryKey, capiGPUCountKey, capiGPUTypeKey, labelsKey, taintsKey} {
g.Expect(a).ToNot(HaveKey(k))
}
g.Expect(a).To(HaveKeyWithValue("custom.io/keep", "preserved"))
Expand All @@ -195,8 +199,9 @@ func TestSetScaleFromZeroAnnotationsOnObject(t *testing.T) {
object: &capiv1.MachineDeployment{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
gpuKey: "2",
taintsKey: "old=stale:NoSchedule",
gpuKey: "2",
capiGPUCountKey: "2",
taintsKey: "old=stale:NoSchedule",
},
},
},
Expand All @@ -205,9 +210,13 @@ func TestSetScaleFromZeroAnnotationsOnObject(t *testing.T) {
validate: func(g Gomega, md *capiv1.MachineDeployment) {
a := md.GetAnnotations()
g.Expect(a).To(HaveKeyWithValue(cpuKey, "2"))
g.Expect(a).To(HaveKeyWithValue(capiCPUKey, "2"))
g.Expect(a).To(HaveKeyWithValue(memoryKey, "8192"))
g.Expect(a).To(HaveKeyWithValue(capiMemoryKey, "8192Mi"))
g.Expect(a).To(HaveKeyWithValue(labelsKey, "kubernetes.io/arch=amd64"))
g.Expect(a).ToNot(HaveKey(gpuKey))
g.Expect(a).ToNot(HaveKey(capiGPUCountKey))
g.Expect(a).ToNot(HaveKey(capiGPUTypeKey))
g.Expect(a).ToNot(HaveKey(taintsKey))
},
},
Expand All @@ -223,9 +232,13 @@ func TestSetScaleFromZeroAnnotationsOnObject(t *testing.T) {
validate: func(g Gomega, md *capiv1.MachineDeployment) {
a := md.GetAnnotations()
g.Expect(a).To(HaveKeyWithValue(cpuKey, "4"))
g.Expect(a).To(HaveKeyWithValue(capiCPUKey, "4"))
g.Expect(a).To(HaveKeyWithValue(memoryKey, "16384"))
g.Expect(a).To(HaveKeyWithValue(capiMemoryKey, "16384Mi"))
g.Expect(a).To(HaveKeyWithValue(labelsKey, "kubernetes.io/arch=amd64"))
g.Expect(a).ToNot(HaveKey(gpuKey))
g.Expect(a).ToNot(HaveKey(capiGPUCountKey))
g.Expect(a).ToNot(HaveKey(capiGPUTypeKey))
},
},
{
Expand Down Expand Up @@ -266,8 +279,12 @@ func TestSetScaleFromZeroAnnotationsOnObject(t *testing.T) {
validate: func(g Gomega, md *capiv1.MachineDeployment) {
a := md.GetAnnotations()
g.Expect(a).To(HaveKeyWithValue(cpuKey, "6"))
g.Expect(a).To(HaveKeyWithValue(capiCPUKey, "6"))
g.Expect(a).To(HaveKeyWithValue(memoryKey, "114688"))
g.Expect(a).To(HaveKeyWithValue(capiMemoryKey, "114688Mi"))
g.Expect(a).To(HaveKeyWithValue(gpuKey, "1"))
g.Expect(a).To(HaveKeyWithValue(capiGPUCountKey, "1"))
g.Expect(a).To(HaveKeyWithValue(capiGPUTypeKey, "nvidia.com/gpu"))
g.Expect(a).To(HaveKeyWithValue(taintsKey, "dedicated=gpu:NoSchedule"))
},
},
Expand Down Expand Up @@ -297,8 +314,12 @@ func TestSetScaleFromZeroAnnotationsOnObject(t *testing.T) {
validate: func(g Gomega, md *capiv1.MachineDeployment) {
a := md.GetAnnotations()
g.Expect(a).To(HaveKeyWithValue(cpuKey, "8"))
g.Expect(a).To(HaveKeyWithValue(capiCPUKey, "8"))
g.Expect(a).To(HaveKeyWithValue(memoryKey, "61440"))
g.Expect(a).To(HaveKeyWithValue(capiMemoryKey, "61440Mi"))
g.Expect(a).To(HaveKeyWithValue(gpuKey, "1"))
g.Expect(a).To(HaveKeyWithValue(capiGPUCountKey, "1"))
g.Expect(a).To(HaveKeyWithValue(capiGPUTypeKey, "nvidia.com/gpu"))
g.Expect(a).To(HaveKeyWithValue(labelsKey, "env=production,kubernetes.io/arch=arm64"))
g.Expect(a).To(HaveKeyWithValue(taintsKey, "dedicated=gpu:NoSchedule"))
g.Expect(a).To(HaveKeyWithValue("custom.io/keep", "preserved"))
Expand Down
24 changes: 17 additions & 7 deletions test/e2e/autoscaling_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,15 +801,22 @@ func testScaleFromZero(ctx context.Context, mgtClient crclient.Client, hostedClu
return true, "native Status.Capacity present with CPU and Memory", nil
}

// Fall back to checking annotations (pre-CAPI 1.11)
// Fall back to checking annotations
t.Logf("No Status.Capacity found, checking for workaround annotations on MachineDeployment")
// CAPI autoscaler keys (--cloud-provider=clusterapi)
if _, ok := md.Annotations["capacity.cluster-autoscaler.kubernetes.io/cpu"]; !ok {
return false, "missing CAPI cpu annotation", nil
}
if _, ok := md.Annotations["capacity.cluster-autoscaler.kubernetes.io/memory"]; !ok {
return false, "missing CAPI memory annotation", nil
}
// Legacy keys (co-existing for backward compatibility)
if _, ok := md.Annotations["machine.openshift.io/vCPU"]; !ok {
return false, "missing both Status.Capacity and vCPU annotation", nil
return false, "missing legacy vCPU annotation", nil
}
if _, ok := md.Annotations["machine.openshift.io/memoryMb"]; !ok {
return false, "missing both Status.Capacity and memoryMb annotation", nil
return false, "missing legacy memoryMb annotation", nil
}
// GPU annotation is optional - only set when instance type has GPUs
labels, ok := md.Annotations["capacity.cluster-autoscaler.kubernetes.io/labels"]
if !ok {
return false, "missing capacity labels annotation", nil
Expand Down Expand Up @@ -844,15 +851,18 @@ func testScaleFromZero(ctx context.Context, mgtClient crclient.Client, hostedClu
memQty.String(),
gpuQty.String())
} else {
gpuValue := md.Annotations["machine.openshift.io/GPU"]
gpuValue := md.Annotations["capacity.cluster-autoscaler.kubernetes.io/gpu-count"]
if gpuValue == "" {
gpuValue = "none (non-GPU instance)"
}
t.Logf("Capacity via %s: vCPU=%s, memoryMb=%s, GPU=%s, labels=%s",
t.Logf("Capacity via %s: CAPI[cpu=%s, memory=%s, gpu=%s] legacy[vCPU=%s, memoryMb=%s, GPU=%s] labels=%s",
capacitySource,
md.Annotations["capacity.cluster-autoscaler.kubernetes.io/cpu"],
md.Annotations["capacity.cluster-autoscaler.kubernetes.io/memory"],
gpuValue,
md.Annotations["machine.openshift.io/vCPU"],
md.Annotations["machine.openshift.io/memoryMb"],
gpuValue,
md.Annotations["machine.openshift.io/GPU"],
md.Annotations["capacity.cluster-autoscaler.kubernetes.io/labels"])
}

Expand Down