Skip to content

🐛 fix(Boxcutter): Use Installing/Upgrading reasons for active operations #2473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
camilamacedo86 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🐛 fix(Boxcutter): Use Installing/Upgrading reasons for active operations #2473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion api/v1/common_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ const (
TypeProgressing = "Progressing"

// Installed reasons
ReasonAbsent = "Absent"
ReasonAbsent = "Absent"
ReasonInstalling = "Installing"
ReasonUpgrading = "Upgrading"

// Progressing reasons
ReasonRollingOut = "RollingOut"
Expand Down
2 changes: 2 additions & 0 deletions internal/operator-controller/conditionsets/conditionsets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ var ConditionReasons = []string{
ocv1.ReasonInvalidConfiguration,
ocv1.ReasonRetrying,
ocv1.ReasonAbsent,
ocv1.ReasonInstalling,
ocv1.ReasonUpgrading,
ocv1.ReasonRollingOut,
ocv1.ReasonProgressDeadlineExceeded,
}
64 changes: 50 additions & 14 deletions internal/operator-controller/controllers/common_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,38 +57,56 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt
// Nothing is installed
if revisionStates.Installed == nil {
setInstallStatus(ext, nil)
reason := determineFailureReason(revisionStates.RollingOut)
reason := determineInstalledReason(revisionStates.RollingOut)
setInstalledStatusConditionFalse(ext, reason, "No bundle installed")
return
}
// Something is installed

// Something is installed - check if upgrade is in progress
installStatus := &ocv1.ClusterExtensionInstallStatus{
Bundle: revisionStates.Installed.BundleMetadata,
}
setInstallStatus(ext, installStatus)

if len(revisionStates.RollingOut) > 0 {
latestRevision := revisionStates.RollingOut[len(revisionStates.RollingOut)-1]
progressingCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)

if progressingCond != nil && progressingCond.Reason == string(ocv1.ReasonRollingOut) {
setInstalledStatusConditionUpgrading(ext, fmt.Sprintf("Upgrading from %s", revisionStates.Installed.Image))
return
}
}

setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", revisionStates.Installed.Image))
}

// determineFailureReason determines the appropriate reason for the Installed condition
// determineInstalledReason determines the appropriate reason for the Installed condition
// when no bundle is installed (Installed: False).
//
// Returns Failed when:
// - No rolling revisions exist (nothing to install)
// - The latest rolling revision has Reason: Retrying (indicates an error occurred)
//
// Returns Installing when:
// - The latest rolling revision explicitly has Reason: RollingOut (healthy installation in progress)
//
// Returns Absent when:
// - Rolling revisions exist with the latest having Reason: RollingOut (healthy phased rollout in progress)
// - Rolling revisions exist but have no conditions set (rollout just started)
//
// Rationale:
// - Failed: Semantically indicates an error prevented installation
// - Absent: Semantically indicates "not there yet" (neutral state, e.g., during healthy rollout)
// - Installing: Semantically indicates a first-time installation is actively in progress
// - Absent: Neutral state when rollout exists but hasn't progressed enough to determine health
// - Retrying reason indicates an error (config validation, apply failure, etc.)
// - RollingOut reason indicates healthy progress (not an error)
// - RollingOut reason indicates confirmed healthy progress
// - Only the LATEST revision matters - old errors superseded by newer healthy revisions should not cause Failed
//
// Note: This function is only called when Installed == nil (first-time installation scenario).
// Note: rollingRevisions are sorted in ascending order by Spec.Revision (oldest to newest),
// so the latest revision is the LAST element in the array.
func determineFailureReason(rollingRevisions []*RevisionMetadata) string {
//
// so the latest revision is the LAST element in the array.
func determineInstalledReason(rollingRevisions []*RevisionMetadata) string {
if len(rollingRevisions) == 0 {
return ocv1.ReasonFailed
}
Expand All @@ -97,14 +115,21 @@ func determineFailureReason(rollingRevisions []*RevisionMetadata) string {
// Latest revision is the last element in the array (sorted ascending by Spec.Revision)
latestRevision := rollingRevisions[len(rollingRevisions)-1]
progressingCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)
if progressingCond != nil && progressingCond.Reason == string(ocv1.ClusterExtensionRevisionReasonRetrying) {
// Retrying indicates an error occurred (config, apply, validation, etc.)
// Use Failed for semantic correctness: installation failed due to error
return ocv1.ReasonFailed
if progressingCond != nil {
if progressingCond.Reason == string(ocv1.ClusterExtensionRevisionReasonRetrying) {
// Retrying indicates an error occurred (config, apply, validation, etc.)
// Use Failed for semantic correctness: installation failed due to error
return ocv1.ReasonFailed
}
if progressingCond.Reason == string(ocv1.ReasonRollingOut) {
// RollingOut indicates healthy progress is confirmed
// Use Installing to communicate that a first-time installation is actively in progress
return ocv1.ReasonInstalling
}
}

// No error detected in latest revision - it's progressing healthily (RollingOut) or no conditions set
// Use Absent for neutral "not installed yet" state
// No progressing condition or unknown reason - rollout just started or hasn't progressed
// Use Absent as neutral state
return ocv1.ReasonAbsent
}

Expand All @@ -119,6 +144,17 @@ func setInstalledStatusConditionSuccess(ext *ocv1.ClusterExtension, message stri
})
}

// setInstalledStatusConditionUpgrading sets the installed status condition to upgrading.
func setInstalledStatusConditionUpgrading(ext *ocv1.ClusterExtension, message string) {
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: ocv1.TypeInstalled,
Status: metav1.ConditionTrue,
Reason: ocv1.ReasonUpgrading,
Message: message,
ObservedGeneration: ext.GetGeneration(),
})
}

// setInstalledStatusConditionFailed sets the installed status condition to failed.
func setInstalledStatusConditionFalse(ext *ocv1.ClusterExtension, reason string, message string) {
SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Expand Down
Loading
Loading