Skip to content

Commit 599fd2f

Browse files
ryanzhang-ossryanzhang-oss
committed
improve the interface and fix test
Signed-off-by: Ryan Zhang <[email protected]>
1 parent bedcd36 commit 599fd2f

File tree

6 files changed

+108
-119
lines changed

6 files changed

+108
-119
lines changed

‎apis/placement/v1beta1/binding_types.go‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,14 @@ var _ BindingObjList = &ResourceBindingList{}
3939
// +kubebuilder:object:generate=false
4040
type BindingSpecGetterSetter interface {
4141
GetBindingSpec() *ResourceBindingSpec
42-
SetBindingSpec(*ResourceBindingSpec)
42+
SetBindingSpec(ResourceBindingSpec)
4343
}
4444

4545
// A BindingStatusGetterSetter offers binding status getter and setter methods.
4646
// +kubebuilder:object:generate=false
4747
type BindingStatusGetterSetter interface {
4848
GetBindingStatus() *ResourceBindingStatus
49-
SetBindingStatus(*ResourceBindingStatus)
49+
SetBindingStatus(ResourceBindingStatus)
5050
}
5151

5252
// A BindingObj offers an abstract way to work with fleet binding objects.
@@ -330,10 +330,8 @@ func (b *ClusterResourceBinding) GetBindingSpec() *ResourceBindingSpec {
330330
}
331331

