Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions pkg/lint/check/result/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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.
Expand Down Expand Up @@ -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,
)
}

Expand Down Expand Up @@ -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
}
Expand Down
159 changes: 159 additions & 0 deletions pkg/lint/check/result/result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Loading