polish based on the comments

Author: osswangxining

pkg/controller/launcher-populator/populator.go

@@ -166,10 +166,6 @@ func (ctl *controller) OnDelete(obj any) {
 		ctl.enqueueLogger.V(5).Info("Enqueuing LauncherPopulationPolicy reference due to notification of delete", "name", typed.Name)
 		item := lppItem{cache.MetaObjectToName(typed)}
 		ctl.Queue.Add(item)
-	case *fmav1alpha1.LauncherConfig:
-		ctl.enqueueLogger.V(5).Info("Enqueuing LauncherConfig reference due to notification of delete", "name", typed.Name)
-		item := lcItem{cache.MetaObjectToName(typed)}
-		ctl.Queue.Add(item)
 	default:
 		ctl.enqueueLogger.V(5).Info("Notified of delete of type of ignored object", "type", fmt.Sprintf("%T", obj))
 		return
@@ -195,53 +191,16 @@ func (ctl *controller) process(ctx context.Context, item queueItem) (error, bool
 
 func (item lppItem) process(ctx context.Context, ctl *controller) (error, bool) {
 	logger := klog.FromContext(ctx)
-	// Get the list of LauncherPopulationPolicies
-	policies, err := ctl.lppLister.List(labels.Everything())
-	if err != nil {
-		logger.Error(err, "Failed to list LauncherPopulationPolicies")
-		return err, true // Return error and retry
-	}
-
-	// If needed, process the retrieved policies here
-	// For example: iterate through policies to perform corresponding business logic
 
-	logger.Info("Successfully listed LauncherPopulationPolicies", "count", len(policies))
-
-	// Build the PopulationPolicy map, storing the maximum count for each (Node, LauncherConfig) pair
-	populationPolicy := make(map[NodeLauncherKey]int32)
-	for _, lpp := range policies {
-		// Get matching nodes
-		nodes, err := ctl.getMatchingNodes(ctx, lpp.Spec.EnhancedNodeSelector)
-		if err != nil {
-			logger.Error(err, "Failed to get matching nodes for policy", "policy", lpp.Name)
-			return err, true
-		}
-		logger.Info("Found matching nodes", "count", len(nodes), "policy", lpp.Name)
-		// For each CountForLauncher rule
-		for _, countRule := range lpp.Spec.CountForLauncher {
-			for _, node := range nodes {
-				key := NodeLauncherKey{
-					NodeName:           node.Name,
-					LauncherConfigName: countRule.LauncherConfigName,
-				}
-				currentCount, exists := populationPolicy[key]
-				logger.Info("Current count for node", "node", node.Name, "launcherConfigName",
-					countRule.LauncherConfigName, "launcherCount", countRule.LauncherCount, "currentCount", currentCount, "exists", exists)
-
-				// Take the maximum value (rule: when multiple CountForLauncher apply to the same pair, take the maximum)
-				if !exists || countRule.LauncherCount > currentCount {
-					populationPolicy[key] = countRule.LauncherCount
-					logger.Info("Updated population policy",
-						"node", node.Name,
-						"config", countRule.LauncherConfigName,
-						"count", countRule.LauncherCount,
-						"policy", lpp.Name)
-				}
-			}
-		}
+	// Build desired state from all policies
+	populationPolicy, err := ctl.buildDesiredStateFromPolicies(ctx, nil)
+	if err != nil {
+		logger.Error(err, "Failed to build desired state from policies")
+		return err, true
 	}
 
 	logger.Info("Final population policy", "policy", MapToLoggable(populationPolicy))
+
 	// Adjust launcher pods according to final requirements
 	if err := ctl.reconcileAllLaunchers(ctx, populationPolicy); err != nil {
 		logger.Error(err, "Failed to reconcile launchers")
@@ -257,21 +216,42 @@ func (item lcItem) process(ctx context.Context, ctl *controller) (error, bool) {
 	lc, err := ctl.lcLister.LauncherConfigs(ctl.namespace).Get(item.Name)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			logger.Info("LauncherConfig no longer exists, skipping reconciliation", "name", item.Name)
+			logger.Info("LauncherConfig does not exist yet, skipping reconciliation", "name", item.Name)
 			return nil, false
 		}
 		logger.Error(err, "Failed to get LauncherConfig", "name", item.Name)
 		return err, true
 	}
 
-	// Get all LauncherPopulationPolicies that reference this LauncherConfig
-	policies, err := ctl.lppLister.List(labels.Everything())
+	// Build desired state only for policies that reference this LauncherConfig
+	populationPolicy, err := ctl.buildDesiredStateFromPolicies(ctx, &item.Name)
 	if err != nil {
-		logger.Error(err, "Failed to list LauncherPopulationPolicies")
+		logger.Error(err, "Failed to build desired state from policies")
 		return err, true
 	}
 
-	// Build desired state for this LauncherConfig across all nodes
+	logger.Info("Final population policy for LauncherConfig", "config", item.Name, "policy", MapToLoggable(populationPolicy))
+
+	// Reconcile launchers for this LauncherConfig
+	if err := ctl.reconcileAllLaunchers(ctx, populationPolicy); err != nil {
+		logger.Error(err, "Failed to reconcile launchers for LauncherConfig", "name", lc.Name)
+		return err, true
+	}
+
+	logger.Info("Successfully reconciled launchers for LauncherConfig", "name", lc.Name)
+	return nil, false
+}
+
+// buildDesiredStateFromPolicies builds the desired state map from all policies.
+// If filterByConfig is provided, only policies referencing that config are considered.
+func (ctl *controller) buildDesiredStateFromPolicies(ctx context.Context, filterByConfig *string) (map[NodeLauncherKey]int32, error) {
+	logger := klog.FromContext(ctx)
+
+	policies, err := ctl.lppLister.List(labels.Everything())
+	if err != nil {
+		return nil, fmt.Errorf("failed to list LauncherPopulationPolicies: %w", err)
+	}
+
 	desired := make(map[NodeLauncherKey]int32)
 	for _, lpp := range policies {
 		nodes, err := ctl.getMatchingNodes(ctx, lpp.Spec.EnhancedNodeSelector)
@@ -281,30 +261,22 @@ func (item lcItem) process(ctx context.Context, ctl *controller) (error, bool) {
 		}
 
 		for _, countRule := range lpp.Spec.CountForLauncher {
-			if countRule.LauncherConfigName != lc.Name {
+			if filterByConfig != nil && countRule.LauncherConfigName != *filterByConfig {
 				continue
 			}
 			for _, node := range nodes {
 				key := NodeLauncherKey{
 					NodeName:           node.Name,
-					LauncherConfigName: lc.Name,
+					LauncherConfigName: countRule.LauncherConfigName,
 				}
-				// Take the maximum count if multiple rules apply
 				if current, exists := desired[key]; !exists || countRule.LauncherCount > current {
 					desired[key] = countRule.LauncherCount
 				}
 			}
 		}
 	}
 
-	// Reconcile launchers for this LauncherConfig
-	if err := ctl.reconcileAllLaunchers(ctx, desired); err != nil {
-		logger.Error(err, "Failed to reconcile launchers for LauncherConfig", "name", lc.Name)
-		return err, true
-	}
-
-	logger.Info("Successfully reconciled launchers for LauncherConfig", "name", lc.Name)
-	return nil, false
+	return desired, nil
 }
 
 // getMatchingNodes returns nodes that match the EnhancedNodeSelector