Skip to content

Commit dc57e28

Browse files
ryanzhang-ossryanzhang-oss
committed
unify all the types and update scheduler
Signed-off-by: Ryan Zhang <[email protected]>
1 parent 0da7106 commit dc57e28

File tree

6 files changed

+342
-89
lines changed

6 files changed

+342
-89
lines changed

‎.github/.copilot/breadcrumbs/2025-06-06-1440-binding-interface-pattern-implementation.md‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
# Binding Interface Pattern Implementation
2+
3+
## Requirements
4+
5+
Create a common interface for ClusterResourceBinding and ResourceBinding by following the same pattern used in resourcesnapshot_types.go and policysnapshot_types.go. The goal is to establish a unified interface pattern similar to how PolicySnapshotObj and ResourceSnapshotObj interfaces work for their respective types.
6+
7+
Key requirements:
8+
1. Follow the exact same pattern as resourcesnapshot_types.go and policysnapshot_types.go
9+
2. Add interface constants, type assertions, and utility methods following the established pattern
10+
3. Add BindingList interface and GetBindingObjs() methods similar to PolicySnapshotList.GetPolicySnapshotObjs()
11+
4. Ensure proper interface implementation with pre-allocated slices to avoid prealloc warnings
12+
5. Move or reorganize binding-related interface definitions to maintain consistency with the established patterns
13+
14+
## Additional comments from user
15+
16+
The task follows the breadcrumb protocol. Need to implement the missing pieces to complete the interface pattern consistency across all placement API types.
17+
18+
## Plan
19+
20+
### Phase 1: Analysis and Interface Design
21+
- [x] **Task 1.1**: Analyze existing interface patterns in resourcesnapshot_types.go and policysnapshot_types.go
22+
- [x] **Task 1.2**: Examine current binding_types.go structure and interface.go binding definitions
23+
- [x] **Task 1.3**: Design binding interface pattern to match resourcesnapshot and policysnapshot patterns
24+
25+
### Phase 2: Interface Implementation
26+
- [ ] **Task 2.1**: Add interface constants and type assertions at the top of binding_types.go
27+
- [ ] **Task 2.2**: Define BindingList interface and BindingListItemGetter interface
28+
- [ ] **Task 2.3**: Implement GetBindingObjs() methods for both ClusterResourceBindingList and ResourceBindingList
29+
- [ ] **Task 2.4**: Move interface definitions from interface.go to binding_types.go for consistency
30+
31+
### Phase 3: Implementation Details and Testing
32+
- [ ] **Task 3.1**: Verify interface implementation with proper type assertions
33+
- [ ] **Task 3.2**: Update init() function to follow the established pattern
34+
- [ ] **Task 3.3**: Run tests to ensure implementation works correctly
35+
- [ ] **Task 3.4**: Update any references if needed
36+
37+
## Decisions
38+
39+
### Interface Pattern Analysis
40+
Based on analysis of existing patterns:
41+
42+
1. **Constants and Type Assertions**: Both resourcesnapshot_types.go and policysnapshot_types.go include:
43+
- Interface variable assertions at the top of the file
44+
- Clear separation between spec and status interfaces
45+
- List interfaces with GetObjs() methods
46+
47+
2. **Interface Structure**: The pattern includes:
48+
- SpecGetSetter interfaces
49+
- StatusGetSetter interfaces
50+
- Main object interface combining client.Object + spec + status interfaces
51+
- ListItemGetter interface for accessing objects from list
52+
- List interface combining client.ObjectList + ListItemGetter
53+
54+
3. **Method Implementation**: Pre-allocated slices in GetObjs() methods to avoid prealloc warnings
55+
56+
### Implementation Strategy
57+
1. Follow resourcesnapshot_types.go pattern exactly
58+
2. Move interface definitions from interface.go to binding_types.go
59+
3. Add BindingList and BindingListItemGetter interfaces
60+
4. Implement GetBindingObjs() methods with pre-allocated slices
61+
5. Add proper type assertions for interface verification
62+
63+
## Implementation Details
64+
65+
### Current State Analysis ✅
66+
- **binding_types.go**: Contains ClusterResourceBinding and ResourceBinding types with method implementations
67+
- **interface.go**: Contains existing BindingObj interface definitions with BindingSpecGetSetter and BindingStatusGetSetter interfaces
68+
- **Interface verification**: Both types already implement the required methods: GetBindingSpec(), SetBindingSpec(), GetBindingStatus(), SetBindingStatus()
69+
70+
### Target Pattern Structure
71+
Based on resourcesnapshot_types.go and policysnapshot_types.go patterns, need to add:
72+
73+
```go
74+
// Interface assertions for verification
75+
var _ BindingObj = &ClusterResourceBinding{}
76+
var _ BindingObj = &ResourceBinding{}
77+
var _ BindingObjList = &ClusterResourceBindingList{}
78+
var _ BindingObjList = &ResourceBindingList{}
79+
80+
// Interface definitions
81+
type BindingSpecGetSetter interface {
82+
GetBindingSpec() *ResourceBindingSpec
83+
SetBindingSpec(*ResourceBindingSpec)
84+
}
85+
86+
type BindingStatusGetSetter interface {
87+
GetBindingStatus() *ResourceBindingStatus
88+
SetBindingStatus(*ResourceBindingStatus)
89+
}
90+
91+
type BindingObj interface {
92+
client.Object
93+
BindingSpecGetSetter
94+
BindingStatusGetSetter
95+
}
96+
97+
type BindingListItemGetter interface {
98+
GetBindingObjs() []BindingObj
99+
}
100+
101+
type BindingObjList interface {
102+
client.ObjectList
103+
BindingListItemGetter
104+
}
105+
106+
// List methods
107+
func (c *ClusterResourceBindingList) GetBindingObjs() []BindingObj {
108+
objs := make([]BindingObj, 0, len(c.Items))
109+
for i := range c.Items {
110+
objs = append(objs, &c.Items[i])
111+
}
112+
return objs
113+
}
114+
115+
func (c *ResourceBindingList) GetBindingObjs() []BindingObj {
116+
objs := make([]BindingObj, 0, len(c.Items))
117+
for i := range c.Items {
118+
objs = append(objs, &c.Items[i])
119+
}
120+
return objs
121+
}
122+
```
123+
124+
## Changes Made
125+
126+
### Phase 1: Analysis and Interface Design ✅
127+
- **Task 1.1**: ✅ Analyzed resourcesnapshot_types.go and policysnapshot_types.go patterns
128+
- **Task 1.2**: ✅ Examined binding_types.go and interface.go current structure
129+
- **Task 1.3**: ✅ Design binding interface pattern to match resourcesnapshot and policysnapshot patterns
130+
131+
### Phase 2: Interface Implementation
132+
- **Task 2.1**: ⬜ Add interface constants and type assertions at the top of binding_types.go
133+
- **Task 2.2**: ⬜ Define BindingList interface and BindingListItemGetter interface
134+
- **Task 2.3**: ⬜ Implement GetBindingObjs() methods for both ClusterResourceBindingList and ResourceBindingList
135+
- **Task 2.4**: ⬜ Move interface definitions from interface.go to binding_types.go for consistency
136+
137+
### Phase 3: Implementation Details and Testing
138+
- **Task 3.1**: ⬜ Verify interface implementation with proper type assertions
139+
- **Task 3.2**: ⬜ Update init() function to follow the established pattern
140+
- **Task 3.3**: ⬜ Run tests to ensure implementation works correctly
141+
- **Task 3.4**: ⬜ Update any references if needed
142+
143+
## Before/After Comparison
144+
145+
### Before
146+
- Interface definitions split between interface.go and binding_types.go
147+
- Missing BindingList interfaces and GetBindingObjs() methods
148+
- Inconsistent pattern compared to resourcesnapshot_types.go and policysnapshot_types.go
149+
150+
### After (Target)
151+
- All binding interfaces consolidated in binding_types.go following established pattern
152+
- Complete BindingList interface with GetBindingObjs() methods
153+
- Consistent interface pattern across all placement API types
154+
- Proper type assertions for interface verification
155+
156+
## References
157+
158+
- **Domain Knowledge Files**: N/A (no specific domain knowledge files referenced)
159+
- **Specification Files**: N/A (no specific specification files referenced)
160+
- **Code Analysis**:
161+
- `/home/zhangryan/github/kubefleet/kubefleet/apis/placement/v1beta1/resourcesnapshot_types.go` - Reference pattern for interface implementation
162+
- `/home/zhangryan/github/kubefleet/kubefleet/apis/placement/v1beta1/policysnapshot_types.go` - Reference pattern for interface implementation
163+
- `/home/zhangryan/github/kubefleet/kubefleet/apis/placement/v1beta1/binding_types.go` - Current implementation to be extended
164+
- `/home/zhangryan/github/kubefleet/kubefleet/apis/placement/v1beta1/interface.go` - Current interface definitions to be moved
165+
166+
## Success Criteria
167+
168+
1. ✅ Interface pattern analysis completed
169+
2. ⬜ binding_types.go follows exact same pattern as resourcesnapshot_types.go and policysnapshot_types.go
170+
3. ⬜ BindingList interfaces implemented with GetBindingObjs() methods
171+
4. ⬜ Interface definitions moved from interface.go to binding_types.go
172+
5. ⬜ Type assertions added for interface verification
173+
6. ⬜ Tests pass without errors
174+
7. ⬜ Implementation maintains backward compatibility

