Skip to content

Bug 1884334: UpdateError: enhance for ability to determine when upgrade failing #486

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
merged 1 commit into from
Feb 3, 2021
Merged

Bug 1884334: UpdateError: enhance for ability to determine when upgrade failing #486

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
10 changes: 6 additions & 4 deletions pkg/cvo/cvo_scenarios_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2563,13 +2563,15 @@ func TestCVO_ParallelError(t *testing.T) {
worker := o.configSync.(*SyncWorker)
b := &errorResourceBuilder{errors: map[string]error{
"0000_10_a_file.yaml": &payload.UpdateError{
Reason: "ClusterOperatorNotAvailable",
Name: "operator-1",
Reason: "ClusterOperatorNotAvailable",
UpdateEffect: payload.UpdateEffectNone,
Name: "operator-1",
},
"0000_20_a_file.yaml": nil,
"0000_20_b_file.yaml": &payload.UpdateError{
Reason: "ClusterOperatorNotAvailable",
Name: "operator-2",
Reason: "ClusterOperatorNotAvailable",
UpdateEffect: payload.UpdateEffectNone,
Name: "operator-2",
},
}}
worker.builder = b
Expand Down
59 changes: 39 additions & 20 deletions pkg/cvo/internal/operatorstatus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ func (b *clusterOperatorBuilder) WithModifier(f resourcebuilder.MetaV1ObjectModi

func (b *clusterOperatorBuilder) Do(ctx context.Context) error {
os := readClusterOperatorV1OrDie(b.raw)

// add cluster operator's start time if not already there
payload.COUpdateStartTimesEnsureName(os.Name)

if b.modifier != nil {
b.modifier(os)
}
Expand Down Expand Up @@ -128,10 +132,11 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration,
actual, err := client.Get(ctx, expected.Name)
if err != nil {
lastErr = &payload.UpdateError{
Nested: err,
Reason: "ClusterOperatorNotAvailable",
Message: fmt.Sprintf("Cluster operator %s has not yet reported success", expected.Name),
Name: expected.Name,
Nested: err,
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: fmt.Sprintf("Cluster operator %s has not yet reported success", expected.Name),
Name: expected.Name,
}
return false, nil
}
Expand Down Expand Up @@ -160,10 +165,11 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration,

message := fmt.Sprintf("Cluster operator %s is still updating", actual.Name)
lastErr = &payload.UpdateError{
Nested: errors.New(lowerFirst(message)),
Reason: "ClusterOperatorNotAvailable",
Message: message,
Name: actual.Name,
Nested: errors.New(lowerFirst(message)),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: message,
Name: actual.Name,
}
return false, nil
}
Expand Down Expand Up @@ -210,31 +216,44 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration,
}
}

nestedMessage := fmt.Errorf("cluster operator %s conditions: available=%v, progressing=%v, degraded=%v",
actual.Name, available, progressing, degraded)

if !available {
lastErr = &payload.UpdateError{
Nested: nestedMessage,
UpdateEffect: payload.UpdateEffectFail,
Reason: "ClusterOperatorNotAvailable",
Message: fmt.Sprintf("Cluster operator %s is not available", actual.Name),
Name: actual.Name,
}
return false, nil
}

condition := failingCondition
if degradedCondition != nil {
condition = degradedCondition
}
if condition != nil && condition.Status == configv1.ConditionTrue {
message := fmt.Sprintf("Cluster operator %s is reporting a failure", actual.Name)
if len(condition.Message) > 0 {
message = fmt.Sprintf("Cluster operator %s is reporting a failure: %s", actual.Name, condition.Message)
nestedMessage = fmt.Errorf("cluster operator %s is reporting a message: %s", actual.Name, condition.Message)
}
lastErr = &payload.UpdateError{
Nested: errors.New(lowerFirst(message)),
Reason: "ClusterOperatorDegraded",
Message: message,
Name: actual.Name,
Nested: nestedMessage,
UpdateEffect: payload.UpdateEffectFailAfterInterval,
Reason: "ClusterOperatorDegraded",
Message: fmt.Sprintf("Cluster operator %s is degraded", actual.Name),
Name: actual.Name,
}
return false, nil
}

lastErr = &payload.UpdateError{
Nested: fmt.Errorf("cluster operator %s is not done; it is available=%v, progressing=%v, degraded=%v",
actual.Name, available, progressing, degraded,
),
Reason: "ClusterOperatorNotAvailable",
Message: fmt.Sprintf("Cluster operator %s has not yet reported success", actual.Name),
Name: actual.Name,
Nested: nestedMessage,
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Copy link
Member

@wking wking Feb 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We might want a different reason for this, because we know by this point that it is available and not degraded.
But we can twiddle that in follow-up work.

Message: fmt.Sprintf("Cluster operator %s is updating versions", actual.Name),
Name: actual.Name,
}
return false, nil
}, ctx.Done())
Expand Down
142 changes: 78 additions & 64 deletions pkg/cvo/internal/operatorstatus_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/diff"
clientgotesting "k8s.io/client-go/testing"

