Skip to content

Commit 5c38041

Browse files
zhiying-linzhiying-lin
authored
fix: fix the service export condition (#281)
1 parent bf4930e commit 5c38041

File tree

14 files changed

+280
-219
lines changed

14 files changed

+280
-219
lines changed

‎pkg/common/condition/condition.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func UnconflictedServiceExportConflictCondition(internalServiceExport fleetnetv1
7676
Type: string(fleetnetv1alpha1.ServiceExportConflict),
7777
Status: metav1.ConditionFalse,
7878
Reason: conditionReasonNoConflictFound,
79-
ObservedGeneration: internalServiceExport.Spec.ServiceReference.Generation, // use the generation of the original object
79+
ObservedGeneration: internalServiceExport.Generation,
8080
Message: fmt.Sprintf("service %s is exported without conflict", svcName),
8181
}
8282
}
@@ -91,7 +91,7 @@ func ConflictedServiceExportConflictCondition(internalServiceExport fleetnetv1al
9191
Type: string(fleetnetv1alpha1.ServiceExportConflict),
9292
Status: metav1.ConditionTrue,
9393
Reason: conditionReasonConflictFound,
94-
ObservedGeneration: internalServiceExport.Spec.ServiceReference.Generation, // use the generation of the original object
94+
ObservedGeneration: internalServiceExport.Generation,
9595
Message: fmt.Sprintf("service %s is in conflict with other exported services", svcName),
9696
}
9797
}