‎apis/placement/v1beta1/binding_types.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package v1beta1
1919
import (
2020
"k8s.io/apimachinery/pkg/api/meta"
2121
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
22+
"sigs.k8s.io/controller-runtime/pkg/client"
2223
)
2324

2425
const (
@@ -246,6 +247,24 @@ type ResourceBindingList struct {
246247
Items []ResourceBinding `json:"items"`
247248
}
248249

250+
// GetBindingObjs returns the binding objects in the list.
251+
func (c *ClusterResourceBindingList) GetBindingObjs() []BindingObj {
252+
objs := make([]BindingObj, 0, len(c.Items))
253+
for i := range c.Items {
254+
objs = append(objs, &c.Items[i])
255+
}
256+
return objs
257+
}
258+
259+
// GetBindingObjs returns the binding objects in the list.
260+
func (r *ResourceBindingList) GetBindingObjs() []BindingObj {
261+
objs := make([]BindingObj, 0, len(r.Items))
262+
for i := range r.Items {
263+
objs = append(objs, &r.Items[i])
264+
}
265+
return objs
266+
}
267+
249268
// SetConditions set the given conditions on the ClusterResourceBinding.
250269
func (b *ClusterResourceBinding) SetConditions(conditions ...metav1.Condition) {
251270
for _, c := range conditions {
@@ -328,6 +347,48 @@ func (b *ResourceBinding) SetBindingStatus(status *ResourceBindingStatus) {
328347
}
329348
}
330349

350+
// make sure the BindingObj and BindingObjList interfaces are implemented by the
351+
// ClusterResourceBinding and ResourceBinding types.
352+
var _ BindingObj = &ClusterResourceBinding{}
353+
var _ BindingObj = &ResourceBinding{}
354+
var _ BindingObjList = &ClusterResourceBindingList{}
355+
var _ BindingObjList = &ResourceBindingList{}
356+
357+
// A BindingSpecGetSetter contains binding spec
358+
// +kubebuilder:object:generate=false
359+
type BindingSpecGetSetter interface {
360+
GetBindingSpec() *ResourceBindingSpec
361+
SetBindingSpec(*ResourceBindingSpec)
362+
}
363+
364+
// A BindingStatusGetSetter contains binding status
365+
// +kubebuilder:object:generate=false
366+
type BindingStatusGetSetter interface {
367+
GetBindingStatus() *ResourceBindingStatus
368+
SetBindingStatus(*ResourceBindingStatus)
369+
}
370+
371+
// A BindingObj is for kubernetes resource binding object.
372+
// +kubebuilder:object:generate=false
373+
type BindingObj interface {
374+
client.Object
375+
BindingSpecGetSetter
376+
BindingStatusGetSetter
377+
}
378+
379+
// A BindingListItemGetter contains binding list items
380+
// +kubebuilder:object:generate=false
381+
type BindingListItemGetter interface {
382+
GetBindingObjs() []BindingObj
383+
}
384+
385+
// A BindingObjList is for kubernetes resource binding list object.
386+
// +kubebuilder:object:generate=false
387+
type BindingObjList interface {
388+
client.ObjectList
389+
BindingListItemGetter
390+
}
391+
331392
func init() {
332393
SchemeBuilder.Register(&ClusterResourceBinding{}, &ClusterResourceBindingList{}, &ResourceBinding{}, &ResourceBindingList{})
333394
}

‎apis/placement/v1beta1/interface.go‎

Lines changed: 0 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -47,88 +47,3 @@ type PlacementObj interface {
4747
PlacementSpecGetSetter
4848
PlacementStatusGetSetter
4949
}
50-
51-
// A BindingSpecGetSetter contains bindingSpec
52-
// +kubebuilder:object:generate=false
53-
type BindingSpecGetSetter interface {
54-
GetBindingSpec() *ResourceBindingSpec
55-
SetBindingSpec(*ResourceBindingSpec)
56-
}
57-
58-
// A BindingStatusGetSetter contains bindingStatus
59-
// +kubebuilder:object:generate=false
60-
type BindingStatusGetSetter interface {
61-
GetBindingStatus() *ResourceBindingStatus
62-
SetBindingStatus(*ResourceBindingStatus)
63-
}
64-
65-
var _ BindingObj = &ClusterResourceBinding{}
66-
var _ BindingObj = &ResourceBinding{}
67-
68-
// A BindingObj is for kubernetes resource binding object.
69-
// +kubebuilder:object:generate=false
70-
type BindingObj interface {
71-
client.Object
72-
BindingSpecGetSetter
73-
BindingStatusGetSetter
74-
}
75-
76-
// A PolicySnapshotSpecGetSetter contains policy snapshot spec
77-
// +kubebuilder:object:generate=false
78-
type PolicySnapshotSpecGetSetter interface {
79-
GetPolicySnapshotSpec() *SchedulingPolicySnapshotSpec
80-
SetPolicySnapshotSpec(*SchedulingPolicySnapshotSpec)
81-
}
82-
83-
// A PolicySnapshotStatusGetSetter contains policy snapshot status
84-
// +kubebuilder:object:generate=false
85-
type PolicySnapshotStatusGetSetter interface {
86-
GetPolicySnapshotStatus() *SchedulingPolicySnapshotStatus
87-
SetPolicySnapshotStatus(*SchedulingPolicySnapshotStatus)
88-
}
89-
90-
// A PolicySnapshotObj is for kubernetes policy snapshot object.
91-
// +kubebuilder:object:generate=false
92-
type PolicySnapshotObj interface {
93-
client.Object
94-
PolicySnapshotSpecGetSetter
95-
PolicySnapshotStatusGetSetter
96-
}
97-
98-
// A PolicySnapshotSpec contains policy snapshot spec
99-
// +kubebuilder:object:generate=false
100-
type PolicySnapshotListItemGetter interface {
101-
GetPolicySnapshotObjs() []PolicySnapshotObj
102-
}
103-
104-
// A PolicySnapshotList is for kubernetes policy snapshot list object.
105-
// +kubebuilder:object:generate=false
106-
type PolicySnapshotList interface {
107-
client.ObjectList
108-
PolicySnapshotListItemGetter
109-
}
110-
111-
// A ResourceSnapshotSpecGetSettter contains resource snapshot spec
112-
// +kubebuilder:object:generate=false
113-
type ResourceSnapshotSpecGetSettter interface {
114-
GetResourceSnapshotSpec() *ResourceSnapshotSpec
115-
SetResourceSnapshotSpec(*ResourceSnapshotSpec)
116-
}
117-
118-
// A ResourceSnapshotStatusGetSetter contains resource snapshot status
119-
// +kubebuilder:object:generate=false
120-
type ResourceSnapshotStatusGetSetter interface {
121-
GetResourceSnapshotStatus() *ResourceSnapshotStatus
122-
SetResourceSnapshotStatus(*ResourceSnapshotStatus)
123-
}
124-
125-
var _ ResourceSnapshotObj = &ClusterResourceSnapshot{}
126-
var _ ResourceSnapshotObj = &ResourceSnapshot{}
127-
128-
// A ResourceSnapshotObj is for kubernetes resource snapshot object.
129-
// +kubebuilder:object:generate=false
130-
type ResourceSnapshotObj interface {
131-
client.Object
132-
ResourceSnapshotSpecGetSettter
133-
ResourceSnapshotStatusGetSetter
134-
}

‎apis/placement/v1beta1/policysnapshot_types.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package v1beta1
1919
import (
2020
"k8s.io/apimachinery/pkg/api/meta"
2121
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
22+
"sigs.k8s.io/controller-runtime/pkg/client"
2223
)
2324

2425
const (
@@ -251,6 +252,41 @@ type SchedulingPolicySnapshot struct {
251252
Status SchedulingPolicySnapshotStatus `json:"status,omitempty"`
252253
}
253254

255+
// A PolicySnapshotSpecGetSetter contains policy snapshot spec
256+
// +kubebuilder:object:generate=false
257+
type PolicySnapshotSpecGetSetter interface {
258+
GetPolicySnapshotSpec() *SchedulingPolicySnapshotSpec
259+
SetPolicySnapshotSpec(*SchedulingPolicySnapshotSpec)
260+
}
261+
262+
// A PolicySnapshotStatusGetSetter contains policy snapshot status
263+
// +kubebuilder:object:generate=false
264+
type PolicySnapshotStatusGetSetter interface {
265+
GetPolicySnapshotStatus() *SchedulingPolicySnapshotStatus
266+
SetPolicySnapshotStatus(*SchedulingPolicySnapshotStatus)
267+
}
268+
269+
// A PolicySnapshotObj is for kubernetes policy snapshot object.
270+
// +kubebuilder:object:generate=false
271+
type PolicySnapshotObj interface {
272+
client.Object
273+
PolicySnapshotSpecGetSetter
274+
PolicySnapshotStatusGetSetter
275+
}
276+
277+
// A PolicySnapshotSpec contains policy snapshot spec
278+
// +kubebuilder:object:generate=false
279+
type PolicySnapshotListItemGetter interface {
280+
GetPolicySnapshotObjs() []PolicySnapshotObj
281+
}
282+
283+
// A PolicySnapshotList is for kubernetes policy snapshot list object.
284+
// +kubebuilder:object:generate=false
285+
type PolicySnapshotList interface {
286+
client.ObjectList
287+
PolicySnapshotListItemGetter
288+
}
289+
254290
// GetPolicySnapshotSpec returns the policy snapshot spec.
255291
func (m *SchedulingPolicySnapshot) GetPolicySnapshotSpec() *SchedulingPolicySnapshotSpec {
256292
return &m.Spec

0 commit comments

Comments
 (0)