Skip to content

Commit cd63274

Browse files
author
Arvind Thirumurugan
committed
update computeRunStageStatus function
Signed-off-by: Arvind Thirumurugan <[email protected]>
1 parent d2c342d commit cd63274

File tree

2 files changed

+77
-12
lines changed

2 files changed

+77
-12
lines changed

.github/.copilot/breadcrumbs/2025-09-25-1430-initialize-refactor.md

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,63 @@ func (r *Reconciler) collectScheduledClusters(
309309

310310
**Compatibility Note**: The `initialize` function includes temporary type conversion logic to maintain compatibility with other functions (`generateStagesByStrategy`, `recordOverrideSnapshots`) that haven't been refactored yet. This demonstrates the incremental nature of the interface-based refactoring.
311311

312+
### Phase 7: computeRunStageStatus Refactor ✅
313+
314+
**Target**: `computeRunStageStatus` function - computes cluster stages based on StagedUpdateStrategy
315+
316+
**Implementation**: Applied interface-based pattern consistently:
317+
318+
**Before**:
319+
```go
320+
func (r *Reconciler) computeRunStageStatus(
321+
ctx context.Context,
322+
scheduledBindings []*placementv1beta1.ClusterResourceBinding,
323+
updateRun *placementv1beta1.ClusterStagedUpdateRun,
324+
) error {
325+
// Direct field access
326+
updateStrategyName := updateRun.Spec.StagedUpdateStrategyName
327+
allSelectedClusters[binding.Spec.TargetCluster] = struct{}{}
328+
for _, stage := range updateRun.Status.StagedUpdateStrategySnapshot.Stages {
329+
// Direct status modification
330+
updateRun.Status.StagesStatus = stagesStatus
331+
}
332+
}
333+
```
334+
335+
**After**:
336+
```go
337+
func (r *Reconciler) computeRunStageStatus(
338+
ctx context.Context,
339+
scheduledBindings []placementv1beta1.BindingObj,
340+
updateRun placementv1beta1.StagedUpdateRunObj,
341+
) error {
342+
// Interface-based access
343+
updateRunSpec := updateRun.GetStagedUpdateRunSpec()
344+
updateRunStatus := updateRun.GetStagedUpdateRunStatus()
345+
updateStrategyName := updateRunSpec.StagedUpdateStrategyName
346+
bindingSpec := binding.GetBindingSpec()
347+
allSelectedClusters[bindingSpec.TargetCluster] = struct{}{}
348+
for _, stage := range updateRunStatus.StagedUpdateStrategySnapshot.Stages {
349+
// Interface-based status modification
350+
updateRunStatus.StagesStatus = stagesStatus
351+
updateRun.SetStagedUpdateRunStatus(*updateRunStatus)
352+
}
353+
}
354+
```
355+
356+
**Key Improvements**:
357+
1. **Generic Interface Parameters**: Uses `[]BindingObj` and `StagedUpdateRunObj` instead of concrete types
358+
2. **Interface-Based Access**: Uses `GetStagedUpdateRunSpec()`, `GetStagedUpdateRunStatus()`, `GetBindingSpec()`
359+
3. **Interface-Based Status Updates**: Uses `SetStagedUpdateRunStatus()` instead of direct field assignment
360+
4. **Type Conversion Integration**: Uses `controller.ConvertCRBArrayToBindingObjs()` in caller
361+
5. **Generic Logging**: Updated terminology from cluster-specific to generic
362+
363+
**Integration**: The `generateStagesByStrategy` function now converts concrete binding arrays to interfaces before calling `computeRunStageStatus`, maintaining compatibility while enabling interface-based processing.
364+
312365
## Current Status
313366
-`validatePlacement`: Fully generic, uses utility functions and interfaces
314367
-`determinePolicySnapshot`: Fully generic, uses utility functions and interfaces, unnecessary switch removed
315368
-`collectScheduledClusters`: Fully generic, uses utility functions and interfaces
316-
- ⚠️ `initialize`: Still takes concrete ClusterStagedUpdateRun type, but now calls interface-based functions
369+
-`computeRunStageStatus`: Fully generic, uses interface-based access patterns
370+
- ⚠️ `initialize`: Still takes concrete ClusterStagedUpdateRun type, but now calls interface-based functions
371+
- ⚠️ `generateStagesByStrategy`: Still takes concrete types but converts to interfaces for `computeRunStageStatus`

pkg/controllers/updaterun/initialization.go

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,9 @@ func (r *Reconciler) generateStagesByStrategy(
322322
removeWaitTimeFromUpdateRunStatus(updateRun)
323323

324324
// Compute the update stages.
325-
if err := r.computeRunStageStatus(ctx, scheduledBindings, updateRun); err != nil {
325+
// Convert concrete binding arrays to interface arrays
326+
scheduledBindingObjs := controller.ConvertCRBArrayToBindingObjs(scheduledBindings)
327+
if err := r.computeRunStageStatus(ctx, scheduledBindingObjs, updateRun); err != nil {
326328
return err
327329
}
328330
toBeDeletedClusters := make([]placementv1beta1.ClusterUpdatingStatus, len(toBeDeletedBindings))
@@ -341,25 +343,30 @@ func (r *Reconciler) generateStagesByStrategy(
341343
return nil
342344
}
343345

344-
// computeRunStageStatus computes the stages based on the ClusterStagedUpdateStrategy and records them in the ClusterStagedUpdateRun status.
346+
// computeRunStageStatus computes the stages based on the StagedUpdateStrategy and records them in the StagedUpdateRun status.
345347
func (r *Reconciler) computeRunStageStatus(
346348
ctx context.Context,
347-
scheduledBindings []*placementv1beta1.ClusterResourceBinding,
348-
updateRun *placementv1beta1.ClusterStagedUpdateRun,
349+
scheduledBindings []placementv1beta1.BindingObj,
350+
updateRun placementv1beta1.StagedUpdateRunObj,
349351
) error {
350352
updateRunRef := klog.KObj(updateRun)
351-
updateStrategyName := updateRun.Spec.StagedUpdateStrategyName
353+
354+
// Use interface methods to access updateRun data
355+
updateRunSpec := updateRun.GetStagedUpdateRunSpec()
356+
updateRunStatus := updateRun.GetStagedUpdateRunStatus()
357+
updateStrategyName := updateRunSpec.StagedUpdateStrategyName
352358

353359
// Map to track clusters and ensure they appear in one and only one stage.
354360
allSelectedClusters := make(map[string]struct{}, len(scheduledBindings))
355361
allPlacedClusters := make(map[string]struct{})
356362
for _, binding := range scheduledBindings {
357-
allSelectedClusters[binding.Spec.TargetCluster] = struct{}{}
363+
bindingSpec := binding.GetBindingSpec()
364+
allSelectedClusters[bindingSpec.TargetCluster] = struct{}{}
358365
}
359-
stagesStatus := make([]placementv1beta1.StageUpdatingStatus, 0, len(updateRun.Status.StagedUpdateStrategySnapshot.Stages))
366+
stagesStatus := make([]placementv1beta1.StageUpdatingStatus, 0, len(updateRunStatus.StagedUpdateStrategySnapshot.Stages))
360367

361-
// Apply the label selectors from the ClusterStagedUpdateStrategy to filter the clusters.
362-
for _, stage := range updateRun.Status.StagedUpdateStrategySnapshot.Stages {
368+
// Apply the label selectors from the StagedUpdateStrategy to filter the clusters.
369+
for _, stage := range updateRunStatus.StagedUpdateStrategySnapshot.Stages {
363370
if err := validateAfterStageTask(stage.AfterStageTasks); err != nil {
364371
klog.ErrorS(err, "Failed to validate the after stage tasks", "clusterStagedUpdateStrategy", updateStrategyName, "stage name", stage.Name, "clusterStagedUpdateRun", updateRunRef)
365372
// no more retries here.
@@ -439,12 +446,15 @@ func (r *Reconciler) computeRunStageStatus(
439446
for i, task := range stage.AfterStageTasks {
440447
curStageUpdatingStatus.AfterStageTaskStatus[i].Type = task.Type
441448
if task.Type == placementv1beta1.AfterStageTaskTypeApproval {
442-
curStageUpdatingStatus.AfterStageTaskStatus[i].ApprovalRequestName = fmt.Sprintf(placementv1beta1.ApprovalTaskNameFmt, updateRun.Name, stage.Name)
449+
curStageUpdatingStatus.AfterStageTaskStatus[i].ApprovalRequestName = fmt.Sprintf(placementv1beta1.ApprovalTaskNameFmt, updateRun.GetName(), stage.Name)
443450
}
444451
}
445452
stagesStatus = append(stagesStatus, curStageUpdatingStatus)
446453
}
447-
updateRun.Status.StagesStatus = stagesStatus
454+
455+
// Update the stages status using interface methods
456+
updateRunStatus.StagesStatus = stagesStatus
457+
updateRun.SetStagedUpdateRunStatus(*updateRunStatus)
448458

449459
// Check if the clusters are all placed.
450460
if len(allPlacedClusters) != len(allSelectedClusters) {

0 commit comments

Comments
 (0)