Skip to content

OCPBUGS-3714: pkg/cli/admin/upgrade: Report on Failing!=False conditions #900

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

Merged
openshift-merge-robot merged 1 commit into from
Nov 18, 2022
Merged

OCPBUGS-3714: pkg/cli/admin/upgrade: Report on Failing!=False conditions #900

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
41 changes: 26 additions & 15 deletions pkg/cli/admin/upgrade/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ import (
"github.com/openshift/oc/pkg/cli/admin/upgrade/channel"
)

const (
// clusterStatusFailing is set on the ClusterVersion status when a cluster
// cannot reach the desired state. It is considered more serious than Degraded
// and indicates the cluster is not healthy.
clusterStatusFailing = configv1.ClusterStatusConditionType("Failing")
)

var upgradeExample = templates.Examples(`
# Review the available cluster updates
oc adm upgrade
Expand Down Expand Up @@ -340,15 +347,12 @@ func (o *Options) Run() error {
return nil

default:
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorDegraded); c != nil && c.Status == configv1.ConditionTrue {
prefix := "No upgrade is possible due to an error"
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorProgressing); c != nil && c.Status == configv1.ConditionTrue && len(c.Message) > 0 {
prefix = c.Message
}
if len(c.Message) > 0 {
return fmt.Errorf("%s:\n\n Reason: %s\n Message: %s\n\n", prefix, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, clusterStatusFailing); c != nil {
if c.Status != configv1.ConditionFalse {
Comment thread
wking marked this conversation as resolved.
fmt.Fprintf(o.Out, "%s=%s:\n\n Reason: %s\n Message: %s\n\n", c.Type, c.Status, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
}
return fmt.Errorf("The cluster can't be upgraded, see `oc describe clusterversion`")
} else {
fmt.Fprintf(o.ErrOut, "warning: No current %s info, see `oc describe clusterversion` for more details.\n", clusterStatusFailing)
Comment thread
wking marked this conversation as resolved.
}

if c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorProgressing); c != nil && len(c.Message) > 0 {
Expand All @@ -358,12 +362,12 @@ func (o *Options) Run() error {
fmt.Fprintln(o.Out, c.Message)
}
} else {
fmt.Fprintln(o.ErrOut, "warning: No current status info, see `oc describe clusterversion` for more details")
fmt.Fprintf(o.ErrOut, "warning: No current %s info, see `oc describe clusterversion` for more details.\n", configv1.OperatorProgressing)
}
fmt.Fprintln(o.Out)
Comment thread
wking marked this conversation as resolved.

if c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorUpgradeable); c != nil && c.Status == configv1.ConditionFalse {
fmt.Fprintf(o.Out, "Upgradeable=False\n\n Reason: %s\n Message: %s\n\n", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
fmt.Fprintf(o.Out, "%s=%s\n\n Reason: %s\n Message: %s\n\n", c.Type, c.Status, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n "))
}

if c := findClusterOperatorStatusCondition(cv.Status.Conditions, "ReleaseAccepted"); c != nil && c.Status != configv1.ConditionTrue {
Expand Down Expand Up @@ -560,20 +564,27 @@ func findClusterOperatorStatusCondition(conditions []configv1.ClusterOperatorSta
func checkForUpgrade(cv *configv1.ClusterVersion) error {
results := []string{}
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, "Invalid"); c != nil && c.Status == configv1.ConditionTrue {
results = append(results, fmt.Sprintf("the cluster version object is invalid, you must correct the invalid state first:\n\n Reason: %s\n Message: %s\n\n", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n ")))
results = append(results, fmt.Sprintf("the cluster version object is invalid, you must correct the invalid state first:\n\n Reason: %s\n Message: %s", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n ")))
}
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorDegraded); c != nil && c.Status == configv1.ConditionTrue {
results = append(results, fmt.Sprintf("the cluster is experiencing an upgrade-blocking error:\n\n Reason: %s\n Message: %s\n\n", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n ")))
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, clusterStatusFailing); c != nil && c.Status == configv1.ConditionTrue {
target := cv.Status.Desired.Version
if target == "" {
target = cv.Status.Desired.Image
Comment thread
wking marked this conversation as resolved.
if target == "" {
target = "ClusterVersion status.desired has neither a version nor image; please open a bug"
}
}
results = append(results, fmt.Sprintf("the cluster is experiencing an error reconciling %q:\n\n Reason: %s\n Message: %s", target, c.Reason, strings.ReplaceAll(c.Message, "\n", "\n ")))
}
if c := findClusterOperatorStatusCondition(cv.Status.Conditions, configv1.OperatorProgressing); c != nil && c.Status == configv1.ConditionTrue {
results = append(results, fmt.Sprintf("the cluster is already upgrading:\n\n Reason: %s\n Message: %s\n\n", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n ")))
results = append(results, fmt.Sprintf("the cluster is already upgrading:\n\n Reason: %s\n Message: %s", c.Reason, strings.ReplaceAll(c.Message, "\n", "\n ")))
}

if len(results) == 0 {
return nil
}

return errors.New(strings.Join(results, ""))
return errors.New(strings.Join(results, "\n\n"))
}

// targetMatch returns true if the target release matches the target
Expand Down
17 changes: 12 additions & 5 deletions pkg/cli/admin/upgrade/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,33 @@ func TestCheckForUpgrade(t *testing.T) {
Reason: "SomeReason",
Message: "Some message.",
}},
expected: errors.New("the cluster version object is invalid, you must correct the invalid state first:\n\n Reason: SomeReason\n Message: Some message.\n\n"),
expected: errors.New("the cluster version object is invalid, you must correct the invalid state first:\n\n Reason: SomeReason\n Message: Some message."),
},
{
name: "degraded and progressing",
name: "failing and progressing",
conditions: []configv1.ClusterOperatorStatusCondition{{
Type: configv1.OperatorProgressing,
Status: configv1.ConditionTrue,
Reason: "RollingOut",
Message: "Updating to v2.",
}, {
Type: configv1.OperatorDegraded,
Type: clusterStatusFailing,
Status: configv1.ConditionTrue,
Reason: "BadStuff",
Message: "The widgets are slow.",
}},
expected: errors.New("the cluster is experiencing an upgrade-blocking error:\n\n Reason: BadStuff\n Message: The widgets are slow.\n\nthe cluster is already upgrading:\n\n Reason: RollingOut\n Message: Updating to v2.\n\n"),
expected: errors.New("the cluster is experiencing an error reconciling \"4.1.0\":\n\n Reason: BadStuff\n Message: The widgets are slow.\n\nthe cluster is already upgrading:\n\n Reason: RollingOut\n Message: Updating to v2."),
},
} {
t.Run(testCase.name, func(t *testing.T) {
clusterVersion := &configv1.ClusterVersion{}
clusterVersion := &configv1.ClusterVersion{
Status: configv1.ClusterVersionStatus{
Desired: configv1.Release{
Version: "4.1.0",
Image: "quay.io/openshift-release-dev/ocp-release@sha256:b8307ac0f3ec4ac86c3f3b52846425205022da52c16f56ec31cbe428501001d6",
},
},
}
clusterVersion.Status.Conditions = testCase.conditions
actual := checkForUpgrade(clusterVersion)
if !reflect.DeepEqual(actual, testCase.expected) {
Expand Down