diff --git a/pkg/lint/check/result/result.go b/pkg/lint/check/result/result.go index a89223c4..9cd79a6e 100644 --- a/pkg/lint/check/result/result.go +++ b/pkg/lint/check/result/result.go @@ -16,6 +16,19 @@ const ( // (e.g., "notebooks.kubeflow.org"). Automatically set by SetImpactedObjects and // AddImpactedObjects from the ResourceType. AnnotationResourceCRDName = "resource.opendatahub.io/crd-name" + + // AnnotationObjectContext is an optional per-object annotation key that provides + // brief clarifying context about why a specific object is impacted. When set on + // an ImpactedObject's ObjectMeta.Annotations, the EnhancedVerboseFormatter renders + // it as a sub-bullet beneath the object reference. + AnnotationObjectContext = "result.opendatahub.io/context" + + // AnnotationObjectCRDName is an optional per-object annotation key for the CRD + // fully-qualified name (e.g., "notebooks.kubeflow.org"). Set from ResourceType.CRDFQN() + // when building impacted objects. Used by buildCRDFQNByKind for authoritative plural + // forms, avoiding naive derivation from Kind. Especially useful for multi-kind results + // where the result-level AnnotationResourceCRDName cannot represent all types. + AnnotationObjectCRDName = "result.opendatahub.io/crd-name" ) const ( @@ -33,11 +46,12 @@ const ( // Impact represents the upgrade impact level of a diagnostic condition. type Impact string -// Impact levels for diagnostic conditions. +// Impact levels for diagnostic conditions, ordered by severity (highest first). const ( - ImpactBlocking Impact = "blocking" // Upgrade CANNOT proceed - ImpactAdvisory Impact = "advisory" // Upgrade CAN proceed with warning - ImpactNone Impact = "" // No impact (omitted from JSON/YAML) + ImpactProhibited Impact = "prohibited" // Upgrade is NOT POSSIBLE + ImpactBlocking Impact = "blocking" // Upgrade CANNOT proceed + ImpactAdvisory Impact = "advisory" // Upgrade CAN proceed with warning + ImpactNone Impact = "" // No impact (omitted from JSON/YAML) ) // Condition represents a diagnostic condition with severity level. @@ -71,16 +85,16 @@ func (c Condition) Validate() error { // False/Unknown status must have impact specified. if c.Impact == ImpactNone || c.Impact == "" { return fmt.Errorf( - "condition with Status=%q must have Impact specified (Blocking or Advisory), got Impact=%q", + "condition with Status=%q must have Impact specified (Prohibited, Blocking, or Advisory), got Impact=%q", c.Status, c.Impact, ) } // Validate impact values. - if c.Impact != ImpactBlocking && c.Impact != ImpactAdvisory { + if c.Impact != ImpactProhibited && c.Impact != ImpactBlocking && c.Impact != ImpactAdvisory { return fmt.Errorf( - "invalid Impact=%q, must be %q or %q", - c.Impact, ImpactBlocking, ImpactAdvisory, + "invalid Impact=%q, must be %q, %q, or %q", + c.Impact, ImpactProhibited, ImpactBlocking, ImpactAdvisory, ) } @@ -254,10 +268,14 @@ func (r *DiagnosticResult) GetImpact() Impact { for _, cond := range r.Status.Conditions { switch cond.Impact { + case ImpactProhibited: + return ImpactProhibited case ImpactBlocking: - return ImpactBlocking + maxImpact = ImpactBlocking case ImpactAdvisory: - maxImpact = ImpactAdvisory + if maxImpact == ImpactNone { + maxImpact = ImpactAdvisory + } case ImpactNone: // No impact - continue checking other conditions } diff --git a/pkg/lint/check/result/result_test.go b/pkg/lint/check/result/result_test.go index 1b8b33d9..dfdf8118 100644 --- a/pkg/lint/check/result/result_test.go +++ b/pkg/lint/check/result/result_test.go @@ -307,6 +307,53 @@ func TestDiagnosticResult_Validate_EmptyConditionReason(t *testing.T) { g.Expect(err.Error()).To(ContainSubstring("has empty reason")) } +func TestConditionValidate_ProhibitedWithStatusFalse(t *testing.T) { + g := NewWithT(t) + + cond := result.Condition{ + Condition: metav1.Condition{ + Type: "Consistency", + Status: metav1.ConditionFalse, + Reason: "Inconsistent", + }, + Impact: result.ImpactProhibited, + } + + g.Expect(cond.Validate()).ToNot(HaveOccurred()) +} + +func TestConditionValidate_ProhibitedWithStatusUnknown(t *testing.T) { + g := NewWithT(t) + + cond := result.Condition{ + Condition: metav1.Condition{ + Type: "Consistency", + Status: metav1.ConditionUnknown, + Reason: "CannotDetermine", + }, + Impact: result.ImpactProhibited, + } + + g.Expect(cond.Validate()).ToNot(HaveOccurred()) +} + +func TestConditionValidate_ProhibitedWithStatusTrue_Invalid(t *testing.T) { + g := NewWithT(t) + + cond := result.Condition{ + Condition: metav1.Condition{ + Type: "Consistency", + Status: metav1.ConditionTrue, + Reason: "Consistent", + }, + Impact: result.ImpactProhibited, + } + + err := cond.Validate() + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("Status=True must have Impact=None")) +} + // T023: Multiple conditions array tests func TestMultipleConditions_OrderedByExecutionSequence(t *testing.T) { @@ -790,6 +837,118 @@ func TestSetCondition_MultipleConditionTypes(t *testing.T) { })) } +// GetImpact tests + +func TestGetImpact_ReturnsProhibitedAsHighest(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "kueue", "data-integrity", "test") + dr.SetCondition(check.NewCondition( + "Advisory", + metav1.ConditionFalse, + check.WithReason("Reason"), + check.WithMessage("advisory finding"), + check.WithImpact(result.ImpactAdvisory), + )) + dr.SetCondition(check.NewCondition( + "Blocking", + metav1.ConditionFalse, + check.WithReason("Reason"), + check.WithMessage("blocking finding"), + check.WithImpact(result.ImpactBlocking), + )) + dr.SetCondition(check.NewCondition( + "Prohibited", + metav1.ConditionFalse, + check.WithReason("Reason"), + check.WithMessage("prohibited finding"), + check.WithImpact(result.ImpactProhibited), + )) + + g.Expect(dr.GetImpact()).To(Equal(result.ImpactProhibited)) +} + +func TestGetImpact_ProhibitedShortCircuits(t *testing.T) { + g := NewWithT(t) + + // Prohibited condition first — should return immediately without checking others. + dr := result.New("workload", "kueue", "data-integrity", "test") + dr.SetCondition(check.NewCondition( + "Prohibited", + metav1.ConditionFalse, + check.WithReason("Reason"), + check.WithMessage("prohibited finding"), + check.WithImpact(result.ImpactProhibited), + )) + dr.SetCondition(check.NewCondition( + "Passing", + metav1.ConditionTrue, + check.WithReason("Reason"), + check.WithMessage("all good"), + )) + + g.Expect(dr.GetImpact()).To(Equal(result.ImpactProhibited)) +} + +func TestGetImpact_BlockingWithoutProhibited(t *testing.T) { + g := NewWithT(t) + + dr := result.New("component", "kserve", "removal", "test") + dr.SetCondition(check.NewCondition( + "Blocking", + metav1.ConditionFalse, + check.WithReason("Reason"), + check.WithMessage("blocking finding"), + check.WithImpact(result.ImpactBlocking), + )) + dr.SetCondition(check.NewCondition( + "Advisory", + metav1.ConditionFalse, + check.WithReason("Reason"), + check.WithMessage("advisory finding"), + check.WithImpact(result.ImpactAdvisory), + )) + + g.Expect(dr.GetImpact()).To(Equal(result.ImpactBlocking)) +} + +func TestGetImpact_AdvisoryWithoutBlockingOrProhibited(t *testing.T) { + g := NewWithT(t) + + dr := result.New("component", "dashboard", "migration", "test") + dr.SetCondition(check.NewCondition( + "Advisory", + metav1.ConditionFalse, + check.WithReason("Reason"), + check.WithMessage("advisory finding"), + check.WithImpact(result.ImpactAdvisory), + )) + + g.Expect(dr.GetImpact()).To(Equal(result.ImpactAdvisory)) +} + +func TestGetImpact_NoneWhenAllPassing(t *testing.T) { + g := NewWithT(t) + + dr := result.New("component", "dashboard", "status", "test") + dr.SetCondition(check.NewCondition( + "Available", + metav1.ConditionTrue, + check.WithReason("Ready"), + check.WithMessage("all good"), + )) + + g.Expect(dr.GetImpact()).To(Equal(result.ImpactNone)) +} + +func TestGetImpact_NoneWhenNoConditions(t *testing.T) { + g := NewWithT(t) + + dr := result.New("component", "test", "check", "test") + + g.Expect(dr.GetImpact()).To(Equal(result.ImpactNone)) +} + // SetImpactedObjects and AddImpactedObjects tests func TestSetImpactedObjects(t *testing.T) { diff --git a/pkg/lint/check/verbose_formatter.go b/pkg/lint/check/verbose_formatter.go index 2bf60d18..8af8de62 100644 --- a/pkg/lint/check/verbose_formatter.go +++ b/pkg/lint/check/verbose_formatter.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "sort" + "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -59,6 +60,13 @@ type namespaceGroup struct { objects []metav1.PartialObjectMetadata } +// qualifiedObject holds a CRD-qualified object reference with optional context. +type qualifiedObject struct { + name string + crdFQN string + context string +} + // formatImpactedObject returns the display string for an impacted object. // Includes the Kind from TypeMeta when available to help identify the resource type. func formatImpactedObject(obj metav1.PartialObjectMetadata) string { @@ -95,3 +103,203 @@ func groupByNamespace(objects []metav1.PartialObjectMetadata) []namespaceGroup { return groups } + +// EnhancedVerboseFormatter provides namespace-grouped rendering with CRD-qualified +// resource names and requester info. Embed this in any check struct to get a +// FormatVerboseOutput that lists objects grouped by namespace. +// +// Output format: +// +// namespace: | requester: +// - ./ +// - ./ +// +// The CRD FQN is derived per object from its TypeMeta, so results with mixed resource +// types (e.g. Notebook and RayCluster) render each object with its correct CRD prefix. +// When all objects share the same type, the AnnotationResourceCRDName annotation is +// preferred for an accurate plural form. +// +// Checks that need custom formatting (e.g. grouping by image or status) +// should define their own FormatVerboseOutput method instead. +type EnhancedVerboseFormatter struct { + // NamespaceRequesters maps namespace names to their openshift.io/requester annotation value. + // Populated automatically by the output renderer before FormatVerboseOutput is called. + NamespaceRequesters map[string]string +} + +// SetNamespaceRequesters sets the namespace-to-requester mapping. +// Called by the output renderer before FormatVerboseOutput. +func (f *EnhancedVerboseFormatter) SetNamespaceRequesters(m map[string]string) { + f.NamespaceRequesters = m +} + +// FormatVerboseOutput implements VerboseOutputFormatter. +// Renders impacted objects grouped by namespace with requester info and CRD FQN. +// Each object's CRD FQN is derived from its own TypeMeta, so mixed-kind results +// render correctly (e.g. notebooks.kubeflow.org/nb-1 alongside rayclusters.ray.io/rc-1). +func (f *EnhancedVerboseFormatter) FormatVerboseOutput(out io.Writer, dr *result.DiagnosticResult) { + // Build a per-object qualified name. For single-kind results, the annotation + // provides the most accurate CRD FQN. For mixed-kind results, each object's + // TypeMeta is used to derive its own prefix. + crdFQNByKind := buildCRDFQNByKind(dr) + + // Group objects by namespace, preserving per-object kind and optional context. + nsMap := make(map[string][]qualifiedObject) + for _, obj := range dr.ImpactedObjects { + fqn := crdFQNByKind[obj.Kind] + + var ctx string + if obj.Annotations != nil { + ctx = obj.Annotations[result.AnnotationObjectContext] + } + + nsMap[obj.Namespace] = append(nsMap[obj.Namespace], qualifiedObject{ + name: obj.Name, + crdFQN: fqn, + context: ctx, + }) + } + + namespaces := make([]string, 0, len(nsMap)) + for ns := range nsMap { + namespaces = append(namespaces, ns) + } + sort.Strings(namespaces) + + for idx, ns := range namespaces { + objects := nsMap[ns] + sort.Slice(objects, func(i, j int) bool { + if objects[i].crdFQN != objects[j].crdFQN { + return objects[i].crdFQN < objects[j].crdFQN + } + + return objects[i].name < objects[j].name + }) + + if ns == "" { + // Cluster-scoped objects listed without namespace header. + writeQualifiedObjects(out, objects, " ") + } else { + nsHeader := f.namespaceHeader(ns) + + _, _ = fmt.Fprintf(out, " %s\n", nsHeader) + writeQualifiedObjects(out, objects, " ") + } + + // Blank line between namespace groups (except after last). + if idx < len(namespaces)-1 { + _, _ = fmt.Fprintln(out) + } + } +} + +// namespaceHeader returns the formatted namespace header, including requester info if available. +func (f *EnhancedVerboseFormatter) namespaceHeader(ns string) string { + header := "namespace: " + ns + if f.NamespaceRequesters != nil { + if requester, ok := f.NamespaceRequesters[ns]; ok && requester != "" { + header = fmt.Sprintf("namespace: %s | requester: %s", ns, requester) + } + } + + return header +} + +// writeQualifiedObjects writes a list of qualified objects with the given indent prefix. +// Objects with a non-empty context annotation get a sub-bullet on the following line. +func writeQualifiedObjects(out io.Writer, objects []qualifiedObject, indent string) { + for _, obj := range objects { + _, _ = fmt.Fprintf(out, "%s- %s/%s\n", indent, obj.crdFQN, obj.name) + if obj.context != "" { + _, _ = fmt.Fprintf(out, "%s — (%s)\n", indent, obj.context) + } + } +} + +// buildCRDFQNByKind returns a map from Kind to CRD FQN for the impacted objects. +// Resolution priority per kind: +// 1. Per-object AnnotationObjectCRDName (authoritative, set from ResourceType.CRDFQN()) +// 2. Result-level AnnotationResourceCRDName (single-kind results only) +// 3. Naive derivation from TypeMeta (fallback) +func buildCRDFQNByKind(dr *result.DiagnosticResult) map[string]string { + fqnByKind := make(map[string]string) + + for _, obj := range dr.ImpactedObjects { + if _, ok := fqnByKind[obj.Kind]; ok { + continue + } + + // Prefer per-object annotation set from ResourceType.CRDFQN(). + if name, ok := obj.Annotations[result.AnnotationObjectCRDName]; ok && name != "" { + fqnByKind[obj.Kind] = name + + continue + } + + fqnByKind[obj.Kind] = DeriveCRDFQNFromTypeMeta([]metav1.PartialObjectMetadata{obj}) + } + + // When there is exactly one kind and the result-level annotation is set, + // prefer it over naive derivation (backward compatibility for callers + // that use SetImpactedObjects/AddImpactedObjects). + if len(fqnByKind) == 1 { + if name, ok := dr.Annotations[result.AnnotationResourceCRDName]; ok && name != "" { + for kind := range fqnByKind { + fqnByKind[kind] = name + } + } + } + + return fqnByKind +} + +// CRDFullyQualifiedName returns the CRD fully-qualified name for the impacted objects +// in a DiagnosticResult. It first checks the AnnotationResourceCRDName annotation +// (automatically set by SetImpactedObjects and AddImpactedObjects from ResourceType.CRDFQN()). +// Falls back to deriving the FQN from the first impacted object's TypeMeta if the +// annotation is absent. +func CRDFullyQualifiedName(dr *result.DiagnosticResult) string { + // Prefer the annotation set by SetImpactedObjects — uses ResourceType.Resource + // for the correct plural form (e.g. "inferenceservices", not a naive pluralization). + if name, ok := dr.Annotations[result.AnnotationResourceCRDName]; ok && name != "" { + return name + } + + // Fallback: derive from TypeMeta (for callers that set ImpactedObjects directly). + return DeriveCRDFQNFromTypeMeta(dr.ImpactedObjects) +} + +// DeriveCRDFQNFromTypeMeta derives the CRD FQN from the first impacted object's TypeMeta. +// For example, APIVersion "kubeflow.org/v1" with Kind "Notebook" produces "notebooks.kubeflow.org". +// Falls back to the lowercase Kind if TypeMeta is not populated. +func DeriveCRDFQNFromTypeMeta(objects []metav1.PartialObjectMetadata) string { + if len(objects) == 0 { + return "" + } + + obj := objects[0] + + kind := obj.Kind + if kind == "" { + return "" + } + + // Naive plural as fallback — callers should set AnnotationResourceCRDName + // via ResourceType.CRDFQN() for authoritative plural forms. + plural := strings.ToLower(kind) + "s" + + // Extract API group from APIVersion (e.g. "kubeflow.org/v1" → "kubeflow.org"). + // Core resources have no group (APIVersion is just "v1"), so return plural only. + apiVersion := obj.APIVersion + if apiVersion == "" { + return plural + } + + group, _, found := strings.Cut(apiVersion, "/") + if !found { + // Core API (e.g. "v1") — no group prefix. + return plural + } + + return plural + "." + group +} diff --git a/pkg/lint/check/verbose_formatter_test.go b/pkg/lint/check/verbose_formatter_test.go index bd52e42f..b36aacca 100644 --- a/pkg/lint/check/verbose_formatter_test.go +++ b/pkg/lint/check/verbose_formatter_test.go @@ -161,6 +161,436 @@ func TestVerboseOutputFormatter_TypeAssertion_PlainCheck(t *testing.T) { g.Expect(ok).To(BeFalse()) } +// --- EnhancedVerboseFormatter tests --- + +func TestEnhancedVerboseFormatter_NamespacedObjects(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "notebook", "impacted-workloads", "test description") + dr.Annotations[result.AnnotationResourceCRDName] = "notebooks.kubeflow.org" + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + {ObjectMeta: metav1.ObjectMeta{Namespace: "ns-b", Name: "nb-2"}}, + {ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "nb-1"}}, + {ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "nb-3"}}, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " namespace: ns-a\n" + + " - notebooks.kubeflow.org/nb-1\n" + + " - notebooks.kubeflow.org/nb-3\n" + + "\n" + + " namespace: ns-b\n" + + " - notebooks.kubeflow.org/nb-2\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_ClusterScopedObjects(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "test", "check", "test description") + dr.Annotations[result.AnnotationResourceCRDName] = "widgets.example.io" + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + {ObjectMeta: metav1.ObjectMeta{Name: "cluster-widget"}}, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := " - widgets.example.io/cluster-widget\n" + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_WithNamespaceRequesters(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "notebook", "check", "test description") + dr.Annotations[result.AnnotationResourceCRDName] = "notebooks.kubeflow.org" + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + {ObjectMeta: metav1.ObjectMeta{Namespace: "user-ns", Name: "my-nb"}}, + } + + formatter := &check.EnhancedVerboseFormatter{} + formatter.SetNamespaceRequesters(map[string]string{ + "user-ns": "jdoe", + }) + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " namespace: user-ns | requester: jdoe\n" + + " - notebooks.kubeflow.org/my-nb\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_FallsBackToTypeMeta(t *testing.T) { + g := NewWithT(t) + + // No AnnotationResourceCRDName — should derive from TypeMeta. + dr := result.New("workload", "notebook", "check", "test description") + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + TypeMeta: metav1.TypeMeta{Kind: "Notebook", APIVersion: "kubeflow.org/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "nb-1"}, + }, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " namespace: ns\n" + + " - notebooks.kubeflow.org/nb-1\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_MixedKinds(t *testing.T) { + g := NewWithT(t) + + // Mixed resource types — each object should get its own CRD FQN prefix. + dr := result.New("workload", "kueue", "data-integrity", "test description") + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + TypeMeta: metav1.TypeMeta{Kind: "Notebook", APIVersion: "kubeflow.org/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "my-nb"}, + }, + { + TypeMeta: metav1.TypeMeta{Kind: "RayCluster", APIVersion: "ray.io/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "my-ray"}, + }, + { + TypeMeta: metav1.TypeMeta{Kind: "Notebook", APIVersion: "kubeflow.org/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns-b", Name: "other-nb"}, + }, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " namespace: ns-a\n" + + " - notebooks.kubeflow.org/my-nb\n" + + " - rayclusters.ray.io/my-ray\n" + + "\n" + + " namespace: ns-b\n" + + " - notebooks.kubeflow.org/other-nb\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_MixedKindsIgnoresAnnotation(t *testing.T) { + g := NewWithT(t) + + // When multiple kinds are present, the AnnotationResourceCRDName annotation + // should be ignored — it only applies to single-kind results. + dr := result.New("workload", "kueue", "data-integrity", "test description") + dr.Annotations[result.AnnotationResourceCRDName] = "notebooks.kubeflow.org" + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + TypeMeta: metav1.TypeMeta{Kind: "Notebook", APIVersion: "kubeflow.org/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "my-nb"}, + }, + { + TypeMeta: metav1.TypeMeta{Kind: "RayCluster", APIVersion: "ray.io/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "my-ray"}, + }, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + output := buf.String() + + // RayCluster should use derived FQN, not the annotation value. + g.Expect(output).To(ContainSubstring("rayclusters.ray.io/my-ray")) + g.Expect(output).To(ContainSubstring("notebooks.kubeflow.org/my-nb")) +} + +func TestEnhancedVerboseFormatter_MixedKindsSortedByCRDThenName(t *testing.T) { + g := NewWithT(t) + + // Objects within the same namespace should sort by CRD FQN, then by name. + dr := result.New("workload", "kueue", "data-integrity", "test description") + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + TypeMeta: metav1.TypeMeta{Kind: "RayCluster", APIVersion: "ray.io/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "rc-b"}, + }, + { + TypeMeta: metav1.TypeMeta{Kind: "Notebook", APIVersion: "kubeflow.org/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "nb-a"}, + }, + { + TypeMeta: metav1.TypeMeta{Kind: "RayCluster", APIVersion: "ray.io/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "rc-a"}, + }, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " namespace: ns\n" + + " - notebooks.kubeflow.org/nb-a\n" + + " - rayclusters.ray.io/rc-a\n" + + " - rayclusters.ray.io/rc-b\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_SingleKindPrefersAnnotation(t *testing.T) { + g := NewWithT(t) + + // When all objects share one kind, the annotation should be preferred + // over TypeMeta derivation for an accurate plural form. + dr := result.New("workload", "kserve", "check", "test description") + dr.Annotations[result.AnnotationResourceCRDName] = "inferenceservices.serving.kserve.io" + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + TypeMeta: metav1.TypeMeta{Kind: "InferenceService", APIVersion: "serving.kserve.io/v1beta1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "isvc-1"}, + }, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " namespace: ns\n" + + " - inferenceservices.serving.kserve.io/isvc-1\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_EmptyObjects(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "test", "check", "test description") + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + g.Expect(buf.String()).To(BeEmpty()) +} + +func TestEnhancedVerboseFormatter_ImplementsVerboseOutputFormatter(t *testing.T) { + g := NewWithT(t) + + // A check embedding EnhancedVerboseFormatter should satisfy VerboseOutputFormatter. + chk := &mockEnhancedCheck{} + + var c check.Check = chk + _, ok := c.(check.VerboseOutputFormatter) + g.Expect(ok).To(BeTrue()) +} + +// --- AnnotationObjectContext tests --- + +func TestEnhancedVerboseFormatter_ObjectContextNamespaced(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "kueue", "data-integrity", "test description") + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + TypeMeta: metav1.TypeMeta{Kind: "Notebook", APIVersion: "kubeflow.org/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "nb-1", Annotations: map[string]string{result.AnnotationObjectContext: "missing queue-name label in kueue-managed namespace"}}, + }, + { + TypeMeta: metav1.TypeMeta{Kind: "RayCluster", APIVersion: "ray.io/v1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "ns-a", Name: "rc-1", Annotations: map[string]string{result.AnnotationObjectContext: "queue-name label mismatch in owner tree"}}, + }, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " namespace: ns-a\n" + + " - notebooks.kubeflow.org/nb-1\n" + + " — (missing queue-name label in kueue-managed namespace)\n" + + " - rayclusters.ray.io/rc-1\n" + + " — (queue-name label mismatch in owner tree)\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_ObjectContextClusterScoped(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "test", "check", "test description") + dr.Annotations[result.AnnotationResourceCRDName] = "widgets.example.io" + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + ObjectMeta: metav1.ObjectMeta{Name: "widget-1", Annotations: map[string]string{result.AnnotationObjectContext: "some context"}}, + }, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " - widgets.example.io/widget-1\n" + + " — (some context)\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +func TestEnhancedVerboseFormatter_ObjectContextMixedPresence(t *testing.T) { + g := NewWithT(t) + + // Only some objects have context — others should render without sub-bullet. + dr := result.New("workload", "notebook", "check", "test description") + dr.Annotations[result.AnnotationResourceCRDName] = "notebooks.kubeflow.org" + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "nb-with-context", Annotations: map[string]string{result.AnnotationObjectContext: "has context"}}, + }, + { + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "nb-without-context"}, + }, + } + + formatter := &check.EnhancedVerboseFormatter{} + + var buf bytes.Buffer + formatter.FormatVerboseOutput(&buf, dr) + + expected := "" + + " namespace: ns\n" + + " - notebooks.kubeflow.org/nb-with-context\n" + + " — (has context)\n" + + " - notebooks.kubeflow.org/nb-without-context\n" + + g.Expect(buf.String()).To(Equal(expected)) +} + +// --- CRDFullyQualifiedName and DeriveCRDFQNFromTypeMeta tests --- + +func TestCRDFullyQualifiedName_PrefersAnnotation(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "notebook", "check", "test description") + dr.Annotations[result.AnnotationResourceCRDName] = "notebooks.kubeflow.org" + // TypeMeta would produce a different result — annotation should win. + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + TypeMeta: metav1.TypeMeta{Kind: "Widget", APIVersion: "other.io/v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "obj"}, + }, + } + + g.Expect(check.CRDFullyQualifiedName(dr)).To(Equal("notebooks.kubeflow.org")) +} + +func TestCRDFullyQualifiedName_FallsBackToTypeMeta(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "notebook", "check", "test description") + dr.ImpactedObjects = []metav1.PartialObjectMetadata{ + { + TypeMeta: metav1.TypeMeta{Kind: "Notebook", APIVersion: "kubeflow.org/v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "obj"}, + }, + } + + g.Expect(check.CRDFullyQualifiedName(dr)).To(Equal("notebooks.kubeflow.org")) +} + +func TestCRDFullyQualifiedName_EmptyObjects(t *testing.T) { + g := NewWithT(t) + + dr := result.New("workload", "test", "check", "test description") + + g.Expect(check.CRDFullyQualifiedName(dr)).To(BeEmpty()) +} + +func TestDeriveCRDFQNFromTypeMeta_GroupedAPIVersion(t *testing.T) { + g := NewWithT(t) + + objects := []metav1.PartialObjectMetadata{ + {TypeMeta: metav1.TypeMeta{Kind: "InferenceService", APIVersion: "serving.kserve.io/v1beta1"}}, + } + + g.Expect(check.DeriveCRDFQNFromTypeMeta(objects)).To(Equal("inferenceservices.serving.kserve.io")) +} + +func TestDeriveCRDFQNFromTypeMeta_CoreAPIVersion(t *testing.T) { + g := NewWithT(t) + + // Core API (e.g. "v1") has no group — should return plural only. + objects := []metav1.PartialObjectMetadata{ + {TypeMeta: metav1.TypeMeta{Kind: "ConfigMap", APIVersion: "v1"}}, + } + + g.Expect(check.DeriveCRDFQNFromTypeMeta(objects)).To(Equal("configmaps")) +} + +func TestDeriveCRDFQNFromTypeMeta_EmptyAPIVersion(t *testing.T) { + g := NewWithT(t) + + objects := []metav1.PartialObjectMetadata{ + {TypeMeta: metav1.TypeMeta{Kind: "Notebook"}}, + } + + g.Expect(check.DeriveCRDFQNFromTypeMeta(objects)).To(Equal("notebooks")) +} + +func TestDeriveCRDFQNFromTypeMeta_EmptyKind(t *testing.T) { + g := NewWithT(t) + + objects := []metav1.PartialObjectMetadata{ + {TypeMeta: metav1.TypeMeta{APIVersion: "kubeflow.org/v1"}}, + } + + g.Expect(check.DeriveCRDFQNFromTypeMeta(objects)).To(BeEmpty()) +} + +func TestDeriveCRDFQNFromTypeMeta_EmptyObjects(t *testing.T) { + g := NewWithT(t) + + g.Expect(check.DeriveCRDFQNFromTypeMeta(nil)).To(BeEmpty()) +} + +// mockEnhancedCheck is a check that embeds EnhancedVerboseFormatter. +type mockEnhancedCheck struct { + check.BaseCheck + check.EnhancedVerboseFormatter +} + +func (c *mockEnhancedCheck) CanApply(_ context.Context, _ check.Target) (bool, error) { + return true, nil +} + +func (c *mockEnhancedCheck) Validate(_ context.Context, _ check.Target) (*result.DiagnosticResult, error) { + return c.NewResult(), nil +} + // mockFormatterCheck is a check that implements VerboseOutputFormatter. type mockFormatterCheck struct { check.BaseCheck diff --git a/pkg/lint/checks/components/kueue/kueue_operator_installed.go b/pkg/lint/checks/components/kueue/kueue_operator_installed.go index dc3c3dd5..a2dc9380 100644 --- a/pkg/lint/checks/components/kueue/kueue_operator_installed.go +++ b/pkg/lint/checks/components/kueue/kueue_operator_installed.go @@ -19,12 +19,14 @@ const ( checkTypeOperatorInstalled = "operator-installed" subscriptionName = "kueue-operator" annotationInstalledVersion = "operator.opendatahub.io/installed-version" + msgManagedNotSupported = "Kueue managementState is Managed — migration to the Red Hat build of Kueue operator is required before upgrading" ) // OperatorInstalledCheck validates the Red Hat build of Kueue operator installation status against the Kueue // component management state: -// - Managed + operator present: blocking — the two cannot coexist +// - Managed: prohibited — no supported upgrade path from embedded Kueue, must migrate to RHBoK first // - Unmanaged + operator absent: blocking — Unmanaged requires the Red Hat build of Kueue operator +// - Unmanaged + operator present: pass type OperatorInstalledCheck struct { check.BaseCheck } @@ -57,6 +59,15 @@ func (c *OperatorInstalledCheck) CanApply(ctx context.Context, target check.Targ func (c *OperatorInstalledCheck) Validate(ctx context.Context, target check.Target) (*result.DiagnosticResult, error) { return validate.Component(c, target). Run(ctx, func(ctx context.Context, req *validate.ComponentRequest) error { + // Managed state is unconditionally prohibited — no supported upgrade path + // from embedded Kueue. Emit the condition without querying OLM since the + // operator presence is irrelevant for this state. + if req.ManagementState == constants.ManagementStateManaged { + c.validateManaged(req) + + return nil + } + info, err := olm.FindOperator(ctx, req.Client, func(sub *olm.SubscriptionInfo) bool { return sub.Name == subscriptionName }) @@ -68,39 +79,26 @@ func (c *OperatorInstalledCheck) Validate(ctx context.Context, target check.Targ req.Result.Annotations[annotationInstalledVersion] = info.GetVersion() } - switch req.ManagementState { - case constants.ManagementStateManaged: - c.validateManaged(req, info) - case constants.ManagementStateUnmanaged: - c.validateUnmanaged(req, info) - } + c.validateUnmanaged(req, info) return nil }) } -// validateManaged checks that the Red Hat build of Kueue operator is NOT installed when Kueue is Managed. +// validateManaged flags Kueue Managed state as a prohibited condition. +// There is no supported upgrade path from embedded Kueue — migration to the +// Red Hat build of Kueue operator is required before upgrading. func (c *OperatorInstalledCheck) validateManaged( req *validate.ComponentRequest, - info *olm.SubscriptionInfo, ) { - switch { - case info.Found(): - req.Result.SetCondition(check.NewCondition( - check.ConditionTypeCompatible, - metav1.ConditionFalse, - check.WithReason(check.ReasonVersionIncompatible), - check.WithMessage("Red Hat build of Kueue operator (%s) is installed but Kueue managementState is Managed — the two cannot coexist", info.GetVersion()), - check.WithImpact(result.ImpactBlocking), - )) - default: - req.Result.SetCondition(check.NewCondition( - check.ConditionTypeCompatible, - metav1.ConditionTrue, - check.WithReason(check.ReasonVersionCompatible), - check.WithMessage("Red Hat build of Kueue operator is not installed — consistent with Managed state"), - )) - } + req.Result.SetCondition(check.NewCondition( + check.ConditionTypeCompatible, + metav1.ConditionFalse, + check.WithReason(check.ReasonVersionIncompatible), + check.WithMessage(msgManagedNotSupported), + check.WithImpact(result.ImpactProhibited), + check.WithRemediation(managementStateRemediation), + )) } // validateUnmanaged checks that the Red Hat build of Kueue operator IS installed when Kueue is Unmanaged. diff --git a/pkg/lint/checks/components/kueue/kueue_operator_installed_test.go b/pkg/lint/checks/components/kueue/kueue_operator_installed_test.go index ab6766cb..a8354839 100644 --- a/pkg/lint/checks/components/kueue/kueue_operator_installed_test.go +++ b/pkg/lint/checks/components/kueue/kueue_operator_installed_test.go @@ -82,7 +82,7 @@ func TestOperatorInstalledCheck_UnmanagedNotInstalled(t *testing.T) { g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking)) } -// Managed + operator NOT installed = pass. +// Managed + operator NOT installed = prohibited. func TestOperatorInstalledCheck_ManagedNotInstalled(t *testing.T) { g := NewWithT(t) ctx := t.Context() @@ -101,13 +101,14 @@ func TestOperatorInstalledCheck_ManagedNotInstalled(t *testing.T) { g.Expect(result.Status.Conditions).To(HaveLen(1)) g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ "Type": Equal(check.ConditionTypeCompatible), - "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(check.ReasonVersionCompatible), - "Message": ContainSubstring("Managed"), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal(check.ReasonVersionIncompatible), + "Message": ContainSubstring("migration to the Red Hat build of Kueue operator is required"), })) + g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactProhibited)) } -// Managed + operator installed = blocking. +// Managed + operator installed = prohibited. func TestOperatorInstalledCheck_ManagedInstalled(t *testing.T) { g := NewWithT(t) ctx := t.Context() @@ -128,10 +129,11 @@ func TestOperatorInstalledCheck_ManagedInstalled(t *testing.T) { "Type": Equal(check.ConditionTypeCompatible), "Status": Equal(metav1.ConditionFalse), "Reason": Equal(check.ReasonVersionIncompatible), - "Message": And(ContainSubstring("Red Hat build of Kueue operator"), ContainSubstring("cannot coexist")), + "Message": ContainSubstring("migration to the Red Hat build of Kueue operator is required"), })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Annotations).To(HaveKeyWithValue("operator.opendatahub.io/installed-version", "kueue-operator.v0.6.0")) + g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactProhibited)) + // Managed state skips OLM lookup entirely, so no installed-version annotation is set. + g.Expect(result.Annotations).ToNot(HaveKey("operator.opendatahub.io/installed-version")) } func TestOperatorInstalledCheck_CanApply_ManagementState(t *testing.T) { diff --git a/pkg/lint/checks/workloads/kueue/check.go b/pkg/lint/checks/workloads/kueue/check.go index e937a471..d863b105 100644 --- a/pkg/lint/checks/workloads/kueue/check.go +++ b/pkg/lint/checks/workloads/kueue/check.go @@ -3,63 +3,42 @@ package kueue import ( "context" "fmt" + "strconv" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/opendatahub-io/odh-cli/pkg/constants" "github.com/opendatahub-io/odh-cli/pkg/lint/check" "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/validate" - "github.com/opendatahub-io/odh-cli/pkg/resources" -) - -const ( - remediationLabeledInNonKueueNs = "Add the kueue-managed or kueue.openshift.io/managed label to the affected namespaces, " + - "or remove the kueue.x-k8s.io/queue-name label from the workloads if kueue integration is not intended" - - remediationMissingLabelInKueueNs = "Add the kueue.x-k8s.io/queue-name label to the workloads in kueue-enabled namespaces, " + - "or remove the kueue-managed or kueue.openshift.io/managed label from the namespaces if kueue integration is not intended for those workloads" + "github.com/opendatahub-io/odh-cli/pkg/util/client" ) -// CheckConfig holds the per-resource parameters that differentiate each kueue label check. -type CheckConfig struct { - Kind string - Resource resources.ResourceType - ConditionType string - MissingLabelConditionType string - KindLabel string - CheckID string - CheckName string -} - -// KueueLabelCheck verifies that workloads with the kueue queue label are in -// kueue-enabled namespaces, and that workloads in kueue-enabled namespaces -// have the kueue queue label. -type KueueLabelCheck struct { +// DataIntegrityCheck verifies that the cluster is in a consistent state +// with respect to kueue labels. It checks three invariants: +// 1. Every workload in a kueue-managed namespace has the queue-name label +// 2. Every workload with the queue-name label is in a kueue-managed namespace +// 3. Within each top-level CR's ownership tree, all resources agree on the queue-name label +type DataIntegrityCheck struct { check.BaseCheck - - resource resources.ResourceType - conditionType string - missingLabelConditionType string - kindLabel string + check.EnhancedVerboseFormatter } -func NewCheck(cfg CheckConfig) *KueueLabelCheck { - return &KueueLabelCheck{ +func NewDataIntegrityCheck() *DataIntegrityCheck { + return &DataIntegrityCheck{ BaseCheck: check.BaseCheck{ CheckGroup: check.GroupWorkload, - Kind: cfg.Kind, + Kind: constants.ComponentKueue, Type: check.CheckTypeDataIntegrity, - CheckID: cfg.CheckID, - CheckName: cfg.CheckName, - CheckDescription: fmt.Sprintf("Verifies that %ss with the kueue queue label are in kueue-enabled namespaces", cfg.KindLabel), - CheckRemediation: remediationLabeledInNonKueueNs, + CheckID: "workloads.kueue.data-integrity", + CheckName: "Workloads :: Kueue :: Data Integrity", + CheckDescription: "Verifies that kueue namespace labels and workload queue-name labels are consistent across the cluster", + CheckRemediation: remediationConsistency, }, - resource: cfg.Resource, - conditionType: cfg.ConditionType, - missingLabelConditionType: cfg.MissingLabelConditionType, - kindLabel: cfg.KindLabel, } } -func (c *KueueLabelCheck) CanApply(ctx context.Context, target check.Target) (bool, error) { +func (c *DataIntegrityCheck) CanApply(ctx context.Context, target check.Target) (bool, error) { ok, err := IsKueueActive(ctx, target) if err != nil { return false, fmt.Errorf("checking kueue state: %w", err) @@ -68,15 +47,271 @@ func (c *KueueLabelCheck) CanApply(ctx context.Context, target check.Target) (bo return ok, nil } -func (c *KueueLabelCheck) Validate( +// Validate does not use the existing validate.Workloads or validate.WorkloadsMetadata builders +// because this check spans multiple resource types, performs namespace-level grouping with a +// shared ownership graph, and emits a single cluster-wide condition rather than per-resource +// conditions. If more cluster-wide consistency checks emerge, a validate.ClusterWide builder +// should be introduced to capture this pattern. +func (c *DataIntegrityCheck) Validate( ctx context.Context, target check.Target, ) (*result.DiagnosticResult, error) { - return validate.WorkloadsMetadata(c, target, c.resource). - Run(ctx, ValidateFn( - c.resource, - c.conditionType, - c.missingLabelConditionType, - c.kindLabel, + dr := c.NewResult() + + if target.TargetVersion != nil { + dr.Annotations[check.AnnotationCheckTargetVersion] = target.TargetVersion.String() + } + + // Phase 1: determine relevant namespaces. + kueueNamespaces, err := kueueEnabledNamespaces(ctx, target.Client) + if err != nil { + return nil, fmt.Errorf("finding kueue-enabled namespaces: %w", err) + } + + workloadNamespaces, err := workloadLabeledNamespaces(ctx, target.Client) + if err != nil { + return nil, fmt.Errorf("finding workload-labeled namespaces: %w", err) + } + + relevantNamespaces := kueueNamespaces.Union(workloadNamespaces) + + if relevantNamespaces.Len() == 0 { + dr.SetCondition(check.NewCondition( + conditionTypeKueueConsistency, + metav1.ConditionTrue, + check.WithReason(check.ReasonRequirementsMet), + check.WithMessage(msgNoRelevantNamespaces), )) + dr.Annotations[check.AnnotationImpactedWorkloadCount] = "0" + + return dr, nil + } + + // Phase 2 & 3: check invariants per namespace. + var violations []violation + + for _, namespace := range sets.List(relevantNamespaces) { + namespaceViolations, err := c.checkNamespace(ctx, target.Client, namespace, kueueNamespaces) + if err != nil { + return nil, fmt.Errorf("checking namespace %s: %w", namespace, err) + } + + violations = append(violations, namespaceViolations...) + } + + // Phase 4: emit result. + impacted := uniqueResources(violations) + dr.Annotations[check.AnnotationImpactedWorkloadCount] = strconv.Itoa(len(impacted)) + + if len(violations) == 0 { + dr.SetCondition(check.NewCondition( + conditionTypeKueueConsistency, + metav1.ConditionTrue, + check.WithReason(check.ReasonRequirementsMet), + check.WithMessage(msgConsistent), + )) + + return dr, nil + } + + dr.SetCondition(check.NewCondition( + conditionTypeKueueConsistency, + metav1.ConditionFalse, + check.WithReason(check.ReasonConfigurationInvalid), + check.WithMessage(msgInconsistent, len(violations)), + check.WithImpact(result.ImpactProhibited), + check.WithRemediation(c.CheckRemediation), + )) + + // Populate impacted objects — only top-level CRs. + populateImpactedObjects(dr, impacted) + + return dr, nil +} + +// checkNamespace checks all three invariants for workloads in a single namespace. +func (c *DataIntegrityCheck) checkNamespace( + ctx context.Context, + r client.Reader, + namespace string, + kueueNamespaces sets.Set[string], +) ([]violation, error) { + // List all top-level CRs in this namespace (metadata-only). + workloads, err := listWorkloadsInNamespace(ctx, r, namespace) + if err != nil { + return nil, err + } + + if len(workloads) == 0 { + return nil, nil + } + + // Build ownership graph for invariant 3. + graph, err := buildGraph(ctx, r, namespace) + if err != nil { + return nil, fmt.Errorf("building ownership graph: %w", err) + } + + var violations []violation + + for _, cr := range workloads { + // Invariant 1: namespace → workload. + if v := checkNamespaceToWorkload(cr, kueueNamespaces); v != nil { + violations = append(violations, *v) + + continue + } + + // Invariant 2: workload → namespace. + if v := checkWorkloadToNamespace(cr, kueueNamespaces); v != nil { + violations = append(violations, *v) + + continue + } + + // Invariant 3: owner tree consistency. + if v := checkOwnerTreeConsistency(cr, graph); v != nil { + violations = append(violations, *v) + } + } + + return violations, nil +} + +// listWorkloadsInNamespace lists all monitored workload types in the given namespace. +func listWorkloadsInNamespace( + ctx context.Context, + r client.Reader, + namespace string, +) ([]*metav1.PartialObjectMetadata, error) { + var all []*metav1.PartialObjectMetadata + + for _, rt := range monitoredWorkloadTypes { + items, err := r.ListMetadata(ctx, rt, client.WithNamespace(namespace)) + if err != nil { + // A missing CRD means the resource type is not installed on this cluster, + // so there are zero instances. Ideally ListMetadata would handle this + // the same way it handles permission errors (return empty list). + if client.IsResourceTypeNotFound(err) { + continue + } + + return nil, fmt.Errorf("listing %s in namespace %s: %w", rt.Kind, namespace, err) + } + + // ListMetadata returns PartialObjectMetadata items whose Kind and APIVersion + // reflect the metadata wrapper ("PartialObjectMetadata" / "meta.k8s.io/v1") + // rather than the actual resource type. Copy each item with the correct + // TypeMeta so violation messages reference the real kind (e.g. "Notebook") + // and we avoid mutating pointers owned by the caller. + for _, item := range items { + obj := &metav1.PartialObjectMetadata{ + TypeMeta: rt.TypeMeta(), + ObjectMeta: *item.ObjectMeta.DeepCopy(), + } + all = append(all, obj) + } + } + + return all, nil +} + +// impactedResource holds the identity, type, and violation context of a top-level CR +// that failed one or more kueue consistency invariants. +type impactedResource struct { + Namespace string + Name string + Kind string + APIVersion string + Message string +} + +// uniqueResources deduplicates violations by resource identity (kind, apiVersion, namespace, name), +// returning the unique set of impacted top-level CRs. +func uniqueResources(violations []violation) []impactedResource { + type resourceKey struct { + Kind string + APIVersion string + Namespace string + Name string + } + + seen := make(map[resourceKey]struct{}) + + var unique []impactedResource + + for i := range violations { + key := resourceKey{ + Kind: violations[i].Kind, + APIVersion: violations[i].APIVersion, + Namespace: violations[i].Resource.Namespace, + Name: violations[i].Resource.Name, + } + + if _, exists := seen[key]; !exists { + seen[key] = struct{}{} + unique = append(unique, impactedResource{ + Namespace: violations[i].Resource.Namespace, + Name: violations[i].Resource.Name, + Kind: violations[i].Kind, + APIVersion: violations[i].APIVersion, + Message: violations[i].Message, + }) + } + } + + return unique +} + +// populateImpactedObjects sets impacted objects on the diagnostic result. +// Only top-level CRs from the monitored list appear as impacted objects. +// Each object carries per-object annotations for context and CRD FQN. +func populateImpactedObjects( + dr *result.DiagnosticResult, + impacted []impactedResource, +) { + // Build lookup from APIVersion+Kind to authoritative CRD FQN + // so the verbose formatter doesn't need to derive plurals naively. + crdfqnLookup := buildCRDFQNLookup() + + dr.ImpactedObjects = make([]metav1.PartialObjectMetadata, 0, len(impacted)) + + for _, r := range impacted { + annotations := make(map[string]string) + + if r.Message != "" { + annotations[result.AnnotationObjectContext] = r.Message + } + + if fqn, ok := crdfqnLookup[r.APIVersion+"/"+r.Kind]; ok { + annotations[result.AnnotationObjectCRDName] = fqn + } + + obj := metav1.PartialObjectMetadata{ + TypeMeta: metav1.TypeMeta{ + Kind: r.Kind, + APIVersion: r.APIVersion, + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.Namespace, + Name: r.Name, + Annotations: annotations, + }, + } + + dr.ImpactedObjects = append(dr.ImpactedObjects, obj) + } +} + +// buildCRDFQNLookup builds a map from "apiVersion/kind" to CRD FQN +// using the authoritative ResourceType definitions from monitoredWorkloadTypes. +func buildCRDFQNLookup() map[string]string { + lookup := make(map[string]string, len(monitoredWorkloadTypes)) + + for _, rt := range monitoredWorkloadTypes { + key := rt.APIVersion() + "/" + rt.Kind + lookup[key] = rt.CRDFQN() + } + + return lookup } diff --git a/pkg/lint/checks/workloads/kueue/check_test.go b/pkg/lint/checks/workloads/kueue/check_test.go new file mode 100644 index 00000000..41d3f6ef --- /dev/null +++ b/pkg/lint/checks/workloads/kueue/check_test.go @@ -0,0 +1,768 @@ +package kueue_test + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/opendatahub-io/odh-cli/pkg/lint/check" + resultpkg "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" + "github.com/opendatahub-io/odh-cli/pkg/lint/check/testutil" + kueuecheck "github.com/opendatahub-io/odh-cli/pkg/lint/checks/workloads/kueue" + "github.com/opendatahub-io/odh-cli/pkg/resources" + + . "github.com/onsi/gomega" +) + +//nolint:gochecknoglobals // Test fixture - shared across test functions. +var listKinds = map[schema.GroupVersionResource]string{ + resources.DataScienceCluster.GVR(): resources.DataScienceCluster.ListKind(), + resources.Namespace.GVR(): resources.Namespace.ListKind(), + resources.Notebook.GVR(): resources.Notebook.ListKind(), + resources.InferenceService.GVR(): resources.InferenceService.ListKind(), + resources.LLMInferenceService.GVR(): resources.LLMInferenceService.ListKind(), + resources.RayCluster.GVR(): resources.RayCluster.ListKind(), + resources.RayJob.GVR(): resources.RayJob.ListKind(), + resources.PyTorchJob.GVR(): resources.PyTorchJob.ListKind(), + resources.Pod.GVR(): resources.Pod.ListKind(), + resources.Deployment.GVR(): resources.Deployment.ListKind(), + resources.StatefulSet.GVR(): resources.StatefulSet.ListKind(), + resources.ReplicaSet.GVR(): resources.ReplicaSet.ListKind(), + resources.DaemonSet.GVR(): resources.DaemonSet.ListKind(), + resources.Job.GVR(): resources.Job.ListKind(), + resources.CronJob.GVR(): resources.CronJob.ListKind(), +} + +func newNamespace(name string, labels map[string]string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": resources.Namespace.APIVersion(), + "kind": resources.Namespace.Kind, + "metadata": map[string]any{ + "name": name, + "labels": toAnyMap(labels), + }, + }, + } +} + +func newWorkload( + rt resources.ResourceType, + namespace string, + name string, + uid string, + labels map[string]string, +) *unstructured.Unstructured { + meta := map[string]any{ + "name": name, + "namespace": namespace, + "uid": uid, + } + if labels != nil { + meta["labels"] = toAnyMap(labels) + } + + return &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": rt.APIVersion(), + "kind": rt.Kind, + "metadata": meta, + }, + } +} + +func newOwnedResource( + rt resources.ResourceType, + namespace string, + name string, + uid string, + ownerUID string, + ownerKind string, + labels map[string]string, +) *unstructured.Unstructured { + meta := map[string]any{ + "name": name, + "namespace": namespace, + "uid": uid, + "ownerReferences": []any{ + map[string]any{ + "apiVersion": "v1", + "kind": ownerKind, + "name": "owner", + "uid": ownerUID, + }, + }, + } + if labels != nil { + meta["labels"] = toAnyMap(labels) + } + + return &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": rt.APIVersion(), + "kind": rt.Kind, + "metadata": meta, + }, + } +} + +func toAnyMap(m map[string]string) map[string]any { + result := make(map[string]any, len(m)) + for k, v := range m { + result[k] = v + } + + return result +} + +func TestDataIntegrityCheck_CanApply_KueueNotActive(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Removed", + }) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + applies, err := chk.CanApply(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(applies).To(BeFalse()) +} + +func TestDataIntegrityCheck_CanApply_KueueActive(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + applies, err := chk.CanApply(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(applies).To(BeTrue()) +} + +func TestDataIntegrityCheck_CanApply_KueueUnmanaged(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Unmanaged", + }) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + applies, err := chk.CanApply(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(applies).To(BeTrue()) +} + +func TestDataIntegrityCheck_BenignWorkload_NoViolations(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + // Non-kueue namespace. + ns := newNamespace("team-b", nil) + + // Workload without queue-name label in non-kueue namespace — completely benign. + nb := newWorkload(resources.Notebook, "team-b", "my-notebook", "nb-uid-1", nil) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions).To(HaveLen(1)) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionTrue)), + )) + g.Expect(result.ImpactedObjects).To(BeEmpty()) +} + +func TestDataIntegrityCheck_NoRelevantNamespaces(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions).To(HaveLen(1)) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionTrue)), + )) + g.Expect(result.ImpactedObjects).To(BeEmpty()) +} + +func TestDataIntegrityCheck_FullyConsistent(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", + map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"}) + + // Pod owned by the notebook with matching label. + pod := newOwnedResource(resources.Pod, "team-a", "my-notebook-0", "pod-uid-1", + "nb-uid-1", "Notebook", + map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"}) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb, pod}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions).To(HaveLen(1)) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionTrue)), + HaveField("Reason", Equal(check.ReasonRequirementsMet)), + )) + g.Expect(result.ImpactedObjects).To(BeEmpty()) +} + +func TestDataIntegrityCheck_Invariant1_MissingLabelInKueueNamespace(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + // Notebook in kueue-managed namespace but WITHOUT queue-name label. + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", nil) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions).To(HaveLen(1)) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionFalse)), + )) + g.Expect(result.ImpactedObjects).To(HaveLen(1)) + g.Expect(result.ImpactedObjects[0]).To(And( + HaveField("Namespace", Equal("team-a")), + HaveField("Name", Equal("my-notebook")), + )) +} + +func TestDataIntegrityCheck_Invariant2_LabeledInNonKueueNamespace(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + // Namespace WITHOUT kueue-managed label. + ns := newNamespace("team-b", nil) + + // Notebook WITH queue-name label in non-kueue namespace. + nb := newWorkload(resources.Notebook, "team-b", "my-notebook", "nb-uid-1", + map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"}) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions).To(HaveLen(1)) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionFalse)), + )) + g.Expect(result.ImpactedObjects).To(HaveLen(1)) +} + +func TestDataIntegrityCheck_Invariant3_OwnerTreeMismatch(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + // Notebook with queue-name label. + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", + map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"}) + + // Pod owned by notebook but MISSING queue-name label. + pod := newOwnedResource(resources.Pod, "team-a", "my-notebook-0", "pod-uid-1", + "nb-uid-1", "Notebook", nil) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb, pod}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions).To(HaveLen(1)) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionFalse)), + )) + g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) + g.Expect(result.ImpactedObjects).To(HaveLen(1)) +} + +func TestDataIntegrityCheck_Invariant3_ChildHasLabelRootDoesNot(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + // Notebook WITHOUT queue-name label (but in kueue namespace — so invariant 1 fires first). + // To isolate invariant 3, the notebook needs the label but pod has a different value. + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", + map[string]string{"kueue.x-k8s.io/queue-name": "queue-a"}) + + // Pod owned by notebook with DIFFERENT queue-name value. + pod := newOwnedResource(resources.Pod, "team-a", "my-notebook-0", "pod-uid-1", + "nb-uid-1", "Notebook", + map[string]string{"kueue.x-k8s.io/queue-name": "queue-b"}) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb, pod}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionFalse)), + )) + g.Expect(result.ImpactedObjects).To(HaveLen(1)) +} + +func TestDataIntegrityCheck_Invariant3_SingleNodeTreeConsistent(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + // Notebook with label, no children — trivially consistent. + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", + map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"}) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions).To(HaveLen(1)) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionTrue)), + )) + g.Expect(result.ImpactedObjects).To(BeEmpty()) +} + +func TestDataIntegrityCheck_MultiLevelOwnerChain(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + queueLabels := map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"} + + // InferenceService → Deployment → ReplicaSet → Pod, all with matching labels. + isvc := newWorkload(resources.InferenceService, "team-a", "my-isvc", "isvc-uid-1", queueLabels) + + deploy := newOwnedResource(resources.Deployment, "team-a", "my-isvc-deploy", "deploy-uid-1", + "isvc-uid-1", "InferenceService", queueLabels) + + rs := newOwnedResource(resources.ReplicaSet, "team-a", "my-isvc-rs", "rs-uid-1", + "deploy-uid-1", "Deployment", queueLabels) + + pod := newOwnedResource(resources.Pod, "team-a", "my-isvc-pod", "pod-uid-1", + "rs-uid-1", "ReplicaSet", queueLabels) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, isvc, deploy, rs, pod}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionTrue)), + )) +} + +func TestDataIntegrityCheck_MixedViolationsAcrossNamespaces(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + kueueNs := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + // Non-kueue namespace. + regularNs := newNamespace("team-b", nil) + + // Invariant 1 violation: notebook in kueue-ns missing label. + nb1 := newWorkload(resources.Notebook, "team-a", "unlabeled-nb", "nb-uid-1", nil) + + // Invariant 2 violation: notebook in non-kueue-ns with label. + nb2 := newWorkload(resources.Notebook, "team-b", "labeled-nb", "nb-uid-2", + map[string]string{"kueue.x-k8s.io/queue-name": "some-queue"}) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, kueueNs, regularNs, nb1, nb2}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionFalse)), + )) + // Two distinct violations across two namespaces. + g.Expect(result.ImpactedObjects).To(HaveLen(2)) + g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "2")) +} + +func TestDataIntegrityCheck_BlockingImpact(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + // Invariant 1 violation. + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", nil) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions[0]).To(HaveField("Impact", Equal(resultpkg.ImpactProhibited))) +} + +func TestDataIntegrityCheck_AnnotationCheckTargetVersion(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationCheckTargetVersion, "3.0.0")) +} + +func TestDataIntegrityCheck_CheckMetadata(t *testing.T) { + g := NewWithT(t) + + chk := kueuecheck.NewDataIntegrityCheck() + + g.Expect(chk.ID()).To(Equal("workloads.kueue.data-integrity")) + g.Expect(chk.Name()).To(Equal("Workloads :: Kueue :: Data Integrity")) + g.Expect(chk.Group()).To(Equal(check.GroupWorkload)) + g.Expect(chk.CheckKind()).To(Equal("kueue")) + g.Expect(chk.CheckType()).To(Equal("data-integrity")) + g.Expect(chk.Description()).ToNot(BeEmpty()) + g.Expect(chk.Remediation()).ToNot(BeEmpty()) +} + +func TestDataIntegrityCheck_OpenshiftManagedLabel(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + // Uses the kueue.openshift.io/managed label instead of kueue-managed. + ns := newNamespace("team-a", map[string]string{ + "kueue.openshift.io/managed": "true", + }) + + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", + map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"}) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions[0]).To(And( + HaveField("Type", Equal("KueueConsistency")), + HaveField("Status", Equal(metav1.ConditionTrue)), + )) +} + +func TestDataIntegrityCheck_Invariant1_ObjectContextAnnotation(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + // Invariant 1: notebook in kueue namespace missing label. + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", nil) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.ImpactedObjects).To(HaveLen(1)) + g.Expect(result.ImpactedObjects[0].Annotations).To( + HaveKeyWithValue(resultpkg.AnnotationObjectContext, + "Notebook team-a/my-notebook is in kueue-managed namespace team-a but missing kueue.x-k8s.io/queue-name label"), + ) +} + +func TestDataIntegrityCheck_Invariant2_ObjectContextAnnotation(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-b", nil) + + // Invariant 2: notebook with label in non-kueue namespace. + nb := newWorkload(resources.Notebook, "team-b", "my-notebook", "nb-uid-1", + map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"}) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.ImpactedObjects).To(HaveLen(1)) + g.Expect(result.ImpactedObjects[0].Annotations).To( + HaveKeyWithValue(resultpkg.AnnotationObjectContext, + "Notebook team-b/my-notebook has kueue.x-k8s.io/queue-name=default-queue but namespace is not kueue-managed"), + ) +} + +func TestDataIntegrityCheck_Invariant3_ObjectContextAnnotation(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + nb := newWorkload(resources.Notebook, "team-a", "my-notebook", "nb-uid-1", + map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"}) + + // Pod missing the label — triggers invariant 3. + pod := newOwnedResource(resources.Pod, "team-a", "my-notebook-0", "pod-uid-1", + "nb-uid-1", "Notebook", nil) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb, pod}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.ImpactedObjects).To(HaveLen(1)) + g.Expect(result.ImpactedObjects[0].Annotations).To( + HaveKeyWithValue(resultpkg.AnnotationObjectContext, + "Notebook team-a/my-notebook has kueue.x-k8s.io/queue-name=default-queue but descendant Pod team-a/my-notebook-0 is missing the label"), + ) +} + +func TestDataIntegrityCheck_MultipleWorkloadTypes(t *testing.T) { + g := NewWithT(t) + + dsc := testutil.NewDSC(map[string]string{ + "kueue": "Managed", + }) + + ns := newNamespace("team-a", map[string]string{ + "kueue-managed": "true", + }) + + queueLabels := map[string]string{"kueue.x-k8s.io/queue-name": "default-queue"} + + nb := newWorkload(resources.Notebook, "team-a", "nb1", "nb-uid-1", queueLabels) + isvc := newWorkload(resources.InferenceService, "team-a", "isvc1", "isvc-uid-1", queueLabels) + ray := newWorkload(resources.RayCluster, "team-a", "ray1", "ray-uid-1", queueLabels) + + target := testutil.NewTarget(t, testutil.TargetConfig{ + ListKinds: listKinds, + Objects: []*unstructured.Unstructured{dsc, ns, nb, isvc, ray}, + CurrentVersion: "2.17.0", + TargetVersion: "3.0.0", + }) + + chk := kueuecheck.NewDataIntegrityCheck() + result, err := chk.Validate(t.Context(), target) + + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(result.Status.Conditions[0]).To(HaveField("Status", Equal(metav1.ConditionTrue))) +} diff --git a/pkg/lint/checks/workloads/kueue/graph.go b/pkg/lint/checks/workloads/kueue/graph.go new file mode 100644 index 00000000..efcc008b --- /dev/null +++ b/pkg/lint/checks/workloads/kueue/graph.go @@ -0,0 +1,106 @@ +package kueue + +import ( + "context" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "github.com/opendatahub-io/odh-cli/pkg/util/client" +) + +// graphNode represents a single resource in the ownership graph, +// holding only the metadata needed for label consistency checks. +type graphNode struct { + UID types.UID + Name string + Namespace string + Kind string + Labels map[string]string +} + +// ownershipGraph maps parent UIDs to their direct children. +// Built once per namespace and reused for all top-level CRs in that namespace. +type ownershipGraph struct { + children map[types.UID][]graphNode +} + +// buildGraph lists all intermediate resource types in the given namespace +// and builds a parent→children map keyed by ownerReference UID. +func buildGraph( + ctx context.Context, + r client.Reader, + namespace string, +) (*ownershipGraph, error) { + graph := &ownershipGraph{ + children: make(map[types.UID][]graphNode), + } + + for _, rt := range intermediateTypes { + items, err := r.ListMetadata(ctx, rt, client.WithNamespace(namespace)) + if err != nil { + // A missing CRD means the resource type is not installed on this cluster, + // so there are zero instances. Ideally ListMetadata would handle this + // the same way it handles permission errors (return empty list). + if client.IsResourceTypeNotFound(err) { + continue + } + + return nil, fmt.Errorf("listing %s in namespace %s: %w", rt.Kind, namespace, err) + } + + // ListMetadata returns PartialObjectMetadata whose Kind is + // "PartialObjectMetadata" rather than the real resource kind. + // Pass the real kind from the resource type when building nodes + // to avoid mutating pointers owned by the caller. + for _, item := range items { + node := newGraphNode(item, rt.Kind) + + for _, ref := range item.GetOwnerReferences() { + graph.children[ref.UID] = append(graph.children[ref.UID], node) + } + } + } + + return graph, nil +} + +// walkSubtree collects all descendants of the given root UID recursively. +// Returns an empty slice if the root has no children (single-node tree). +// Uses a visited set to guard against owner-reference cycles that could +// arise from stale cached data or buggy controllers. +func (g *ownershipGraph) walkSubtree(rootUID types.UID) []graphNode { + var result []graphNode + + visited := make(map[types.UID]struct{}) + queue := []types.UID{rootUID} + + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + + if _, seen := visited[current]; seen { + continue + } + + visited[current] = struct{}{} + + for _, child := range g.children[current] { + result = append(result, child) + queue = append(queue, child.UID) + } + } + + return result +} + +func newGraphNode(item *metav1.PartialObjectMetadata, kind string) graphNode { + return graphNode{ + UID: item.GetUID(), + Name: item.GetName(), + Namespace: item.GetNamespace(), + Kind: kind, + Labels: item.GetLabels(), + } +} diff --git a/pkg/lint/checks/workloads/kueue/invariants.go b/pkg/lint/checks/workloads/kueue/invariants.go new file mode 100644 index 00000000..3b1dfe4a --- /dev/null +++ b/pkg/lint/checks/workloads/kueue/invariants.go @@ -0,0 +1,154 @@ +package kueue + +import ( + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/opendatahub-io/odh-cli/pkg/constants" +) + +// violation describes a single consistency failure for a top-level CR. +type violation struct { + // Resource identifies the top-level CR that is in violation. + Resource types.NamespacedName + + // Kind is the Kubernetes Kind of the top-level CR (e.g. "Notebook"). + Kind string + + // APIVersion is the API version of the top-level CR (e.g. "kubeflow.org/v1"). + APIVersion string + + // Message is a detailed, human-readable description of the violation. + // Rendered as per-object context in verbose output via AnnotationObjectContext. + Message string +} + +// checkNamespaceToWorkload checks invariant 1: a CR in a kueue-managed namespace +// must have the kueue.x-k8s.io/queue-name label. +// Returns a violation if the CR is in a kueue-managed namespace but missing the label. +func checkNamespaceToWorkload( + cr *metav1.PartialObjectMetadata, + kueueNamespaces sets.Set[string], +) *violation { + if !kueueNamespaces.Has(cr.GetNamespace()) { + return nil + } + + if _, ok := cr.GetLabels()[constants.LabelKueueQueueName]; ok { + return nil + } + + return &violation{ + Resource: types.NamespacedName{ + Namespace: cr.GetNamespace(), + Name: cr.GetName(), + }, + Kind: cr.Kind, + APIVersion: cr.APIVersion, + Message: fmt.Sprintf( + msgInvariant1, cr.Kind, cr.GetNamespace(), cr.GetName(), cr.GetNamespace(), + ), + } +} + +// checkWorkloadToNamespace checks invariant 2: a CR with the kueue.x-k8s.io/queue-name +// label must reside in a kueue-managed namespace. +// Returns a violation if the CR has the label but its namespace is not kueue-managed. +func checkWorkloadToNamespace( + cr *metav1.PartialObjectMetadata, + kueueNamespaces sets.Set[string], +) *violation { + queueName, ok := cr.GetLabels()[constants.LabelKueueQueueName] + if !ok { + return nil + } + + if kueueNamespaces.Has(cr.GetNamespace()) { + return nil + } + + return &violation{ + Resource: types.NamespacedName{ + Namespace: cr.GetNamespace(), + Name: cr.GetName(), + }, + Kind: cr.Kind, + APIVersion: cr.APIVersion, + Message: fmt.Sprintf( + msgInvariant2, cr.Kind, cr.GetNamespace(), cr.GetName(), queueName, + ), + } +} + +// checkOwnerTreeConsistency checks invariant 3: within the ownership tree of a +// top-level CR, all resources must agree on the kueue queue-name label. +// Either every resource has the label with the same value, or none have it. +// Returns a violation on the first disagreement found (first-violation-wins). +func checkOwnerTreeConsistency( + cr *metav1.PartialObjectMetadata, + graph *ownershipGraph, +) *violation { + descendants := graph.walkSubtree(cr.GetUID()) + if len(descendants) == 0 { + // Single-node tree is trivially consistent. + return nil + } + + // Collect the label state from the root CR. + rootValue, rootHas := cr.GetLabels()[constants.LabelKueueQueueName] + + // Check each descendant for agreement with the root. + for i := range descendants { + childValue, childHas := descendants[i].Labels[constants.LabelKueueQueueName] + + switch { + case rootHas && !childHas: + return &violation{ + Resource: types.NamespacedName{ + Namespace: cr.GetNamespace(), + Name: cr.GetName(), + }, + Kind: cr.Kind, + APIVersion: cr.APIVersion, + Message: fmt.Sprintf( + msgInvariant3Missing, + cr.Kind, cr.GetNamespace(), cr.GetName(), rootValue, + descendants[i].Kind, descendants[i].Namespace, descendants[i].Name, + ), + } + case !rootHas && childHas: + return &violation{ + Resource: types.NamespacedName{ + Namespace: cr.GetNamespace(), + Name: cr.GetName(), + }, + Kind: cr.Kind, + APIVersion: cr.APIVersion, + Message: fmt.Sprintf( + msgInvariant3Unexpected, + descendants[i].Kind, descendants[i].Namespace, descendants[i].Name, childValue, + cr.Kind, cr.GetNamespace(), cr.GetName(), + ), + } + case rootHas && childHas && rootValue != childValue: + return &violation{ + Resource: types.NamespacedName{ + Namespace: cr.GetNamespace(), + Name: cr.GetName(), + }, + Kind: cr.Kind, + APIVersion: cr.APIVersion, + Message: fmt.Sprintf( + msgInvariant3Mismatch, + descendants[i].Kind, descendants[i].Namespace, descendants[i].Name, childValue, + cr.Kind, cr.GetNamespace(), cr.GetName(), rootValue, + ), + } + } + } + + return nil +} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_isvc.go b/pkg/lint/checks/workloads/kueue/kueue_labels_isvc.go deleted file mode 100644 index cda37f3d..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_isvc.go +++ /dev/null @@ -1,23 +0,0 @@ -package kueue - -import ( - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/resources" -) - -const ( - ConditionTypeISVCKueueLabels = "ISVCKueueLabels" - ConditionTypeISVCKueueMissingLabels = "ISVCKueueMissingLabels" -) - -func NewKueueLabelsISVCCheck() *KueueLabelCheck { - return NewCheck(CheckConfig{ - Kind: constants.ComponentKueue, - Resource: resources.InferenceService, - ConditionType: ConditionTypeISVCKueueLabels, - MissingLabelConditionType: ConditionTypeISVCKueueMissingLabels, - KindLabel: "InferenceService", - CheckID: "workloads.kueue.inferenceservice-labels", - CheckName: "Workloads :: Kueue :: InferenceService Labels", - }) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_isvc_test.go b/pkg/lint/checks/workloads/kueue/kueue_labels_isvc_test.go deleted file mode 100644 index 0de11492..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_isvc_test.go +++ /dev/null @@ -1,405 +0,0 @@ -package kueue_test - -import ( - "fmt" - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/lint/check" - resultpkg "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/testutil" - "github.com/opendatahub-io/odh-cli/pkg/lint/checks/workloads/kueue" - "github.com/opendatahub-io/odh-cli/pkg/resources" - - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" -) - -//nolint:gochecknoglobals -var kueueLabelsISVCListKinds = map[schema.GroupVersionResource]string{ - resources.InferenceService.GVR(): resources.InferenceService.ListKind(), - resources.Namespace.GVR(): resources.Namespace.ListKind(), - resources.DataScienceCluster.GVR(): resources.DataScienceCluster.ListKind(), -} - -func newKueueNamespace(name string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{ - "name": name, - } - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.Namespace.APIVersion(), - "kind": resources.Namespace.Kind, - "metadata": metadata, - }, - } -} - -func newISVC(name string, namespace string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{ - "name": name, - "namespace": namespace, - } - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.InferenceService.APIVersion(), - "kind": resources.InferenceService.Kind, - "metadata": metadata, - }, - } -} - -func TestKueueLabelsISVCCheck_Metadata(t *testing.T) { - g := NewWithT(t) - - chk := kueue.NewKueueLabelsISVCCheck() - - g.Expect(chk.ID()).To(Equal("workloads.kueue.inferenceservice-labels")) - g.Expect(chk.Name()).To(Equal("Workloads :: Kueue :: InferenceService Labels")) - g.Expect(chk.Group()).To(Equal(check.GroupWorkload)) - g.Expect(chk.CheckKind()).To(Equal("kueue")) - g.Expect(chk.CheckType()).To(Equal(string(check.CheckTypeDataIntegrity))) - g.Expect(chk.Description()).ToNot(BeEmpty()) - g.Expect(chk.Remediation()).To(ContainSubstring("kueue.x-k8s.io/queue-name")) -} - -func TestKueueLabelsISVCCheck_CanApply_KueueManaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsISVCCheck_CanApply_KueueUnmanaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Unmanaged"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsISVCCheck_CanApply_KueueRemoved(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Removed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeFalse()) -} - -func TestKueueLabelsISVCCheck_NoInferenceServices(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeISVCKueueLabels), - "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(check.ReasonRequirementsMet), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloads, "InferenceService")), - })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactNone)) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeISVCKueueMissingLabels), - "Status": Equal(metav1.ConditionTrue), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "InferenceService")), - })) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsISVCCheck_WithoutQueueLabel(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("user-ns", nil) - isvc := newISVC("my-isvc", "user-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{ns, isvc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "InferenceService"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "InferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsISVCCheck_WithQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - - isvc := newISVC("good-isvc", "kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{ns, isvc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "InferenceService"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "InferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsISVCCheck_WithoutQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - - isvc := newISVC("unlabeled-isvc", "kueue-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{ns, isvc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "InferenceService"))) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeISVCKueueMissingLabels), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(check.ReasonConfigurationInvalid), - "Message": Equal(fmt.Sprintf(kueue.MsgMissingLabelInKueueNs, 1, "InferenceService")), - })) - g.Expect(result.Status.Conditions[1].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("unlabeled-isvc")) - g.Expect(result.ImpactedObjects[0].Namespace).To(Equal("kueue-ns")) -} - -func TestKueueLabelsISVCCheck_LabeledInNonKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("plain-ns", nil) - - isvc := newISVC("bad-isvc", "plain-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{ns, isvc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeISVCKueueLabels), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(check.ReasonConfigurationInvalid), - "Message": Equal(fmt.Sprintf(kueue.MsgNsNotKueueEnabled, 1, "InferenceService")), - })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Status.Conditions[0].Remediation).To(ContainSubstring("kueue.x-k8s.io/queue-name")) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "InferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-isvc")) - g.Expect(result.ImpactedObjects[0].Namespace).To(Equal("plain-ns")) -} - -func TestKueueLabelsISVCCheck_MixedLabeledInferenceServices(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - nsKueue := newKueueNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - nsPlain := newKueueNamespace("plain-ns", nil) - - isvcGood := newISVC("good-isvc", "kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - isvcBad := newISVC("bad-isvc", "plain-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - isvcPlain := newISVC("plain-isvc", "plain-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{nsKueue, nsPlain, isvcGood, isvcBad, isvcPlain}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionFalse)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNsNotKueueEnabled, 1, "InferenceService"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "InferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-isvc")) -} - -func TestKueueLabelsISVCCheck_OpenshiftKueueLabel(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("ocp-kueue-ns", map[string]any{ - constants.LabelKueueOpenshiftManaged: "true", - }) - - isvc := newISVC("good-isvc", "ocp-kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{ns, isvc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "InferenceService"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "InferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsISVCCheck_CustomQueueName(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - - isvc := newISVC("custom-queue-isvc", "kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "team-queue", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - Objects: []*unstructured.Unstructured{ns, isvc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "InferenceService"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "InferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsISVCCheck_AnnotationTargetVersion(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsISVCListKinds, - CurrentVersion: "2.17.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsISVCCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationCheckTargetVersion, "3.0.0")) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_llmisvc.go b/pkg/lint/checks/workloads/kueue/kueue_labels_llmisvc.go deleted file mode 100644 index 2e131bbc..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_llmisvc.go +++ /dev/null @@ -1,23 +0,0 @@ -package kueue - -import ( - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/resources" -) - -const ( - ConditionTypeLLMISVCKueueLabels = "LLMISVCKueueLabels" - ConditionTypeLLMISVCKueueMissingLabels = "LLMISVCKueueMissingLabels" -) - -func NewKueueLabelsLLMCheck() *KueueLabelCheck { - return NewCheck(CheckConfig{ - Kind: constants.ComponentKueue, - Resource: resources.LLMInferenceService, - ConditionType: ConditionTypeLLMISVCKueueLabels, - MissingLabelConditionType: ConditionTypeLLMISVCKueueMissingLabels, - KindLabel: "LLMInferenceService", - CheckID: "workloads.kueue.llminferenceservice-labels", - CheckName: "Workloads :: Kueue :: LLMInferenceService Labels", - }) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_llmisvc_test.go b/pkg/lint/checks/workloads/kueue/kueue_labels_llmisvc_test.go deleted file mode 100644 index 4076c35d..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_llmisvc_test.go +++ /dev/null @@ -1,313 +0,0 @@ -package kueue_test - -import ( - "fmt" - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/lint/check" - resultpkg "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/testutil" - "github.com/opendatahub-io/odh-cli/pkg/lint/checks/workloads/kueue" - "github.com/opendatahub-io/odh-cli/pkg/resources" - - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" -) - -//nolint:gochecknoglobals -var kueueLabelsLLMListKinds = map[schema.GroupVersionResource]string{ - resources.LLMInferenceService.GVR(): resources.LLMInferenceService.ListKind(), - resources.Namespace.GVR(): resources.Namespace.ListKind(), - resources.DataScienceCluster.GVR(): resources.DataScienceCluster.ListKind(), -} - -func newLLMISVC(name string, namespace string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{ - "name": name, - "namespace": namespace, - } - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.LLMInferenceService.APIVersion(), - "kind": resources.LLMInferenceService.Kind, - "metadata": metadata, - }, - } -} - -func TestKueueLabelsLLMCheck_Metadata(t *testing.T) { - g := NewWithT(t) - - chk := kueue.NewKueueLabelsLLMCheck() - - g.Expect(chk.ID()).To(Equal("workloads.kueue.llminferenceservice-labels")) - g.Expect(chk.Name()).To(Equal("Workloads :: Kueue :: LLMInferenceService Labels")) - g.Expect(chk.Group()).To(Equal(check.GroupWorkload)) - g.Expect(chk.CheckKind()).To(Equal("kueue")) - g.Expect(chk.CheckType()).To(Equal(string(check.CheckTypeDataIntegrity))) - g.Expect(chk.Description()).ToNot(BeEmpty()) - g.Expect(chk.Remediation()).To(ContainSubstring("kueue.x-k8s.io/queue-name")) -} - -func TestKueueLabelsLLMCheck_CanApply_KueueManaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsLLMCheck_CanApply_KueueRemoved(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Removed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeFalse()) -} - -func TestKueueLabelsLLMCheck_CanApply_KueueUnmanaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Unmanaged"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsLLMCheck_NoLLMInferenceServices(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeLLMISVCKueueLabels), - "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(check.ReasonRequirementsMet), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloads, "LLMInferenceService")), - })) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeLLMISVCKueueMissingLabels), - "Status": Equal(metav1.ConditionTrue), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "LLMInferenceService")), - })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactNone)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsLLMCheck_WithoutQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - - llm := newLLMISVC("unlabeled-llm", "kueue-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - Objects: []*unstructured.Unstructured{ns, llm}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "LLMInferenceService"))) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeLLMISVCKueueMissingLabels), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(check.ReasonConfigurationInvalid), - "Message": Equal(fmt.Sprintf(kueue.MsgMissingLabelInKueueNs, 1, "LLMInferenceService")), - })) - g.Expect(result.Status.Conditions[1].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("unlabeled-llm")) - g.Expect(result.ImpactedObjects[0].Namespace).To(Equal("kueue-ns")) -} - -func TestKueueLabelsLLMCheck_LabeledInNonKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("plain-ns", nil) - - llm := newLLMISVC("bad-llm", "plain-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - Objects: []*unstructured.Unstructured{ns, llm}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeLLMISVCKueueLabels), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(check.ReasonConfigurationInvalid), - "Message": Equal(fmt.Sprintf(kueue.MsgNsNotKueueEnabled, 1, "LLMInferenceService")), - })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Status.Conditions[0].Remediation).To(ContainSubstring("kueue.x-k8s.io/queue-name")) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "LLMInferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-llm")) - g.Expect(result.ImpactedObjects[0].Namespace).To(Equal("plain-ns")) -} - -func TestKueueLabelsLLMCheck_WithQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - - llm := newLLMISVC("good-llm", "kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - Objects: []*unstructured.Unstructured{ns, llm}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "LLMInferenceService"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "LLMInferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsLLMCheck_MixedLabeledLLMInferenceServices(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - nsKueue := newKueueNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - nsPlain := newKueueNamespace("plain-ns", nil) - - llmGood := newLLMISVC("good-llm", "kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - llmBad := newLLMISVC("bad-llm", "plain-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - llmPlain := newLLMISVC("plain-llm", "plain-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - Objects: []*unstructured.Unstructured{nsKueue, nsPlain, llmGood, llmBad, llmPlain}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionFalse)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNsNotKueueEnabled, 1, "LLMInferenceService"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "LLMInferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-llm")) -} - -func TestKueueLabelsLLMCheck_OpenshiftKueueLabel(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNamespace("ocp-kueue-ns", map[string]any{ - constants.LabelKueueOpenshiftManaged: "true", - }) - - llm := newLLMISVC("good-llm", "ocp-kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsLLMListKinds, - Objects: []*unstructured.Unstructured{ns, llm}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsLLMCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "LLMInferenceService"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "LLMInferenceService"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_notebook.go b/pkg/lint/checks/workloads/kueue/kueue_labels_notebook.go deleted file mode 100644 index 7759b9f3..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_notebook.go +++ /dev/null @@ -1,23 +0,0 @@ -package kueue - -import ( - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/resources" -) - -const ( - ConditionTypeNotebookKueueLabels = "NotebookKueueLabels" - ConditionTypeNotebookKueueMissingLabels = "NotebookKueueMissingLabels" -) - -func NewKueueLabelsNotebookCheck() *KueueLabelCheck { - return NewCheck(CheckConfig{ - Kind: constants.ComponentKueue, - Resource: resources.Notebook, - ConditionType: ConditionTypeNotebookKueueLabels, - MissingLabelConditionType: ConditionTypeNotebookKueueMissingLabels, - KindLabel: "Notebook", - CheckID: "workloads.kueue.notebook-labels", - CheckName: "Workloads :: Kueue :: Notebook Labels", - }) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_notebook_test.go b/pkg/lint/checks/workloads/kueue/kueue_labels_notebook_test.go deleted file mode 100644 index 7026639c..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_notebook_test.go +++ /dev/null @@ -1,407 +0,0 @@ -package kueue_test - -import ( - "fmt" - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/lint/check" - resultpkg "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/testutil" - "github.com/opendatahub-io/odh-cli/pkg/lint/checks/workloads/kueue" - "github.com/opendatahub-io/odh-cli/pkg/resources" - - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" -) - -//nolint:gochecknoglobals -var kueueLabelsNotebookListKinds = map[schema.GroupVersionResource]string{ - resources.Notebook.GVR(): resources.Notebook.ListKind(), - resources.Namespace.GVR(): resources.Namespace.ListKind(), - resources.DataScienceCluster.GVR(): resources.DataScienceCluster.ListKind(), -} - -func newNamespace(name string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{ - "name": name, - } - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.Namespace.APIVersion(), - "kind": resources.Namespace.Kind, - "metadata": metadata, - }, - } -} - -func newNotebook(name string, namespace string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{ - "name": name, - "namespace": namespace, - } - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.Notebook.APIVersion(), - "kind": resources.Notebook.Kind, - "metadata": metadata, - }, - } -} - -func TestKueueLabelsNotebookCheck_Metadata(t *testing.T) { - g := NewWithT(t) - - chk := kueue.NewKueueLabelsNotebookCheck() - - g.Expect(chk.ID()).To(Equal("workloads.kueue.notebook-labels")) - g.Expect(chk.Name()).To(Equal("Workloads :: Kueue :: Notebook Labels")) - g.Expect(chk.Group()).To(Equal(check.GroupWorkload)) - g.Expect(chk.CheckKind()).To(Equal("kueue")) - g.Expect(chk.CheckType()).To(Equal(string(check.CheckTypeDataIntegrity))) - g.Expect(chk.Description()).ToNot(BeEmpty()) - g.Expect(chk.Remediation()).To(ContainSubstring("kueue.x-k8s.io/queue-name")) -} - -func TestKueueLabelsNotebookCheck_CanApply_KueueManaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsNotebookCheck_CanApply_KueueUnmanaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Unmanaged"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsNotebookCheck_CanApply_KueueRemoved(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Removed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeFalse()) -} - -func TestKueueLabelsNotebookCheck_NoNotebooks(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeNotebookKueueLabels), - "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(check.ReasonRequirementsMet), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloads, "Notebook")), - })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactNone)) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeNotebookKueueMissingLabels), - "Status": Equal(metav1.ConditionTrue), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "Notebook")), - })) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsNotebookCheck_NotebookWithoutQueueLabel(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newNamespace("user-ns", nil) - nb := newNotebook("my-notebook", "user-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"}), ns, nb}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "Notebook"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "Notebook"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsNotebookCheck_NotebookWithQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - - nb := newNotebook("good-notebook", "kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"}), ns, nb}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "Notebook"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "Notebook"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsNotebookCheck_NotebookWithoutQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - - nb := newNotebook("unlabeled-notebook", "kueue-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"}), ns, nb}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "Notebook"))) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeNotebookKueueMissingLabels), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(check.ReasonConfigurationInvalid), - "Message": Equal(fmt.Sprintf(kueue.MsgMissingLabelInKueueNs, 1, "Notebook")), - })) - g.Expect(result.Status.Conditions[1].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("unlabeled-notebook")) - g.Expect(result.ImpactedObjects[0].Namespace).To(Equal("kueue-ns")) -} - -func TestKueueLabelsNotebookCheck_LabeledNotebookInNonKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newNamespace("plain-ns", nil) - - nb := newNotebook("bad-notebook", "plain-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{ns, nb}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeNotebookKueueLabels), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(check.ReasonConfigurationInvalid), - "Message": Equal(fmt.Sprintf(kueue.MsgNsNotKueueEnabled, 1, "Notebook")), - })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Status.Conditions[0].Remediation).To(ContainSubstring("kueue.x-k8s.io/queue-name")) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "Notebook"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-notebook")) - g.Expect(result.ImpactedObjects[0].Namespace).To(Equal("plain-ns")) -} - -func TestKueueLabelsNotebookCheck_MixedLabeledNotebooks(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - nsKueue := newNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - nsPlain := newNamespace("plain-ns", nil) - - nbGood := newNotebook("good-notebook", "kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - nbBad := newNotebook("bad-notebook", "plain-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - nbPlain := newNotebook("plain-notebook", "plain-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{nsKueue, nsPlain, nbGood, nbBad, nbPlain}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionFalse)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNsNotKueueEnabled, 1, "Notebook"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "Notebook"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-notebook")) -} - -func TestKueueLabelsNotebookCheck_OpenshiftKueueNamespaceLabel(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newNamespace("ocp-kueue-ns", map[string]any{ - constants.LabelKueueOpenshiftManaged: "true", - }) - - nb := newNotebook("good-notebook", "ocp-kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "default", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"}), ns, nb}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "Notebook"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "Notebook"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsNotebookCheck_NotebookWithCustomQueueName(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newNamespace("kueue-ns", map[string]any{ - constants.LabelKueueManaged: "true", - }) - - nb := newNotebook("custom-queue-notebook", "kueue-ns", map[string]any{ - constants.LabelKueueQueueName: "team-queue", - }) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"}), ns, nb}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "Notebook"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "Notebook"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsNotebookCheck_AnnotationTargetVersion(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsNotebookListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"})}, - CurrentVersion: "2.17.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsNotebookCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationCheckTargetVersion, "3.0.0")) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_pytorchjob.go b/pkg/lint/checks/workloads/kueue/kueue_labels_pytorchjob.go deleted file mode 100644 index fe9a66e3..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_pytorchjob.go +++ /dev/null @@ -1,23 +0,0 @@ -package kueue - -import ( - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/resources" -) - -const ( - ConditionTypePyTorchJobKueueLabels = "PyTorchJobKueueLabels" - ConditionTypePyTorchJobKueueMissingLabels = "PyTorchJobKueueMissingLabels" -) - -func NewKueueLabelsPyTorchJobCheck() *KueueLabelCheck { - return NewCheck(CheckConfig{ - Kind: constants.ComponentKueue, - Resource: resources.PyTorchJob, - ConditionType: ConditionTypePyTorchJobKueueLabels, - MissingLabelConditionType: ConditionTypePyTorchJobKueueMissingLabels, - KindLabel: "PyTorchJob", - CheckID: "workloads.kueue.pytorchjob-labels", - CheckName: "Workloads :: Kueue :: PyTorchJob Labels", - }) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_pytorchjob_test.go b/pkg/lint/checks/workloads/kueue/kueue_labels_pytorchjob_test.go deleted file mode 100644 index 81b9e265..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_pytorchjob_test.go +++ /dev/null @@ -1,266 +0,0 @@ -package kueue_test - -import ( - "fmt" - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/lint/check" - resultpkg "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/testutil" - "github.com/opendatahub-io/odh-cli/pkg/lint/checks/workloads/kueue" - "github.com/opendatahub-io/odh-cli/pkg/resources" - - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" -) - -//nolint:gochecknoglobals -var kueueLabelsPyTorchJobListKinds = map[schema.GroupVersionResource]string{ - resources.PyTorchJob.GVR(): resources.PyTorchJob.ListKind(), - resources.Namespace.GVR(): resources.Namespace.ListKind(), - resources.DataScienceCluster.GVR(): resources.DataScienceCluster.ListKind(), -} - -func newPyTorchJobNamespace(name string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{"name": name} - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.Namespace.APIVersion(), - "kind": resources.Namespace.Kind, - "metadata": metadata, - }, - } -} - -func newPyTorchJob(name string, namespace string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{ - "name": name, - "namespace": namespace, - } - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.PyTorchJob.APIVersion(), - "kind": resources.PyTorchJob.Kind, - "metadata": metadata, - }, - } -} - -func TestKueueLabelsPyTorchJobCheck_Metadata(t *testing.T) { - g := NewWithT(t) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - - g.Expect(chk.ID()).To(Equal("workloads.kueue.pytorchjob-labels")) - g.Expect(chk.Name()).To(Equal("Workloads :: Kueue :: PyTorchJob Labels")) - g.Expect(chk.Group()).To(Equal(check.GroupWorkload)) - g.Expect(chk.CheckKind()).To(Equal("kueue")) - g.Expect(chk.CheckType()).To(Equal(string(check.CheckTypeDataIntegrity))) - g.Expect(chk.Remediation()).To(ContainSubstring("kueue.x-k8s.io/queue-name")) -} - -func TestKueueLabelsPyTorchJobCheck_CanApply_KueueManaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsPyTorchJobListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsPyTorchJobCheck_CanApply_KueueRemoved(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsPyTorchJobListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Removed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeFalse()) -} - -func TestKueueLabelsPyTorchJobCheck_CanApply_KueueUnmanaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsPyTorchJobListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Unmanaged"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsPyTorchJobCheck_NoPyTorchJobs(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsPyTorchJobListKinds, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypePyTorchJobKueueLabels), - "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(check.ReasonRequirementsMet), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloads, "PyTorchJob")), - })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactNone)) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypePyTorchJobKueueMissingLabels), - "Status": Equal(metav1.ConditionTrue), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "PyTorchJob")), - })) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsPyTorchJobCheck_WithoutQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newPyTorchJobNamespace("kueue-ns", map[string]any{constants.LabelKueueManaged: "true"}) - job := newPyTorchJob("unlabeled-job", "kueue-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsPyTorchJobListKinds, - Objects: []*unstructured.Unstructured{ns, job}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "PyTorchJob"))) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypePyTorchJobKueueMissingLabels), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(check.ReasonConfigurationInvalid), - "Message": Equal(fmt.Sprintf(kueue.MsgMissingLabelInKueueNs, 1, "PyTorchJob")), - })) - g.Expect(result.Status.Conditions[1].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("unlabeled-job")) - g.Expect(result.ImpactedObjects[0].Namespace).To(Equal("kueue-ns")) -} - -func TestKueueLabelsPyTorchJobCheck_LabeledInNonKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newPyTorchJobNamespace("plain-ns", nil) - job := newPyTorchJob("bad-job", "plain-ns", map[string]any{constants.LabelKueueQueueName: "default"}) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsPyTorchJobListKinds, - Objects: []*unstructured.Unstructured{ns, job}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionFalse)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNsNotKueueEnabled, 1, "PyTorchJob"))) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "PyTorchJob"))) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-job")) -} - -func TestKueueLabelsPyTorchJobCheck_WithQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newPyTorchJobNamespace("kueue-ns", map[string]any{constants.LabelKueueManaged: "true"}) - job := newPyTorchJob("good-job", "kueue-ns", map[string]any{constants.LabelKueueQueueName: "default"}) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsPyTorchJobListKinds, - Objects: []*unstructured.Unstructured{ns, job}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "PyTorchJob"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "PyTorchJob"))) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsPyTorchJobCheck_WithoutQueueLabel(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newPyTorchJobNamespace("plain-ns", nil) - job := newPyTorchJob("my-job", "plain-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsPyTorchJobListKinds, - Objects: []*unstructured.Unstructured{ns, job}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsPyTorchJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "PyTorchJob"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "PyTorchJob"))) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_raycluster.go b/pkg/lint/checks/workloads/kueue/kueue_labels_raycluster.go deleted file mode 100644 index 33f7c614..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_raycluster.go +++ /dev/null @@ -1,23 +0,0 @@ -package kueue - -import ( - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/resources" -) - -const ( - ConditionTypeRayClusterKueueLabels = "RayClusterKueueLabels" - ConditionTypeRayClusterKueueMissingLabels = "RayClusterKueueMissingLabels" -) - -func NewKueueLabelsRayClusterCheck() *KueueLabelCheck { - return NewCheck(CheckConfig{ - Kind: constants.ComponentKueue, - Resource: resources.RayCluster, - ConditionType: ConditionTypeRayClusterKueueLabels, - MissingLabelConditionType: ConditionTypeRayClusterKueueMissingLabels, - KindLabel: "RayCluster", - CheckID: "workloads.kueue.raycluster-labels", - CheckName: "Workloads :: Kueue :: RayCluster Labels", - }) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_raycluster_test.go b/pkg/lint/checks/workloads/kueue/kueue_labels_raycluster_test.go deleted file mode 100644 index b0c35f8c..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_raycluster_test.go +++ /dev/null @@ -1,263 +0,0 @@ -package kueue_test - -import ( - "fmt" - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/lint/check" - resultpkg "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/testutil" - "github.com/opendatahub-io/odh-cli/pkg/lint/checks/workloads/kueue" - "github.com/opendatahub-io/odh-cli/pkg/resources" - - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" -) - -//nolint:gochecknoglobals -var kueueLabelsRayClusterListKinds = map[schema.GroupVersionResource]string{ - resources.RayCluster.GVR(): resources.RayCluster.ListKind(), - resources.Namespace.GVR(): resources.Namespace.ListKind(), - resources.DataScienceCluster.GVR(): resources.DataScienceCluster.ListKind(), -} - -func newKueueNs(name string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{"name": name} - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.Namespace.APIVersion(), - "kind": resources.Namespace.Kind, - "metadata": metadata, - }, - } -} - -func newRayCluster(name string, namespace string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{ - "name": name, - "namespace": namespace, - } - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.RayCluster.APIVersion(), - "kind": resources.RayCluster.Kind, - "metadata": metadata, - }, - } -} - -func TestKueueLabelsRayClusterCheck_Metadata(t *testing.T) { - g := NewWithT(t) - - chk := kueue.NewKueueLabelsRayClusterCheck() - - g.Expect(chk.ID()).To(Equal("workloads.kueue.raycluster-labels")) - g.Expect(chk.Name()).To(Equal("Workloads :: Kueue :: RayCluster Labels")) - g.Expect(chk.Group()).To(Equal(check.GroupWorkload)) - g.Expect(chk.CheckKind()).To(Equal("kueue")) - g.Expect(chk.CheckType()).To(Equal(string(check.CheckTypeDataIntegrity))) - g.Expect(chk.Remediation()).To(ContainSubstring("kueue.x-k8s.io/queue-name")) -} - -func TestKueueLabelsRayClusterCheck_CanApply_KueueManaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayClusterListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayClusterCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsRayClusterCheck_CanApply_KueueRemoved(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayClusterListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Removed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayClusterCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeFalse()) -} - -func TestKueueLabelsRayClusterCheck_CanApply_KueueUnmanaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayClusterListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Unmanaged"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayClusterCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsRayClusterCheck_NoRayClusters(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayClusterListKinds, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayClusterCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeRayClusterKueueLabels), - "Status": Equal(metav1.ConditionTrue), - "Reason": Equal(check.ReasonRequirementsMet), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloads, "RayCluster")), - })) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactNone)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "0")) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeRayClusterKueueMissingLabels), - "Status": Equal(metav1.ConditionTrue), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "RayCluster")), - })) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsRayClusterCheck_WithoutQueueLabel(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNs("plain-ns", nil) - rc := newRayCluster("my-rc", "plain-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayClusterListKinds, - Objects: []*unstructured.Unstructured{ns, rc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayClusterCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "RayCluster"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "RayCluster"))) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsRayClusterCheck_WithoutQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNs("kueue-ns", map[string]any{constants.LabelKueueManaged: "true"}) - rc := newRayCluster("unlabeled-rc", "kueue-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayClusterListKinds, - Objects: []*unstructured.Unstructured{ns, rc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayClusterCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "RayCluster"))) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeRayClusterKueueMissingLabels), - "Status": Equal(metav1.ConditionFalse), - "Reason": Equal(check.ReasonConfigurationInvalid), - "Message": Equal(fmt.Sprintf(kueue.MsgMissingLabelInKueueNs, 1, "RayCluster")), - })) - g.Expect(result.Status.Conditions[1].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("unlabeled-rc")) -} - -func TestKueueLabelsRayClusterCheck_WithQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNs("kueue-ns", map[string]any{constants.LabelKueueManaged: "true"}) - rc := newRayCluster("good-rc", "kueue-ns", map[string]any{constants.LabelKueueQueueName: "default"}) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayClusterListKinds, - Objects: []*unstructured.Unstructured{ns, rc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayClusterCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "RayCluster"))) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsRayClusterCheck_LabeledInNonKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNs("plain-ns", nil) - rc := newRayCluster("bad-rc", "plain-ns", map[string]any{constants.LabelKueueQueueName: "default"}) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayClusterListKinds, - Objects: []*unstructured.Unstructured{ns, rc}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayClusterCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionFalse)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNsNotKueueEnabled, 1, "RayCluster"))) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-rc")) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "RayCluster"))) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_rayjob.go b/pkg/lint/checks/workloads/kueue/kueue_labels_rayjob.go deleted file mode 100644 index 7d59c659..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_rayjob.go +++ /dev/null @@ -1,23 +0,0 @@ -package kueue - -import ( - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/resources" -) - -const ( - ConditionTypeRayJobKueueLabels = "RayJobKueueLabels" - ConditionTypeRayJobKueueMissingLabels = "RayJobKueueMissingLabels" -) - -func NewKueueLabelsRayJobCheck() *KueueLabelCheck { - return NewCheck(CheckConfig{ - Kind: constants.ComponentKueue, - Resource: resources.RayJob, - ConditionType: ConditionTypeRayJobKueueLabels, - MissingLabelConditionType: ConditionTypeRayJobKueueMissingLabels, - KindLabel: "RayJob", - CheckID: "workloads.kueue.rayjob-labels", - CheckName: "Workloads :: Kueue :: RayJob Labels", - }) -} diff --git a/pkg/lint/checks/workloads/kueue/kueue_labels_rayjob_test.go b/pkg/lint/checks/workloads/kueue/kueue_labels_rayjob_test.go deleted file mode 100644 index 5e010f21..00000000 --- a/pkg/lint/checks/workloads/kueue/kueue_labels_rayjob_test.go +++ /dev/null @@ -1,211 +0,0 @@ -package kueue_test - -import ( - "fmt" - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - - "github.com/opendatahub-io/odh-cli/pkg/constants" - "github.com/opendatahub-io/odh-cli/pkg/lint/check" - resultpkg "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/testutil" - "github.com/opendatahub-io/odh-cli/pkg/lint/checks/workloads/kueue" - "github.com/opendatahub-io/odh-cli/pkg/resources" - - . "github.com/onsi/gomega" - . "github.com/onsi/gomega/gstruct" -) - -//nolint:gochecknoglobals -var kueueLabelsRayJobListKinds = map[schema.GroupVersionResource]string{ - resources.RayJob.GVR(): resources.RayJob.ListKind(), - resources.Namespace.GVR(): resources.Namespace.ListKind(), - resources.DataScienceCluster.GVR(): resources.DataScienceCluster.ListKind(), -} - -func newRayJob(name string, namespace string, labels map[string]any) *unstructured.Unstructured { - metadata := map[string]any{ - "name": name, - "namespace": namespace, - } - - if len(labels) > 0 { - metadata["labels"] = labels - } - - return &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": resources.RayJob.APIVersion(), - "kind": resources.RayJob.Kind, - "metadata": metadata, - }, - } -} - -func TestKueueLabelsRayJobCheck_Metadata(t *testing.T) { - g := NewWithT(t) - - chk := kueue.NewKueueLabelsRayJobCheck() - - g.Expect(chk.ID()).To(Equal("workloads.kueue.rayjob-labels")) - g.Expect(chk.Name()).To(Equal("Workloads :: Kueue :: RayJob Labels")) - g.Expect(chk.Group()).To(Equal(check.GroupWorkload)) - g.Expect(chk.CheckKind()).To(Equal("kueue")) - g.Expect(chk.Remediation()).To(ContainSubstring("kueue.x-k8s.io/queue-name")) -} - -func TestKueueLabelsRayJobCheck_CanApply_KueueManaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayJobListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Managed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayJobCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsRayJobCheck_CanApply_KueueRemoved(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayJobListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Removed"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayJobCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeFalse()) -} - -func TestKueueLabelsRayJobCheck_CanApply_KueueUnmanaged(t *testing.T) { - g := NewWithT(t) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayJobListKinds, - Objects: []*unstructured.Unstructured{testutil.NewDSC(map[string]string{"kueue": "Unmanaged"})}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayJobCheck() - canApply, err := chk.CanApply(t.Context(), target) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(canApply).To(BeTrue()) -} - -func TestKueueLabelsRayJobCheck_NoRayJobs(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayJobListKinds, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions).To(HaveLen(2)) - g.Expect(result.Status.Conditions[0].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeRayJobKueueLabels), - "Status": Equal(metav1.ConditionTrue), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloads, "RayJob")), - })) - g.Expect(result.Status.Conditions[1].Condition).To(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(kueue.ConditionTypeRayJobKueueMissingLabels), - "Status": Equal(metav1.ConditionTrue), - "Message": Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "RayJob")), - })) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsRayJobCheck_WithQueueLabelInKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNs("kueue-ns", map[string]any{constants.LabelKueueManaged: "true"}) - rj := newRayJob("good-rj", "kueue-ns", map[string]any{constants.LabelKueueQueueName: "default"}) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayJobListKinds, - Objects: []*unstructured.Unstructured{ns, rj}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgAllValid, 1, "RayJob"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgAllInKueueNsLabeled, 1, "RayJob"))) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} - -func TestKueueLabelsRayJobCheck_LabeledInNonKueueNamespace(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNs("plain-ns", nil) - rj := newRayJob("bad-rj", "plain-ns", map[string]any{constants.LabelKueueQueueName: "default"}) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayJobListKinds, - Objects: []*unstructured.Unstructured{ns, rj}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionFalse)) - g.Expect(result.Status.Conditions[0].Impact).To(Equal(resultpkg.ImpactBlocking)) - g.Expect(result.Annotations).To(HaveKeyWithValue(check.AnnotationImpactedWorkloadCount, "1")) - g.Expect(result.ImpactedObjects).To(HaveLen(1)) - g.Expect(result.ImpactedObjects[0].Name).To(Equal("bad-rj")) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "RayJob"))) -} - -func TestKueueLabelsRayJobCheck_WithoutQueueLabel(t *testing.T) { - g := NewWithT(t) - ctx := t.Context() - - ns := newKueueNs("plain-ns", nil) - rj := newRayJob("unlabeled-rj", "plain-ns", nil) - - target := testutil.NewTarget(t, testutil.TargetConfig{ - ListKinds: kueueLabelsRayJobListKinds, - Objects: []*unstructured.Unstructured{ns, rj}, - CurrentVersion: "3.0.0", - TargetVersion: "3.0.0", - }) - - chk := kueue.NewKueueLabelsRayJobCheck() - result, err := chk.Validate(ctx, target) - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(result.Status.Conditions[0].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[0].Message).To(Equal(fmt.Sprintf(kueue.MsgNoLabeledWorkloads, "RayJob"))) - g.Expect(result.Status.Conditions[1].Status).To(Equal(metav1.ConditionTrue)) - g.Expect(result.Status.Conditions[1].Message).To(Equal(fmt.Sprintf(kueue.MsgNoWorkloadsInKueueNs, "RayJob"))) - g.Expect(result.ImpactedObjects).To(BeEmpty()) -} diff --git a/pkg/lint/checks/workloads/kueue/support.go b/pkg/lint/checks/workloads/kueue/support.go index f349497c..b3189bb0 100644 --- a/pkg/lint/checks/workloads/kueue/support.go +++ b/pkg/lint/checks/workloads/kueue/support.go @@ -3,35 +3,74 @@ package kueue import ( "context" "fmt" - "strconv" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "github.com/opendatahub-io/odh-cli/pkg/constants" "github.com/opendatahub-io/odh-cli/pkg/lint/check" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" - "github.com/opendatahub-io/odh-cli/pkg/lint/check/validate" "github.com/opendatahub-io/odh-cli/pkg/resources" "github.com/opendatahub-io/odh-cli/pkg/util/client" "github.com/opendatahub-io/odh-cli/pkg/util/components" - "github.com/opendatahub-io/odh-cli/pkg/util/kube" ) -// Messages for the "labeled in non-kueue namespace" condition. +// Top-level CR types that we monitor for kueue label consistency. +// These are the only resource types that appear in ImpactedObjects. +// +//nolint:gochecknoglobals // Static configuration for monitored workload types. +var monitoredWorkloadTypes = []resources.ResourceType{ + resources.Notebook, + resources.InferenceService, + resources.LLMInferenceService, + resources.RayCluster, + resources.RayJob, + resources.PyTorchJob, +} + +// Intermediate resource types used to build the ownership graph. +// These appear in ownership chains between top-level CRs and Pods. +// +//nolint:gochecknoglobals // Static configuration for intermediate resource types. +var intermediateTypes = []resources.ResourceType{ + resources.Deployment, + resources.StatefulSet, + resources.ReplicaSet, + resources.DaemonSet, + resources.Job, + resources.CronJob, + resources.Pod, +} + +// Condition type for the consolidated data-integrity check. +const ( + conditionTypeKueueConsistency = "KueueConsistency" +) + +// Remediation guidance for kueue consistency violations. const ( - MsgNoWorkloads = "No %s instances found" - MsgNoLabeledWorkloads = "No %s(s) found with the kueue.x-k8s.io/queue-name label" - MsgAllValid = "All %d %s(s) with the kueue.x-k8s.io/queue-name label are in kueue-enabled namespaces" - MsgNsNotKueueEnabled = "Found %d %s(s) with the kueue.x-k8s.io/queue-name label in namespaces not enabled for kueue" + remediationConsistency = "Ensure kueue-managed namespaces and workload kueue.x-k8s.io/queue-name labels are consistent. " + + "Add the kueue-managed or kueue.openshift.io/managed label to namespaces with kueue workloads, " + + "or add the kueue.x-k8s.io/queue-name label to all workloads in kueue-enabled namespaces" ) -// Messages for the "missing label in kueue namespace" condition. +// Messages for the consolidated KueueConsistency condition. const ( - MsgNoWorkloadsInKueueNs = "No %s(s) found in kueue-enabled namespaces" - MsgAllInKueueNsLabeled = "All %d %s(s) in kueue-enabled namespaces have the kueue.x-k8s.io/queue-name label" - MsgMissingLabelInKueueNs = "Found %d %s(s) in kueue-enabled namespaces without the kueue.x-k8s.io/queue-name label" + msgConsistent = "All monitored workloads are consistent with kueue namespace configuration" + msgNoRelevantNamespaces = "No kueue-managed namespaces or kueue-labeled workloads found" + msgInconsistent = "Found %d kueue consistency violation(s) across monitored workloads" +) + +// Messages for individual violation descriptions. +const ( + // Invariant 1: workload in kueue namespace missing queue-name label. + msgInvariant1 = "%s %s/%s is in kueue-managed namespace %s but missing kueue.x-k8s.io/queue-name label" + + // Invariant 2: workload with queue-name label in non-kueue namespace. + msgInvariant2 = "%s %s/%s has kueue.x-k8s.io/queue-name=%s but namespace is not kueue-managed" + + // Invariant 3: owner tree label disagreement. + msgInvariant3Missing = "%s %s/%s has kueue.x-k8s.io/queue-name=%s but descendant %s %s/%s is missing the label" + msgInvariant3Unexpected = "%s %s/%s has kueue.x-k8s.io/queue-name=%s but ancestor %s %s/%s does not have the label" + msgInvariant3Mismatch = "%s %s/%s has kueue.x-k8s.io/queue-name=%s but root %s %s/%s has kueue.x-k8s.io/queue-name=%s" ) // IsKueueActive returns true when Kueue is active (Managed or Unmanaged) on the DSC. @@ -54,163 +93,6 @@ func IsKueueActive( return true, nil } -// ValidateFn returns a validation function that classifies workloads into four categories -// based on kueue label presence and namespace enablement, then emits two conditions: -// - labeled workloads in non-kueue namespaces (conditionType) -// - unlabeled workloads in kueue-enabled namespaces (missingLabelConditionType) -func ValidateFn( - resourceType resources.ResourceType, - conditionType string, - missingLabelConditionType string, - kindLabel string, -) validate.WorkloadValidateFn[*metav1.PartialObjectMetadata] { - return func(ctx context.Context, req *validate.WorkloadRequest[*metav1.PartialObjectMetadata]) error { - dr := req.Result - - if len(req.Items) == 0 { - dr.Annotations[check.AnnotationImpactedWorkloadCount] = "0" - dr.SetCondition(newLabeledInNonKueueCondition(conditionType, kindLabel, len(req.Items), 0, 0)) - dr.SetCondition(newMissingLabelCondition(missingLabelConditionType, kindLabel, 0, 0)) - dr.SetImpactedObjects(resourceType, nil) - - return nil - } - - allKueueNs, err := kueueEnabledNamespaces(ctx, req.Client) - if err != nil { - return err - } - - // Classify workloads into four categories based on label presence and namespace. - var ( - labeled []types.NamespacedName - labeledInNonKueueNs []types.NamespacedName - inKueueNs []types.NamespacedName - unlabeledInKueueNs []types.NamespacedName - ) - - for _, item := range req.Items { - nn := types.NamespacedName{ - Namespace: item.GetNamespace(), - Name: item.GetName(), - } - - hasLabel := kube.ContainsLabel(item, constants.LabelKueueQueueName) - inKueue := allKueueNs.Has(item.GetNamespace()) - - if hasLabel { - labeled = append(labeled, nn) - } - - if inKueue { - inKueueNs = append(inKueueNs, nn) - } - - switch { - case hasLabel && !inKueue: - labeledInNonKueueNs = append(labeledInNonKueueNs, nn) - case !hasLabel && inKueue: - unlabeledInKueueNs = append(unlabeledInKueueNs, nn) - } - } - - totalImpacted := len(labeledInNonKueueNs) + len(unlabeledInKueueNs) - dr.Annotations[check.AnnotationImpactedWorkloadCount] = strconv.Itoa(totalImpacted) - - dr.SetCondition(newLabeledInNonKueueCondition( - conditionType, kindLabel, - len(req.Items), len(labeled), len(labeledInNonKueueNs), - )) - dr.SetCondition(newMissingLabelCondition( - missingLabelConditionType, kindLabel, - len(inKueueNs), len(unlabeledInKueueNs), - )) - - dr.SetImpactedObjects(resourceType, labeledInNonKueueNs) - dr.AddImpactedObjects(resourceType, unlabeledInKueueNs) - - return nil - } -} - -// newLabeledInNonKueueCondition builds the condition for workloads that have the -// kueue queue label but are in namespaces not enabled for kueue. -func newLabeledInNonKueueCondition( - conditionType string, - kindLabel string, - totalWorkloads int, - labeledCount int, - impactedCount int, -) result.Condition { - switch { - case totalWorkloads == 0: - return check.NewCondition( - conditionType, - metav1.ConditionTrue, - check.WithReason(check.ReasonRequirementsMet), - check.WithMessage(MsgNoWorkloads, kindLabel), - ) - case labeledCount == 0: - return check.NewCondition( - conditionType, - metav1.ConditionTrue, - check.WithReason(check.ReasonRequirementsMet), - check.WithMessage(MsgNoLabeledWorkloads, kindLabel), - ) - case impactedCount == 0: - return check.NewCondition( - conditionType, - metav1.ConditionTrue, - check.WithReason(check.ReasonRequirementsMet), - check.WithMessage(MsgAllValid, labeledCount, kindLabel), - ) - default: - return check.NewCondition( - conditionType, - metav1.ConditionFalse, - check.WithReason(check.ReasonConfigurationInvalid), - check.WithMessage(MsgNsNotKueueEnabled, impactedCount, kindLabel), - check.WithImpact(result.ImpactBlocking), - check.WithRemediation(remediationLabeledInNonKueueNs), - ) - } -} - -// newMissingLabelCondition builds the condition for workloads in kueue-enabled -// namespaces that are missing the kueue queue label. -func newMissingLabelCondition( - conditionType string, - kindLabel string, - totalInKueueNs int, - missingCount int, -) result.Condition { - switch { - case totalInKueueNs == 0: - return check.NewCondition( - conditionType, - metav1.ConditionTrue, - check.WithReason(check.ReasonRequirementsMet), - check.WithMessage(MsgNoWorkloadsInKueueNs, kindLabel), - ) - case missingCount == 0: - return check.NewCondition( - conditionType, - metav1.ConditionTrue, - check.WithReason(check.ReasonRequirementsMet), - check.WithMessage(MsgAllInKueueNsLabeled, totalInKueueNs, kindLabel), - ) - default: - return check.NewCondition( - conditionType, - metav1.ConditionFalse, - check.WithReason(check.ReasonConfigurationInvalid), - check.WithMessage(MsgMissingLabelInKueueNs, missingCount, kindLabel), - check.WithImpact(result.ImpactBlocking), - check.WithRemediation(remediationMissingLabelInKueueNs), - ) - } -} - // kueueEnabledNamespaces returns the set of namespaces that have a kueue-managed label. // Uses two ListMetadata calls with label selectors for server-side filtering, // giving a fixed cost regardless of how many namespaces exist. @@ -237,3 +119,33 @@ func kueueEnabledNamespaces( return enabled, nil } + +// workloadLabeledNamespaces returns the set of namespaces that contain at least one +// monitored workload with the kueue.x-k8s.io/queue-name label. +func workloadLabeledNamespaces( + ctx context.Context, + r client.Reader, +) (sets.Set[string], error) { + namespaces := sets.New[string]() + selector := constants.LabelKueueQueueName + + for _, rt := range monitoredWorkloadTypes { + items, err := r.ListMetadata(ctx, rt, client.WithLabelSelector(selector)) + if err != nil { + // A missing CRD means the resource type is not installed on this cluster, + // so there are zero instances. Ideally ListMetadata would handle this + // the same way it handles permission errors (return empty list). + if client.IsResourceTypeNotFound(err) { + continue + } + + return nil, fmt.Errorf("listing %s with kueue label: %w", rt.Kind, err) + } + + for _, item := range items { + namespaces.Insert(item.GetNamespace()) + } + } + + return namespaces, nil +} diff --git a/pkg/lint/checks/workloads/notebook/accelerator_migration.go b/pkg/lint/checks/workloads/notebook/accelerator_migration.go index b1197841..256babc5 100644 --- a/pkg/lint/checks/workloads/notebook/accelerator_migration.go +++ b/pkg/lint/checks/workloads/notebook/accelerator_migration.go @@ -18,7 +18,7 @@ import ( // that will be auto-migrated to HardwareProfiles (infrastructure.opendatahub.io) during RHOAI 3.x upgrade. type AcceleratorMigrationCheck struct { check.BaseCheck - NotebookVerboseFormatter + check.EnhancedVerboseFormatter } func NewAcceleratorMigrationCheck() *AcceleratorMigrationCheck { diff --git a/pkg/lint/checks/workloads/notebook/connection_integrity.go b/pkg/lint/checks/workloads/notebook/connection_integrity.go index 7b7c213f..3fa5fd4e 100644 --- a/pkg/lint/checks/workloads/notebook/connection_integrity.go +++ b/pkg/lint/checks/workloads/notebook/connection_integrity.go @@ -24,7 +24,7 @@ import ( // notebook's namespace. type ConnectionIntegrityCheck struct { check.BaseCheck - NotebookVerboseFormatter + check.EnhancedVerboseFormatter } func NewConnectionIntegrityCheck() *ConnectionIntegrityCheck { diff --git a/pkg/lint/checks/workloads/notebook/container_name.go b/pkg/lint/checks/workloads/notebook/container_name.go index 35d1a20e..91edd383 100644 --- a/pkg/lint/checks/workloads/notebook/container_name.go +++ b/pkg/lint/checks/workloads/notebook/container_name.go @@ -16,7 +16,7 @@ import ( // Dashboard-managed annotations (accelerator profile or size selection) are checked. type ContainerNameCheck struct { check.BaseCheck - NotebookVerboseFormatter + check.EnhancedVerboseFormatter } // NewContainerNameCheck creates a new ContainerNameCheck. diff --git a/pkg/lint/checks/workloads/notebook/hardware_profile_integrity.go b/pkg/lint/checks/workloads/notebook/hardware_profile_integrity.go index 757f5206..3255fb54 100644 --- a/pkg/lint/checks/workloads/notebook/hardware_profile_integrity.go +++ b/pkg/lint/checks/workloads/notebook/hardware_profile_integrity.go @@ -23,7 +23,7 @@ import ( // via annotations point to HardwareProfiles that actually exist on the cluster. type HardwareProfileIntegrityCheck struct { check.BaseCheck - NotebookVerboseFormatter + check.EnhancedVerboseFormatter } func NewHardwareProfileIntegrityCheck() *HardwareProfileIntegrityCheck { diff --git a/pkg/lint/checks/workloads/notebook/hardwareprofile_migration.go b/pkg/lint/checks/workloads/notebook/hardwareprofile_migration.go index 985f68c4..9c8ea6be 100644 --- a/pkg/lint/checks/workloads/notebook/hardwareprofile_migration.go +++ b/pkg/lint/checks/workloads/notebook/hardwareprofile_migration.go @@ -17,7 +17,7 @@ import ( // opendatahub.io/legacy-hardware-profile-name annotation that may need attention. type HardwareProfileMigrationCheck struct { check.BaseCheck - NotebookVerboseFormatter + check.EnhancedVerboseFormatter } // NewHardwareProfileMigrationCheck creates a new HardwareProfileMigrationCheck. diff --git a/pkg/lint/checks/workloads/notebook/impacted.go b/pkg/lint/checks/workloads/notebook/impacted.go index 53a696b2..7e2ace67 100644 --- a/pkg/lint/checks/workloads/notebook/impacted.go +++ b/pkg/lint/checks/workloads/notebook/impacted.go @@ -133,7 +133,7 @@ func NewImpactedWorkloadsCheck() *ImpactedWorkloadsCheck { // - namespace: // - / func (c *ImpactedWorkloadsCheck) FormatVerboseOutput(out iolib.Writer, dr *result.DiagnosticResult) { - crdName := crdFQN(dr) + crdName := check.CRDFullyQualifiedName(dr) // Group notebooks by image reference, preserving insertion order. // Within each image group, track notebooks per namespace. @@ -1406,7 +1406,7 @@ func isCompliantBuildRef(buildRef string) bool { // mustParseVersionParts parses a "X.Y" version string into its major and minor // integer components. Panics if the format is invalid. // -//nolint:revive // unnamed-result conflicts with nonamedreturns linter +//nolint:revive // confusing-results: unnamed (int, int) conflicts with nonamedreturns linter. func mustParseVersionParts(v string) (int, int) { majorStr, minorStr, ok := strings.Cut(v, ".") if !ok { diff --git a/pkg/lint/checks/workloads/notebook/running_workloads.go b/pkg/lint/checks/workloads/notebook/running_workloads.go index 1623733d..4c7eeb07 100644 --- a/pkg/lint/checks/workloads/notebook/running_workloads.go +++ b/pkg/lint/checks/workloads/notebook/running_workloads.go @@ -17,7 +17,7 @@ import ( // A Notebook is considered running when it does not have the kubeflow-resource-stopped annotation. type RunningWorkloadsCheck struct { check.BaseCheck - NotebookVerboseFormatter + check.EnhancedVerboseFormatter } func NewRunningWorkloadsCheck() *RunningWorkloadsCheck { diff --git a/pkg/lint/checks/workloads/notebook/verbose_formatter.go b/pkg/lint/checks/workloads/notebook/verbose_formatter.go deleted file mode 100644 index 2811aa60..00000000 --- a/pkg/lint/checks/workloads/notebook/verbose_formatter.go +++ /dev/null @@ -1,139 +0,0 @@ -package notebook - -import ( - "fmt" - "io" - "sort" - "strings" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/opendatahub-io/odh-cli/pkg/lint/check/result" -) - -// NotebookVerboseFormatter provides a namespace-grouped rendering for impacted objects -// in verbose table output. Embed this in any check struct to get a default -// FormatVerboseOutput that lists objects grouped by namespace with requester info -// and CRD-qualified resource names. -// -// Output format: -// -// namespace: | requester: -// - ./ -// - ./ -// -// The CRD FQN (e.g. "notebooks.kubeflow.org") is read from the DiagnosticResult's -// AnnotationResourceCRDName annotation, which is automatically set by SetImpactedObjects -// and AddImpactedObjects. This makes the formatter adoptable by any resource type -// without hardcoding. -// -// Checks that need custom formatting (e.g. ImpactedWorkloadsCheck groups by image) -// should define their own FormatVerboseOutput method instead. -type NotebookVerboseFormatter struct { - // NamespaceRequesters maps namespace names to their openshift.io/requester annotation value. - // Populated automatically by the output renderer before FormatVerboseOutput is called. - NamespaceRequesters map[string]string -} - -// SetNamespaceRequesters sets the namespace-to-requester mapping. -// Called by the output renderer before FormatVerboseOutput. -func (f *NotebookVerboseFormatter) SetNamespaceRequesters(m map[string]string) { - f.NamespaceRequesters = m -} - -// FormatVerboseOutput implements check.VerboseOutputFormatter. -// Renders impacted objects grouped by namespace with requester info and CRD FQN. -func (f *NotebookVerboseFormatter) FormatVerboseOutput(out io.Writer, dr *result.DiagnosticResult) { - crdName := crdFQN(dr) - - nsMap := make(map[string][]string) - for _, obj := range dr.ImpactedObjects { - nsMap[obj.Namespace] = append(nsMap[obj.Namespace], obj.Name) - } - - namespaces := make([]string, 0, len(nsMap)) - for ns := range nsMap { - namespaces = append(namespaces, ns) - } - sort.Strings(namespaces) - - for idx, ns := range namespaces { - names := make([]string, len(nsMap[ns])) - copy(names, nsMap[ns]) - sort.Strings(names) - - if ns == "" { - // Cluster-scoped objects listed without namespace header. - for _, name := range names { - _, _ = fmt.Fprintf(out, " - %s/%s\n", crdName, name) - } - } else { - nsHeader := "namespace: " + ns - if f.NamespaceRequesters != nil { - if requester, ok := f.NamespaceRequesters[ns]; ok && requester != "" { - nsHeader = fmt.Sprintf("namespace: %s | requester: %s", ns, requester) - } - } - - _, _ = fmt.Fprintf(out, " %s\n", nsHeader) - for _, name := range names { - _, _ = fmt.Fprintf(out, " - %s/%s\n", crdName, name) - } - } - - // Blank line between namespace groups (except after last). - if idx < len(namespaces)-1 { - _, _ = fmt.Fprintln(out) - } - } -} - -// crdFQN returns the CRD fully-qualified name for the impacted objects in a DiagnosticResult. -// It first checks the AnnotationResourceCRDName annotation (automatically set by SetImpactedObjects -// and AddImpactedObjects from ResourceType.CRDFQN()). Falls back to deriving the FQN from the -// first impacted object's TypeMeta if the annotation is absent. -func crdFQN(dr *result.DiagnosticResult) string { - // Prefer the annotation set by SetImpactedObjects — uses ResourceType.Resource - // for the correct plural form (e.g. "inferenceservices", not "inferenceservices" from naive pluralization). - if name, ok := dr.Annotations[result.AnnotationResourceCRDName]; ok && name != "" { - return name - } - - // Fallback: derive from TypeMeta (for callers that set ImpactedObjects directly). - return deriveCRDFQNFromTypeMeta(dr.ImpactedObjects) -} - -// deriveCRDFQNFromTypeMeta derives the CRD FQN from the first impacted object's TypeMeta. -// For example, APIVersion "kubeflow.org/v1" with Kind "Notebook" produces "notebooks.kubeflow.org". -// Falls back to the lowercase Kind if TypeMeta is not populated. -func deriveCRDFQNFromTypeMeta(objects []metav1.PartialObjectMetadata) string { - if len(objects) == 0 { - return "" - } - - obj := objects[0] - - kind := obj.Kind - if kind == "" { - return "" - } - - // Kubernetes CRD plural convention: lowercase kind + "s" - // (e.g. Notebook → notebooks, InferenceService → inferenceservices, RayCluster → rayclusters). - plural := strings.ToLower(kind) + "s" - - // Extract API group from APIVersion (e.g. "kubeflow.org/v1" → "kubeflow.org"). - // Core resources have no group (APIVersion is just "v1"), so return plural only. - apiVersion := obj.APIVersion - if apiVersion == "" { - return plural - } - - group, _, found := strings.Cut(apiVersion, "/") - if !found { - // Core API (e.g. "v1") — no group prefix. - return plural - } - - return plural + "." + group -} diff --git a/pkg/lint/command.go b/pkg/lint/command.go index dd9b3e6a..71d8d79c 100644 --- a/pkg/lint/command.go +++ b/pkg/lint/command.go @@ -2,6 +2,7 @@ package lint import ( "context" + "errors" "fmt" "slices" @@ -105,7 +106,7 @@ func NewCommand( registry.MustRegister(certmanager.NewCheck()) registry.MustRegister(openshift.NewCheck()) - // Workloads (25) + // Workloads (20) registry.MustRegister(ray.NewAppWrapperCleanupCheck()) registry.MustRegister(datasciencepipelinesworkloads.NewInstructLabRemovalCheck()) registry.MustRegister(datasciencepipelinesworkloads.NewStoredVersionRemovalCheck()) @@ -115,21 +116,16 @@ func NewCommand( registry.MustRegister(kserveworkloads.NewAcceleratorMigrationCheck()) registry.MustRegister(kserveworkloads.NewHardwareProfileMigrationCheck()) registry.MustRegister(kserveworkloads.NewImpactedWorkloadsCheck()) - registry.MustRegister(kueueworkloads.NewKueueLabelsISVCCheck()) - registry.MustRegister(kueueworkloads.NewKueueLabelsLLMCheck()) + registry.MustRegister(kueueworkloads.NewDataIntegrityCheck()) registry.MustRegister(llamastackworkloads.NewConfigCheck()) registry.MustRegister(notebook.NewAcceleratorMigrationCheck()) registry.MustRegister(notebook.NewContainerNameCheck()) registry.MustRegister(notebook.NewHardwareProfileMigrationCheck()) registry.MustRegister(notebook.NewConnectionIntegrityCheck()) registry.MustRegister(notebook.NewHardwareProfileIntegrityCheck()) - registry.MustRegister(kueueworkloads.NewKueueLabelsNotebookCheck()) registry.MustRegister(notebook.NewImpactedWorkloadsCheck()) registry.MustRegister(notebook.NewRunningWorkloadsCheck()) - registry.MustRegister(kueueworkloads.NewKueueLabelsRayClusterCheck()) - registry.MustRegister(kueueworkloads.NewKueueLabelsRayJobCheck()) registry.MustRegister(ray.NewImpactedWorkloadsCheck()) - registry.MustRegister(kueueworkloads.NewKueueLabelsPyTorchJobCheck()) registry.MustRegister(trainingoperatorworkloads.NewImpactedWorkloadsCheck()) c := &Command{ @@ -328,7 +324,7 @@ func (c *Command) runUpgradeMode(ctx context.Context, currentVersion *semver.Ver // printVerdictAndExit prints a prominent result verdict for table output and returns // an error if fail-on conditions are met (to control exit code). func (c *Command) printVerdictAndExit(results []check.CheckExecution) error { - var hasBlocking, hasAdvisory bool + var hasProhibited, hasBlocking, hasAdvisory bool for _, exec := range results { if exec.Result == nil { @@ -336,6 +332,8 @@ func (c *Command) printVerdictAndExit(results []check.CheckExecution) error { } switch exec.Result.GetImpact() { + case resultpkg.ImpactProhibited: + hasProhibited = true case resultpkg.ImpactBlocking: hasBlocking = true case resultpkg.ImpactAdvisory: @@ -346,7 +344,11 @@ func (c *Command) printVerdictAndExit(results []check.CheckExecution) error { } if c.OutputFormat == OutputFormatTable { - printVerdict(c.IO.Out(), hasBlocking, hasAdvisory) + printVerdict(c.IO.Out(), hasProhibited, hasBlocking, hasAdvisory) + } + + if hasProhibited { + return errors.New("prohibited findings detected: upgrade is not possible") } return nil diff --git a/pkg/lint/command_options.go b/pkg/lint/command_options.go index ce1d1521..53c285b8 100644 --- a/pkg/lint/command_options.go +++ b/pkg/lint/command_options.go @@ -36,9 +36,10 @@ const ( type SeverityLevel string const ( - SeverityLevelCritical SeverityLevel = "critical" // Show only blocking (critical) conditions - SeverityLevelWarning SeverityLevel = "warning" // Show blocking and advisory conditions - SeverityLevelInfo SeverityLevel = "info" // Show all conditions (default) + SeverityLevelProhibited SeverityLevel = "prohibited" // Show only prohibited conditions + SeverityLevelCritical SeverityLevel = "critical" // Show prohibited and blocking conditions + SeverityLevelWarning SeverityLevel = "warning" // Show prohibited, blocking, and advisory conditions + SeverityLevelInfo SeverityLevel = "info" // Show all conditions (default) ) // Validate checks if the output format is valid. @@ -54,10 +55,10 @@ func (o OutputFormat) Validate() error { // Validate checks if the severity level is valid. func (s SeverityLevel) Validate() error { switch s { - case SeverityLevelCritical, SeverityLevelWarning, SeverityLevelInfo: + case SeverityLevelProhibited, SeverityLevelCritical, SeverityLevelWarning, SeverityLevelInfo: return nil default: - return fmt.Errorf("invalid severity level: %s (must be one of: critical, warning, info)", s) + return fmt.Errorf("invalid severity level: %s (must be one of: prohibited, critical, warning, info)", s) } } @@ -317,10 +318,12 @@ func FilterBySeverity(results []check.CheckExecution, minLevel SeverityLevel) [] // minimum severity threshold. func meetsMinSeverity(impact result.Impact, minLevel SeverityLevel) bool { switch minLevel { + case SeverityLevelProhibited: + return impact == result.ImpactProhibited case SeverityLevelCritical: - return impact == result.ImpactBlocking + return impact == result.ImpactProhibited || impact == result.ImpactBlocking case SeverityLevelWarning: - return impact == result.ImpactBlocking || impact == result.ImpactAdvisory + return impact == result.ImpactProhibited || impact == result.ImpactBlocking || impact == result.ImpactAdvisory case SeverityLevelInfo: return true } @@ -345,12 +348,15 @@ func checkMaxImpact(exec check.CheckExecution) result.Impact { // getImpactString determines the display string from a condition's impact. func getImpactString( condition *result.Condition, + prohibitedStr string, blockingStr string, advisoryStr string, noneStr string, ) string { // Use Impact field directly (always set by NewCondition). switch condition.Impact { + case result.ImpactProhibited: + return prohibitedStr case result.ImpactBlocking: return blockingStr case result.ImpactAdvisory: @@ -364,15 +370,19 @@ func getImpactString( // Impact sort priorities (lower = higher severity). const ( - impactPriorityBlocking = iota + impactPriorityProhibited = iota + impactPriorityBlocking impactPriorityAdvisory impactPriorityNone ) -// impactSortPriority returns a numeric priority so blocking (critical) sorts -// before advisory (warning) which sorts before none (info). +// impactSortPriority returns a numeric priority so prohibited sorts before +// blocking (critical) which sorts before advisory (warning) which sorts +// before none (info). func impactSortPriority(impact result.Impact) int { switch impact { + case result.ImpactProhibited: + return impactPriorityProhibited case result.ImpactBlocking: return impactPriorityBlocking case result.ImpactAdvisory: diff --git a/pkg/lint/command_options_test.go b/pkg/lint/command_options_test.go index 7f0d7142..2d82f68f 100644 --- a/pkg/lint/command_options_test.go +++ b/pkg/lint/command_options_test.go @@ -171,6 +171,7 @@ func TestSeverityLevelValidate(t *testing.T) { level lint.SeverityLevel wantErr bool }{ + {name: "prohibited valid", level: lint.SeverityLevelProhibited, wantErr: false}, {name: "critical valid", level: lint.SeverityLevelCritical, wantErr: false}, {name: "warning valid", level: lint.SeverityLevelWarning, wantErr: false}, {name: "info valid", level: lint.SeverityLevelInfo, wantErr: false}, @@ -194,6 +195,8 @@ func TestSeverityLevelValidate(t *testing.T) { func makeCondition(impact result.Impact, msg string) result.Condition { status := metav1.ConditionTrue switch impact { + case result.ImpactProhibited: + status = metav1.ConditionFalse case result.ImpactBlocking: status = metav1.ConditionFalse case result.ImpactAdvisory: @@ -256,6 +259,39 @@ func TestFilterBySeverity_WarningHidesInfo(t *testing.T) { g.Expect(filtered[1].Result.Kind).To(Equal("dashboard")) } +func TestFilterBySeverity_ProhibitedKeepsOnlyProhibited(t *testing.T) { + g := NewWithT(t) + + results := []check.CheckExecution{ + makeExec("kueue", makeCondition(result.ImpactProhibited, "prohibited")), + makeExec("kserve", makeCondition(result.ImpactBlocking, "crit")), + makeExec("dashboard", makeCondition(result.ImpactAdvisory, "warn")), + makeExec("notebook", makeCondition(result.ImpactNone, "info")), + } + + filtered := lint.FilterBySeverity(results, lint.SeverityLevelProhibited) + + g.Expect(filtered).To(HaveLen(1)) + g.Expect(filtered[0].Result.Kind).To(Equal("kueue")) +} + +func TestFilterBySeverity_CriticalRetainsProhibited(t *testing.T) { + g := NewWithT(t) + + results := []check.CheckExecution{ + makeExec("kueue", makeCondition(result.ImpactProhibited, "prohibited")), + makeExec("kserve", makeCondition(result.ImpactBlocking, "crit")), + makeExec("dashboard", makeCondition(result.ImpactAdvisory, "warn")), + makeExec("notebook", makeCondition(result.ImpactNone, "info")), + } + + filtered := lint.FilterBySeverity(results, lint.SeverityLevelCritical) + + g.Expect(filtered).To(HaveLen(2)) + g.Expect(filtered[0].Result.Kind).To(Equal("kueue")) + g.Expect(filtered[1].Result.Kind).To(Equal("kserve")) +} + func TestFilterBySeverity_CriticalHidesWarningAndInfo(t *testing.T) { g := NewWithT(t) diff --git a/pkg/lint/constants.go b/pkg/lint/constants.go index da170a8b..a9bb4c0a 100644 --- a/pkg/lint/constants.go +++ b/pkg/lint/constants.go @@ -4,7 +4,7 @@ package lint const ( flagDescTargetVersion = "target version for upgrade readiness checks (e.g., 2.25.0, 3.0.0)" flagDescOutput = "output format (table|json|yaml)" - flagDescSeverity = "minimum severity level to display (critical|warning|info)" + flagDescSeverity = "minimum severity level to display (prohibited|critical|warning|info)" flagDescVerbose = "show impacted objects and summary information" flagDescDebug = "show detailed diagnostic logs for troubleshooting" flagDescTimeout = "operation timeout (e.g., 10m, 30m)" diff --git a/pkg/lint/output_table.go b/pkg/lint/output_table.go index bd3fc28f..0a6e773b 100644 --- a/pkg/lint/output_table.go +++ b/pkg/lint/output_table.go @@ -19,14 +19,16 @@ import ( //nolint:gochecknoglobals var ( // Table output symbols. - statusPass = color.New(color.FgGreen).Sprint("✓") - statusWarn = color.New(color.FgYellow).Sprint("⚠") - statusFail = color.New(color.FgRed).Sprint("✗") + statusPass = color.New(color.FgGreen).Sprint("✓") + statusWarn = color.New(color.FgYellow).Sprint("⚠") + statusFail = color.New(color.FgRed).Sprint("✗") + statusProhibited = color.New(color.FgRed, color.Bold).Sprint("‼") // Severity level formatting. - severityCrit = color.New(color.FgRed).Sprint("critical") - severityWarn = color.New(color.FgYellow).Add(color.Bold).Sprint("warning") // Bold yellow (orange-ish) - severityInfo = color.New(color.FgCyan).Sprint("info") + severityProhibited = color.New(color.FgRed, color.Bold).Sprint("prohibited") + severityCrit = color.New(color.FgRed).Sprint("critical") + severityWarn = color.New(color.FgYellow).Sprint("warning") + severityInfo = color.New(color.FgCyan).Sprint("info") // Table headers. tableHeaders = []string{"STATUS", "KIND", "GROUP", "CHECK", "IMPACT", "MESSAGE"} @@ -37,11 +39,14 @@ var ( ) // printVerdict prints the Result section after the summary. -func printVerdict(out io.Writer, hasBlocking bool, hasAdvisory bool) { +func printVerdict(out io.Writer, hasProhibited bool, hasBlocking bool, hasAdvisory bool) { _, _ = fmt.Fprintln(out) _, _ = fmt.Fprintln(out, "Result:") switch { + case hasProhibited: + verdict := color.New(color.FgRed, color.Bold).Sprint("PROHIBITED") + _, _ = fmt.Fprintf(out, " %s - upgrade is not possible\n", verdict) case hasBlocking: verdict := color.New(color.FgRed, color.Bold).Sprint("FAIL") _, _ = fmt.Fprintf(out, " %s - blocking findings detected\n", verdict) @@ -54,6 +59,29 @@ func printVerdict(out io.Writer, hasBlocking bool, hasAdvisory bool) { } } +// outputProhibitedBanner renders a prominent warning banner above the summary table +// listing all prohibited findings. Each prohibited condition is shown so that none +// can be overlooked when multiple checks report prohibited-level impact. +func outputProhibitedBanner(out io.Writer, findings []sortableRow) { + bold := color.New(color.FgRed, color.Bold) + + _, _ = fmt.Fprintln(out) + bannerText := " Prohibited Violations Detected: Upgrade is NOT POSSIBLE " + bannerWidth := visibleLen(bannerText) + hLine := strings.Repeat("═", bannerWidth) + + _, _ = fmt.Fprintln(out, bold.Sprintf("╔%s╗", hLine)) + _, _ = fmt.Fprintln(out, bold.Sprintf("║%s║", bannerText)) + _, _ = fmt.Fprintln(out, bold.Sprintf("╚%s╝", hLine)) + + for _, f := range findings { + _, _ = fmt.Fprintf(out, " %s [%s / %s] %s\n", + statusProhibited, f.row.Group, f.row.Check, f.row.Message) + } + + _, _ = fmt.Fprintln(out) +} + // sortableRow pairs a table row with the raw impact for sort comparisons. type sortableRow struct { row CheckResultTableRow @@ -86,7 +114,7 @@ func collectSortedRows(results []check.CheckExecution) []sortableRow { Kind: exec.Result.Kind, Group: exec.Result.Group, Check: exec.Result.Name, - Impact: getImpactString(&condition, severityCrit, severityWarn, severityInfo), + Impact: getImpactString(&condition, severityProhibited, severityCrit, severityWarn, severityInfo), Message: condition.Message, Description: exec.Result.Spec.Description, }, @@ -119,6 +147,8 @@ func collectSortedRows(results []check.CheckExecution) []sortableRow { // statusSymbol returns the colored status symbol for the given impact level. func statusSymbol(impact result.Impact) string { switch impact { + case result.ImpactProhibited: + return statusProhibited case result.ImpactBlocking: return statusFail case result.ImpactAdvisory: @@ -151,6 +181,18 @@ func padRight(s string, visibleWidth int) string { func OutputTable(out io.Writer, results []check.CheckExecution, opts TableOutputOptions) error { rows := collectSortedRows(results) + // Collect prohibited findings for the warning banner before the table. + var prohibitedFindings []sortableRow + for _, sr := range rows { + if sr.impact == result.ImpactProhibited { + prohibitedFindings = append(prohibitedFindings, sr) + } + } + + if len(prohibitedFindings) > 0 { + outputProhibitedBanner(out, prohibitedFindings) + } + renderer := table.NewRenderer[CheckResultTableRow]( table.WithWriter[CheckResultTableRow](out), table.WithHeaders[CheckResultTableRow](tableHeaders...), @@ -161,11 +203,14 @@ func OutputTable(out io.Writer, results []check.CheckExecution, opts TableOutput totalPassed := 0 totalWarnings := 0 totalFailed := 0 + totalProhibited := 0 for _, sr := range rows { totalChecks++ switch sr.impact { + case result.ImpactProhibited: + totalProhibited++ case result.ImpactBlocking: totalFailed++ case result.ImpactAdvisory: @@ -190,7 +235,7 @@ func OutputTable(out io.Writer, results []check.CheckExecution, opts TableOutput _, _ = fmt.Fprintln(out) _, _ = fmt.Fprintln(out, "Summary:") - _, _ = fmt.Fprintf(out, " Total: %d | Passed: %d | Warnings: %d | Failed: %d\n", totalChecks, totalPassed, totalWarnings, totalFailed) + _, _ = fmt.Fprintf(out, " Total: %d | Passed: %d | Warnings: %d | Failed: %d | Prohibited: %d\n", totalChecks, totalPassed, totalWarnings, totalFailed, totalProhibited) if opts.ShowImpactedObjects { outputImpactedObjects(out, results, opts.NamespaceRequesters) @@ -215,7 +260,7 @@ func outputVersionInfo(out io.Writer, info *VersionInfo) { } // namespaceRequesterSetter is implemented by verbose formatters that need -// namespace-to-requester mappings (e.g. NotebookVerboseFormatter). +// namespace-to-requester mappings (e.g. EnhancedVerboseFormatter). type namespaceRequesterSetter interface { SetNamespaceRequesters(requesters map[string]string) } @@ -259,7 +304,7 @@ func buildVerboseRows( check: exec.Result.Name, impact: getImpactString( &result.Condition{Impact: maxImpact}, - severityCrit, severityWarn, severityInfo, + severityProhibited, severityCrit, severityWarn, severityInfo, ), exec: exec, } diff --git a/pkg/lint/output_table_test.go b/pkg/lint/output_table_test.go index 1f9b8f2a..22c41109 100644 --- a/pkg/lint/output_table_test.go +++ b/pkg/lint/output_table_test.go @@ -575,3 +575,148 @@ func TestOutputTable_VersionInfoAppearsBetweenTableAndSummary(t *testing.T) { g.Expect(tableIdx).To(BeNumerically("<", envIdx)) g.Expect(envIdx).To(BeNumerically("<", summaryIdx)) } + +func TestOutputTable_ProhibitedBannerAppearsBeforeTable(t *testing.T) { + g := NewWithT(t) + + results := []check.CheckExecution{ + { + Result: &result.DiagnosticResult{ + Group: "workload", + Kind: "kueue", + Name: "data-integrity", + Status: result.DiagnosticStatus{ + Conditions: []result.Condition{ + { + Condition: metav1.Condition{ + Type: "KueueConsistency", + Status: metav1.ConditionFalse, + Reason: "ConfigurationInvalid", + Message: "Found 3 kueue consistency violations", + }, + Impact: result.ImpactProhibited, + }, + }, + }, + }, + }, + { + Result: &result.DiagnosticResult{ + Group: "component", + Kind: "dashboard", + Name: "version-check", + Status: result.DiagnosticStatus{ + Conditions: []result.Condition{passCondition()}, + }, + }, + }, + } + + var buf bytes.Buffer + err := lint.OutputTable(&buf, results, lint.TableOutputOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + + output := buf.String() + + // Banner should be present and appear before the table data. + g.Expect(output).To(ContainSubstring("Prohibited Violations Detected")) + g.Expect(output).To(ContainSubstring("Found 3 kueue consistency violations")) + + bannerIdx := strings.Index(output, "Prohibited Violations Detected") + tableIdx := strings.Index(output, "STATUS") + g.Expect(bannerIdx).To(BeNumerically("<", tableIdx)) + + // Summary should include prohibited count. + g.Expect(output).To(ContainSubstring("Prohibited: 1")) +} + +func TestOutputTable_ProhibitedBannerShowsMultipleProhibitedFindings(t *testing.T) { + g := NewWithT(t) + + results := []check.CheckExecution{ + { + Result: &result.DiagnosticResult{ + Group: "workload", + Kind: "kueue", + Name: "data-integrity", + Status: result.DiagnosticStatus{ + Conditions: []result.Condition{ + { + Condition: metav1.Condition{ + Type: "KueueConsistency", + Status: metav1.ConditionFalse, + Reason: "ConfigurationInvalid", + Message: "kueue label inconsistencies found", + }, + Impact: result.ImpactProhibited, + }, + }, + }, + }, + }, + { + Result: &result.DiagnosticResult{ + Group: "workload", + Kind: "other", + Name: "other-integrity", + Status: result.DiagnosticStatus{ + Conditions: []result.Condition{ + { + Condition: metav1.Condition{ + Type: "DataConsistency", + Status: metav1.ConditionFalse, + Reason: "Inconsistent", + Message: "second prohibited violation detected", + }, + Impact: result.ImpactProhibited, + }, + }, + }, + }, + }, + } + + var buf bytes.Buffer + err := lint.OutputTable(&buf, results, lint.TableOutputOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + + output := buf.String() + + // Both prohibited findings should appear in the banner. + g.Expect(output).To(ContainSubstring("kueue label inconsistencies found")) + g.Expect(output).To(ContainSubstring("second prohibited violation detected")) + g.Expect(output).To(ContainSubstring("Prohibited: 2")) +} + +func TestOutputTable_NoBannerWhenNoProhibitedFindings(t *testing.T) { + g := NewWithT(t) + + results := []check.CheckExecution{ + { + Result: &result.DiagnosticResult{ + Group: "component", + Kind: "dashboard", + Name: "version-check", + Status: result.DiagnosticStatus{ + Conditions: []result.Condition{ + { + Condition: metav1.Condition{ + Type: "Compatible", + Status: metav1.ConditionFalse, + Reason: "Incompatible", + Message: "blocking but not prohibited", + }, + Impact: result.ImpactBlocking, + }, + }, + }, + }, + }, + } + + var buf bytes.Buffer + err := lint.OutputTable(&buf, results, lint.TableOutputOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + + g.Expect(buf.String()).ToNot(ContainSubstring("Prohibited Violations Detected")) +} diff --git a/pkg/resources/types.go b/pkg/resources/types.go index 92dfe3b8..4bec69c9 100644 --- a/pkg/resources/types.go +++ b/pkg/resources/types.go @@ -108,6 +108,30 @@ var ( Resource: "datasciencepipelinesapplications", } + // StatefulSet is the Kubernetes StatefulSet resource. + StatefulSet = ResourceType{ + Group: "apps", + Version: "v1", + Kind: "StatefulSet", + Resource: "statefulsets", + } + + // ReplicaSet is the Kubernetes ReplicaSet resource. + ReplicaSet = ResourceType{ + Group: "apps", + Version: "v1", + Kind: "ReplicaSet", + Resource: "replicasets", + } + + // DaemonSet is the Kubernetes DaemonSet resource. + DaemonSet = ResourceType{ + Group: "apps", + Version: "v1", + Kind: "DaemonSet", + Resource: "daemonsets", + } + // Deployment is the Kubernetes Deployment resource. Deployment = ResourceType{ Group: "apps", @@ -116,6 +140,22 @@ var ( Resource: "deployments", } + // Job is the Kubernetes Job resource. + Job = ResourceType{ + Group: "batch", + Version: "v1", + Kind: "Job", + Resource: "jobs", + } + + // CronJob is the Kubernetes CronJob resource. + CronJob = ResourceType{ + Group: "batch", + Version: "v1", + Kind: "CronJob", + Resource: "cronjobs", + } + // Namespace is the core Kubernetes Namespace resource. Namespace = ResourceType{ Group: "", diff --git a/pkg/util/kube/resources.go b/pkg/util/kube/resources.go index 9d518dd8..e13694ea 100644 --- a/pkg/util/kube/resources.go +++ b/pkg/util/kube/resources.go @@ -33,11 +33,13 @@ func ToPartialObjectMetadata(objs ...*unstructured.Unstructured) []runtime.Objec Kind: obj.GetKind(), }, ObjectMeta: metav1.ObjectMeta{ - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - Labels: obj.GetLabels(), - Annotations: obj.GetAnnotations(), - Finalizers: obj.GetFinalizers(), + Name: obj.GetName(), + Namespace: obj.GetNamespace(), + UID: obj.GetUID(), + Labels: obj.GetLabels(), + Annotations: obj.GetAnnotations(), + Finalizers: obj.GetFinalizers(), + OwnerReferences: obj.GetOwnerReferences(), }, } result = append(result, pom)