Skip to content

Commit 3c35f49

Browse files
author
Ryan Zhang
committed
test
1 parent 23c55bc commit 3c35f49

File tree

5 files changed

+737
-114
lines changed

5 files changed

+737
-114
lines changed

‎pkg/controllers/rollout/controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ func determineBindingsToUpdate(
500500
readyBindings, canBeReadyBindings, canBeUnavailableBindings []placementv1beta1.BindingObj,
501501
) ([]toBeUpdatedBinding, []toBeUpdatedBinding) {
502502
toBeUpdatedBindingList := make([]toBeUpdatedBinding, 0)
503-
// TODO: Fix the bug that we don't shrink to zero when there are bindings that are not
503+
// TODO: Fix the bug that we don't shrink to zero when there are bindings that are not ready yet.
504504
// calculate the max number of bindings that can be unavailable according to user specified maxUnavailable
505505
maxNumberToRemove := calculateMaxToRemove(placementObj, targetNumber, readyBindings, canBeUnavailableBindings)
506506
// we can still update the bindings that are failed to apply already regardless of the maxNumberToRemove

‎pkg/utils/annotations/annotations.go‎

Lines changed: 24 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,30 @@ func ExtractNumOfClustersFromPolicySnapshot(policy fleetv1beta1.PolicySnapshotOb
4242
return numOfClusters, nil
4343
}
4444

45+
// ExtractSubindexFromResourceSnapshot is a helper function to extract subindex from ResourceSnapshot objects.
46+
func ExtractSubindexFromResourceSnapshot(snapshot fleetv1beta1.ResourceSnapshotObj) (doesExist bool, subindex int, err error) {
47+
annotations := snapshot.GetAnnotations()
48+
if annotations == nil {
49+
return false, 0, nil
50+
}
51+
52+
subindexStr, exists := annotations[fleetv1beta1.SubindexOfResourceSnapshotAnnotation]
53+
if !exists || subindexStr == "" {
54+
return false, 0, nil
55+
}
56+
57+
subindex, err = strconv.Atoi(subindexStr)
58+
if err != nil {
59+
return false, 0, fmt.Errorf("invalid subindex annotation value %q: %w", subindexStr, err)
60+
}
61+
62+
if subindex < 0 {
63+
return false, 0, fmt.Errorf("subindex cannot be negative: %d", subindex)
64+
}
65+
66+
return true, subindex, nil
67+
}
68+
4569
// ExtractObservedCRPGenerationFromPolicySnapshot extracts the observed CRP generation from policySnapshot.
4670
func ExtractObservedCRPGenerationFromPolicySnapshot(policy fleetv1beta1.PolicySnapshotObj) (int64, error) {
4771
crpGenerationStr, ok := policy.GetAnnotations()[fleetv1beta1.CRPGenerationAnnotation]
@@ -58,20 +82,6 @@ func ExtractObservedCRPGenerationFromPolicySnapshot(policy fleetv1beta1.PolicySn
5882
return int64(observedCRPGeneration), nil
5983
}
6084

61-
// ExtractSubindexFromClusterResourceSnapshot extracts the subindex value from the annotations of a clusterResourceSnapshot.
62-
func ExtractSubindexFromClusterResourceSnapshot(snapshot fleetv1beta1.ResourceSnapshotObj) (doesExist bool, subindex int, err error) {
63-
subindexStr, ok := snapshot.GetAnnotations()[fleetv1beta1.SubindexOfResourceSnapshotAnnotation]
64-
if !ok {
65-
return false, -1, nil
66-
}
67-
subindex, err = strconv.Atoi(subindexStr)
68-
if err != nil || subindex < 0 {
69-
return true, -1, fmt.Errorf("invalid annotation %s: %s is invalid: %w", fleetv1beta1.SubindexOfResourceSnapshotAnnotation, subindexStr, err)
70-
}
71-
72-
return true, subindex, nil
73-
}
74-
7585
// ExtractNumberOfResourceSnapshotsFromResourceSnapshot extracts the number of clusterResourceSnapshots in a group from the master clusterResourceSnapshot.
7686
func ExtractNumberOfResourceSnapshotsFromResourceSnapshot(snapshot fleetv1beta1.ResourceSnapshotObj) (int, error) {
7787
countAnnotation := snapshot.GetAnnotations()[fleetv1beta1.NumberOfResourceSnapshotsAnnotation]
@@ -125,27 +135,3 @@ func ParseResourceGroupHashFromAnnotation(resourceSnapshot fleetv1beta1.Resource
125135
}
126136
return v, nil
127137
}
128-
129-
// ExtractSubindexFromResourceSnapshot is a helper function to extract subindex from ResourceSnapshot objects.
130-
func ExtractSubindexFromResourceSnapshot(snapshot fleetv1beta1.ResourceSnapshotObj) (doesExist bool, subindex int, err error) {
131-
annotations := snapshot.GetAnnotations()
132-
if annotations == nil {
133-
return false, 0, nil
134-
}
135-
136-
subindexStr, exists := annotations[fleetv1beta1.SubindexOfResourceSnapshotAnnotation]
137-
if !exists || subindexStr == "" {
138-
return false, 0, nil
139-
}
140-
141-
subindex, err = strconv.Atoi(subindexStr)
142-
if err != nil {
143-
return false, 0, fmt.Errorf("invalid subindex annotation value %q: %w", subindexStr, err)
144-
}
145-
146-
if subindex < 0 {
147-
return false, 0, fmt.Errorf("subindex cannot be negative: %d", subindex)
148-
}
149-
150-
return true, subindex, nil
151-
}

‎pkg/utils/annotations/annotations_test.go‎

Lines changed: 177 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -102,80 +102,6 @@ func TestExtractNumOfClustersFromPolicySnapshot(t *testing.T) {
102102
}
103103
}
104104

105-
func TestExtractSubindexFromClusterResourceSnapshot(t *testing.T) {
106-
testCases := []struct {
107-
name string
108-
snapshot *fleetv1beta1.ClusterResourceSnapshot
109-
wantExist bool
110-
wantSubindex int
111-
wantError bool
112-
}{
113-
{
114-
name: "valid annotation",
115-
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
116-
ObjectMeta: metav1.ObjectMeta{
117-
Name: snapshotName,
118-
Annotations: map[string]string{
119-
fleetv1beta1.SubindexOfResourceSnapshotAnnotation: "1",
120-
},
121-
},
122-
},
123-
wantExist: true,
124-
wantSubindex: 1,
125-
},
126-
{
127-
name: "no annotation",
128-
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
129-
ObjectMeta: metav1.ObjectMeta{
130-
Name: snapshotName,
131-
},
132-
},
133-
wantExist: false,
134-
wantSubindex: -1,
135-
},
136-
{
137-
name: "invalid annotation: not an integer",
138-
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
139-
ObjectMeta: metav1.ObjectMeta{
140-
Name: snapshotName,
141-
Annotations: map[string]string{
142-
fleetv1beta1.SubindexOfResourceSnapshotAnnotation: "abc",
143-
},
144-
},
145-
},
146-
wantError: true,
147-
},
148-
{
149-
name: "invalid annotation: negative integer",
150-
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
151-
ObjectMeta: metav1.ObjectMeta{
152-
Name: snapshotName,
153-
Annotations: map[string]string{
154-
fleetv1beta1.SubindexOfResourceSnapshotAnnotation: "-1",
155-
},
156-
},
157-
},
158-
wantError: true,
159-
},
160-
}
161-
162-
for _, tc := range testCases {
163-
t.Run(tc.name, func(t *testing.T) {
164-
gotExist, gotSubindex, err := ExtractSubindexFromClusterResourceSnapshot(tc.snapshot)
165-
if tc.wantError {
166-
if err == nil {
167-
t.Fatalf("ExtractSubindexFromClusterResourceSnapshot() = %v, %v, want error", gotExist, gotSubindex)
168-
}
169-
return
170-
}
171-
172-
if gotExist != tc.wantExist || gotSubindex != tc.wantSubindex {
173-
t.Fatalf("ExtractSubindexFromClusterResourceSnapshot() = %v, %v, want %v, %v", gotExist, gotSubindex, tc.wantExist, tc.wantSubindex)
174-
}
175-
})
176-
}
177-
}
178-
179105
// TestExtractObservedCRPGenerationFromPolicySnapshot tests the ExtractObservedCRPGenerationFromPolicySnapshot function.
180106
func TestExtractObservedCRPGenerationFromPolicySnapshot(t *testing.T) {
181107
testCases := []struct {
@@ -451,3 +377,180 @@ func TestExtractNextResourceSnapshotCandidateDetectionTimeFromResourceSnapshot(t
451377
})
452378
}
453379
}
380+
func TestParseResourceGroupHashFromAnnotation(t *testing.T) {
381+
testCases := []struct {
382+
name string
383+
snapshot *fleetv1beta1.ClusterResourceSnapshot
384+
want string
385+
wantError bool
386+
}{
387+
{
388+
name: "valid annotation",
389+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
390+
ObjectMeta: metav1.ObjectMeta{
391+
Name: snapshotName,
392+
Annotations: map[string]string{
393+
fleetv1beta1.ResourceGroupHashAnnotation: "abc123",
394+
},
395+
},
396+
},
397+
want: "abc123",
398+
},
399+
{
400+
name: "no annotations",
401+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
402+
ObjectMeta: metav1.ObjectMeta{
403+
Name: snapshotName,
404+
},
405+
},
406+
wantError: true,
407+
},
408+
{
409+
name: "annotation not set",
410+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
411+
ObjectMeta: metav1.ObjectMeta{
412+
Name: snapshotName,
413+
Annotations: map[string]string{
414+
"other-annotation": "value",
415+
},
416+
},
417+
},
418+
wantError: true,
419+
},
420+
{
421+
name: "empty annotation value",
422+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
423+
ObjectMeta: metav1.ObjectMeta{
424+
Name: snapshotName,
425+
Annotations: map[string]string{
426+
fleetv1beta1.ResourceGroupHashAnnotation: "",
427+
},
428+
},
429+
},
430+
want: "",
431+
},
432+
}
433+
434+
for _, tc := range testCases {
435+
t.Run(tc.name, func(t *testing.T) {
436+
got, err := ParseResourceGroupHashFromAnnotation(tc.snapshot)
437+
if gotErr := err != nil; gotErr != tc.wantError {
438+
t.Fatalf("ParseResourceGroupHashFromAnnotation() got err %v, want err %v", err, tc.wantError)
439+
}
440+
if !tc.wantError && got != tc.want {
441+
t.Fatalf("ParseResourceGroupHashFromAnnotation() got %s, want %s", got, tc.want)
442+
}
443+
})
444+
}
445+
}
446+
447+
func TestExtractSubindexFromResourceSnapshot(t *testing.T) {
448+
testCases := []struct {
449+
name string
450+
snapshot *fleetv1beta1.ClusterResourceSnapshot
451+
wantExist bool
452+
wantSubindex int
453+
wantError bool
454+
}{
455+
{
456+
name: "valid annotation",
457+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
458+
ObjectMeta: metav1.ObjectMeta{
459+
Name: snapshotName,
460+
Annotations: map[string]string{
461+
fleetv1beta1.SubindexOfResourceSnapshotAnnotation: "1",
462+
},
463+
},
464+
},
465+
wantExist: true,
466+
wantSubindex: 1,
467+
},
468+
{
469+
name: "no annotations",
470+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
471+
ObjectMeta: metav1.ObjectMeta{
472+
Name: snapshotName,
473+
},
474+
},
475+
wantExist: false,
476+
wantSubindex: 0,
477+
},
478+
{
479+
name: "annotation not set",
480+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
481+
ObjectMeta: metav1.ObjectMeta{
482+
Name: snapshotName,
483+
Annotations: map[string]string{
484+
"other-annotation": "value",
485+
},
486+
},
487+
},
488+
wantExist: false,
489+
wantSubindex: 0,
490+
},
491+
{
492+
name: "empty annotation value",
493+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
494+
ObjectMeta: metav1.ObjectMeta{
495+
Name: snapshotName,
496+
Annotations: map[string]string{
497+
fleetv1beta1.SubindexOfResourceSnapshotAnnotation: "",
498+
},
499+
},
500+
},
501+
wantExist: false,
502+
wantSubindex: 0,
503+
},
504+
{
505+
name: "invalid annotation: not an integer",
506+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
507+
ObjectMeta: metav1.ObjectMeta{
508+
Name: snapshotName,
509+
Annotations: map[string]string{
510+
fleetv1beta1.SubindexOfResourceSnapshotAnnotation: "abc",
511+
},
512+
},
513+
},
514+
wantError: true,
515+
},
516+
{
517+
name: "invalid annotation: negative integer",
518+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
519+
ObjectMeta: metav1.ObjectMeta{
520+
Name: snapshotName,
521+
Annotations: map[string]string{
522+
fleetv1beta1.SubindexOfResourceSnapshotAnnotation: "-1",
523+
},
524+
},
525+
},
526+
wantError: true,
527+
},
528+
{
529+
name: "valid annotation with zero value",
530+
snapshot: &fleetv1beta1.ClusterResourceSnapshot{
531+
ObjectMeta: metav1.ObjectMeta{
532+
Name: snapshotName,
533+
Annotations: map[string]string{
534+
fleetv1beta1.SubindexOfResourceSnapshotAnnotation: "0",
535+
},
536+
},
537+
},
538+
wantExist: true,
539+
wantSubindex: 0,
540+
},
541+
}
542+
543+
for _, tc := range testCases {
544+
t.Run(tc.name, func(t *testing.T) {
545+
gotExist, gotSubindex, err := ExtractSubindexFromResourceSnapshot(tc.snapshot)
546+
if gotErr := err != nil; gotErr != tc.wantError {
547+
t.Fatalf("ExtractSubindexFromResourceSnapshot() got err %v, want err %v", err, tc.wantError)
548+
}
549+
if !tc.wantError {
550+
if gotExist != tc.wantExist || gotSubindex != tc.wantSubindex {
551+
t.Fatalf("ExtractSubindexFromResourceSnapshot() = %v, %v, want %v, %v", gotExist, gotSubindex, tc.wantExist, tc.wantSubindex)
552+
}
553+
}
554+
})
555+
}
556+
}

‎pkg/utils/controller/controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ func CollectResourceIdentifiersFromResourceSnapshot(
408408
}
409409
if masterResourceSnapshot == nil {
410410
err := NewUnexpectedBehaviorError(fmt.Errorf("no master resourceSnapshot found for placement `%s`", placementKey))
411-
klog.ErrorS(err, "Found resourceSnapshots without master snapshot", "placement", placementKey, "resourceSnapshotIndex", resourceSnapshotIndex, "resourceSnapshotCount", len(items))
411+
klog.ErrorS(err, "Found resourceSnapshots without master resource Snapshot", "placement", placementKey, "resourceSnapshotIndex", resourceSnapshotIndex, "resourceSnapshotCount", len(items))
412412
return nil, err
413413
}
414414

0 commit comments

Comments
 (0)