cluster topology controller - deletion prevention

Author: shayasoolin

docs/designs/topology.md

@@ -229,7 +229,7 @@ Deletion Workflow:
 2. Kubernetes blocks deletion (finalizer `grove.io/clustertopology` present)
 3. Controller reconciles:
    - Detects deletion request (deletion timestamp set)
-   - Checks if any PodCliqueSet (in any namespace) references this ClusterTopology (via `grove.io/topology-name` label)
+   - Checks if any PodCliqueSet (in any namespace) references this ClusterTopology (via `grove.io/cluster-topology-name` label)
    - Checks if topology is enabled in operator config (`topology.enabled: true`) and this specific topology is configured (considering both the default topology name and the `topology.name` setting)
    - If ANY PodCliqueSet references this topology OR topology is enabled and set to this topology: Keeps finalizer, deletion blocked
    - If NO PodCliqueSet references this topology AND topology is disabled or not set to this topology: Removes finalizer, deletion proceeds
@@ -238,7 +238,7 @@ Deletion Workflow:
 Key Points:
 
 - Admin must satisfy BOTH conditions before deletion:
-    - Delete all PodCliqueSet resources that reference this ClusterTopology (check `grove.io/topology-name` label)
+    - Delete all PodCliqueSet resources that reference this ClusterTopology (check `grove.io/cluster-topology-name` label)
     - Disable TAS in operator config (`topology.enabled: false`) and restart operator
 - Controller checks both conditions before allowing deletion
 - Controller continuously reconciles deletion requests
@@ -280,11 +280,11 @@ topology:
 3. Admin restarts operator (see Startup Behavior section for details)
 4. Operator validates ClusterTopology CR exists
 5. For existing workloads:
-    - Workloads with topology constraints: operator uses topology from PodCliqueSet label (`grove.io/topology-name`)
+    - Workloads with topology constraints: operator uses topology from PodCliqueSet label (`grove.io/cluster-topology-name`)
     - Workloads without topology constraints: no impact (checked via label presence)
 6. For new workloads:
     - Topology constraints validated against ClusterTopology CR
-   - Mutation webhook adds `grove.io/topology-name` label to PodCliqueSet (see Mutation Webhook section)
+   - Mutation webhook adds `grove.io/cluster-topology-name` label to PodCliqueSet (see Mutation Webhook section)
 
 **Disabling Topology (topology.enabled: true → false):**
 