configv1 "github.com/openshift/api/config/v1"
Expand Down Expand Up @@ -43,10 +42,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: apierrors.NewNotFound(schema.GroupResource{"", "clusteroperator"}, "test-co"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co has not yet reported success",
Name: "test-co",
Nested: apierrors.NewNotFound(schema.GroupResource{"", "clusteroperator"}, "test-co"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co has not yet reported success",
Name: "test-co",
},
}, {
name: "cluster operator reporting no versions with no operands",
Expand All @@ -62,10 +62,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is still updating"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is still updating"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
},
}, {
name: "cluster operator reporting no versions",
Expand All @@ -83,10 +84,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is still updating"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is still updating"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
},
}, {
name: "cluster operator reporting no versions for operand",
Expand All @@ -109,10 +111,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is still updating"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is still updating"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
},
}, {
name: "cluster operator reporting old versions",
Expand All @@ -137,10 +140,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is still updating"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is still updating"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
},
}, {
name: "cluster operator reporting mix of desired and old versions",
Expand All @@ -165,10 +169,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is still updating"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is still updating"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
},
}, {
name: "cluster operator reporting desired operator and old versions for 2 operands",
Expand Down Expand Up @@ -197,10 +202,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is still updating"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is still updating"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
},
}, {
name: "cluster operator reporting desired operator and mix of old and desired versions for 2 operands",
Expand Down Expand Up @@ -229,10 +235,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is still updating"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is still updating"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is still updating",
Name: "test-co",
},
}, {
name: "cluster operator reporting desired versions and no conditions",
Expand All @@ -257,10 +264,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is not done; it is available=false, progressing=true, degraded=true"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co has not yet reported success",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co conditions: available=false, progressing=true, degraded=true"),
UpdateEffect: payload.UpdateEffectFail,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is not available",
Name: "test-co",
},
}, {
name: "cluster operator reporting progressing=true",
Expand All @@ -286,13 +294,14 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is not done; it is available=false, progressing=true, degraded=true"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co has not yet reported success",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co conditions: available=false, progressing=true, degraded=true"),
UpdateEffect: payload.UpdateEffectFail,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is not available",
Name: "test-co",
},
}, {
name: "cluster operator reporting degraded=true",
name: "cluster operator reporting available=false degraded=true",
actual: &configv1.ClusterOperator{
ObjectMeta: metav1.ObjectMeta{Name: "test-co"},
Status: configv1.ClusterOperatorStatus{
Expand All @@ -301,7 +310,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
}, {
Name: "operand-1", Version: "v1",
}},
Conditions: []configv1.ClusterOperatorStatusCondition{{Type: configv1.OperatorDegraded, Status: configv1.ConditionTrue, Message: "random error"}},
Conditions: []configv1.ClusterOperatorStatusCondition{{Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse}, {Type: configv1.OperatorDegraded, Status: configv1.ConditionTrue}},
},
},
exp: &configv1.ClusterOperator{
Expand All @@ -315,10 +324,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
Reason: "ClusterOperatorDegraded",
Message: "Cluster operator test-co is reporting a failure: random error",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co conditions: available=false, progressing=true, degraded=true"),
UpdateEffect: payload.UpdateEffectFail,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is not available",
Name: "test-co",
},
}, {
name: "cluster operator reporting available=true progressing=true",
Expand All @@ -344,10 +354,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is not done; it is available=true, progressing=true, degraded=true"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co has not yet reported success",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co conditions: available=true, progressing=true, degraded=true"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is updating versions",
Name: "test-co",
},
}, {
name: "cluster operator reporting available=true degraded=true",
Expand All @@ -373,10 +384,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
Reason: "ClusterOperatorDegraded",
Message: "Cluster operator test-co is reporting a failure: random error",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is reporting a message: random error"),
UpdateEffect: payload.UpdateEffectFailAfterInterval,
Reason: "ClusterOperatorDegraded",
Message: "Cluster operator test-co is degraded",
Name: "test-co",
},
}, {
name: "cluster operator reporting available=true progressing=true degraded=true",
Expand All @@ -402,10 +414,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
Reason: "ClusterOperatorDegraded",
Message: "Cluster operator test-co is reporting a failure: random error",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co is reporting a message: random error"),
UpdateEffect: payload.UpdateEffectFailAfterInterval,
Reason: "ClusterOperatorDegraded",
Message: "Cluster operator test-co is degraded",
Name: "test-co",
},
}, {
name: "cluster operator reporting available=true no progressing or degraded",
Expand All @@ -431,10 +444,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
},
},
expErr: &payload.UpdateError{
Nested: fmt.Errorf("cluster operator test-co is not done; it is available=true, progressing=true, degraded=true"),
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co has not yet reported success",
Name: "test-co",
Nested: fmt.Errorf("cluster operator test-co conditions: available=true, progressing=true, degraded=true"),
UpdateEffect: payload.UpdateEffectNone,
Reason: "ClusterOperatorNotAvailable",
Message: "Cluster operator test-co is updating versions",
Name: "test-co",
},
}, {
name: "cluster operator reporting available=true progressing=false degraded=false",
Expand Down Expand Up @@ -484,7 +498,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
t.Fatalf("unexpected error: %v", err)
}
if !reflect.DeepEqual(test.expErr, err) {
t.Fatalf("unexpected: %s", diff.ObjectReflectDiff(test.expErr, err))
t.Fatalf("Incorrect value returned -\nexpected: %#v\nreturned: %#v", test.expErr, err)
}
})
}
Expand Down
Loading