Iniitialize status conditions #191
Conversation
- k8s api server is rejecting status condition with no values. So update initialize conditions func to initialize all fields in all sub status.
Codecov Report
@@ Coverage Diff @@
## main #191 +/- ##
==========================================
- Coverage 94.38% 94.34% -0.04%
==========================================
Files 16 16
Lines 641 637 -4
==========================================
- Hits 605 601 -4
Misses 25 25
Partials 11 11
Continue to review full report at Codecov.
|
| serviceAvailable := metav1.Condition{Type: ServiceBindingConditionServiceAvailable} | ||
| projectionReady := metav1.Condition{Type: ServiceBindingConditionProjectionReady} | ||
| func (bs *ServiceBindingStatus) InitializeConditions(now metav1.Time) { | ||
| ready := metav1.Condition{Type: ServiceBindingConditionReady, Status: metav1.ConditionUnknown, LastTransitionTime: now, Reason: InitializeConditionReason} |
There was a problem hiding this comment.
Wondering if it would be simpler to set the metav1.time inside the method. Perhaps the unit test will be challenging!
There was a problem hiding this comment.
Yup you guessed it right
| @@ -442,7 +456,7 @@ func TestServiceBindingStatus_MarkServiceAvailable(t *testing.T) { | |||
| { | |||
| Type: ServiceBindingConditionReady, | |||
| Status: metav1.ConditionUnknown, | |||
| Reason: "Unknown", | |||
| Reason: "ProjectionReadyUnknown", | |||
There was a problem hiding this comment.
Is that Reason ProjectionReadyUnknown correct? Can you please help me with some detail for this reason value?
There was a problem hiding this comment.
aggregare func builds the reason string with sub condition type + condition reason. Same for other comment
| @@ -572,11 +587,21 @@ func TestServiceBindingStatus_aggregateReadyCondition(t *testing.T) { | |||
| { | |||
| Type: ServiceBindingConditionReady, | |||
| Status: "Unknown", | |||
| Reason: "ServiceAvailableUnknown", | |||
There was a problem hiding this comment.
ServiceAvailableUnknown - same question here as well
|
Still seeing this behavior in v0.6.0 |
Pull request
What this PR does / why we need it
K8s API server is rejecting status condition with no values. So update initialize conditions func to initialize all fields in status resource.
Which issue(s) this PR fixes
Fixes #177
Describe testing done for PR
Use the testcase from #177 issue.
Additional information or special notes for your reviewer