@@ -383,7 +383,7 @@ type PodCliqueTemplateSpec struct {
 
 The mutation webhook automatically modifies PodCliqueSet resources on CREATE operations:
 
-- **Label Addition**: Automatically adds `grove.io/topology-name` label to PodCliqueSet
+- **Label Addition**: Automatically adds `grove.io/cluster-topology-name` label to PodCliqueSet
 - **Label Value**: Set to the configured ClusterTopology name from operator config (e.g., "grove-topology")
 - **Condition**: Label only added when `topology.enabled: true` in operator configuration
 - **Purpose**: Enables workload-to-topology mapping for controller lifecycle management and deletion protection
@@ -407,7 +407,7 @@ The mutation webhook automatically modifies PodCliqueSet resources on CREATE ope
 
 **Label Immutability:**
 
-- Webhook rejects changes to `grove.io/topology-name` label on PodCliqueSet after creation
+- Webhook rejects changes to `grove.io/cluster-topology-name` label on PodCliqueSet after creation
 - Label can only be set during PodCliqueSet creation (see Mutation Webhook section above)
 - Ensures topology reference remains consistent throughout resource lifecycle
 
@@ -450,7 +450,7 @@ The operator adds topology information to PodGang metadata via annotation:
 # Annotation added to PodGang
 metadata:
   annotations:
-    grove.io/topology-name: "<user-configured-name>"
+    grove.io/cluster-topology-name: "<user-configured-name>"
 ```
 
 This annotation allows the scheduler to locate the Kueue Topology resource without requiring a spec field, providing
@@ -530,7 +530,7 @@ Fields Added:
 
 Annotations Added:
 
-- `grove.io/topology-name: "<user-configured-name>"` - Annotation on PodGang metadata referencing topology name
+- `grove.io/cluster-topology-name: "<user-configured-name>"` - Annotation on PodGang metadata referencing topology name
 
 Fields Removed:
 
@@ -544,7 +544,7 @@ The operator translates Grove operator API to Grove Scheduler API with three-lev
 
 **Topology Annotation:**
 
-- Operator adds annotation `grove.io/topology-name: "<topology-name>"` to PodGang metadata
+- Operator adds annotation `grove.io/cluster-topology-name: "<topology-name>"` to PodGang metadata
 - Annotation value matches the ClusterTopology name from operator configuration
 - KAI scheduler uses this annotation to locate the corresponding Kueue Topology CRD
 - Annotation approach provides API flexibility for future changes without breaking spec

operator/internal/controller/clustertopology/reconciledelete.go

@@ -18,6 +18,7 @@ package clustertopology
 
 import (
 	"context"
+	"fmt"
 
 	apicommon "github.com/ai-dynamo/grove/operator/api/common"
 	"github.com/ai-dynamo/grove/operator/api/common/constants"
@@ -97,11 +98,11 @@ func (r *Reconciler) removeFinalizer(ctx context.Context, logger logr.Logger, ct
 	if err := ctrlutils.RemoveAndPatchFinalizer(ctx, r.client, ct, constants.FinalizerClusterTopology); err != nil {
 		logger.Error(err, "failed to remove finalizer", "ClusterTopology", ct.Name, "finalizerName", constants.FinalizerClusterTopology)
 		r.eventRecorder.Eventf(ct, corev1.EventTypeWarning, groveconstants.ReasonClusterTopologyDeleteFailed,
-			"Cannot delete ClusterTopology %s: error removing finalizer: %s from ClusterTopology: %s: %w", constants.FinalizerClusterTopology, ct.Name, err)
-		return ctrlcommon.ReconcileWithErrors("failed to remove finalizer", err)
+			"Failed to remove finalizer %s from ClusterTopology %s: %v", constants.FinalizerClusterTopology, ct.Name, err)
+		return ctrlcommon.ReconcileWithErrors("error removing finalizer", fmt.Errorf("failed to remove finalizer %s from ClusterTopology %s: %v", constants.FinalizerClusterTopology, ct.Name, err))
 	}
 	logger.Info("Finalizer removed", "ClusterTopology", ct.Name, "finalizerName", constants.FinalizerClusterTopology)
 	r.eventRecorder.Eventf(ct, corev1.EventTypeNormal, groveconstants.ReasonClusterTopologyDeleteSuccessful,
-		"ClusterTopology %s deleted successfully", ct.Name)
+		"Successfully removed finalizer %s from ClusterTopology %s", constants.FinalizerClusterTopology, ct.Name)
 	return ctrlcommon.ContinueReconcile()
 }

operator/internal/controller/clustertopology/reconcilespec.go

@@ -56,7 +56,7 @@ func (r *Reconciler) ensureFinalizer(ctx context.Context, logger logr.Logger, ct
 		if err := ctrlutils.AddAndPatchFinalizer(ctx, r.client, ct, constants.FinalizerClusterTopology); err != nil {
 			r.eventRecorder.Eventf(ct, corev1.EventTypeWarning, groveconstants.ReasonClusterTopologyCreateOrUpdateFailed,
 				"Failed to add finalizer %s to ClusterTopology %s: %v", constants.FinalizerClusterTopology, ct.Name, err)
-			return ctrlcommon.ReconcileWithErrors("error adding finalizer", fmt.Errorf("failed to add finalizer: %s to ClusterTopology: %s: %w", constants.FinalizerClusterTopology, ct.Name, err))
+			return ctrlcommon.ReconcileWithErrors("error adding finalizer", fmt.Errorf("failed to add finalizer %s to ClusterTopology %s: %v", constants.FinalizerClusterTopology, ct.Name, err))
 		}
 		r.eventRecorder.Eventf(ct, corev1.EventTypeNormal, groveconstants.ReasonClusterTopologyCreateOrUpdateSuccessful,
 			"Successfully added finalizer %s to ClusterTopology %s", constants.FinalizerClusterTopology, ct.Name)