Skip to content

Commit 2595ea2

Browse files
jottofarjottofar
committed
UpdateError: enhance for ability to determine when upgrade failing
1 parent f9c42b2 commit 2595ea2

File tree

6 files changed

+224
-108
lines changed

6 files changed

+224
-108
lines changed

‎pkg/cvo/cvo_scenarios_test.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2563,13 +2563,15 @@ func TestCVO_ParallelError(t *testing.T) {
25632563
worker := o.configSync.(*SyncWorker)
25642564
b := &errorResourceBuilder{errors: map[string]error{
25652565
"0000_10_a_file.yaml": &payload.UpdateError{
2566-
Reason: "ClusterOperatorNotAvailable",
2567-
Name: "operator-1",
2566+
Reason: "ClusterOperatorNotAvailable",
2567+
UpdateEffect: payload.UpdateEffectNone,
2568+
Name: "operator-1",
25682569
},
25692570
"0000_20_a_file.yaml": nil,
25702571
"0000_20_b_file.yaml": &payload.UpdateError{
2571-
Reason: "ClusterOperatorNotAvailable",
2572-
Name: "operator-2",
2572+
Reason: "ClusterOperatorNotAvailable",
2573+
UpdateEffect: payload.UpdateEffectNone,
2574+
Name: "operator-2",
25732575
},
25742576
}}
25752577
worker.builder = b

‎pkg/cvo/internal/operatorstatus.go

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ func (b *clusterOperatorBuilder) WithModifier(f resourcebuilder.MetaV1ObjectModi
9696

9797
func (b *clusterOperatorBuilder) Do(ctx context.Context) error {
9898
os := readClusterOperatorV1OrDie(b.raw)
99+
100+
// add cluster operator's start time if not already there
101+
payload.COUpdateStartTimesEnsureName(os.Name)
102+
99103
if b.modifier != nil {
100104
b.modifier(os)
101105
}
@@ -128,10 +132,11 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration,
128132
actual, err := client.Get(ctx, expected.Name)
129133
if err != nil {
130134
lastErr = &payload.UpdateError{
131-
Nested: err,
132-
Reason: "ClusterOperatorNotAvailable",
133-
Message: fmt.Sprintf("Cluster operator %s has not yet reported success", expected.Name),
134-
Name: expected.Name,
135+
Nested: err,
136+
UpdateEffect: payload.UpdateEffectNone,
137+
Reason: "ClusterOperatorNotAvailable",
138+
Message: fmt.Sprintf("Cluster operator %s has not yet reported success", expected.Name),
139+
Name: expected.Name,
135140
}
136141
return false, nil
137142
}
@@ -160,10 +165,11 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration,
160165

161166
message := fmt.Sprintf("Cluster operator %s is still updating", actual.Name)
162167
lastErr = &payload.UpdateError{
163-
Nested: errors.New(lowerFirst(message)),
164-
Reason: "ClusterOperatorNotAvailable",
165-
Message: message,
166-
Name: actual.Name,
168+
Nested: errors.New(lowerFirst(message)),
169+
UpdateEffect: payload.UpdateEffectNone,
170+
Reason: "ClusterOperatorNotAvailable",
171+
Message: message,
172+
Name: actual.Name,
167173
}
168174
return false, nil
169175
}
@@ -210,31 +216,44 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration,
210216
}
211217
}
212218

219+
nestedMessage := fmt.Errorf("cluster operator %s conditions: available=%v, progressing=%v, degraded=%v",
220+
actual.Name, available, progressing, degraded)
221+
222+
if !available {
223+
lastErr = &payload.UpdateError{
224+
Nested: nestedMessage,
225+
UpdateEffect: payload.UpdateEffectFail,
226+
Reason: "ClusterOperatorNotAvailable",
227+
Message: fmt.Sprintf("Cluster operator %s is not available", actual.Name),
228+
Name: actual.Name,
229+
}
230+
return false, nil
231+
}
232+
213233
condition := failingCondition
214234
if degradedCondition != nil {
215235
condition = degradedCondition
216236
}
217237
if condition != nil && condition.Status == configv1.ConditionTrue {
218-
message := fmt.Sprintf("Cluster operator %s is reporting a failure", actual.Name)
219238
if len(condition.Message) > 0 {
220-
message = fmt.Sprintf("Cluster operator %s is reporting a failure: %s", actual.Name, condition.Message)
239+
nestedMessage = fmt.Errorf("cluster operator %s is reporting a message: %s", actual.Name, condition.Message)
221240
}
222241
lastErr = &payload.UpdateError{
223-
Nested: errors.New(lowerFirst(message)),
224-
Reason: "ClusterOperatorDegraded",
225-
Message: message,
226-
Name: actual.Name,
242+
Nested: nestedMessage,
243+
UpdateEffect: payload.UpdateEffectFailAfterInterval,
244+
Reason: "ClusterOperatorDegraded",
245+
Message: fmt.Sprintf("Cluster operator %s is degraded", actual.Name),
246+
Name: actual.Name,
227247
}
228248
return false, nil
229249
}
230250

