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
8 changes: 4 additions & 4 deletions api/v1/clusterobjectset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const (
ClusterObjectSetKind = "ClusterObjectSet"

// Condition Types
ClusterObjectSetTypeAvailable = "Available"
ClusterObjectSetTypeReady = "Ready"

// Condition Reasons
ClusterObjectSetReasonArchived = "Archived"
Expand Down Expand Up @@ -494,13 +494,13 @@ type ClusterObjectSetStatus struct {
// conditions is an optional list of status conditions describing the state of the
// ClusterObjectSet.
//
// The Available condition represents the state of the revision.
// The Ready condition represents the state of the revision.
// True means all objects are at the desired state; False means one or more
// objects are not at the desired state; Unknown is the initial state, before
// the first reconciliation has evaluated the revision.
// - True with reason ProbesSucceeded: the revision has rolled out and all objects pass their readiness probes.
// - False with reason ProbeFailure: one or more objects are failing their readiness probes during rollout.
// - False with reason RollingOut: the revision is actively rolling out and has not yet become available.
// - False with reason RollingOut: the revision is actively rolling out and has not yet become ready.
// - False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery.
// - False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes.
// - False with reason Reconciling: the revision encountered an error that prevented it from observing the probes.
Expand Down Expand Up @@ -560,7 +560,7 @@ type ObservedPhase struct {
// +kubebuilder:object:root=true
// +kubebuilder:resource:scope=Cluster
// +kubebuilder:subresource:status
// +kubebuilder:printcolumn:name="Available",type=string,JSONPath=`.status.conditions[?(@.type=='Available')].status`
// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=='Ready')].status`
// +kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp`

// ClusterObjectSet represents an immutable snapshot of Kubernetes objects
Expand Down
4 changes: 4 additions & 0 deletions api/v1/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ package v1
const (
TypeInstalled = "Installed"
TypeProgressing = "Progressing"
// TypeAvailable is the ClusterExtension condition that surfaces the health of the
// full set of managed objects across all active revisions. This condition may flap
// during upgrades and configuration changes.
TypeAvailable = "Available"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding Available here also changes ensureFailureConditionsWithReason()🔗, which loops over this list and creates missing conditions as False. So we now get a new default Available=False condition.

That helper is shared by the Helm and Boxcutter runtimes. If catalog resolution fails during a Helm installation, it now creates Available=False/Retrying. When resolution recovers and installation succeeds, Helm updates Installed and Progressing but never updates Available, leaving the extension reporting unavailable with the old error indefinitely.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a great catch! Thank you! I've removed it from the condition types slice!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm skipping the Available condition in ensureFailureConditionsWithReason for now. I guess this is the current behavior before introducing the TypeAvailable condition here anyway.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like something I may have wrote or contributed to early on to satisfy the API convention of "always populate all conditions, even if it is just to set Unknown.

But honestly, we should consider dropping or at least majorly reconsidering the approach of automatically filling in Conditions with a shared reason. Seems like we should have an explicit set of possible condition Type/Status/Reason tuples that are valid, and there should be no "just fill in the blanks" sort of logic.


// Installed reasons
ReasonAbsent = "Absent"
Expand Down
4 changes: 2 additions & 2 deletions applyconfigurations/api/v1/clusterobjectsetstatus.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cmd/object-controller/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func TestStandaloneController(t *testing.T) {
return
}
assert.False(collect, cos.Status.CompletedAt.IsZero(), "completedAt should be set after rollout; conditions: %v", cos.Status.Conditions)
available := meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
available := meta.FindStatusCondition(cos.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
if assert.NotNil(collect, available) {
assert.Equal(collect, metav1.ConditionTrue, available.Status)
assert.Equal(collect, ocv1.ClusterObjectSetReasonProbesSucceeded, available.Reason)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ spec:
scope: Cluster
versions:
- additionalPrinterColumns:
- jsonPath: .status.conditions[?(@.type=='Available')].status
name: Available
- jsonPath: .status.conditions[?(@.type=='Ready')].status
name: Ready
type: string
- jsonPath: .metadata.creationTimestamp
name: Age
Expand Down Expand Up @@ -554,13 +554,13 @@ spec:
conditions is an optional list of status conditions describing the state of the
ClusterObjectSet.

The Available condition represents the state of the revision.
The Ready condition represents the state of the revision.
True means all objects are at the desired state; False means one or more
objects are not at the desired state; Unknown is the initial state, before
the first reconciliation has evaluated the revision.
- True with reason ProbesSucceeded: the revision has rolled out and all objects pass their readiness probes.
- False with reason ProbeFailure: one or more objects are failing their readiness probes during rollout.
- False with reason RollingOut: the revision is actively rolling out and has not yet become available.
- False with reason RollingOut: the revision is actively rolling out and has not yet become ready.
- False with reason Blocked: the revision has encountered an error that requires manual intervention for recovery.
- False with reason ProgressDeadlineExceeded: the revision did not roll out within spec.progressDeadlineMinutes.
- False with reason Reconciling: the revision encountered an error that prevented it from observing the probes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ func setAvailableWithDeadline(cos *ocv1.ClusterObjectSet, status metav1.Conditio
return
}
meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeAvailable,
Type: ocv1.ClusterObjectSetTypeReady,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Status: status,
Reason: reason,
Message: message,
Expand All @@ -676,7 +676,7 @@ func setRetryingConditions(cos *ocv1.ClusterObjectSet, message string, isDeadlin

func markAsAvailable(cos *ocv1.ClusterObjectSet, reason, message string) bool {
return meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeAvailable,
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionTrue,
Reason: reason,
Message: message,
Expand All @@ -686,7 +686,7 @@ func markAsAvailable(cos *ocv1.ClusterObjectSet, reason, message string) bool {

func markAsUnavailable(cos *ocv1.ClusterObjectSet, reason, message string) bool {
return meta.SetStatusCondition(&cos.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeAvailable,
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionFalse,
Reason: reason,
Message: message,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
Expand All @@ -100,7 +100,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
ext := newTestClusterExtension()
rev1 := newTestClusterObjectSet(t, clusterObjectSetName, ext, testScheme)
meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeAvailable,
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionTrue,
Reason: ocv1.ClusterObjectSetReasonProbesSucceeded,
Message: "Revision 1 is rolled out.",
Expand All @@ -114,7 +114,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
Expand All @@ -139,7 +139,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
Expand All @@ -161,7 +161,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ReasonRollingOut, cond.Reason)
Expand Down Expand Up @@ -249,7 +249,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonProbeFailure, cond.Reason)
Expand Down Expand Up @@ -337,7 +337,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonProbeFailure, cond.Reason)
Expand All @@ -362,7 +362,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ReasonRollingOut, cond.Reason)
Expand All @@ -387,7 +387,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_RevisionReconciliation(t *testing
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionTrue, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonProbesSucceeded, cond.Reason)
Expand Down Expand Up @@ -750,7 +750,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
Expand Down Expand Up @@ -785,7 +785,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonArchived, cond.Reason)
Expand Down Expand Up @@ -819,7 +819,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
Expand Down Expand Up @@ -853,7 +853,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
Expand Down Expand Up @@ -886,7 +886,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
Expand All @@ -907,7 +907,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ArchivalAndDeletion(t *testing.T)
}
rev1.Spec.LifecycleState = ocv1.ClusterObjectSetLifecycleStateArchived
meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeAvailable,
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionFalse,
Reason: ocv1.ClusterObjectSetReasonArchived,
Message: "revision is archived",
Expand Down Expand Up @@ -1012,7 +1012,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.Equal(t, metav1.ConditionFalse, cnd.Status)
require.Equal(t, ocv1.ReasonProgressDeadlineExceeded, cnd.Reason)
},
Expand Down Expand Up @@ -1066,7 +1066,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cnd)
require.Equal(t, metav1.ConditionFalse, cnd.Status)
require.Equal(t, ocv1.ReasonProgressDeadlineExceeded, cnd.Reason)
Expand Down Expand Up @@ -1096,7 +1096,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.Equal(t, metav1.ConditionFalse, cnd.Status)
require.Equal(t, ocv1.ReasonRollingOut, cnd.Reason)
},
Expand All @@ -1109,7 +1109,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
rev1.Spec.ProgressDeadlineMinutes = 1
rev1.CreationTimestamp = metav1.NewTime(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC))
meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeAvailable,
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionFalse,
Reason: ocv1.ReasonProgressDeadlineExceeded,
Message: "Revision has not rolled out for 1 minute(s). Last status: Revision 1 is rolling out.",
Expand All @@ -1127,7 +1127,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cnd)
require.Equal(t, metav1.ConditionTrue, cnd.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonProbesSucceeded, cnd.Reason)
Expand All @@ -1142,7 +1142,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
rev1.Spec.ProgressDeadlineMinutes = 1
rev1.CreationTimestamp = metav1.NewTime(time.Now().Add(-2 * time.Minute))
meta.SetStatusCondition(&rev1.Status.Conditions, metav1.Condition{
Type: ocv1.ClusterObjectSetTypeAvailable,
Type: ocv1.ClusterObjectSetTypeReady,
Status: metav1.ConditionTrue,
Reason: ocv1.ClusterObjectSetReasonProbesSucceeded,
ObservedGeneration: rev1.Generation,
Expand All @@ -1159,7 +1159,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ProgressDeadline(t *testing.T) {
Name: clusterObjectSetName,
}, rev)
require.NoError(t, err)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cnd := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.Equal(t, metav1.ConditionFalse, cnd.Status)
require.Equal(t, ocv1.ReasonRollingOut, cnd.Reason)
},
Expand Down Expand Up @@ -1623,7 +1623,7 @@ func Test_ClusterObjectSetReconciler_Reconcile_ForeignRevisionCollision(t *testi

rev := &ocv1.ClusterObjectSet{}
require.NoError(t, testClient.Get(t.Context(), client.ObjectKey{Name: tc.reconcilingRevisionName}, rev))
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeAvailable)
cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterObjectSetTypeReady)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1.ClusterObjectSetReasonReconciling, cond.Reason)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var ConditionTypes = []string{
ocv1.TypeChannelDeprecated,
ocv1.TypeBundleDeprecated,
ocv1.TypeProgressing,
ocv1.TypeAvailable,
}

var ConditionReasons = []string{
Expand Down
Loading
Loading