Skip to content

Commit 41bca85

Browse files
robotmaxtronlburgazzoli
authored andcommitted
enhancement: adding workload validation logic that mirrors the ComponentBuilder.InState logic. additional workload tests added.
Signed-off-by: Max Whittingham <mwhittingham@redhat.com>
1 parent 42a9907 commit 41bca85

File tree

2 files changed

+325
-5
lines changed

2 files changed

+325
-5
lines changed

pkg/lint/check/validate/workload.go

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@ import (
55
"fmt"
66
"strconv"
77

8+
apierrors "k8s.io/apimachinery/pkg/api/errors"
89
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
910
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1011

12+
"github.com/opendatahub-io/odh-cli/pkg/constants"
1113
"github.com/opendatahub-io/odh-cli/pkg/lint/check"
1214
"github.com/opendatahub-io/odh-cli/pkg/lint/check/result"
1315
"github.com/opendatahub-io/odh-cli/pkg/resources"
1416
"github.com/opendatahub-io/odh-cli/pkg/util/client"
17+
"github.com/opendatahub-io/odh-cli/pkg/util/components"
1518
"github.com/opendatahub-io/odh-cli/pkg/util/kube"
1619
)
1720

@@ -40,11 +43,12 @@ type WorkloadConditionFn[T any] func(ctx context.Context, req *WorkloadRequest[T
4043
// It handles resource listing, CRD-not-found handling, filtering, annotation population,
4144
// and auto-populating ImpactedObjects.
4245
type WorkloadBuilder[T kube.NamespacedNamer] struct {
43-
check check.Check
44-
target check.Target
45-
resourceType resources.ResourceType
46-
listFn func(ctx context.Context) ([]T, error)
47-
filterFn func(T) (bool, error)
46+
check check.Check
47+
target check.Target
48+
resourceType resources.ResourceType
49+
listFn func(ctx context.Context) ([]T, error)
50+
filterFn func(T) (bool, error)
51+
componentNames []string
4852
}
4953

5054
// Workloads creates a WorkloadBuilder that lists full unstructured objects.
@@ -90,6 +94,17 @@ func (b *WorkloadBuilder[T]) Filter(fn func(T) (bool, error)) *WorkloadBuilder[T
9094
return b
9195
}
9296

97+
// ForComponent specifies the DSC component(s) this workload check requires.
98+
// If set, Run() verifies at least one component is not in "Removed" state
99+
// before listing resources. If all components are Removed (or DSC is not found),
100+
// a passing result is returned indicating no validation is needed.
101+
// Multiple names use OR semantics (at least one must be active).
102+
func (b *WorkloadBuilder[T]) ForComponent(names ...string) *WorkloadBuilder[T] {
103+
b.componentNames = names
104+
105+
return b
106+
}
107+
93108
// Run lists resources, applies the filter, populates annotations, calls the validation function,
94109
// and auto-populates ImpactedObjects if the mapper did not set them.
95110
func (b *WorkloadBuilder[T]) Run(
@@ -107,6 +122,18 @@ func (b *WorkloadBuilder[T]) Run(
107122
dr.Annotations[check.AnnotationCheckTargetVersion] = b.target.TargetVersion.String()
108123
}
109124

125+
// Check component state precondition if ForComponent was called.
126+
if len(b.componentNames) > 0 {
127+
earlyResult, err := b.checkComponentState(ctx, dr)
128+
if err != nil {
129+
return nil, err
130+
}
131+
132+
if earlyResult != nil {
133+
return earlyResult, nil
134+
}
135+
}
136+
110137
// List resources; treat CRD-not-found as empty list.
111138
items, err := b.listFn(ctx)
112139
if err != nil && !client.IsResourceTypeNotFound(err) {
@@ -152,6 +179,44 @@ func (b *WorkloadBuilder[T]) Run(
152179
return dr, nil
153180
}
154181

182+
// checkComponentState verifies at least one component is not in Removed state.
183+
// Returns (result, nil) if validation should short-circuit, or (nil, nil) to continue.
184+
func (b *WorkloadBuilder[T]) checkComponentState(
185+
ctx context.Context,
186+
dr *result.DiagnosticResult,
187+
) (*result.DiagnosticResult, error) {
188+
dsc, err := client.GetDataScienceCluster(ctx, b.target.Client)
189+
switch {
190+
case apierrors.IsNotFound(err):
191+
dr.SetCondition(check.NewCondition(
192+
check.ConditionTypeAvailable,
193+
metav1.ConditionFalse,
194+
check.WithReason(check.ReasonResourceNotFound),
195+
check.WithMessage("No DataScienceCluster found"),
196+
))
197+
198+
return dr, nil
199+
case err != nil:
200+
return nil, fmt.Errorf("getting DataScienceCluster: %w", err)
201+
}
202+
203+
// Check if at least one component is active (not Removed).
204+
for _, name := range b.componentNames {
205+
if !components.HasManagementState(dsc, name, constants.ManagementStateRemoved) {
206+
return nil, nil
207+
}
208+
}
209+
210+
// All components are Removed - skip workload validation.
211+
dr.SetCondition(check.NewCondition(
212+
check.ConditionTypeConfigured,
213+
metav1.ConditionTrue,
214+
check.WithReason(check.ReasonRequirementsMet),
215+
))
216+
217+
return dr, nil
218+
}
219+
155220
// Complete is a convenience alternative to Run for checks that only need to set conditions.
156221
// It calls fn to obtain conditions, sets each on the result, and returns.
157222
func (b *WorkloadBuilder[T]) Complete(

pkg/lint/check/validate/workload_test.go

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
dynamicfake "k8s.io/client-go/dynamic/fake"
1515
metadatafake "k8s.io/client-go/metadata/fake"
1616

17+
"github.com/opendatahub-io/odh-cli/pkg/constants"
1718
"github.com/opendatahub-io/odh-cli/pkg/lint/check"
1819
"github.com/opendatahub-io/odh-cli/pkg/lint/check/result"
1920
"github.com/opendatahub-io/odh-cli/pkg/lint/check/validate"
@@ -34,6 +35,12 @@ var pytorchJobListKinds = map[schema.GroupVersionResource]string{
3435
resources.PyTorchJob.GVR(): resources.PyTorchJob.ListKind(),
3536
}
3637

38+
//nolint:gochecknoglobals // Test fixture - merged DSC + Notebook list kinds for ForComponent tests
39+
var dscAndNotebookListKinds = map[schema.GroupVersionResource]string{
40+
resources.DataScienceCluster.GVR(): resources.DataScienceCluster.ListKind(),
41+
resources.Notebook.GVR(): resources.Notebook.ListKind(),
42+
}
43+
3744
func newWorkloadTestCheck() *testCheck {
3845
return &testCheck{
3946
BaseCheck: check.BaseCheck{
@@ -577,3 +584,251 @@ func TestWorkloadBuilder_Complete_ErrorPropagated(t *testing.T) {
577584

578585
g.Expect(err).To(MatchError(expectedErr))
579586
}
587+
588+
func TestWorkloadBuilder_ForComponent_RemovedReturnsPassingResult(t *testing.T) {
589+
g := NewWithT(t)
590+
ctx := t.Context()
591+
592+
dsc := createDSCWithComponent("kserve", constants.ManagementStateRemoved)
593+
594+
scheme := runtime.NewScheme()
595+
_ = metav1.AddMetaToScheme(scheme)
596+
dynamicClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, dscAndNotebookListKinds, dsc)
597+
metadataClient := metadatafake.NewSimpleMetadataClient(scheme)
598+
599+
c := client.NewForTesting(client.TestClientConfig{
600+
Dynamic: dynamicClient,
601+
Metadata: metadataClient,
602+
})
603+
604+
chk := newWorkloadTestCheck()
605+
chk.Kind = "kserve"
606+
607+
targetVer := semver.MustParse("3.0.0")
608+
target := check.Target{
609+
Client: c,
610+
TargetVersion: &targetVer,
611+
}
612+
613+
validationCalled := false
614+
615+
dr, err := validate.WorkloadsMetadata(chk, target, resources.Notebook).
616+
ForComponent("kserve").
617+
Run(ctx, func(_ context.Context, _ *validate.WorkloadRequest[*metav1.PartialObjectMetadata]) error {
618+
validationCalled = true
619+
620+
return nil
621+
})
622+
623+
g.Expect(err).ToNot(HaveOccurred())
624+
g.Expect(dr).ToNot(BeNil())
625+
g.Expect(validationCalled).To(BeFalse())
626+
g.Expect(dr.Status.Conditions).To(HaveLen(1))
627+
g.Expect(dr.Status.Conditions[0].Type).To(Equal(check.ConditionTypeConfigured))
628+
g.Expect(dr.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue))
629+
g.Expect(dr.Status.Conditions[0].Reason).To(Equal(check.ReasonRequirementsMet))
630+
g.Expect(dr.Annotations).To(HaveKeyWithValue(check.AnnotationCheckTargetVersion, "3.0.0"))
631+
}
632+
633+
func TestWorkloadBuilder_ForComponent_ManagedProceedsNormally(t *testing.T) {
634+
g := NewWithT(t)
635+
ctx := t.Context()
636+
637+
dsc := createDSCWithComponent("kserve", constants.ManagementStateManaged)
638+
nb := &unstructured.Unstructured{
639+
Object: map[string]any{
640+
"apiVersion": resources.Notebook.APIVersion(),
641+
"kind": resources.Notebook.Kind,
642+
"metadata": map[string]any{"name": "nb-1", "namespace": "ns1"},
643+
},
644+
}
645+
646+
scheme := runtime.NewScheme()
647+
_ = metav1.AddMetaToScheme(scheme)
648+
dynamicClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, dscAndNotebookListKinds, dsc, nb)
649+
metadataClient := metadatafake.NewSimpleMetadataClient(scheme, kube.ToPartialObjectMetadata(nb)...)
650+
651+
c := client.NewForTesting(client.TestClientConfig{
652+
Dynamic: dynamicClient,
653+
Metadata: metadataClient,
654+
})
655+
656+
chk := newWorkloadTestCheck()
657+
chk.Kind = "kserve"
658+
659+
targetVer := semver.MustParse("3.0.0")
660+
target := check.Target{
661+
Client: c,
662+
TargetVersion: &targetVer,
663+
}
664+
665+
validationCalled := false
666+
667+
dr, err := validate.WorkloadsMetadata(chk, target, resources.Notebook).
668+
ForComponent("kserve").
669+
Run(ctx, func(_ context.Context, req *validate.WorkloadRequest[*metav1.PartialObjectMetadata]) error {
670+
validationCalled = true
671+
g.Expect(req.Items).To(HaveLen(1))
672+
req.Result.SetCondition(check.NewCondition(
673+
check.ConditionTypeCompatible,
674+
metav1.ConditionTrue,
675+
check.WithReason(check.ReasonVersionCompatible),
676+
check.WithMessage("Found %d notebooks", len(req.Items)),
677+
))
678+
679+
return nil
680+
})
681+
682+
g.Expect(err).ToNot(HaveOccurred())
683+
g.Expect(dr).ToNot(BeNil())
684+
g.Expect(validationCalled).To(BeTrue())
685+
g.Expect(dr.Status.Conditions).To(HaveLen(1))
686+
g.Expect(dr.Status.Conditions[0].Type).To(Equal(check.ConditionTypeCompatible))
687+
}
688+
689+
func TestWorkloadBuilder_ForComponent_DSCNotFound(t *testing.T) {
690+
g := NewWithT(t)
691+
ctx := t.Context()
692+
693+
// No DSC in the cluster.
694+
scheme := runtime.NewScheme()
695+
_ = metav1.AddMetaToScheme(scheme)
696+
dynamicClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, dscAndNotebookListKinds)
697+
metadataClient := metadatafake.NewSimpleMetadataClient(scheme)
698+
699+
c := client.NewForTesting(client.TestClientConfig{
700+
Dynamic: dynamicClient,
701+
Metadata: metadataClient,
702+
})
703+
704+
chk := newWorkloadTestCheck()
705+
chk.Kind = "kserve"
706+
707+
target := check.Target{
708+
Client: c,
709+
}
710+
711+
validationCalled := false
712+
713+
dr, err := validate.WorkloadsMetadata(chk, target, resources.Notebook).
714+
ForComponent("kserve").
715+
Run(ctx, func(_ context.Context, _ *validate.WorkloadRequest[*metav1.PartialObjectMetadata]) error {
716+
validationCalled = true
717+
718+
return nil
719+
})
720+
721+
g.Expect(err).ToNot(HaveOccurred())
722+
g.Expect(dr).ToNot(BeNil())
723+
g.Expect(validationCalled).To(BeFalse())
724+
g.Expect(dr.Status.Conditions).To(HaveLen(1))
725+
g.Expect(dr.Status.Conditions[0].Type).To(Equal(check.ConditionTypeAvailable))
726+
g.Expect(dr.Status.Conditions[0].Status).To(Equal(metav1.ConditionFalse))
727+
g.Expect(dr.Status.Conditions[0].Reason).To(Equal(check.ReasonResourceNotFound))
728+
}
729+
730+
func TestWorkloadBuilder_ForComponent_MultipleComponentsORSemantics(t *testing.T) {
731+
g := NewWithT(t)
732+
ctx := t.Context()
733+
734+
// DSC with kserve Removed but modelmesh Managed - at least one is active.
735+
dsc := &unstructured.Unstructured{
736+
Object: map[string]any{
737+
"apiVersion": resources.DataScienceCluster.APIVersion(),
738+
"kind": resources.DataScienceCluster.Kind,
739+
"metadata": map[string]any{"name": "default-dsc"},
740+
"spec": map[string]any{
741+
"components": map[string]any{
742+
"kserve": map[string]any{"managementState": constants.ManagementStateRemoved},
743+
"modelmesh": map[string]any{"managementState": constants.ManagementStateManaged},
744+
},
745+
},
746+
},
747+
}
748+
749+
scheme := runtime.NewScheme()
750+
_ = metav1.AddMetaToScheme(scheme)
751+
dynamicClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, dscAndNotebookListKinds, dsc)
752+
metadataClient := metadatafake.NewSimpleMetadataClient(scheme)
753+
754+
c := client.NewForTesting(client.TestClientConfig{
755+
Dynamic: dynamicClient,
756+
Metadata: metadataClient,
757+
})
758+
759+
chk := newWorkloadTestCheck()
760+
761+
target := check.Target{
762+
Client: c,
763+
}
764+
765+
validationCalled := false
766+
767+
dr, err := validate.WorkloadsMetadata(chk, target, resources.Notebook).
768+
ForComponent("kserve", "modelmesh").
769+
Run(ctx, func(_ context.Context, req *validate.WorkloadRequest[*metav1.PartialObjectMetadata]) error {
770+
validationCalled = true
771+
req.Result.SetCondition(check.NewCondition(
772+
check.ConditionTypeCompatible,
773+
metav1.ConditionTrue,
774+
check.WithReason(check.ReasonVersionCompatible),
775+
check.WithMessage("Validation proceeded"),
776+
))
777+
778+
return nil
779+
})
780+
781+
g.Expect(err).ToNot(HaveOccurred())
782+
g.Expect(dr).ToNot(BeNil())
783+
g.Expect(validationCalled).To(BeTrue())
784+
}
785+
786+
func TestWorkloadBuilder_NoForComponent_BackwardCompatible(t *testing.T) {
787+
g := NewWithT(t)
788+
ctx := t.Context()
789+
790+
nb := &unstructured.Unstructured{
791+
Object: map[string]any{
792+
"apiVersion": resources.Notebook.APIVersion(),
793+
"kind": resources.Notebook.Kind,
794+
"metadata": map[string]any{"name": "nb-1", "namespace": "ns1"},
795+
},
796+
}
797+
798+
scheme := runtime.NewScheme()
799+
_ = metav1.AddMetaToScheme(scheme)
800+
dynamicClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, notebookListKinds, nb)
801+
metadataClient := metadatafake.NewSimpleMetadataClient(scheme, kube.ToPartialObjectMetadata(nb)...)
802+
803+
c := client.NewForTesting(client.TestClientConfig{
804+
Dynamic: dynamicClient,
805+
Metadata: metadataClient,
806+
})
807+
808+
chk := newWorkloadTestCheck()
809+
target := check.Target{
810+
Client: c,
811+
}
812+
813+
// No ForComponent() called - should proceed without checking DSC.
814+
validationCalled := false
815+
816+
dr, err := validate.WorkloadsMetadata(chk, target, resources.Notebook).
817+
Run(ctx, func(_ context.Context, req *validate.WorkloadRequest[*metav1.PartialObjectMetadata]) error {
818+
validationCalled = true
819+
g.Expect(req.Items).To(HaveLen(1))
820+
req.Result.SetCondition(check.NewCondition(
821+
check.ConditionTypeCompatible,
822+
metav1.ConditionTrue,
823+
check.WithReason(check.ReasonVersionCompatible),
824+
check.WithMessage("No component check"),
825+
))
826+
827+
return nil
828+
})
829+
830+
g.Expect(err).ToNot(HaveOccurred())
831+
g.Expect(dr).ToNot(BeNil())
832+
g.Expect(validationCalled).To(BeTrue())
833+
g.Expect(dr.Status.Conditions).To(HaveLen(1))
834+
}

0 commit comments

Comments
 (0)