datum-cloud · scotwells · Sep 25, 2026
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"sort"
 	"strings"
-	"unicode"
 
 	"github.com/spf13/cobra"
 	corev1 "k8s.io/api/core/v1"
@@ -166,13 +165,7 @@ func runList(cmd *cobra.Command, opts *listOptions) error {
 			wlName = labelWLName
 		} else {
 			// At least one label absent — fall back to WorkloadDeployment lookup.
-			// Prefer the explicit WorkloadDeploymentNameLabel; fall back to
-			// deriving the WD name from the Instance name for existing instances
-			// that predate the label.
-			depName := inst.Labels[computev1alpha.WorkloadDeploymentNameLabel]
-			if depName == "" {
-				depName = wdNameFromInstanceName(inst.Name)
-			}
+			depName := deploymentNameForInstance(&inst)
 			if dep, ok := deploymentMap[depName]; ok {
 				location = dep.Spec.LocationRef.Name
 				if labelWLName != "" {
@@ -325,12 +318,7 @@ func runDescribe(cmd *cobra.Command, args []string) error {
 		placementName = labelPlacement
 	} else {
 		// At least one label absent — fall back to WorkloadDeployment Get.
-		// Prefer the WorkloadDeploymentNameLabel; fall back to deriving the WD
-		// name from the Instance name for existing instances that lack the label.
-		depName := inst.Labels[computev1alpha.WorkloadDeploymentNameLabel]
-		if depName == "" {
-			depName = wdNameFromInstanceName(inst.Name)
-		}
+		depName := deploymentNameForInstance(&inst)
 		if depName != "" {
 			var dep computev1alpha.WorkloadDeployment
 			if err := c.Get(ctx, types.NamespacedName{Namespace: util.ResourceNamespace, Name: depName}, &dep); err == nil {
@@ -450,30 +438,20 @@ func networkSummary(ifaces []computev1alpha.InstanceNetworkInterfaceStatus) stri
 	return fmt.Sprintf("External: %s  Internal: %s", extIP, intIP)
 }
 
-// wdNameFromInstanceName derives the WorkloadDeployment name from an Instance
-// name by stripping the trailing "-<ordinal>" suffix. Instance names follow the
-// convention "<wd-name>-<ordinal>" (e.g. "my-api-default-dfw-0" → "my-api-default-dfw").
-// This is used as a fallback when WorkloadDeploymentNameLabel is absent on older
-// instances that predate that label.
-//
-// If the name has no trailing numeric segment (not a standard instance name),
-// the original name is returned unchanged so callers can handle it gracefully.
-func wdNameFromInstanceName(instanceName string) string {
-	idx := strings.LastIndex(instanceName, "-")
-	if idx < 0 {
-		return instanceName
+// deploymentNameForInstance returns the name of the WorkloadDeployment that
+// owns the instance, read from WorkloadDeploymentNameLabel or, failing that,
+// the instance's WorkloadDeployment owner reference. Returns "" when neither is
+// set.
+func deploymentNameForInstance(inst *computev1alpha.Instance) string {
+	if name := inst.Labels[computev1alpha.WorkloadDeploymentNameLabel]; name != "" {
+		return name
 	}
-	suffix := instanceName[idx+1:]
-	// The suffix must be entirely numeric digits to qualify as an ordinal.
-	for _, r := range suffix {
-		if !unicode.IsDigit(r) {
-			return instanceName
+	for _, owner := range inst.OwnerReferences {
+		if owner.Kind == "WorkloadDeployment" && owner.APIVersion == computev1alpha.GroupVersion.String() {
+			return owner.Name
 		}
 	}
-	if suffix == "" {
-		return instanceName
-	}
-	return instanceName[:idx]
+	return ""
 }
 
 // formatEnvVar renders a single EnvVar for display.

@@ -0,0 +1,60 @@
+package instances
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	computev1alpha "go.datum.net/compute/api/v1alpha"
+)
+
+func TestDeploymentNameForInstance(t *testing.T) {
+	const deploymentName = "checkout-api-7c1e04b9a2"
+
+	ownerRef := func(apiVersion, kind, name string) metav1.OwnerReference {
+		return metav1.OwnerReference{APIVersion: apiVersion, Kind: kind, Name: name}
+	}
+
+	tests := []struct {
+		name   string
+		labels map[string]string
+		owners []metav1.OwnerReference
+		want   string
+	}{
+		{
+			name:   "label",
+			labels: map[string]string{computev1alpha.WorkloadDeploymentNameLabel: deploymentName},
+			owners: []metav1.OwnerReference{ownerRef(computev1alpha.GroupVersion.String(), "WorkloadDeployment", "other")},
+			want:   deploymentName,
+		},
+		{
+			name:   "owner reference",
+			owners: []metav1.OwnerReference{ownerRef(computev1alpha.GroupVersion.String(), "WorkloadDeployment", deploymentName)},
+			want:   deploymentName,
+		},
+		{
+			name:   "owner of another kind",
+			owners: []metav1.OwnerReference{ownerRef("apps/v1", "WorkloadDeployment", deploymentName)},
+			want:   "",
+		},
+		{
+			name: "name is never parsed",
+			want: "",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			inst := &computev1alpha.Instance{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            deploymentName + "-0",
+					Labels:          test.labels,
+					OwnerReferences: test.owners,
+				},
+			}
+			if got := deploymentNameForInstance(inst); got != test.want {
+				t.Errorf("deploymentNameForInstance() = %q, want %q", got, test.want)
+			}
+		})
+	}
+}
@@ -3,6 +3,7 @@ package stateful
 import (
 	"context"
 	"fmt"
+	"maps"
 	"slices"
 	"strconv"
 
@@ -74,15 +75,24 @@ func (c *statefulControl) GetActions(
 	// highest -> lowest
 	var deleteActions []instancecontrol.Action
 
+	// Instances that cannot hold a slot. Deleted before anything else so a
+	// replacement can take the name.
+	var strayActions []instancecontrol.Action
+
 	// Instances that are desired to exist. We do not currently support the
 	// concept of a partition, so will fill the entire slice.
 	desiredInstances := make([]*v1alpha.Instance, desiredReplicas)
 
 	for _, instance := range currentInstances {
-		instanceIndex := getInstanceOrdinal(instance.Name)
-		if instanceIndex >= len(desiredInstances) {
+		instanceIndex := getInstanceOrdinal(&instance)
+		switch {
+		case instanceIndex < 0:
+			strayActions = append(strayActions, instancecontrol.NewDeleteAction(&instance))
+		case instanceIndex >= len(desiredInstances):
 			deleteActions = append(deleteActions, instancecontrol.NewDeleteAction(&instance))
-		} else {
+		case desiredInstances[instanceIndex] != nil:
+			strayActions = append(strayActions, instancecontrol.NewDeleteAction(&instance))
+		default:
 			desiredInstances[instanceIndex] = &instance
 		}
 	}
@@ -93,7 +103,7 @@ func (c *statefulControl) GetActions(
 		if desiredInstances[i] == nil {
 			desiredInstances[i] = &v1alpha.Instance{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels:      deployment.Spec.Template.Labels,
+					Labels:      maps.Clone(deployment.Spec.Template.Labels),
 					Annotations: deployment.Spec.Template.Annotations,
 					Name:        fmt.Sprintf("%s-%d", deployment.Name, i),
 					Namespace:   deployment.Namespace,
@@ -128,7 +138,7 @@ func (c *statefulControl) GetActions(
 				SchedulingGates: gates,
 			}
 
-			addInstanceControllerLabels(desiredInstances[i], getInstanceOrdinal(desiredInstances[i].Name), deployment)
+			addInstanceControllerLabels(desiredInstances[i], int(i), deployment)
 
 			if err := controllerutil.SetControllerReference(deployment, desiredInstances[i], scheme); err != nil {
 				return nil, fmt.Errorf("failed to set controller reference: %w", err)
@@ -177,13 +187,13 @@ func (c *statefulControl) GetActions(
 	// and is emitted outside the ordered rollout decision so it never gates or
 	// reorders instance creation/updates.
 	var patchLabelActions []instancecontrol.Action
-	for _, instance := range desiredInstances {
+	for i, instance := range desiredInstances {
 		if instance.CreationTimestamp.IsZero() || !instance.DeletionTimestamp.IsZero() {
 			// Skip instances that don't exist yet or are being deleted.
 			continue
 		}
 
-		desiredLabels := desiredControllerLabels(getInstanceOrdinal(instance.Name), deployment)
+		desiredLabels := desiredControllerLabels(i, deployment)
 		if labelsNeedBackfill(instance.Labels, desiredLabels) {
 			base := instance.DeepCopy()
 			patched := instance.DeepCopy()
@@ -200,7 +210,7 @@ func (c *statefulControl) GetActions(
 	slices.SortFunc(recreateActions, descendingOrdinal)
 	slices.SortFunc(deleteActions, descendingOrdinal)
 
-	actions := make([]instancecontrol.Action, 0, len(createActions)+len(waitActions)+len(recreateActions)+len(deleteActions)+len(patchLabelActions))
+	actions := make([]instancecontrol.Action, 0, len(strayActions)+len(createActions)+len(waitActions)+len(recreateActions)+len(deleteActions)+len(patchLabelActions))
 
 	switch deployment.Spec.ScaleSettings.InstanceManagementPolicy {
 	case v1alpha.OrderedReadyInstanceManagementPolicyType:
@@ -210,11 +220,11 @@ func (c *statefulControl) GetActions(
 		//
 		// For instance, we may have instance 0 that needs to wait to be ready, but
 		// instance 1 wants to be created.
-		actions = append(actions, createActions...)
-		actions = append(actions, waitActions...)
-
-		slices.SortFunc(actions, ascendingOrdinal)
+		ordered := slices.Concat(createActions, waitActions)
+		slices.SortFunc(ordered, ascendingOrdinal)
 
+		actions = append(actions, strayActions...)
+		actions = append(actions, ordered...)
 		actions = append(actions, recreateActions...)
 		actions = append(actions, deleteActions...)
 

@@ -710,3 +710,100 @@ func getInstanceTemplate(name string, ordinal int) *v1alpha.Instance {
 
 	return instance
 }
+
+// TestOrdinal_ReadFromLabelNotName verifies that an existing instance fills the
+// slot recorded in its ordinal label, whatever its name ends with.
+func TestOrdinal_ReadFromLabelNotName(t *testing.T) {
+	ctx := context.Background()
+	control := NewWithOptions(Options{})
+
+	deployment := getWorkloadDeployment("test-ordinal-label", 2)
+
+	instance := getInstanceForDeployment(deployment, 1)
+	instance.Name = "test-ordinal-label-renamed"
+
+	actions, err := control.GetActions(ctx, scheme, deployment, deployment.Spec.ScaleSettings.MinReplicas, []v1alpha.Instance{*instance})
+	require.NoError(t, err)
+
+	for _, a := range actions {
+		assert.NotEqual(t, instancecontrol.ActionTypeDelete, a.ActionType(),
+			"an instance with a valid ordinal label must keep its slot")
+	}
+	require.Len(t, actions, 1)
+	assert.Equal(t, instancecontrol.ActionTypeCreate, actions[0].ActionType())
+	assert.Equal(t, "test-ordinal-label-0", actions[0].Object.GetName())
+}
+
+// TestOrdinal_InvalidLabelDeleted verifies that an instance without a usable
+// ordinal label is deleted instead of being placed by its name.
+func TestOrdinal_InvalidLabelDeleted(t *testing.T) {
+	for _, value := range []string{"", "foo", "-1"} {
+		t.Run(fmt.Sprintf("label=%q", value), func(t *testing.T) {
+			ctx := context.Background()
+			control := NewWithOptions(Options{})
+
+			deployment := getWorkloadDeployment("test-ordinal-invalid", 1)
+
+			instance := getInstanceForDeployment(deployment, 0)
+			if value == "" {
+				delete(instance.Labels, v1alpha.InstanceIndexLabel)
+			} else {
+				instance.Labels[v1alpha.InstanceIndexLabel] = value
+			}
+
+			actions, err := control.GetActions(ctx, scheme, deployment, deployment.Spec.ScaleSettings.MinReplicas, []v1alpha.Instance{*instance})
+			require.NoError(t, err)
+
+			// The delete must run first: the replacement reuses the same name.
+			require.Len(t, actions, 2)
+			assert.Equal(t, instancecontrol.ActionTypeDelete, actions[0].ActionType())
+			assert.Equal(t, "test-ordinal-invalid-0", actions[0].Object.GetName())
+			assert.False(t, actions[0].IsSkipped())
+			assert.Equal(t, instancecontrol.ActionTypeCreate, actions[1].ActionType())
+			assert.Equal(t, "test-ordinal-invalid-0", actions[1].Object.GetName())
+			assert.True(t, actions[1].IsSkipped())
+		})
+	}
+}
+
+// TestOrdinal_DuplicateLabelDeleted verifies that when two instances claim the
+// same ordinal, only one keeps the slot and the other is deleted.
+func TestOrdinal_DuplicateLabelDeleted(t *testing.T) {
+	ctx := context.Background()
+	control := NewWithOptions(Options{})
+
+	deployment := getWorkloadDeployment("test-ordinal-duplicate", 1)
+
+	first := getInstanceForDeployment(deployment, 0)
+	second := getInstanceForDeployment(deployment, 0)
+	second.Name = "test-ordinal-duplicate-other"
+
+	actions, err := control.GetActions(ctx, scheme, deployment, deployment.Spec.ScaleSettings.MinReplicas, []v1alpha.Instance{*first, *second})
+	require.NoError(t, err)
+
+	require.Len(t, actions, 1)
+	assert.Equal(t, instancecontrol.ActionTypeDelete, actions[0].ActionType())
+	assert.Equal(t, "test-ordinal-duplicate-other", actions[0].Object.GetName())
+	assert.False(t, actions[0].IsSkipped())
+}
+
+// TestOrdinal_NewInstancesDoNotShareTemplateLabels verifies that each new
+// instance gets its own label map, so the ordinal stamped on one instance does
+// not leak onto another or onto the deployment template.
+func TestOrdinal_NewInstancesDoNotShareTemplateLabels(t *testing.T) {
+	ctx := context.Background()
+	control := NewWithOptions(Options{})
+
+	deployment := getWorkloadDeployment("test-ordinal-template", 3)
+	deployment.Spec.Template.Labels = map[string]string{"app": "checkout"}
+
+	actions, err := control.GetActions(ctx, scheme, deployment, deployment.Spec.ScaleSettings.MinReplicas, nil)
+	require.NoError(t, err)
+	require.Len(t, actions, 3)
+
+	for i, a := range actions {
+		assert.Equal(t, strconv.Itoa(i), a.Object.GetLabels()[v1alpha.InstanceIndexLabel])
+		assert.Equal(t, "checkout", a.Object.GetLabels()["app"])
+	}
+	assert.Equal(t, map[string]string{"app": "checkout"}, deployment.Spec.Template.Labels)
+}
@@ -2,7 +2,8 @@ package stateful
 
 import (
 	"strconv"
-	"strings"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"go.datum.net/compute/api/v1alpha"
 	"go.datum.net/compute/internal/controller/instancecontrol"
@@ -13,32 +14,33 @@ func needsUpdate(instance *v1alpha.Instance, instanceTemplateHash string) bool {
 		instance.Spec.Controller.TemplateHash != instanceTemplateHash
 }
 
-// getInstanceOrdinal returns the ordinal of the instance, or -1 if the instance
-// does not have an ordinal.
-func getInstanceOrdinal(name string) int {
-	lastDash := strings.LastIndex(name, "-")
-	if lastDash == -1 {
+// getInstanceOrdinal returns the ordinal recorded in the instance's
+// InstanceIndexLabel, or -1 if the label is absent or not a non-negative
+// integer.
+func getInstanceOrdinal(obj metav1.Object) int {
+	value, ok := obj.GetLabels()[v1alpha.InstanceIndexLabel]
+	if !ok {
 		return -1
 	}
 
-	ordinal := -1
-	if i, err := strconv.Atoi(name[lastDash+1:]); err == nil {
-		ordinal = i
+	ordinal, err := strconv.Atoi(value)
+	if err != nil || ordinal < 0 {
+		return -1
 	}
 
 	return ordinal
 }
 
 func ascendingOrdinal(a, b instancecontrol.Action) int {
-	if getInstanceOrdinal(a.Object.GetName()) < getInstanceOrdinal(b.Object.GetName()) {
+	if getInstanceOrdinal(a.Object) < getInstanceOrdinal(b.Object) {
 		return -1
 	} else {
 		return 1
 	}
 }
 
 func descendingOrdinal(a, b instancecontrol.Action) int {
-	if getInstanceOrdinal(a.Object.GetName()) > getInstanceOrdinal(b.Object.GetName()) {
+	if getInstanceOrdinal(a.Object) > getInstanceOrdinal(b.Object) {
 		return -1
 	} else {
 		return 1