fix(resources): omit ephemeral-disk autoscaler annotation when 0

Address review feedback on PR #1014.

For server_type=bm, the boot volume default returns 0 (Ironic boots
from local disk, not Cinder), and the fallback flavor.disk is
typically also 0 because root_gb lives on the Ironic node rather
than the Nova flavor. Previously the autoscaler annotation
capacity.cluster-autoscaler.kubernetes.io/ephemeral-disk was set
to "0", which causes cluster-autoscaler to reject any pod that
requests ephemeral-storage even if the Ironic node has plenty of
local disk.

Skip the annotation entirely in that case so the autoscaler can fall
back to cpu/memory-only scheduling decisions, which is correct for
BM nodes whose disk capacity we cannot reliably determine here.

Also document the auto-default in docs/user/labels.md.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: ricolin <rlin@vexxhost.com>

Author: ricolin

docs/user/labels.md

@@ -17,7 +17,11 @@ specify the volume size and type using the following labels:
 
 :   The size in gigabytes of the boot volume.  If you set this value, it will
     enable boot from volume.
-    **Default value**: Unset
+    **Default value**: Unset for `server_type=vm` clusters; automatically
+    `0` for `server_type=bm` clusters (baremetal nodes boot from local disk
+    via Ironic and cannot use a Cinder root volume). Operators can still
+    override explicitly on `server_type=bm` by setting this label on the
+    cluster_template, cluster, or node_group.
 
 `boot_volume_type`
 

magnum_cluster_api/resources.py

@@ -749,21 +749,30 @@ def mutate_machine_deployment(
             boot_volume_size = flavor.disk
 
         machine_deployment["replicas"] = None
-        machine_deployment["metadata"]["annotations"] = {
+        annotations = {
             AUTOSCALE_ANNOTATION_MIN: str(node_group.min_node_count),
             AUTOSCALE_ANNOTATION_MAX: str(
                 utils.get_node_group_max_node_count(node_group)
             ),
             "capacity.cluster-autoscaler.kubernetes.io/memory": f"{math.ceil(flavor.ram / 1024)}G",
             "capacity.cluster-autoscaler.kubernetes.io/cpu": str(flavor.vcpus),
-            "capacity.cluster-autoscaler.kubernetes.io/ephemeral-disk": str(
-                boot_volume_size
-            ),
             "capacity.cluster-autoscaler.kubernetes.io/labels": (
                 f"node-role.kubernetes.io/{node_group.role}=,"
                 f"node.cluster.x-k8s.io/nodegroup={node_group.name}"
             ),
         }
+        # Only emit the ephemeral-disk capacity hint when we can resolve a
+        # non-zero value. For server_type=bm the flavor.disk is typically
+        # 0 (root_gb lives on the Ironic node, not the Nova flavor) and
+        # the Cinder root volume default is also 0; advertising "0" here
+        # would mislead the autoscaler into rejecting pods that request
+        # ephemeral-storage. Omitting the annotation lets the autoscaler
+        # fall back to scheduling on cpu/memory only.
+        if boot_volume_size > 0:
+            annotations["capacity.cluster-autoscaler.kubernetes.io/ephemeral-disk"] = (
+                str(boot_volume_size)
+            )
+        machine_deployment["metadata"]["annotations"] = annotations
     else:
         machine_deployment["replicas"] = node_group.node_count
         machine_deployment["metadata"]["annotations"] = {}

magnum_cluster_api/tests/unit/test_resources.py

@@ -96,6 +96,14 @@ class TestExistingMutateMachineDeployment:
     @pytest.fixture(autouse=True)
     def setup(self, auto_scaling_enabled, auto_healing_enabled, context, mocker):
         self.cluster = utils.get_test_cluster(context, labels={})
+        # mutate_machine_deployment now consults cluster.cluster_template.server_type
+        # (via utils.get_default_boot_volume_size) which lazy-loads from the DB
+        # in this test context. Stub the helper so existing assertions are
+        # unaffected.
+        mocker.patch(
+            "magnum_cluster_api.utils.get_default_boot_volume_size",
+            side_effect=lambda cluster, default: default,
+        )
         if auto_scaling_enabled is not None:
             self.cluster.labels["auto_scaling_enabled"] = str(auto_scaling_enabled)
 
@@ -161,3 +169,68 @@ def test_mutate_machine_deployment(self, context, auto_scaling_enabled):
         else:
             assert md["replicas"] == self.node_group.node_count
             assert md["metadata"]["annotations"] == {}
+
+
+def test_mutate_machine_deployment_bm_omits_ephemeral_disk_annotation(context, mocker):
+    """For server_type=bm flavors with disk=0 the autoscaler annotation
+    should be omitted rather than emitted as "0", which would make the
+    autoscaler reject pods that request ephemeral-storage."""
+    cluster = utils.get_test_cluster(context, labels={})
+    mocker.patch(
+        "magnum_cluster_api.utils.get_default_boot_volume_size",
+        return_value=0,
+    )
+    cluster.labels["auto_scaling_enabled"] = "true"
+    node_group = utils.get_test_nodegroup(context, labels={})
+    node_group.min_node_count = 1
+    node_group.max_node_count = 3
+
+    mocker.patch("magnum_cluster_api.utils.lookup_image", return_value={"id": "foo"})
+    mocker.patch(
+        "magnum_cluster_api.utils.lookup_flavor",
+        return_value=flavors.Flavor(
+            None,
+            {"name": "bm-flavor", "disk": 0, "ram": 4096, "vcpus": 4},
+        ),
+    )
+
+    md = resources.mutate_machine_deployment(
+        context, cluster, node_group, {"name": node_group.name}
+    )
+
+    annotations = md["metadata"]["annotations"]
+    assert "capacity.cluster-autoscaler.kubernetes.io/ephemeral-disk" not in annotations
+    # The other autoscaler hints must still be set so the autoscaler can
+    # schedule on cpu/memory.
+    assert annotations["capacity.cluster-autoscaler.kubernetes.io/cpu"] == "4"
+    assert annotations["capacity.cluster-autoscaler.kubernetes.io/memory"] == "4G"
+
+
+def test_mutate_machine_deployment_vm_keeps_ephemeral_disk_annotation(context, mocker):
+    cluster = utils.get_test_cluster(context, labels={})
+    mocker.patch(
+        "magnum_cluster_api.utils.get_default_boot_volume_size",
+        side_effect=lambda cluster, default: default,
+    )
+    cluster.labels["auto_scaling_enabled"] = "true"
+    node_group = utils.get_test_nodegroup(context, labels={})
+    node_group.min_node_count = 1
+    node_group.max_node_count = 3
+
+    mocker.patch("magnum_cluster_api.utils.lookup_image", return_value={"id": "foo"})
+    mocker.patch(
+        "magnum_cluster_api.utils.lookup_flavor",
+        return_value=flavors.Flavor(
+            None,
+            {"name": "vm-flavor", "disk": 20, "ram": 1024, "vcpus": 1},
+        ),
+    )
+
+    md = resources.mutate_machine_deployment(
+        context, cluster, node_group, {"name": node_group.name}
+    )
+
+    annotations = md["metadata"]["annotations"]
+    assert (
+        annotations["capacity.cluster-autoscaler.kubernetes.io/ephemeral-disk"] == "20"
+    )