Skip to content

Commit 7ff2b44

Browse files
author
Ryan Zhang
committed
test
1 parent 3e62d30 commit 7ff2b44

File tree

5 files changed

+361
-135
lines changed

5 files changed

+361
-135
lines changed

‎.github/.copilot/breadcrumbs/2025-07-17-1200-clusterresourceplacement-controller-interface-refactor.md‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
# ClusterResourcePlacement Controller Interface Refactor
2+
3+
## Requirements
4+
5+
Refactor the ClusterResourcePlacement controller (`pkg/controllers/clusterresourceplacement/controller.go`) to use interface objects instead of concrete types like ClusterResourcePlacement. Instead, use PlacementObj. The controller should use interface types like BindingObj, ResourceSnapshotObj, and PolicySnapshotObj throughout instead of concrete implementations.
6+
7+
**Specific Requirements**:
8+
- Replace all function signatures that use `*fleetv1beta1.ClusterResourcePlacement` with `fleetv1beta1.PlacementObj`
9+
- Replace all concrete types like `*fleetv1beta1.ClusterResourceBinding`, `*fleetv1beta1.ClusterResourceSnapshot`, `*fleetv1beta1.ClusterSchedulingPolicySnapshot` with their respective interface types: `BindingObj`, `ResourceSnapshotObj`, `PolicySnapshotObj`
10+
- Use helper functions defined in `pkg/utils/controller` when need to get the concrete type from the clusters
11+
- Update integration and unit tests to work with interface types
12+
- Follow the patterns established in previous breadcrumb refactoring work, particularly `2025-06-24-0900-rollout-controller-binding-interface-refactor.md`
13+
14+
## Additional comments from user
15+
16+
This follows the breadcrumb protocol and builds on previous successful interface refactoring work. The goal is to make the controller work with both cluster-scoped and namespace-scoped placement objects through unified interfaces.
17+
18+
## Plan
19+
20+
### Phase 1: Analysis and Preparation
21+
- [x] Task 1.1: Analyze current concrete type usage patterns in the controller.go
22+
- [x] Task 1.2: Identify all functions that need signature updates for PlacementObj interface
23+
- [x] Task 1.3: Review available interface methods (PlacementObj, BindingObj, ResourceSnapshotObj, PolicySnapshotObj)
24+
- [x] Task 1.4: Examine helper functions in pkg/utils/controller for fetching concrete types
25+
- [x] Task 1.5: Identify test files that will need updates
26+
27+
### Phase 2: Core Reconciler Function Updates
28+
- [x] Task 2.1: Update `Reconcile` function to use PlacementObj interface ✅
29+
- [x] Task 2.2: Update `handleDelete` function signature to use PlacementObj interface ✅
30+
- [x] Task 2.3: Update `handleUpdate` function signature to use PlacementObj interface ✅
31+
- [x] Task 2.4: Update placement fetch logic to use helper functions from pkg/utils/controller ✅
32+
33+
### Functions Updated So Far ✅
34+
- `Reconcile()` - Now uses `FetchPlacementFromKey()` and works with `PlacementObj`
35+
- `handleDelete()` - Updated to use `PlacementObj` interface
36+
- `deleteClusterSchedulingPolicySnapshots()` - Updated to use `PlacementObj` interface
37+
- `deleteClusterResourceSnapshots()` - Updated to use `PlacementObj` interface
38+
- `selectResourcesForPlacement()` - Updated to use `PlacementObj` interface
39+
- `emitPlacementStatusMetric()` - Updated to use `PlacementObj` interface
40+
- `determineExpectedCRPAndResourcePlacementStatusCondType()` - Updated to use `PlacementObj` interface
41+
- `handleUpdate()` - COMPLETED ✅ Updated to use `PlacementObj` interface with type assertions for snapshot management
42+
- `getOrCreateClusterSchedulingPolicySnapshot()` - Updated to accept `PlacementObj` interface
43+
- `setPlacementStatus()` - Updated to use `PlacementObj` interface
44+
45+
### Phase 3: Update Remaining Utility Functions ✅
46+
- [x] Task 3.1: Update `isRolloutCompleted()` function to use PlacementObj interface ✅
47+
- [x] Task 3.2: Update `isCRPScheduled()` function to use PlacementObj interface ✅
48+
- [x] Task 3.3: Update `buildScheduledCondition()` function to use PlacementObj interface ✅
49+
- [x] Task 3.4: Update `buildResourcePlacementStatusMap()` function to use PlacementObj interface ✅
50+
- [x] Task 3.5: Update `calculateFailedToScheduleClusterCount()` function to use PlacementObj interface ✅
51+
52+
### Additional Functions Updated in Phase 3 ✅
53+
- `isRolloutCompleted()` - Now uses `PlacementObj` interface with `GetCondition()` and `GetGeneration()`
54+
- `isCRPScheduled()` - Now uses `PlacementObj` interface with `GetCondition()` and `GetGeneration()`
55+
- `buildScheduledCondition()` - Now uses `PlacementObj` interface with `GetGeneration()`
56+
- `buildResourcePlacementStatusMap()` - Now uses `PlacementObj` interface with `GetPlacementStatus()`
57+
- `calculateFailedToScheduleClusterCount()` - Now uses `PlacementObj` interface with `GetPlacementSpec()`
58+
59+
### Interface Pattern Successfully Established 🎯
60+
✅ **No Type Assertions Needed**: All functions now work with `PlacementObj` interface
61+
✅ **Automatic Interface Satisfaction**: Concrete types (`*ClusterResourcePlacement`) automatically satisfy the interface
62+
✅ **Clean Separation**: Interface methods (`GetPlacementSpec()`, `GetPlacementStatus()`, `GetCondition()`, `GetGeneration()`) used throughout
63+
✅ **Future Ready**: Ready to support namespace-scoped placements when they implement the same interface
64+
65+
### Current Compilation Status ✅
66+
- controller.go: ✅ No errors
67+
- placement_status.go: ✅ No errors
68+
- resource_selector.go: ✅ No errors (already updated in Phase 2)
69+
70+
### Phase 3: Snapshot Management Function Updates
71+
- [ ] Task 3.1: Update `getOrCreateClusterSchedulingPolicySnapshot` to return PolicySnapshotObj
72+
- [ ] Task 3.2: Update `getOrCreateClusterResourceSnapshot` to return ResourceSnapshotObj
73+
- [ ] Task 3.3: Update all snapshot listing and deletion functions to use interface types
74+
- [ ] Task 3.4: Update snapshot creation and management helper functions
75+
76+
### Phase 4: Status and Condition Management Updates
77+
- [ ] Task 4.1: Update `setPlacementStatus` function to use PlacementObj interface
78+
- [ ] Task 4.2: Update condition building functions to use interface types
79+
- [ ] Task 4.3: Update resource placement status calculation functions
80+
81+
### Phase 5: Builder and Utility Function Updates
82+
- [ ] Task 5.1: Update snapshot builder functions to return interface types
83+
- [ ] Task 5.2: Update parsing and utility functions to work with interface types
84+
- [ ] Task 5.3: Update all logging references to use interface objects
85+
86+
### Phase 6: Test Updates
87+
- [ ] Task 6.1: Update unit tests in the same package to use interface types
88+
- [ ] Task 6.2: Update integration tests to use interface types
89+
- [ ] Task 6.3: Run tests to ensure all interface usage is correct
90+
91+
### Phase 7: Interface Method Integration
92+
- [ ] Task 7.1: Replace direct field access with interface methods where applicable
93+
- [ ] Task 7.2: Ensure proper use of `GetPlacementSpec()`, `SetPlacementSpec()` methods
94+
- [ ] Task 7.3: Verify all interface implementations are working correctly
95+
96+
## Analysis
97+
98+
### Current State Analysis ✅
99+
100+
Based on the grep search, identified 20+ functions that need refactoring:
101+
102+
**Main reconciler functions**:
103+
- `handleDelete(ctx context.Context, crp *fleetv1beta1.ClusterResourcePlacement)`
104+
- `handleUpdate(ctx context.Context, crp *fleetv1beta1.ClusterResourcePlacement)`
105+
106+
**Snapshot management functions**:
107+
- `getOrCreateClusterSchedulingPolicySnapshot(ctx context.Context, crp *fleetv1beta1.ClusterResourcePlacement, revisionHistoryLimit int) (*fleetv1beta1.ClusterSchedulingPolicySnapshot, error)`
108+
- `getOrCreateClusterResourceSnapshot(ctx context.Context, crp *fleetv1beta1.ClusterResourcePlacement, envelopeObjCount int, resourceSnapshotSpec *fleetv1beta1.ResourceSnapshotSpec, revisionHistoryLimit int) (ctrl.Result, *fleetv1beta1.ClusterResourceSnapshot, error)`
109+
110+
**Builder functions**:
111+
- `buildMasterClusterResourceSnapshot(...) *fleetv1beta1.ClusterResourceSnapshot`
112+
- `buildSubIndexResourceSnapshot(...) *fleetv1beta1.ClusterResourceSnapshot`
113+
114+
**Status and condition functions**:
115+
- `buildScheduledCondition(crp *fleetv1beta1.ClusterResourcePlacement, latestSchedulingPolicySnapshot *fleetv1beta1.ClusterSchedulingPolicySnapshot) metav1.Condition`
116+
- `buildResourcePlacementStatusMap(crp *fleetv1beta1.ClusterResourcePlacement) map[string][]metav1.Condition`
117+
118+
### Interface Patterns Available ✅
119+
120+
**PlacementObj interface** provides:
121+
- `GetPlacementSpec() *ResourcePlacementSpec`
122+
- `SetPlacementSpec(*ResourcePlacementSpec)`
123+
- `GetPlacementStatus() *ResourcePlacementStatus`
124+
- `SetPlacementStatus(*ResourcePlacementStatus)`
125+
126+
**ResourceSnapshotObj interface** provides:
127+
- `GetResourceSnapshotSpec() *ResourceSnapshotSpec`
128+
- `SetResourceSnapshotSpec(*ResourceSnapshotSpec)`
129+
- `GetResourceSnapshotStatus() *ResourceSnapshotStatus`
130+
- `SetResourceSnapshotStatus(*ResourceSnapshotStatus)`
131+
132+
**PolicySnapshotObj interface** provides:
133+
- `GetPolicySnapshotSpec() *SchedulingPolicySnapshotSpec`
134+
- `SetPolicySnapshotSpec(*SchedulingPolicySnapshotSpec)`
135+
- `GetPolicySnapshotStatus() *SchedulingPolicySnapshotStatus`
136+
- `SetPolicySnapshotStatus(*SchedulingPolicySnapshotStatus)`
137+
138+
### Helper Functions Available ✅
139+
140+
From `pkg/utils/controller`:
141+
- `FetchPlacementFromKey(ctx context.Context, c client.Reader, placementKey queue.PlacementKey) (fleetv1beta1.PlacementObj, error)`
142+
- `FetchLatestMasterResourceSnapshot(ctx context.Context, k8Client client.Reader, placementKey types.NamespacedName) (fleetv1beta1.ResourceSnapshotObj, error)`
143+
- Various resolver and conversion helper functions
144+
145+
## Decisions
146+
147+
### Refactoring Strategy
148+
1. Follow the exact same pattern as the rollout controller refactor in breadcrumb `2025-06-24-0900-rollout-controller-binding-interface-refactor.md`
149+
2. Replace concrete types with interfaces in function signatures
150+
3. Use interface methods instead of direct field access
151+
4. Keep type assertions only at system boundaries (controller-runtime event handling)
152+
5. Use helper functions from `pkg/utils/controller` for fetching concrete types when needed
153+
154+
### Implementation Order
155+
1. Start with the main reconciler entry point and work downward
156+
2. Update function signatures first, then implement interface method usage
157+
3. Handle tests after core functionality is complete
158+
4. Follow the principle of minimal changes - avoid large complex refactoring
159+
160+
## Implementation Status
161+
162+
### Analysis Complete ✅
163+
- Identified all functions requiring refactoring
164+
- Confirmed available interface types and methods
165+
- Located helper functions for type conversion
166+
- Planned implementation strategy based on successful previous patterns
167+
168+
### Key Understanding ✅
169+
170+
**Current Controller Logic**:
171+
1. `Reconcile()` function assumes key is a simple string (cluster-scoped name only)
172+
2. Directly fetches `ClusterResourcePlacement` using `types.NamespacedName{Name: name}`
173+
3. Uses concrete types throughout all functions (`*fleetv1beta1.ClusterResourcePlacement`, `*fleetv1beta1.ClusterResourceSnapshot`, etc.)
174+
4. All business logic operates on concrete types
175+
176+
**Target Interface Pattern**:
177+
1. `Reconcile()` should accept placement keys that include namespace/name format
178+
2. Use `FetchPlacementFromKey()` helper to get appropriate placement type (cluster-scoped or namespaced)
179+
3. Work with `PlacementObj` interface throughout business logic
180+
4. Use interface methods like `GetPlacementSpec()`, `SetPlacementSpec()` instead of direct field access
181+
5. Keep type assertions only at system boundaries (controller-runtime interactions)
182+
183+
**Critical Functions That Need Interface Refactoring**:
184+
- `Reconcile()` - entry point, needs to use placement resolver
185+
- `handleDelete()` / `handleUpdate()` - core business logic, use PlacementObj
186+
- `selectResourcesForPlacement()` - resource selection, use PlacementObj
187+
- `getOrCreateClusterSchedulingPolicySnapshot()` - return PolicySnapshotObj
188+
- `getOrCreateClusterResourceSnapshot()` - return ResourceSnapshotObj
189+
- `setPlacementStatus()` - status management, use PlacementObj + interface snapshots