231251
lastErr = &payload.UpdateError{
232-
Nested: fmt.Errorf("cluster operator %s is not done; it is available=%v, progressing=%v, degraded=%v",
233-
actual.Name, available, progressing, degraded,
234-
),
235-
Reason: "ClusterOperatorNotAvailable",
236-
Message: fmt.Sprintf("Cluster operator %s has not yet reported success", actual.Name),
237-
Name: actual.Name,
252+
Nested: nestedMessage,
253+
UpdateEffect: payload.UpdateEffectNone,
254+
Reason: "ClusterOperatorNotAvailable",
255+
Message: fmt.Sprintf("Cluster operator %s is updating versions", actual.Name),
256+
Name: actual.Name,
238257
}
239258
return false, nil
240259
}, ctx.Done())

‎pkg/cvo/internal/operatorstatus_test.go

Lines changed: 78 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1212
"k8s.io/apimachinery/pkg/runtime"
1313
"k8s.io/apimachinery/pkg/runtime/schema"
14-
"k8s.io/apimachinery/pkg/util/diff"
1514
clientgotesting "k8s.io/client-go/testing"
1615

1716
configv1 "github.com/openshift/api/config/v1"
@@ -43,10 +42,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
4342
},
4443
},
4544
expErr: &payload.UpdateError{
46-
Nested: apierrors.NewNotFound(schema.GroupResource{"", "clusteroperator"}, "test-co"),
47-
Reason: "ClusterOperatorNotAvailable",
48-
Message: "Cluster operator test-co has not yet reported success",
49-
Name: "test-co",
45+
Nested: apierrors.NewNotFound(schema.GroupResource{"", "clusteroperator"}, "test-co"),
46+
UpdateEffect: payload.UpdateEffectNone,
47+
Reason: "ClusterOperatorNotAvailable",
48+
Message: "Cluster operator test-co has not yet reported success",
49+
Name: "test-co",
5050
},
5151
}, {
5252
name: "cluster operator reporting no versions with no operands",
@@ -62,10 +62,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
6262
},
6363
},
6464
expErr: &payload.UpdateError{
65-
Nested: fmt.Errorf("cluster operator test-co is still updating"),
66-
Reason: "ClusterOperatorNotAvailable",
67-
Message: "Cluster operator test-co is still updating",
68-
Name: "test-co",
65+
Nested: fmt.Errorf("cluster operator test-co is still updating"),
66+
UpdateEffect: payload.UpdateEffectNone,
67+
Reason: "ClusterOperatorNotAvailable",
68+
Message: "Cluster operator test-co is still updating",
69+
Name: "test-co",
6970
},
7071
}, {
7172
name: "cluster operator reporting no versions",
@@ -83,10 +84,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
8384
},
8485
},
8586
expErr: &payload.UpdateError{
86-
Nested: fmt.Errorf("cluster operator test-co is still updating"),
87-
Reason: "ClusterOperatorNotAvailable",
88-
Message: "Cluster operator test-co is still updating",
89-
Name: "test-co",
87+
Nested: fmt.Errorf("cluster operator test-co is still updating"),
88+
UpdateEffect: payload.UpdateEffectNone,
89+
Reason: "ClusterOperatorNotAvailable",
90+
Message: "Cluster operator test-co is still updating",
91+
Name: "test-co",
9092
},
9193
}, {
9294
name: "cluster operator reporting no versions for operand",
@@ -109,10 +111,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
109111
},
110112
},
111113
expErr: &payload.UpdateError{
112-
Nested: fmt.Errorf("cluster operator test-co is still updating"),
113-
Reason: "ClusterOperatorNotAvailable",
114-
Message: "Cluster operator test-co is still updating",
115-
Name: "test-co",
114+
Nested: fmt.Errorf("cluster operator test-co is still updating"),
115+
UpdateEffect: payload.UpdateEffectNone,
116+
Reason: "ClusterOperatorNotAvailable",
117+
Message: "Cluster operator test-co is still updating",
118+
Name: "test-co",
116119
},
117120
}, {
118121
name: "cluster operator reporting old versions",
@@ -137,10 +140,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
137140
},
138141
},
139142
expErr: &payload.UpdateError{
140-
Nested: fmt.Errorf("cluster operator test-co is still updating"),
141-
Reason: "ClusterOperatorNotAvailable",
142-
Message: "Cluster operator test-co is still updating",
143-
Name: "test-co",
143+
Nested: fmt.Errorf("cluster operator test-co is still updating"),
144+
UpdateEffect: payload.UpdateEffectNone,
145+
Reason: "ClusterOperatorNotAvailable",
146+
Message: "Cluster operator test-co is still updating",
147+
Name: "test-co",
144148
},
145149
}, {
146150
name: "cluster operator reporting mix of desired and old versions",
@@ -165,10 +169,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
165169
},
166170
},
167171
expErr: &payload.UpdateError{
168-
Nested: fmt.Errorf("cluster operator test-co is still updating"),
169-
Reason: "ClusterOperatorNotAvailable",
170-
Message: "Cluster operator test-co is still updating",
171-
Name: "test-co",
172+
Nested: fmt.Errorf("cluster operator test-co is still updating"),
173+
UpdateEffect: payload.UpdateEffectNone,
174+
Reason: "ClusterOperatorNotAvailable",
175+
Message: "Cluster operator test-co is still updating",
176+
Name: "test-co",
172177
},
173178
}, {
174179
name: "cluster operator reporting desired operator and old versions for 2 operands",
@@ -197,10 +202,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
197202
},
198203
},
199204
expErr: &payload.UpdateError{
200-
Nested: fmt.Errorf("cluster operator test-co is still updating"),
201-
Reason: "ClusterOperatorNotAvailable",
202-
Message: "Cluster operator test-co is still updating",
203-
Name: "test-co",
205+
Nested: fmt.Errorf("cluster operator test-co is still updating"),
206+
UpdateEffect: payload.UpdateEffectNone,
207+
Reason: "ClusterOperatorNotAvailable",
208+
Message: "Cluster operator test-co is still updating",
209+
Name: "test-co",
204210
},
205211
}, {
206212
name: "cluster operator reporting desired operator and mix of old and desired versions for 2 operands",
@@ -229,10 +235,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
229235
},
230236
},
231237
expErr: &payload.UpdateError{
232-
Nested: fmt.Errorf("cluster operator test-co is still updating"),
233-
Reason: "ClusterOperatorNotAvailable",
234-
Message: "Cluster operator test-co is still updating",
235-
Name: "test-co",
238+
Nested: fmt.Errorf("cluster operator test-co is still updating"),
239+
UpdateEffect: payload.UpdateEffectNone,
240+
Reason: "ClusterOperatorNotAvailable",
241+
Message: "Cluster operator test-co is still updating",
242+
Name: "test-co",
236243
},
237244
}, {
238245
name: "cluster operator reporting desired versions and no conditions",
@@ -257,10 +264,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
257264
},
258265
},
259266
expErr: &payload.UpdateError{
260-
Nested: fmt.Errorf("cluster operator test-co is not done; it is available=false, progressing=true, degraded=true"),
261-
Reason: "ClusterOperatorNotAvailable",
262-
Message: "Cluster operator test-co has not yet reported success",
263-
Name: "test-co",
267+
Nested: fmt.Errorf("cluster operator test-co conditions: available=false, progressing=true, degraded=true"),
268+
UpdateEffect: payload.UpdateEffectFail,
269+
Reason: "ClusterOperatorNotAvailable",
270+
Message: "Cluster operator test-co is not available",
271+
Name: "test-co",
264272
},
265273
}, {
266274
name: "cluster operator reporting progressing=true",
@@ -286,13 +294,14 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
286294
},
287295
},
288296
expErr: &payload.UpdateError{
289-
Nested: fmt.Errorf("cluster operator test-co is not done; it is available=false, progressing=true, degraded=true"),
290-
Reason: "ClusterOperatorNotAvailable",
291-
Message: "Cluster operator test-co has not yet reported success",
292-
Name: "test-co",
297+
Nested: fmt.Errorf("cluster operator test-co conditions: available=false, progressing=true, degraded=true"),
298+
UpdateEffect: payload.UpdateEffectFail,
299+
Reason: "ClusterOperatorNotAvailable",
300+
Message: "Cluster operator test-co is not available",
301+
Name: "test-co",
293302
},
294303
}, {
295-
name: "cluster operator reporting degraded=true",
304+
name: "cluster operator reporting available=false degraded=true",
296305
actual: &configv1.ClusterOperator{
297306
ObjectMeta: metav1.ObjectMeta{Name: "test-co"},
298307
Status: configv1.ClusterOperatorStatus{
@@ -301,7 +310,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
301310
}, {
302311
Name: "operand-1", Version: "v1",
303312
}},
304-
Conditions: []configv1.ClusterOperatorStatusCondition{{Type: configv1.OperatorDegraded, Status: configv1.ConditionTrue, Message: "random error"}},
313+
Conditions: []configv1.ClusterOperatorStatusCondition{{Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse}, {Type: configv1.OperatorDegraded, Status: configv1.ConditionTrue}},
305314
},
306315
},
307316
exp: &configv1.ClusterOperator{
@@ -315,10 +324,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
315324
},
316325
},
317326
expErr: &payload.UpdateError{
318-
Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
319-
Reason: "ClusterOperatorDegraded",
320-
Message: "Cluster operator test-co is reporting a failure: random error",
321-
Name: "test-co",
327+
Nested: fmt.Errorf("cluster operator test-co conditions: available=false, progressing=true, degraded=true"),
328+
UpdateEffect: payload.UpdateEffectFail,
329+
Reason: "ClusterOperatorNotAvailable",
330+
Message: "Cluster operator test-co is not available",
331+
Name: "test-co",
322332
},
323333
}, {
324334
name: "cluster operator reporting available=true progressing=true",
@@ -344,10 +354,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
344354
},
345355
},
346356
expErr: &payload.UpdateError{
347-
Nested: fmt.Errorf("cluster operator test-co is not done; it is available=true, progressing=true, degraded=true"),
348-
Reason: "ClusterOperatorNotAvailable",
349-
Message: "Cluster operator test-co has not yet reported success",
350-
Name: "test-co",
357+
Nested: fmt.Errorf("cluster operator test-co conditions: available=true, progressing=true, degraded=true"),
358+
UpdateEffect: payload.UpdateEffectNone,
359+
Reason: "ClusterOperatorNotAvailable",
360+
Message: "Cluster operator test-co is updating versions",
361+
Name: "test-co",
351362
},
352363
}, {
353364
name: "cluster operator reporting available=true degraded=true",
@@ -373,10 +384,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
373384
},
374385
},
375386
expErr: &payload.UpdateError{
376-
Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
377-
Reason: "ClusterOperatorDegraded",
378-
Message: "Cluster operator test-co is reporting a failure: random error",
379-
Name: "test-co",
387+
Nested: fmt.Errorf("cluster operator test-co is reporting a message: random error"),
388+
UpdateEffect: payload.UpdateEffectFailAfterInterval,
389+
Reason: "ClusterOperatorDegraded",
390+
Message: "Cluster operator test-co is degraded",
391+
Name: "test-co",
380392
},
381393
}, {
382394
name: "cluster operator reporting available=true progressing=true degraded=true",
@@ -402,10 +414,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
402414
},
403415
},
404416
expErr: &payload.UpdateError{
405-
Nested: fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
406-
Reason: "ClusterOperatorDegraded",
407-
Message: "Cluster operator test-co is reporting a failure: random error",
408-
Name: "test-co",
417+
Nested: fmt.Errorf("cluster operator test-co is reporting a message: random error"),
418+
UpdateEffect: payload.UpdateEffectFailAfterInterval,
419+
Reason: "ClusterOperatorDegraded",
420+
Message: "Cluster operator test-co is degraded",
421+
Name: "test-co",
409422
},
410423
}, {
411424
name: "cluster operator reporting available=true no progressing or degraded",
@@ -431,10 +444,11 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
431444
},
432445
},
433446
expErr: &payload.UpdateError{
434-
Nested: fmt.Errorf("cluster operator test-co is not done; it is available=true, progressing=true, degraded=true"),
435-
Reason: "ClusterOperatorNotAvailable",
436-
Message: "Cluster operator test-co has not yet reported success",
437-
Name: "test-co",
447+
Nested: fmt.Errorf("cluster operator test-co conditions: available=true, progressing=true, degraded=true"),
448+
UpdateEffect: payload.UpdateEffectNone,
449+
Reason: "ClusterOperatorNotAvailable",
450+
Message: "Cluster operator test-co is updating versions",
451+
Name: "test-co",
438452
},
439453
}, {
440454
name: "cluster operator reporting available=true progressing=false degraded=false",
@@ -484,7 +498,7 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
484498
t.Fatalf("unexpected error: %v", err)
485499
}
486500
if !reflect.DeepEqual(test.expErr, err) {
487-
t.Fatalf("unexpected: %s", diff.ObjectReflectDiff(test.expErr, err))
501+
t.Fatalf("Incorrect value returned -\nexpected: %#v\nreturned: %#v", test.expErr, err)
488502
}
489503
})
490504
}

0 commit comments

Comments
 (0)