Skip to content

Commit dfb8d30

Browse files
creydrcreydr
committed
Fix CalculateReadyCondition to handle Unknown conditions
Function is now marked as NotReady when any condition is Unknown, preventing false positive Ready status. False conditions take precedence over Unknown when determining reason and message.
1 parent 5521e1e commit dfb8d30

2 files changed

Lines changed: 175 additions & 5 deletions

File tree

‎api/v1alpha1/function_lifecycle.go‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,25 @@ func (f *Function) CalculateReadyCondition() {
5252
reason := ""
5353
message := ""
5454
for _, condition := range f.Status.Conditions {
55-
if condition.Type != TypeReady && condition.Status == metav1.ConditionFalse {
56-
allReady = false
57-
reason = condition.Reason
58-
message = condition.Message
59-
continue
55+
if condition.Type != TypeReady {
56+
if condition.Status == metav1.ConditionFalse {
57+
allReady = false
58+
reason = condition.Reason
59+
message = condition.Message
60+
continue
61+
} else if condition.Status == metav1.ConditionUnknown {
62+
allReady = false
63+
64+
// override reason & message only if not set already
65+
// (e.g. if set by a ConditionFalse as this takes preference)
66+
if reason == "" {
67+
reason = condition.Reason
68+
}
69+
if message == "" {
70+
message = condition.Message
71+
}
72+
continue
73+
}
6074
}
6175
}
6276

‎api/v1alpha1/function_lifecycle_test.go‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
package v1alpha1
2+
3+
import (
4+
"testing"
5+
6+
"k8s.io/apimachinery/pkg/api/meta"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
)
9+
10+
func TestCalculateReadyCondition(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
conditions []metav1.Condition
14+
expectedStatus metav1.ConditionStatus
15+
expectedReason string
16+
expectedMessage string
17+
}{
18+
{
19+
name: "all conditions true",
20+
conditions: []metav1.Condition{
21+
{Type: TypeSourceReady, Status: metav1.ConditionTrue, Reason: "CloneSucceeded"},
22+
{Type: TypeDeployed, Status: metav1.ConditionTrue, Reason: "DeploySucceeded"},
23+
{Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"},
24+
},
25+
expectedStatus: metav1.ConditionTrue,
26+
expectedReason: "ReconcileSucceeded",
27+
},
28+
{
29+
name: "one condition false",
30+
conditions: []metav1.Condition{
31+
{Type: TypeSourceReady, Status: metav1.ConditionTrue, Reason: "CloneSucceeded"},
32+
{Type: TypeDeployed, Status: metav1.ConditionFalse, Reason: "DeployFailed", Message: "deployment failed"},
33+
{Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"},
34+
},
35+
expectedStatus: metav1.ConditionFalse,
36+
expectedReason: "DeployFailed",
37+
expectedMessage: "deployment failed",
38+
},
39+
{
40+
name: "one condition unknown",
41+
conditions: []metav1.Condition{
42+
{Type: TypeSourceReady, Status: metav1.ConditionTrue, Reason: "CloneSucceeded"},
43+
{Type: TypeDeployed, Status: metav1.ConditionUnknown, Reason: "NotChecked", Message: "deployment not checked yet"},
44+
{Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"},
45+
},
46+
expectedStatus: metav1.ConditionFalse,
47+
expectedReason: "NotChecked",
48+
expectedMessage: "deployment not checked yet",
49+
},
50+
{
51+
name: "multiple conditions unknown",
52+
conditions: []metav1.Condition{
53+
{Type: TypeSourceReady, Status: metav1.ConditionUnknown, Reason: "NotCloned", Message: "source not cloned yet"},
54+
{Type: TypeDeployed, Status: metav1.ConditionUnknown, Reason: "NotDeployed", Message: "not deployed yet"},
55+
{Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"},
56+
},
57+
expectedStatus: metav1.ConditionFalse,
58+
expectedReason: "NotCloned",
59+
expectedMessage: "source not cloned yet",
60+
},
61+
{
62+
name: "false takes precedence over unknown",
63+
conditions: []metav1.Condition{
64+
{Type: TypeSourceReady, Status: metav1.ConditionUnknown, Reason: "NotCloned", Message: "source not cloned yet"},
65+
{Type: TypeDeployed, Status: metav1.ConditionFalse, Reason: "DeployFailed", Message: "deployment failed"},
66+
{Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"},
67+
},
68+
expectedStatus: metav1.ConditionFalse,
69+
expectedReason: "DeployFailed",
70+
expectedMessage: "deployment failed",
71+
},
72+
{
73+
name: "all conditions unknown",
74+
conditions: []metav1.Condition{
75+
{Type: TypeSourceReady, Status: metav1.ConditionUnknown, Reason: "unknown", Message: ""},
76+
{Type: TypeDeployed, Status: metav1.ConditionUnknown, Reason: "unknown", Message: ""},
77+
{Type: TypeMiddlewareUpToDate, Status: metav1.ConditionUnknown, Reason: "unknown", Message: ""},
78+
},
79+
expectedStatus: metav1.ConditionFalse,
80+
expectedReason: "unknown",
81+
},
82+
}
83+
84+
for _, tt := range tests {
85+
t.Run(tt.name, func(t *testing.T) {
86+
f := &Function{
87+
ObjectMeta: metav1.ObjectMeta{
88+
Generation: 1,
89+
},
90+
}
91+
f.Status.Conditions = tt.conditions
92+
93+
f.CalculateReadyCondition()
94+
95+
readyCondition := meta.FindStatusCondition(f.Status.Conditions, TypeReady)
96+
if readyCondition == nil {
97+
t.Fatal("Ready condition not found")
98+
}
99+
100+
if readyCondition.Status != tt.expectedStatus {
101+
t.Errorf("expected status %v, got %v", tt.expectedStatus, readyCondition.Status)
102+
}
103+
104+
if readyCondition.Reason != tt.expectedReason {
105+
t.Errorf("expected reason %q, got %q", tt.expectedReason, readyCondition.Reason)
106+
}
107+
108+
if tt.expectedMessage != "" && readyCondition.Message != tt.expectedMessage {
109+
t.Errorf("expected message %q, got %q", tt.expectedMessage, readyCondition.Message)
110+
}
111+
})
112+
}
113+
}
114+
115+
func TestInitializeConditions(t *testing.T) {
116+
f := &Function{
117+
ObjectMeta: metav1.ObjectMeta{
118+
Generation: 1,
119+
},
120+
}
121+
122+
// Set some existing conditions
123+
f.Status.Conditions = []metav1.Condition{
124+
{Type: TypeSourceReady, Status: metav1.ConditionTrue, Reason: "CloneSucceeded"},
125+
{Type: TypeReady, Status: metav1.ConditionTrue, Reason: "ReconcileSucceeded"},
126+
}
127+
128+
f.InitializeConditions()
129+
130+
// Verify all conditions are reset to Unknown
131+
for _, condType := range FunctionsConditions {
132+
cond := meta.FindStatusCondition(f.Status.Conditions, condType)
133+
if cond == nil {
134+
t.Errorf("condition %s not found", condType)
135+
continue
136+
}
137+
if cond.Status != metav1.ConditionUnknown {
138+
t.Errorf("condition %s: expected status Unknown, got %v", condType, cond.Status)
139+
}
140+
if cond.Reason != "unknown" {
141+
t.Errorf("condition %s: expected reason 'unknown', got %q", condType, cond.Reason)
142+
}
143+
if cond.ObservedGeneration != f.Generation {
144+
t.Errorf("condition %s: expected generation %d, got %d", condType, f.Generation, cond.ObservedGeneration)
145+
}
146+
}
147+
148+
// Verify Ready condition is also set to Unknown
149+
readyCond := meta.FindStatusCondition(f.Status.Conditions, TypeReady)
150+
if readyCond == nil {
151+
t.Fatal("Ready condition not found")
152+
}
153+
if readyCond.Status != metav1.ConditionUnknown {
154+
t.Errorf("Ready condition: expected status Unknown, got %v", readyCond.Status)
155+
}
156+
}

0 commit comments

Comments
 (0)