‎CLAUDE.md‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,19 @@ make clean-e2e-tests
6969
## Architecture Overview
7070

7171
### Core API Types
72-
- **ClusterResourcePlacement (CRP)**: Main API for placing resources across clusters with scheduling policies
73-
- **MemberCluster**: Represents a member cluster with identity and heartbeat settings
74-
- **ClusterResourceBinding**: Represents scheduling decisions binding resources to clusters
72+
- **ClusterResourcePlacement (CRP)**: Main API for placing cluster scoped resources across clusters with scheduling policies. If one namespace is selected, we will place everything in that namespace across clusters.
73+
- **ResourcePlacement (RP)**: Main API for placing namespaced resources across clusters with scheduling policies.
74+
- **MemberCluster**: Represents a member cluster with identity and heartbeat settings
75+
- **ClusterResourceBinding**: Represents scheduling decisions binding cluster scoped resources to clusters
76+
- **ResourceBinding**: Represents scheduling decisions binding resources to clusters
77+
- **ClusterResourceSnapshot**: Immutable snapshot of cluster resource state for rollback and history
78+
- **ResourceSnapshot**: Immutable snapshot of namespaced resource state for rollback and history
7579
- **Work**: Contains manifests to be applied on member clusters
7680

7781
### Key Controllers
7882
- **ClusterResourcePlacement Controller** (`pkg/controllers/clusterresourceplacement/`): Manages CRP lifecycle
7983
- **Scheduler** (`pkg/scheduler/`): Makes placement decisions using pluggable framework
84+
- **Rollout Controller** (`pkg/controllers/rollout/`): Manages rollout the changes to all the clusters that a placement decision has been made for
8085
- **WorkGenerator** (`pkg/controllers/workgenerator/`): Generates Work objects from bindings
8186
- **WorkApplier** (`pkg/controllers/workapplier/`): Applies Work manifests on member clusters
8287

0 commit comments

Comments
 (0)