Skip to content

Commit 87dddec

Browse files
author
Ryan Zhang
committed
test
Signed-off-by: Ryan Zhang <[email protected]>
1 parent 57464c9 commit 87dddec

29 files changed

+1562
-803
lines changed

‎.github/.copilot/breadcrumbs/2025-01-07-0700-extend-getOrCreateClusterResourceSnapshot-for-namespaced-snapshots.md‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# Extend getOrCreateClusterResourceSnapshot to Handle Both Snapshot Types
2+
3+
## Context
4+
5+
Following successful interface refactoring and deletion function optimizations, user requested: "can you modify the function getOrCreateClusterResourceSnapshot to handle both clusterresourcesnapshot and resourcesnapshot?"
6+
7+
## Problem
8+
9+
The `getOrCreateClusterResourceSnapshot` function in `/Users/ryanzhang/Workspace/github/kubefleet/pkg/controllers/clusterresourceplacement/controller.go` (lines 468-650) currently only handles `ClusterResourceSnapshot` (cluster-scoped) resources. We need to extend it to also handle `ResourceSnapshot` (namespace-scoped) resources using the common `ResourceSnapshotObj` interface.
10+
11+
## Research Findings
12+
13+
### ResourceSnapshotObj Interface Structure
14+
```go
15+
type ResourceSnapshotObj interface {
16+
apis.ConditionedObj
17+
ResourceSnapshotSpecGetterSetter
18+
ResourceSnapshotStatusGetterSetter
19+
}
20+
21+
type ResourceSnapshotSpecGetterSetter interface {
22+
GetResourceSnapshotSpec() *ResourceSnapshotSpec
23+
SetResourceSnapshotSpec(ResourceSnapshotSpec)
24+
}
25+
26+
type ResourceSnapshotStatusGetterSetter interface {
27+
GetResourceSnapshotStatus() *ResourceSnapshotStatus
28+
SetResourceSnapshotStatus(ResourceSnapshotStatus)
29+
}
30+
```
31+
32+
### Implementation Types
33+
- **ClusterResourceSnapshot** (cluster-scoped, v1beta1) - implements ResourceSnapshotObj
34+
- **ResourceSnapshot** (namespace-scoped, v1beta1) - implements ResourceSnapshotObj
35+
36+
Both types share:
37+
- Same `ResourceSnapshotSpec` structure
38+
- Same interface methods for spec/status access
39+
- Same labeling and annotation patterns
40+
- Different scoping (cluster vs namespace)
41+
42+
### Pattern Precedent
43+
The delete functions already follow this pattern:
44+
- `deleteClusterSchedulingPolicySnapshots` handles both ClusterSchedulingPolicySnapshot and SchedulingPolicySnapshot
45+
- `deleteResourceSnapshots` uses `DeleteAllOf` with namespace awareness
46+
47+
## Plan
48+
49+
### Phase 1: Function Signature Updates
50+
- [x] Task 1.1: Change function signature to accept `PlacementObj` interface instead of concrete type
51+
- [x] Task 1.2: Update return type to use `ResourceSnapshotObj` interface
52+
- [x] Task 1.3: Update function name to `getOrCreateResourceSnapshot` for clarity
53+
54+
### Phase 2: Namespace Detection Logic
55+
- [x] Task 2.1: Add helper function to determine if placement is namespace-scoped
56+
- [x] Task 2.2: Implement logic to choose between ClusterResourceSnapshot and ResourceSnapshot based on placement scope
57+
58+
### Phase 3: Snapshot Creation Logic
59+
- [ ] Task 3.1: Extend `buildMasterClusterResourceSnapshot` to handle both types
60+
- [ ] Task 3.2: Extend `buildSubIndexResourceSnapshot` to handle both types
61+
- [ ] Task 3.3: Update snapshot creation logic to use proper type based on scope
62+
63+
### Phase 4: Helper Functions Updates
64+
- [ ] Task 4.1: Update `lookupLatestResourceSnapshot` to handle both types
65+
- [ ] Task 4.2: Update `listSortedResourceSnapshots` to handle both types
66+
- [ ] Task 4.3: Update `createResourceSnapshot` to handle both types
67+
68+
### Phase 5: Testing and Validation
69+
- [ ] Task 5.1: Run existing unit tests to ensure no regression
70+
- [ ] Task 5.2: Verify function works with both cluster-scoped and namespace-scoped placements
71+
- [ ] Task 5.3: Ensure interface compatibility throughout the codebase
72+
73+
## Success Criteria
74+
- Function accepts both ClusterResourcePlacement and ResourcePlacement
75+
- Creates appropriate snapshot type based on placement scope
76+
- Maintains all existing functionality for cluster-scoped resources
77+
- Uses ResourceSnapshotObj interface throughout
78+
- No breaking changes to existing API
79+
- All tests pass
80+
81+
## Implementation Notes
82+
- Follow established namespace-aware pattern from delete functions
83+
- Use ResourceSnapshotObj interface consistently
84+
- Maintain backward compatibility
85+
- Apply same labeling/annotation patterns for both types

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

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
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+
### Phase 4: Testing and Validation ✅
66+
- [x] Task 4.1: Verify unit tests still pass after interface refactoring ✅
67+
- [x] Task 4.2: Verify specific refactored functions work correctly ✅
68+
- [x] Task 4.3: Ensure no compilation errors ✅
69+
- [x] Task 4.4: Validate core functionality maintained ✅
70+
71+
### Testing Results ✅
72+
**Unit Tests**: All existing unit tests continue to pass
73+
- ✅ `TestBuildResourcePlacementStatusMap` - Passed all sub-tests
74+
- ✅ `TestGetOrCreateClusterSchedulingPolicySnapshot` - Passed all cases
75+
- ✅ `TestGetOrCreateClusterResourceSnapshot` - Passed all cases
76+
- ✅ General controller tests - All passing
77+
78+
**Compilation**: Clean compilation with no errors
79+
- ✅ `go build ./pkg/controllers/clusterresourceplacement/` - Success
80+
- ✅ `go test -run ^$` - Compilation-only test passed
81+
82+
**Interface Integration**: All refactored functions work correctly with interfaces
83+
- ✅ Functions automatically accept concrete types via interface satisfaction
84+
- ✅ No type assertion issues in main business logic
85+
- ✅ Error handling preserved and working correctly
86+
87+
### Phase 5: Refactor placement_status.go Functions ⚠️
88+
- [ ] Task 5.1: Update `appendFailedToScheduleResourcePlacementStatuses()` to use PlacementObj interface
89+
- [ ] Task 5.2: Update `appendScheduledResourcePlacementStatuses()` to use PlacementObj and interface types
90+
- [ ] Task 5.3: Update `buildClusterResourceBindings()` to use PlacementObj and return BindingObj interfaces
91+
- [ ] Task 5.4: Update `findClusterResourceSnapshotIndexForBindings()` to use interface types
92+
- [ ] Task 5.5: Update `setResourcePlacementStatusPerCluster()` to use interface types
93+
- [ ] Task 5.6: Update `setResourcePlacementStatusBasedOnBinding()` to use interface types
94+
95+
### Functions to Refactor in placement_status.go 🎯
96+
**Current concrete type usage identified:**
97+
- `appendFailedToScheduleResourcePlacementStatuses()` - Takes `*ClusterResourcePlacement`
98+
- `appendScheduledResourcePlacementStatuses()` - Takes `*ClusterResourcePlacement`, `*ClusterSchedulingPolicySnapshot`, `*ClusterResourceSnapshot`
99+
- `buildClusterResourceBindings()` - Takes `*ClusterResourcePlacement`, `*ClusterSchedulingPolicySnapshot`, returns `map[string]*ClusterResourceBinding`
100+
- `findClusterResourceSnapshotIndexForBindings()` - Takes `*ClusterResourcePlacement`, `map[string]*ClusterResourceBinding`
101+
- `setResourcePlacementStatusPerCluster()` - Takes `*ClusterResourcePlacement`, `*ClusterResourceSnapshot`, `*ClusterResourceBinding`
102+
- `setResourcePlacementStatusBasedOnBinding()` - Takes `*ClusterResourcePlacement`, `*ClusterResourceBinding`
103+
104+
### Phase 3: Snapshot Management Function Updates
105+
- [ ] Task 3.1: Update `getOrCreateClusterSchedulingPolicySnapshot` to return PolicySnapshotObj
106+
- [ ] Task 3.2: Update `getOrCreateClusterResourceSnapshot` to return ResourceSnapshotObj
107+
- [ ] Task 3.3: Update all snapshot listing and deletion functions to use interface types
108+
- [ ] Task 3.4: Update snapshot creation and management helper functions
109+
110+
### Phase 4: Status and Condition Management Updates
111+
- [ ] Task 4.1: Update `setPlacementStatus` function to use PlacementObj interface
112+
- [ ] Task 4.2: Update condition building functions to use interface types
113+
- [ ] Task 4.3: Update resource placement status calculation functions
114+
115+
### Phase 5: Builder and Utility Function Updates
116+
- [ ] Task 5.1: Update snapshot builder functions to return interface types
117+
- [ ] Task 5.2: Update parsing and utility functions to work with interface types
118+
- [ ] Task 5.3: Update all logging references to use interface objects
119+
120+
### Phase 6: Test Updates
121+
- [ ] Task 6.1: Update unit tests in the same package to use interface types
122+
- [ ] Task 6.2: Update integration tests to use interface types
123+
- [ ] Task 6.3: Run tests to ensure all interface usage is correct
124+
125+
### Phase 7: Interface Method Integration
126+
- [ ] Task 7.1: Replace direct field access with interface methods where applicable
127+
- [ ] Task 7.2: Ensure proper use of `GetPlacementSpec()`, `SetPlacementSpec()` methods
128+
- [ ] Task 7.3: Verify all interface implementations are working correctly
129+
130+
## Analysis
131+
132+
### Current State Analysis ✅
133+
134+
Based on the grep search, identified 20+ functions that need refactoring:
135+
136+
**Main reconciler functions**:
137+
- `handleDelete(ctx context.Context, crp *fleetv1beta1.ClusterResourcePlacement)`
138+
- `handleUpdate(ctx context.Context, crp *fleetv1beta1.ClusterResourcePlacement)`
139+
140+
**Snapshot management functions**:
141+
- `getOrCreateClusterSchedulingPolicySnapshot(ctx context.Context, crp *fleetv1beta1.ClusterResourcePlacement, revisionHistoryLimit int) (*fleetv1beta1.ClusterSchedulingPolicySnapshot, error)`
142+
- `getOrCreateClusterResourceSnapshot(ctx context.Context, crp *fleetv1beta1.ClusterResourcePlacement, envelopeObjCount int, resourceSnapshotSpec *fleetv1beta1.ResourceSnapshotSpec, revisionHistoryLimit int) (ctrl.Result, *fleetv1beta1.ClusterResourceSnapshot, error)`
143+
144+
**Builder functions**:
145+
- `buildMasterClusterResourceSnapshot(...) *fleetv1beta1.ClusterResourceSnapshot`
146+
- `buildSubIndexResourceSnapshot(...) *fleetv1beta1.ClusterResourceSnapshot`
147+
148+
**Status and condition functions**:
149+
- `buildScheduledCondition(crp *fleetv1beta1.ClusterResourcePlacement, latestSchedulingPolicySnapshot *fleetv1beta1.ClusterSchedulingPolicySnapshot) metav1.Condition`
150+
- `buildResourcePlacementStatusMap(crp *fleetv1beta1.ClusterResourcePlacement) map[string][]metav1.Condition`
151+
152+
### Interface Patterns Available ✅
153+
154+
**PlacementObj interface** provides:
155+
- `GetPlacementSpec() *ResourcePlacementSpec`
156+
- `SetPlacementSpec(*ResourcePlacementSpec)`
157+
- `GetPlacementStatus() *ResourcePlacementStatus`
158+
- `SetPlacementStatus(*ResourcePlacementStatus)`
159+
160+
**ResourceSnapshotObj interface** provides:
161+
- `GetResourceSnapshotSpec() *ResourceSnapshotSpec`
162+
- `SetResourceSnapshotSpec(*ResourceSnapshotSpec)`
163+
- `GetResourceSnapshotStatus() *ResourceSnapshotStatus`
164+
- `SetResourceSnapshotStatus(*ResourceSnapshotStatus)`
165+
166+
**PolicySnapshotObj interface** provides:
167+
- `GetPolicySnapshotSpec() *SchedulingPolicySnapshotSpec`
168+
- `SetPolicySnapshotSpec(*SchedulingPolicySnapshotSpec)`
169+
- `GetPolicySnapshotStatus() *SchedulingPolicySnapshotStatus`
170+
- `SetPolicySnapshotStatus(*SchedulingPolicySnapshotStatus)`
171+
172+
### Helper Functions Available ✅
173+
174+
From `pkg/utils/controller`:
175+
- `FetchPlacementFromKey(ctx context.Context, c client.Reader, placementKey queue.PlacementKey) (fleetv1beta1.PlacementObj, error)`
176+
- `FetchLatestMasterResourceSnapshot(ctx context.Context, k8Client client.Reader, placementKey types.NamespacedName) (fleetv1beta1.ResourceSnapshotObj, error)`
177+
- Various resolver and conversion helper functions
178+
179+
## Decisions
180+
181+
### Refactoring Strategy
182+
1. Follow the exact same pattern as the rollout controller refactor in breadcrumb `2025-06-24-0900-rollout-controller-binding-interface-refactor.md`
183+
2. Replace concrete types with interfaces in function signatures
184+
3. Use interface methods instead of direct field access
185+
4. Keep type assertions only at system boundaries (controller-runtime event handling)
186+
5. Use helper functions from `pkg/utils/controller` for fetching concrete types when needed
187+
188+
### Implementation Order
189+
1. Start with the main reconciler entry point and work downward
190+
2. Update function signatures first, then implement interface method usage
191+
3. Handle tests after core functionality is complete
192+
4. Follow the principle of minimal changes - avoid large complex refactoring
193+
194+
## Implementation Status
195+
196+
### Analysis Complete ✅
197+
- Identified all functions requiring refactoring
198+
- Confirmed available interface types and methods
199+
- Located helper functions for type conversion
200+
- Planned implementation strategy based on successful previous patterns
201+
202+
### Key Understanding ✅
203+
204+
**Current Controller Logic**:
205+
1. `Reconcile()` function assumes key is a simple string (cluster-scoped name only)
206+
2. Directly fetches `ClusterResourcePlacement` using `types.NamespacedName{Name: name}`
207+
3. Uses concrete types throughout all functions (`*fleetv1beta1.ClusterResourcePlacement`, `*fleetv1beta1.ClusterResourceSnapshot`, etc.)
208+
4. All business logic operates on concrete types
209+
210+
**Target Interface Pattern**:
211+
1. `Reconcile()` should accept placement keys that include namespace/name format
212+
2. Use `FetchPlacementFromKey()` helper to get appropriate placement type (cluster-scoped or namespaced)
213+
3. Work with `PlacementObj` interface throughout business logic
214+
4. Use interface methods like `GetPlacementSpec()`, `SetPlacementSpec()` instead of direct field access
215+
5. Keep type assertions only at system boundaries (controller-runtime interactions)
216+
217+
**Critical Functions That Need Interface Refactoring**:
218+
- `Reconcile()` - entry point, needs to use placement resolver
219+
- `handleDelete()` / `handleUpdate()` - core business logic, use PlacementObj
220+
- `selectResourcesForPlacement()` - resource selection, use PlacementObj
221+
- `getOrCreateClusterSchedulingPolicySnapshot()` - return PolicySnapshotObj
222+
- `getOrCreateClusterResourceSnapshot()` - return ResourceSnapshotObj
223+
- `setPlacementStatus()` - status management, use PlacementObj + interface snapshots

‎CLAUDE.md‎

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

7373
### Core API Types
74-
- **ClusterResourcePlacement (CRP)**: Main API for placing resources across clusters with scheduling policies
75-
- **MemberCluster**: Represents a member cluster with identity and heartbeat settings
76-
- **ClusterResourceBinding**: Represents scheduling decisions binding resources to clusters
74+
- **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.
75+
- **ResourcePlacement (RP)**: Main API for placing namespaced resources across clusters with scheduling policies.
76+
- **MemberCluster**: Represents a member cluster with identity and heartbeat settings
77+
- **ClusterResourceBinding**: Represents scheduling decisions binding cluster scoped resources to clusters
78+
- **ResourceBinding**: Represents scheduling decisions binding resources to clusters
79+
- **ClusterResourceSnapshot**: Immutable snapshot of cluster resource state for rollback and history
80+
- **ResourceSnapshot**: Immutable snapshot of namespaced resource state for rollback and history
7781
- **Work**: Contains manifests to be applied on member clusters
7882

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

0 commit comments

Comments
 (0)