From 719180f1794525be4412e21394ce01d830e18601 Mon Sep 17 00:00:00 2001 From: thc1006 Date: Sun, 16 Aug 2026 14:28:31 +0800 Subject: [PATCH] Return ConditionUnknown from Unknown() Unknown() is named, documented and reasoned as reporting an unknown state, but it returned metav1.ConditionFalse. Failed() a few lines below returns the same status, so the two were distinguishable only by their Reason, and a consumer switching on Status read "not known yet" as "definitely failed". metav1.ConditionUnknown exists for exactly the state this constructor names. The generated CRDs already advertise the third state. The manifests under config/crd/bases whose status is built from these packages carry status: description: status of the condition, one of True, False, Unknown. enum: ["True", "False", Unknown] so the published schema promises a tri-state that the one constructor meant to produce it could not emit. ConditionReasonUnknown was declared and used while metav1.ConditionUnknown appeared nowhere in either package. This is not a regression. infra/v1alpha1/condition.go was introduced by 3316d8b (#23) on 2023-05-11 and the line has not been modified since; cfg/v1alpha1/condition.go is a later whole-file copy of it, added by ceb0d10 (#46) on 2023-06-15. Both packages are fixed together. They export a helper of the same name with the same documented contract, so correcting only one would make the returned status depend on which package a caller imports, which is worse than the uniform defect it replaces. cfg/v1alpha1 has no importers I could find while infra/v1alpha1 has many, but both are published API. TestUnknown asserts the status alone, which is the field this change alters. The constructors were already executed by TestConditionedStatusEqual and TestSetConditions, so they were at 100% statement coverage before this change; what was missing was any assertion on the value they return. No in-tree or publicly indexed caller of Unknown() was found, in this repository, in nephio, in oai, or in the public importers of this module. That is a narrower claim than no impact: this changes the observable behaviour of an exported function, and unindexed consumers may exist. Left alone deliberately. ConditionedStatus.GetCondition() collapses the same tri-state in the other direction by synthesising ConditionFalse when no condition of the requested type exists, which is a separate API decision. The whole-file duplication between the two packages is the reason one defect could exist twice, but deduplicating them is a refactor. The pinned nokia/k8s-ipam dependency carries the same defect in its own copy and has active callers that persist it; that belongs upstream, not here. Signed-off-by: thc1006 --- cfg/v1alpha1/condition.go | 2 +- cfg/v1alpha1/condition_test.go | 6 ++++++ infra/v1alpha1/condition.go | 2 +- infra/v1alpha1/condition_test.go | 6 ++++++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cfg/v1alpha1/condition.go b/cfg/v1alpha1/condition.go index cafa13d8f..8c53b2bfb 100644 --- a/cfg/v1alpha1/condition.go +++ b/cfg/v1alpha1/condition.go @@ -159,7 +159,7 @@ func Ready() Condition { func Unknown() Condition { return Condition{metav1.Condition{ Type: string(ConditionTypeReady), - Status: metav1.ConditionFalse, + Status: metav1.ConditionUnknown, LastTransitionTime: metav1.Now(), Reason: string(ConditionReasonUnknown), }} diff --git a/cfg/v1alpha1/condition_test.go b/cfg/v1alpha1/condition_test.go index befb049cc..f2fbd2581 100644 --- a/cfg/v1alpha1/condition_test.go +++ b/cfg/v1alpha1/condition_test.go @@ -218,3 +218,9 @@ func TestConditionWithMessage(t *testing.T) { }) } } + +func TestUnknown(t *testing.T) { + if got := Unknown().Status; got != metav1.ConditionUnknown { + t.Errorf("Unknown().Status = %q, want %q", got, metav1.ConditionUnknown) + } +} diff --git a/infra/v1alpha1/condition.go b/infra/v1alpha1/condition.go index 814753039..6b8d5c311 100644 --- a/infra/v1alpha1/condition.go +++ b/infra/v1alpha1/condition.go @@ -157,7 +157,7 @@ func Ready() Condition { func Unknown() Condition { return Condition{metav1.Condition{ Type: string(ConditionTypeReady), - Status: metav1.ConditionFalse, + Status: metav1.ConditionUnknown, LastTransitionTime: metav1.Now(), Reason: string(ConditionReasonUnknown), }} diff --git a/infra/v1alpha1/condition_test.go b/infra/v1alpha1/condition_test.go index befb049cc..f2fbd2581 100644 --- a/infra/v1alpha1/condition_test.go +++ b/infra/v1alpha1/condition_test.go @@ -218,3 +218,9 @@ func TestConditionWithMessage(t *testing.T) { }) } } + +func TestUnknown(t *testing.T) { + if got := Unknown().Status; got != metav1.ConditionUnknown { + t.Errorf("Unknown().Status = %q, want %q", got, metav1.ConditionUnknown) + } +}