‎pkg/common/condition/condition_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,9 @@ func TestEqualConditionIgnoreReason(t *testing.T) {
236236

237237
func TestUnconflictedServiceExportConflictCondition(t *testing.T) {
238238
input := fleetnetv1alpha1.InternalServiceExport{
239+
ObjectMeta: metav1.ObjectMeta{
240+
Generation: 456,
241+
},
239242
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
240243
Ports: []fleetnetv1alpha1.ServicePort{
241244
{
@@ -267,7 +270,7 @@ func TestUnconflictedServiceExportConflictCondition(t *testing.T) {
267270
Type: string(fleetnetv1alpha1.ServiceExportConflict),
268271
Status: metav1.ConditionFalse,
269272
Reason: conditionReasonNoConflictFound,
270-
ObservedGeneration: 123, // use the generation of the original object
273+
ObservedGeneration: 456,
271274
Message: "service test-ns/test-svc is exported without conflict",
272275
}
273276
got := UnconflictedServiceExportConflictCondition(input)
@@ -278,6 +281,9 @@ func TestUnconflictedServiceExportConflictCondition(t *testing.T) {
278281

279282
func TestConflictedServiceExportConflictCondition(t *testing.T) {
280283
input := fleetnetv1alpha1.InternalServiceExport{
284+
ObjectMeta: metav1.ObjectMeta{
285+
Generation: 123,
286+
},
281287
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
282288
Ports: []fleetnetv1alpha1.ServicePort{
283289
{
@@ -300,7 +306,7 @@ func TestConflictedServiceExportConflictCondition(t *testing.T) {
300306
Namespace: "test-ns",
301307
Name: "test-svc",
302308
ResourceVersion: "0",
303-
Generation: 123,
309+
Generation: 456,
304310
UID: "0",
305311
},
306312
},

‎pkg/controllers/hub/internalserviceexport/controller_integration_test.go‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,15 @@ var _ = Describe("Test InternalServiceExport Controller", func() {
153153

154154
By("Checking internalServiceExportA status")
155155
Eventually(func() string {
156-
want := fleetnetv1alpha1.InternalServiceExportStatus{
157-
Conditions: []metav1.Condition{
158-
unconflictedServiceExportConflictCondition(testNamespace, testServiceName),
159-
},
160-
}
161156
key := types.NamespacedName{Namespace: testMemberClusterA, Name: testName}
162157
if err := k8sClient.Get(ctx, key, internalServiceExportA); err != nil {
163158
return err.Error()
164159
}
160+
want := fleetnetv1alpha1.InternalServiceExportStatus{
161+
Conditions: []metav1.Condition{
162+
unconflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportA.Generation),
163+
},
164+
}
165165
return cmp.Diff(want, internalServiceExportA.Status, options...)
166166
}, timeout, interval).Should(BeEmpty())
167167

@@ -170,15 +170,15 @@ var _ = Describe("Test InternalServiceExport Controller", func() {
170170

171171
By("Checking internalServiceExportB status")
172172
Eventually(func() string {
173-
want := fleetnetv1alpha1.InternalServiceExportStatus{
174-
Conditions: []metav1.Condition{
175-
unconflictedServiceExportConflictCondition(testNamespace, testServiceName),
176-
},
177-
}
178173
key := types.NamespacedName{Namespace: testMemberClusterB, Name: testName}
179174
if err := k8sClient.Get(ctx, key, internalServiceExportB); err != nil {
180175
return err.Error()
181176
}
177+
want := fleetnetv1alpha1.InternalServiceExportStatus{
178+
Conditions: []metav1.Condition{
179+
unconflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportB.Generation),
180+
},
181+
}
182182
return cmp.Diff(want, internalServiceExportB.Status, options...)
183183
}, timeout, interval).Should(BeEmpty())
184184

@@ -313,15 +313,15 @@ var _ = Describe("Test InternalServiceExport Controller", func() {
313313

314314
By("Checking internalServiceExportA status")
315315
Eventually(func() string {
316-
want := fleetnetv1alpha1.InternalServiceExportStatus{
317-
Conditions: []metav1.Condition{
318-
unconflictedServiceExportConflictCondition(testNamespace, testServiceName),
319-
},
320-
}
321316
key := types.NamespacedName{Namespace: testMemberClusterA, Name: testName}
322317
if err := k8sClient.Get(ctx, key, internalServiceExportA); err != nil {
323318
return err.Error()
324319
}
320+
want := fleetnetv1alpha1.InternalServiceExportStatus{
321+
Conditions: []metav1.Condition{
322+
unconflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportA.Generation),
323+
},
324+
}
325325
return cmp.Diff(want, internalServiceExportA.Status, options...)
326326
}, timeout, interval).Should(BeEmpty())
327327

@@ -386,15 +386,15 @@ var _ = Describe("Test InternalServiceExport Controller", func() {
386386

387387
By("Checking internalServiceExportA status")
388388
Eventually(func() string {
389-
want := fleetnetv1alpha1.InternalServiceExportStatus{
390-
Conditions: []metav1.Condition{
391-
conflictedServiceExportConflictCondition(testNamespace, testServiceName),
392-
},
393-
}
394389
key := types.NamespacedName{Namespace: testMemberClusterA, Name: testName}
395390
if err := k8sClient.Get(ctx, key, internalServiceExportA); err != nil {
396391
return err.Error()
397392
}
393+
want := fleetnetv1alpha1.InternalServiceExportStatus{
394+
Conditions: []metav1.Condition{
395+
conflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportA.Generation),
396+
},
397+
}
398398
return cmp.Diff(want, internalServiceExportA.Status, options...)
399399
}, timeout, interval).Should(BeEmpty())
400400

‎pkg/controllers/hub/internalserviceexport/controller_test.go‎

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -92,22 +92,22 @@ func internalServiceExportReconciler(client client.Client) *Reconciler {
9292
}
9393
}
9494

95-
func unconflictedServiceExportConflictCondition(svcNamespace string, svcName string) metav1.Condition {
95+
func unconflictedServiceExportConflictCondition(svcNamespace string, svcName string, observedGeneration int64) metav1.Condition {
9696
return metav1.Condition{
9797
Type: string(fleetnetv1alpha1.ServiceExportConflict),
9898
Status: metav1.ConditionFalse,
99-
ObservedGeneration: 0,
99+
ObservedGeneration: observedGeneration,
100100
LastTransitionTime: metav1.Now(),
101101
Reason: conditionReasonNoConflictFound,
102102
Message: fmt.Sprintf("service %s/%s is exported without conflict", svcNamespace, svcName),
103103
}
104104
}
105105

106-
func conflictedServiceExportConflictCondition(svcNamespace string, svcName string) metav1.Condition {
106+
func conflictedServiceExportConflictCondition(svcNamespace string, svcName string, observedGeneration int64) metav1.Condition {
107107
return metav1.Condition{
108108
Type: string(fleetnetv1alpha1.ServiceExportConflict),
109109
Status: metav1.ConditionTrue,
110-
ObservedGeneration: 0,
110+
ObservedGeneration: observedGeneration,
111111
LastTransitionTime: metav1.Now(),
112112
Reason: conditionReasonConflictFound,
113113
Message: fmt.Sprintf("service %s/%s is in conflict with other exported services", svcNamespace, svcName),
@@ -370,6 +370,7 @@ func TestHandleDelete_EmptyServiceImportSpec(t *testing.T) {
370370
}
371371

372372
func TestHandleUpdate(t *testing.T) {
373+
internalServiceExportGeneration := int64(456)
373374
importServicePorts := []fleetnetv1alpha1.ServicePort{
374375
{
375376
Name: "portA",
@@ -397,8 +398,9 @@ func TestHandleUpdate(t *testing.T) {
397398
name: "no serviceImport exists",
398399
internalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
399400
ObjectMeta: metav1.ObjectMeta{
400-
Name: testName,
401-
Namespace: testMemberNamespace,
401+
Name: testName,
402+
Namespace: testMemberNamespace,
403+
Generation: internalServiceExportGeneration,
402404
},
403405
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
404406
Ports: []fleetnetv1alpha1.ServicePort{
@@ -430,8 +432,9 @@ func TestHandleUpdate(t *testing.T) {
430432
want: ctrl.Result{RequeueAfter: internalserviceexportRetryInterval},
431433
wantInternalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
432434
ObjectMeta: metav1.ObjectMeta{
433-
Name: testName,
434-
Namespace: testMemberNamespace,
435+
Name: testName,
436+
Namespace: testMemberNamespace,
437+
Generation: internalServiceExportGeneration,
435438
},
436439
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
437440
Ports: []fleetnetv1alpha1.ServicePort{
@@ -471,8 +474,9 @@ func TestHandleUpdate(t *testing.T) {
471474
name: "serviceExport just created and has the same spec as serviceImport",
472475
internalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
473476
ObjectMeta: metav1.ObjectMeta{
474-
Name: testName,
475-
Namespace: testMemberNamespace,
477+
Name: testName,
478+
Namespace: testMemberNamespace,
479+
Generation: internalServiceExportGeneration,
476480
},
477481
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
478482
Ports: importServicePorts,
@@ -505,8 +509,9 @@ func TestHandleUpdate(t *testing.T) {
505509
want: ctrl.Result{},
506510
wantInternalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
507511
ObjectMeta: metav1.ObjectMeta{
508-
Name: testName,
509-
Namespace: testMemberNamespace,
512+
Name: testName,
513+
Namespace: testMemberNamespace,
514+
Generation: internalServiceExportGeneration,
510515
},
511516
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
512517
Ports: importServicePorts,
@@ -522,7 +527,7 @@ func TestHandleUpdate(t *testing.T) {
522527
},
523528
Status: fleetnetv1alpha1.InternalServiceExportStatus{
524529
Conditions: []metav1.Condition{
525-
unconflictedServiceExportConflictCondition(testNamespace, testServiceName),
530+
unconflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportGeneration),
526531
},
527532
},
528533
},
@@ -549,8 +554,9 @@ func TestHandleUpdate(t *testing.T) {
549554
name: "serviceExport just created and has the different spec as serviceImport",
550555
internalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
551556
ObjectMeta: metav1.ObjectMeta{
552-
Name: testName,
553-
Namespace: testMemberNamespace,
557+
Name: testName,
558+
Namespace: testMemberNamespace,
559+
Generation: internalServiceExportGeneration,
554560
},
555561
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
556562
Ports: []fleetnetv1alpha1.ServicePort{
@@ -591,8 +597,9 @@ func TestHandleUpdate(t *testing.T) {
591597
want: ctrl.Result{},
592598
wantInternalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
593599
ObjectMeta: metav1.ObjectMeta{
594-
Name: testName,
595-
Namespace: testMemberNamespace,
600+
Name: testName,
601+
Namespace: testMemberNamespace,
602+
Generation: internalServiceExportGeneration,
596603
},
597604
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
598605
Ports: []fleetnetv1alpha1.ServicePort{
@@ -616,7 +623,7 @@ func TestHandleUpdate(t *testing.T) {
616623
},
617624
Status: fleetnetv1alpha1.InternalServiceExportStatus{
618625
Conditions: []metav1.Condition{
619-
conflictedServiceExportConflictCondition(testNamespace, testServiceName),
626+
conflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportGeneration),
620627
},
621628
},
622629
},
@@ -640,8 +647,9 @@ func TestHandleUpdate(t *testing.T) {
640647
name: "update serviceExport and old serviceExport has the same spec as serviceImport",
641648
internalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
642649
ObjectMeta: metav1.ObjectMeta{
643-
Name: testName,
644-
Namespace: testMemberNamespace,
650+
Name: testName,
651+
Namespace: testMemberNamespace,
652+
Generation: internalServiceExportGeneration,
645653
},
646654
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
647655
Ports: []fleetnetv1alpha1.ServicePort{
@@ -665,7 +673,7 @@ func TestHandleUpdate(t *testing.T) {
665673
},
666674
Status: fleetnetv1alpha1.InternalServiceExportStatus{
667675
Conditions: []metav1.Condition{
668-
unconflictedServiceExportConflictCondition(testNamespace, testServiceName),
676+
unconflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportGeneration),
669677
},
670678
},
671679
},
@@ -690,8 +698,9 @@ func TestHandleUpdate(t *testing.T) {
690698
want: ctrl.Result{},
691699
wantInternalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
692700
ObjectMeta: metav1.ObjectMeta{
693-
Name: testName,
694-
Namespace: testMemberNamespace,
701+
Name: testName,
702+
Namespace: testMemberNamespace,
703+
Generation: internalServiceExportGeneration,
695704
},
696705
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
697706
Ports: []fleetnetv1alpha1.ServicePort{
@@ -715,7 +724,7 @@ func TestHandleUpdate(t *testing.T) {
715724
},
716725
Status: fleetnetv1alpha1.InternalServiceExportStatus{
717726
Conditions: []metav1.Condition{
718-
conflictedServiceExportConflictCondition(testNamespace, testServiceName),
727+
conflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportGeneration),
719728
},
720729
},
721730
},
@@ -739,8 +748,9 @@ func TestHandleUpdate(t *testing.T) {
739748
name: "update serviceExport and old serviceExport has the different spec as serviceImport",
740749
internalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
741750
ObjectMeta: metav1.ObjectMeta{
742-
Name: testName,
743-
Namespace: testMemberNamespace,
751+
Name: testName,
752+
Namespace: testMemberNamespace,
753+
Generation: internalServiceExportGeneration,
744754
},
745755
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
746756
Ports: importServicePorts,
@@ -756,7 +766,7 @@ func TestHandleUpdate(t *testing.T) {
756766
},
757767
Status: fleetnetv1alpha1.InternalServiceExportStatus{
758768
Conditions: []metav1.Condition{
759-
conflictedServiceExportConflictCondition(testNamespace, testServiceName),
769+
conflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportGeneration),
760770
},
761771
},
762772
},
@@ -778,8 +788,9 @@ func TestHandleUpdate(t *testing.T) {
778788
want: ctrl.Result{},
779789
wantInternalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
780790
ObjectMeta: metav1.ObjectMeta{
781-
Name: testName,
782-
Namespace: testMemberNamespace,
791+
Name: testName,
792+
Namespace: testMemberNamespace,
793+
Generation: internalServiceExportGeneration,
783794
},
784795
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
785796
Ports: importServicePorts,
@@ -795,7 +806,7 @@ func TestHandleUpdate(t *testing.T) {
795806
},
796807
Status: fleetnetv1alpha1.InternalServiceExportStatus{
797808
Conditions: []metav1.Condition{
798-
unconflictedServiceExportConflictCondition(testNamespace, testServiceName),
809+
unconflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportGeneration),
799810
},
800811
},
801812
},
@@ -822,8 +833,9 @@ func TestHandleUpdate(t *testing.T) {
822833
name: "there is only one serviceExport and port spec has been changed",
823834
internalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
824835
ObjectMeta: metav1.ObjectMeta{
825-
Name: testName,
826-
Namespace: testMemberNamespace,
836+
Name: testName,
837+
Namespace: testMemberNamespace,
838+
Generation: internalServiceExportGeneration,
827839
},
828840
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
829841
Ports: []fleetnetv1alpha1.ServicePort{
@@ -847,7 +859,7 @@ func TestHandleUpdate(t *testing.T) {
847859
},
848860
Status: fleetnetv1alpha1.InternalServiceExportStatus{
849861
Conditions: []metav1.Condition{
850-
unconflictedServiceExportConflictCondition(testNamespace, testServiceName),
862+
unconflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportGeneration),
851863
},
852864
},
853865
},
@@ -869,8 +881,9 @@ func TestHandleUpdate(t *testing.T) {
869881
want: ctrl.Result{RequeueAfter: internalserviceexportRetryInterval},
870882
wantInternalSvcExport: &fleetnetv1alpha1.InternalServiceExport{
871883
ObjectMeta: metav1.ObjectMeta{
872-
Name: testName,
873-
Namespace: testMemberNamespace,
884+
Name: testName,
885+
Namespace: testMemberNamespace,
886+
Generation: internalServiceExportGeneration,
874887
},
875888
Spec: fleetnetv1alpha1.InternalServiceExportSpec{
876889
Ports: []fleetnetv1alpha1.ServicePort{
@@ -894,7 +907,7 @@ func TestHandleUpdate(t *testing.T) {
894907
},
895908
Status: fleetnetv1alpha1.InternalServiceExportStatus{
896909
Conditions: []metav1.Condition{
897-
unconflictedServiceExportConflictCondition(testNamespace, testServiceName),
910+
unconflictedServiceExportConflictCondition(testNamespace, testServiceName, internalServiceExportGeneration),
898911
},
899912
},
900913
},
@@ -928,7 +941,7 @@ func TestHandleUpdate(t *testing.T) {
928941
}
929942
want := tc.want
930943
if !cmp.Equal(got, want) {
931-
t.Errorf("handleDelete() = %+v, want %+v", got, want)
944+
t.Errorf("handleUpdate() = %+v, want %+v", got, want)
932945
}
933946
options := []cmp.Option{
934947
cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion"),

0 commit comments

Comments
 (0)