332332
// SetBindingSpec sets the binding spec.
333-
func (b *ClusterResourceBinding) SetBindingSpec(spec *ResourceBindingSpec) {
334-
if spec != nil {
335-
spec.DeepCopyInto(&b.Spec)
336-
}
333+
func (b *ClusterResourceBinding) SetBindingSpec(spec ResourceBindingSpec) {
334+
spec.DeepCopyInto(&b.Spec)
337335
}
338336

339337
// GetBindingStatus returns the binding status.
@@ -342,10 +340,8 @@ func (b *ClusterResourceBinding) GetBindingStatus() *ResourceBindingStatus {
342340
}
343341

344342
// SetBindingStatus sets the binding status.
345-
func (b *ClusterResourceBinding) SetBindingStatus(status *ResourceBindingStatus) {
346-
if status != nil {
347-
status.DeepCopyInto(&b.Status)
348-
}
343+
func (b *ClusterResourceBinding) SetBindingStatus(status ResourceBindingStatus) {
344+
status.DeepCopyInto(&b.Status)
349345
}
350346

351347
// SetConditions set the given conditions on the ResourceBinding.
@@ -371,10 +367,8 @@ func (b *ResourceBinding) GetBindingSpec() *ResourceBindingSpec {
371367
}
372368

373369
// SetBindingSpec sets the binding spec.
374-
func (b *ResourceBinding) SetBindingSpec(spec *ResourceBindingSpec) {
375-
if spec != nil {
376-
spec.DeepCopyInto(&b.Spec)
377-
}
370+
func (b *ResourceBinding) SetBindingSpec(spec ResourceBindingSpec) {
371+
spec.DeepCopyInto(&b.Spec)
378372
}
379373

380374
// GetBindingStatus returns the binding status.
@@ -383,10 +377,8 @@ func (b *ResourceBinding) GetBindingStatus() *ResourceBindingStatus {
383377
}
384378

385379
// SetBindingStatus sets the binding status.
386-
func (b *ResourceBinding) SetBindingStatus(status *ResourceBindingStatus) {
387-
if status != nil {
388-
status.DeepCopyInto(&b.Status)
389-
}
380+
func (b *ResourceBinding) SetBindingStatus(status ResourceBindingStatus) {
381+
status.DeepCopyInto(&b.Status)
390382
}
391383

392384
func init() {

‎apis/placement/v1beta1/clusterresourceplacement_types.go‎

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ var _ PlacementObjList = &ResourcePlacementList{}
4545
// +kubebuilder:object:generate=false
4646
type PlacementSpecGetterSetter interface {
4747
GetPlacementSpec() *PlacementSpec
48-
SetPlacementSpec(*PlacementSpec)
48+
SetPlacementSpec(PlacementSpec)
4949
}
5050

5151
// PlacementStatusGetterSetter offers the functionality to work with the PlacementStatusGetterSetter.
5252
// +kubebuilder:object:generate=false
5353
type PlacementStatusGetterSetter interface {
5454
GetPlacementStatus() *PlacementStatus
55-
SetPlacementStatus(*PlacementStatus)
55+
SetPlacementStatus(PlacementStatus)
5656
}
5757

5858
// PlacementObj offers the functionality to work with fleet placement object.
@@ -535,8 +535,8 @@ type ApplyStrategy struct {
535535
// managed by Fleet (i.e., specified in the hub cluster manifest). This is the default
536536
// option.
537537
//
538-
// Note that this option would revert any ad-hoc changes made on the member cluster side in
539-
// the managed fields; if you would like to make temporary edits on the member cluster side
538+
// Note that this option would revert any ad-hoc changes made on the member cluster side in the
539+
// managed fields; if you would like to make temporary edits on the member cluster side
540540
// in the managed fields, switch to IfNotDrifted option. Note that changes in unmanaged
541541
// fields will be left alone; if you use the FullDiff compare option, such changes will
542542
// be reported as drifts.
@@ -861,7 +861,7 @@ type RollingUpdateConfig struct {
861861
UnavailablePeriodSeconds *int `json:"unavailablePeriodSeconds,omitempty"`
862862
}
863863

864-
// PlacementStatus defines the observed state of the ClusterResourcePlacement object.
864+
// PlacementStatus defines the observed status of the ClusterResourcePlacement object.
865865
type PlacementStatus struct {
866866
// SelectedResources contains a list of resources selected by ResourceSelectors.
867867
// This field is only meaningful if the `ObservedResourceIndex` is not empty.
@@ -1353,23 +1353,18 @@ func (m *ClusterResourcePlacement) GetPlacementSpec() *PlacementSpec {
13531353
}
13541354

13551355
// SetPlacementSpec sets the placement spec.
1356-
func (m *ClusterResourcePlacement) SetPlacementSpec(spec *PlacementSpec) {
1357-
if spec != nil {
1358-
spec.DeepCopyInto(&m.Spec)
1359-
}
1356+
func (m *ClusterResourcePlacement) SetPlacementSpec(spec PlacementSpec) {
1357+
spec.DeepCopyInto(&m.Spec)
13601358
}
13611359

13621360
// GetPlacementStatus returns the placement status.
13631361
func (m *ClusterResourcePlacement) GetPlacementStatus() *PlacementStatus {
1364-
status := m.Status
1365-
return &status
1362+
return &m.Status
13661363
}
13671364

13681365
// SetPlacementStatus sets the placement status.
1369-
func (m *ClusterResourcePlacement) SetPlacementStatus(status *PlacementStatus) {
1370-
if status != nil {
1371-
status.DeepCopyInto(&m.Status)
1372-
}
1366+
func (m *ClusterResourcePlacement) SetPlacementStatus(status PlacementStatus) {
1367+
status.DeepCopyInto(&m.Status)
13731368
}
13741369

13751370
const (
@@ -1439,10 +1434,8 @@ func (m *ResourcePlacement) GetPlacementSpec() *PlacementSpec {
14391434
}
14401435

14411436
// SetPlacementSpec sets the placement spec.
1442-
func (m *ResourcePlacement) SetPlacementSpec(spec *PlacementSpec) {
1443-
if spec != nil {
1444-
spec.DeepCopyInto(&m.Spec)
1445-
}
1437+
func (m *ResourcePlacement) SetPlacementSpec(spec PlacementSpec) {
1438+
spec.DeepCopyInto(&m.Spec)
14461439
}
14471440

14481441
// GetPlacementStatus returns the placement status.
@@ -1451,10 +1444,8 @@ func (m *ResourcePlacement) GetPlacementStatus() *PlacementStatus {
14511444
}
14521445

14531446
// SetPlacementStatus sets the placement status.
1454-
func (m *ResourcePlacement) SetPlacementStatus(status *PlacementStatus) {
1455-
if status != nil {
1456-
status.DeepCopyInto(&m.Status)
1457-
}
1447+
func (m *ResourcePlacement) SetPlacementStatus(status PlacementStatus) {
1448+
status.DeepCopyInto(&m.Status)
14581449
}
14591450

14601451
// GetPlacementObjs returns the placement objects in the list.

‎apis/placement/v1beta1/policysnapshot_types.go‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ var _ PolicySnapshotList = &SchedulingPolicySnapshotList{}
4444
// +kubebuilder:object:generate=false
4545
type PolicySnapshotSpecGetterSetter interface {
4646
GetPolicySnapshotSpec() *SchedulingPolicySnapshotSpec
47-
SetPolicySnapshotSpec(*SchedulingPolicySnapshotSpec)
47+
SetPolicySnapshotSpec(SchedulingPolicySnapshotSpec)
4848
}
4949

5050
// A PolicySnapshotStatusGetterSetter offers methods to get and set the policy snapshot status.
5151
// +kubebuilder:object:generate=false
5252
type PolicySnapshotStatusGetterSetter interface {
5353
GetPolicySnapshotStatus() *SchedulingPolicySnapshotStatus
54-
SetPolicySnapshotStatus(*SchedulingPolicySnapshotStatus)
54+
SetPolicySnapshotStatus(SchedulingPolicySnapshotStatus)
5555
}
5656

5757
// A PolicySnapshotObj offers an abstract way to work with a fleet policy snapshot object.
@@ -112,10 +112,8 @@ func (m *ClusterSchedulingPolicySnapshot) GetPolicySnapshotSpec() *SchedulingPol
112112
}
113113

114114
// SetPolicySnapshotSpec sets the policy snapshot spec.
115-
func (m *ClusterSchedulingPolicySnapshot) SetPolicySnapshotSpec(spec *SchedulingPolicySnapshotSpec) {
116-
if spec != nil {
117-
spec.DeepCopyInto(&m.Spec)
118-
}
115+
func (m *ClusterSchedulingPolicySnapshot) SetPolicySnapshotSpec(spec SchedulingPolicySnapshotSpec) {
116+
spec.DeepCopyInto(&m.Spec)
119117
}
120118

121119
// GetPolicySnapshotStatus returns the policy snapshot status.
@@ -124,10 +122,8 @@ func (m *ClusterSchedulingPolicySnapshot) GetPolicySnapshotStatus() *SchedulingP
124122
}
125123

126124
// SetPolicySnapshotStatus sets the policy snapshot status.
127-
func (m *ClusterSchedulingPolicySnapshot) SetPolicySnapshotStatus(status *SchedulingPolicySnapshotStatus) {
128-
if status != nil {
129-
status.DeepCopyInto(&m.Status)
130-
}
125+
func (m *ClusterSchedulingPolicySnapshot) SetPolicySnapshotStatus(status SchedulingPolicySnapshotStatus) {
126+
status.DeepCopyInto(&m.Status)
131127
}
132128

133129
// SchedulingPolicySnapshotSpec defines the desired state of SchedulingPolicySnapshot.
@@ -293,10 +289,8 @@ func (m *SchedulingPolicySnapshot) GetPolicySnapshotSpec() *SchedulingPolicySnap
293289
}
294290

295291
// SetPolicySnapshotSpec sets the policy snapshot spec.
296-
func (m *SchedulingPolicySnapshot) SetPolicySnapshotSpec(spec *SchedulingPolicySnapshotSpec) {
297-
if spec != nil {
298-
spec.DeepCopyInto(&m.Spec)
299-
}
292+
func (m *SchedulingPolicySnapshot) SetPolicySnapshotSpec(spec SchedulingPolicySnapshotSpec) {
293+
spec.DeepCopyInto(&m.Spec)
300294
}
301295

302296
// GetPolicySnapshotStatus returns the policy snapshot status.
@@ -305,10 +299,8 @@ func (m *SchedulingPolicySnapshot) GetPolicySnapshotStatus() *SchedulingPolicySn
305299
}
306300

307301
// SetPolicySnapshotStatus sets the policy snapshot status.
308-
func (m *SchedulingPolicySnapshot) SetPolicySnapshotStatus(status *SchedulingPolicySnapshotStatus) {
309-
if status != nil {
310-
status.DeepCopyInto(&m.Status)
311-
}
302+
func (m *SchedulingPolicySnapshot) SetPolicySnapshotStatus(status SchedulingPolicySnapshotStatus) {
303+
status.DeepCopyInto(&m.Status)
312304
}
313305

314306
// SchedulingPolicySnapshotList contains a list of SchedulingPolicySnapshotList.

‎apis/placement/v1beta1/resourcesnapshot_types.go‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ var _ ResourceSnapshotObjList = &ResourceSnapshotList{}
5858
// +kubebuilder:object:generate=false
5959
type ResourceSnapshotSpecGetterSetter interface {
6060
GetResourceSnapshotSpec() *ResourceSnapshotSpec
61-
SetResourceSnapshotSpec(*ResourceSnapshotSpec)
61+
SetResourceSnapshotSpec(ResourceSnapshotSpec)
6262
}
6363

6464
// A ResourceSnapshotStatusGetterSetter offers methods to get and set the resource snapshot status.
6565
// +kubebuilder:object:generate=false
6666
type ResourceSnapshotStatusGetterSetter interface {
6767
GetResourceSnapshotStatus() *ResourceSnapshotStatus
68-
SetResourceSnapshotStatus(*ResourceSnapshotStatus)
68+
SetResourceSnapshotStatus(ResourceSnapshotStatus)
6969
}
7070

7171
// A ResourceSnapshotObj offers an abstract way to work with a resource snapshot object.
@@ -184,10 +184,8 @@ func (m *ClusterResourceSnapshot) GetResourceSnapshotSpec() *ResourceSnapshotSpe
184184
}
185185

186186
// SetResourceSnapshotSpec sets the resource snapshot spec.
187-
func (m *ClusterResourceSnapshot) SetResourceSnapshotSpec(spec *ResourceSnapshotSpec) {
188-
if spec != nil {
189-
spec.DeepCopyInto(&m.Spec)
190-
}
187+
func (m *ClusterResourceSnapshot) SetResourceSnapshotSpec(spec ResourceSnapshotSpec) {
188+
spec.DeepCopyInto(&m.Spec)
191189
}
192190

193191
// GetResourceSnapshotStatus returns the resource snapshot status.
@@ -196,10 +194,8 @@ func (m *ClusterResourceSnapshot) GetResourceSnapshotStatus() *ResourceSnapshotS
196194
}
197195

198196
// SetResourceSnapshotStatus sets the resource snapshot status.
199-
func (m *ClusterResourceSnapshot) SetResourceSnapshotStatus(status *ResourceSnapshotStatus) {
200-
if status != nil {
201-
status.DeepCopyInto(&m.Status)
202-
}
197+
func (m *ClusterResourceSnapshot) SetResourceSnapshotStatus(status ResourceSnapshotStatus) {
198+
status.DeepCopyInto(&m.Status)
203199
}
204200

205201
// ClusterResourceSnapshotList returns the list of ResourceSnapshotObj from the ResourceSnapshotList.
@@ -280,10 +276,8 @@ func (m *ResourceSnapshot) GetResourceSnapshotSpec() *ResourceSnapshotSpec {
280276
}
281277

282278
// SetResourceSnapshotSpec sets the resource snapshot spec.
283-
func (m *ResourceSnapshot) SetResourceSnapshotSpec(spec *ResourceSnapshotSpec) {
284-
if spec != nil {
285-
spec.DeepCopyInto(&m.Spec)
286-
}
279+
func (m *ResourceSnapshot) SetResourceSnapshotSpec(spec ResourceSnapshotSpec) {
280+
spec.DeepCopyInto(&m.Spec)
287281
}
288282

289283
// GetResourceSnapshotStatus returns the resource snapshot status.
@@ -292,10 +286,8 @@ func (m *ResourceSnapshot) GetResourceSnapshotStatus() *ResourceSnapshotStatus {
292286
}
293287

294288
// SetResourceSnapshotStatus sets the resource snapshot status.
295-
func (m *ResourceSnapshot) SetResourceSnapshotStatus(status *ResourceSnapshotStatus) {
296-
if status != nil {
297-
status.DeepCopyInto(&m.Status)
298-
}
289+
func (m *ResourceSnapshot) SetResourceSnapshotStatus(status ResourceSnapshotStatus) {
290+
status.DeepCopyInto(&m.Status)
299291
}
300292

301293
// GetResourceSnapshotObjs returns the list of ResourceSnapshotObj from the ResourceSnapshotList.

‎pkg/utils/controller/placement_resolver.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controller
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"strings"
2223

2324
"k8s.io/apimachinery/pkg/types"
@@ -28,7 +29,7 @@ import (
2829
)
2930

3031
const (
31-
// namespaceSeparator is the separator used between namespace and name in placement keys
32+
// namespaceSeparator is the separator used between namespace and name in placement keys.
3233
namespaceSeparator = "/"
3334
)
3435

@@ -39,9 +40,9 @@ func FetchPlacementFromKey(ctx context.Context, c client.Client, placementKey qu
3940
// Check if the key contains a namespace separator
4041
if strings.Contains(keyStr, namespaceSeparator) {
4142
// This is a namespaced ResourcePlacement
42-
parts := strings.SplitN(keyStr, namespaceSeparator, 2)
43+
parts := strings.Split(keyStr, namespaceSeparator)
4344
if len(parts) != 2 {
44-
return nil, nil
45+
return nil, NewUnexpectedBehaviorError(fmt.Errorf("invalid placement key format: %s", keyStr))
4546
}
4647

4748
namespace := parts[0]
@@ -59,6 +60,9 @@ func FetchPlacementFromKey(ctx context.Context, c client.Client, placementKey qu
5960

6061
return rp, nil
6162
} else {
63+
if len(keyStr) == 0 {
64+
return nil, NewUnexpectedBehaviorError(fmt.Errorf("invalid placement key format: %s", keyStr))
65+
}
6266
// This is a cluster-scoped ClusterResourcePlacement
6367
crp := &fleetv1beta1.ClusterResourcePlacement{}
6468
key := types.NamespacedName{

0 commit comments

Comments
 (0)