Skip to content

Commit 531157a

Browse files
WeichengWang1WeichengWang1
authored
feat: add ValidateOptions (#58)
1 parent 7c8ef2f commit 531157a

6 files changed

Lines changed: 163 additions & 7 deletions

File tree

‎pkg/frame/controller/consister.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -334,13 +334,7 @@ func (r *Consist) ensureExpectedFinalizerNotNeedRecord(ctx context.Context, empl
334334
Succeed: false,
335335
})
336336
}
337-
errPatchEmployees := r.patchPodExpectedFinalizer(ctx, employer, toAdd, toDelete)
338-
for _, deleteExpectedFinalizerOps := range toDelete {
339-
if deleteExpectedFinalizerOps.Succeed {
340-
return true, errPatchEmployees
341-
}
342-
}
343-
return false, errPatchEmployees
337+
return false, r.patchPodExpectedFinalizer(ctx, employer, toAdd, toDelete)
344338
}
345339

346340
func (r *Consist) ensureExpectedFinalizerNeedRecord(ctx context.Context, employer client.Object, selectedEmployeeNames []string) (bool, error) {

‎pkg/frame/controller/consts.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,5 @@ const (
4040
CleanEmployerCleanFinalizerSucceed = "CleanEmployerCleanFinalizerSucceed"
4141
RecordStatusesFailed = "RecordStatusesFailed"
4242
RecordErrorConditionsFailed = "RecordErrorConditionsFailed"
43+
ValidateEmployerFailed = "ValidateEmployerFailed"
4344
)

‎pkg/frame/controller/resourceconsist_controller.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,21 @@ func (r *Consist) Reconcile(ctx context.Context, request reconcile.Request) (rec
159159
return reconcile.Result{}, err
160160
}
161161

162+
// Validate employer if ValidateOptions is implemented
163+
if validateOptions, ok := r.adapter.(ValidateOptions); ok {
164+
isvalid, validateErr := validateOptions.Validate(ctx, employer)
165+
if validateErr != nil {
166+
logger.Error(validateErr, "validate employer failed")
167+
r.recorder.Eventf(employer, corev1.EventTypeWarning, ValidateEmployerFailed,
168+
"validate employer failed: %s", validateErr.Error())
169+
return reconcile.Result{}, validateErr
170+
}
171+
if !isvalid {
172+
logger.Info("employer is not valid, skip reconciliation")
173+
return reconcile.Result{}, nil
174+
}
175+
}
176+
162177
defer func() {
163178
if err != nil {
164179
if recordOptions, ok := r.adapter.(StatusRecordOptions); ok {

‎pkg/frame/controller/resourceconsist_controller_suite_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ type DemoControllerAdapter struct {
3636
var _ ReconcileAdapter = &DemoControllerAdapter{}
3737
var _ ReconcileLifecycleOptions = &DemoControllerAdapter{}
3838
var _ StatusRecordOptions = &DemoControllerAdapter{}
39+
var _ ValidateOptions = &DemoControllerAdapter{}
3940

4041
var needRecordEmployees = false
4142

@@ -102,6 +103,17 @@ func (r *DemoControllerAdapter) GetSelectedEmployeeNames(ctx context.Context, em
102103
return selected, nil
103104
}
104105

106+
func (r *DemoControllerAdapter) Validate(ctx context.Context, employer client.Object) (bool, error) {
107+
// Check if employer has invalid label
108+
labels := employer.GetLabels()
109+
if labels != nil {
110+
if _, exists := labels["resource-consist.kusionstack.io/invalid"]; exists {
111+
return false, nil
112+
}
113+
}
114+
return true, nil
115+
}
116+
105117
func (r *DemoControllerAdapter) GetControllerName() string {
106118
return "demo-controller"
107119
}

‎pkg/frame/controller/resourceconsit_controller_test.go‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,127 @@ var _ = Describe("resource-consist-controller", func() {
614614
})
615615
})
616616

617+
Context("validate employer", func() {
618+
svc4 := corev1.Service{
619+
ObjectMeta: v1.ObjectMeta{
620+
Name: "resource-consist-ut-svc-4",
621+
Namespace: "default",
622+
Labels: map[string]string{
623+
v1alpha1.ControlledByKusionStackLabelKey: "true",
624+
"resource-consist.kusionstack.io/invalid": "true",
625+
},
626+
},
627+
Spec: corev1.ServiceSpec{
628+
Ports: []corev1.ServicePort{
629+
{
630+
Name: "tcp-80",
631+
Port: 80,
632+
Protocol: corev1.ProtocolTCP,
633+
},
634+
},
635+
Selector: map[string]string{
636+
"resource-consist-ut": "resource-consist-ut-4",
637+
},
638+
},
639+
}
640+
641+
It("employer with invalid label should not reconcile", func() {
642+
rc.ExpectedCalls = nil
643+
rc.On("QueryVip", mock.Anything).Return(&DemoResourceVipOps{}, nil)
644+
rc.On("CreateVip", mock.Anything).Return(&DemoResourceVipOps{}, nil)
645+
rc.On("UpdateVip", mock.Anything).Return(&DemoResourceVipOps{}, nil)
646+
rc.On("DeleteVip", mock.Anything).Return(&DemoResourceVipOps{}, nil)
647+
rc.On("QueryRealServer", mock.Anything).Return(&DemoResourceRsOps{}, nil)
648+
rc.On("CreateRealServer", mock.Anything).Return(&DemoResourceRsOps{}, nil)
649+
rc.On("UpdateRealServer", mock.Anything).Return(&DemoResourceRsOps{}, nil)
650+
rc.On("DeleteRealServer", mock.Anything).Return(&DemoResourceRsOps{}, nil)
651+
652+
Expect(mgr.GetClient().Create(context.Background(), &svc4)).Should(BeNil())
653+
654+
// Employer should NOT be created in backend provider because validation failed
655+
Consistently(func() bool {
656+
_, exist := demoResourceVipStatusInProvider.Load(svc4.Name)
657+
return !exist
658+
}, 2*time.Second, 100*time.Millisecond).Should(BeTrue())
659+
660+
// Clean finalizer should NOT be added because validation failed
661+
Consistently(func() bool {
662+
svcTmp := corev1.Service{}
663+
err := mgr.GetClient().Get(context.TODO(), types.NamespacedName{
664+
Name: svc4.Name,
665+
Namespace: svc4.Namespace,
666+
}, &svcTmp)
667+
if err != nil {
668+
return false
669+
}
670+
for _, flz := range svcTmp.GetFinalizers() {
671+
if flz == cleanFinalizer {
672+
return false
673+
}
674+
}
675+
return true
676+
}, 2*time.Second, 100*time.Millisecond).Should(BeTrue())
677+
})
678+
679+
It("after removing invalid label, employer should be reconciled", func() {
680+
// Remove invalid label
681+
Eventually(func() bool {
682+
svcTmp := corev1.Service{}
683+
err := mgr.GetClient().Get(context.TODO(), types.NamespacedName{
684+
Name: svc4.Name,
685+
Namespace: svc4.Namespace,
686+
}, &svcTmp)
687+
if err != nil {
688+
return false
689+
}
690+
delete(svcTmp.Labels, "resource-consist.kusionstack.io/invalid")
691+
return mgr.GetClient().Update(context.TODO(), &svcTmp) == nil
692+
}, 3*time.Second, 100*time.Millisecond).Should(BeTrue())
693+
694+
// Clean finalizer should be added now
695+
Eventually(func() bool {
696+
svcTmp := corev1.Service{}
697+
err := mgr.GetClient().Get(context.TODO(), types.NamespacedName{
698+
Name: svc4.Name,
699+
Namespace: svc4.Namespace,
700+
}, &svcTmp)
701+
if err != nil {
702+
return false
703+
}
704+
for _, flz := range svcTmp.GetFinalizers() {
705+
if flz == cleanFinalizer {
706+
return true
707+
}
708+
}
709+
return false
710+
}, 3*time.Second, 100*time.Millisecond).Should(BeTrue())
711+
712+
// Now employer should be created in backend provider
713+
Eventually(func() bool {
714+
details, exist := demoResourceVipStatusInProvider.Load(svc4.Name)
715+
return exist && details.(DemoServiceDetails).RemoteVIP == "demo-remote-VIP" && details.(DemoServiceDetails).RemoteVIPQPS == 100
716+
}, 3*time.Second, 100*time.Millisecond).Should(BeTrue())
717+
})
718+
719+
It("clean up svc4", func() {
720+
Expect(mgr.GetClient().Delete(context.TODO(), &svc4)).Should(BeNil())
721+
722+
Eventually(func() bool {
723+
svcTmp := corev1.Service{}
724+
err := mgr.GetClient().Get(context.TODO(), types.NamespacedName{
725+
Name: svc4.Name,
726+
Namespace: svc4.Namespace,
727+
}, &svcTmp)
728+
return errors.IsNotFound(err)
729+
}, 3*time.Second, 100*time.Millisecond).Should(BeTrue())
730+
731+
Eventually(func() bool {
732+
_, exist := demoResourceVipStatusInProvider.Load(svc4.Name)
733+
return !exist
734+
}, 3*time.Second, 100*time.Millisecond).Should(BeTrue())
735+
})
736+
})
737+
617738
Context("label/selector change", func() {
618739
svc3 := corev1.Service{
619740
ObjectMeta: v1.ObjectMeta{

‎pkg/frame/controller/types.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,19 @@ type ReconcileRequeueOptions interface {
9797
EmployeeSyncRequeueInterval() time.Duration
9898
}
9999

100+
// ValidateOptions defines validation method for employer resource.
101+
// If not implemented, no validation is performed and the employer is considered valid.
102+
// This is useful when validation logic needs to halt reconciliation until the employer is validated.
103+
type ValidateOptions interface {
104+
// Validate validates the employer resource after it's fetched from the cluster.
105+
// Returns (isvalid, err):
106+
// - valid=true, err=nil: validation passed, continue reconciliation
107+
// - valid=false, err=nil: validation failed but no error, stop reconciliation (return reconcile.Result{}, nil)
108+
// - valid=false, err!=nil: validation failed with error, return error and retry
109+
// - valid=true, err!=nil: should not happen, treated as error
110+
Validate(ctx context.Context, employer client.Object) (valid bool, err error)
111+
}
112+
100113
// ReconcileAdapter is the interface that customized controllers should implement.
101114
type ReconcileAdapter interface {
102115
GetControllerName() string

0 commit comments

Comments
 (0)