Skip to content

Commit 02d2092

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #581 from karlkfi/karl-event-order
fix: Sort reconcile events to fix flakey test
2 parents 59160ba + 59a8300 commit 02d2092

5 files changed

Lines changed: 74 additions & 64 deletions

File tree

‎pkg/apply/applier_test.go‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -343,24 +343,24 @@ func TestApplier(t *testing.T) {
343343
Type: event.Started,
344344
},
345345
},
346-
// Secrets before Deployments (see pkg/ordering)
346+
// Wait events with same status sorted by Identifier (see pkg/testutil)
347347
{
348348
EventType: event.WaitType,
349349
WaitEvent: &testutil.ExpWaitEvent{
350350
GroupName: "wait-0",
351351
Status: event.ReconcilePending,
352-
Identifier: testutil.ToIdentifier(t, resources["secret"]),
352+
Identifier: testutil.ToIdentifier(t, resources["deployment"]),
353353
},
354354
},
355355
{
356356
EventType: event.WaitType,
357357
WaitEvent: &testutil.ExpWaitEvent{
358358
GroupName: "wait-0",
359359
Status: event.ReconcilePending,
360-
Identifier: testutil.ToIdentifier(t, resources["deployment"]),
360+
Identifier: testutil.ToIdentifier(t, resources["secret"]),
361361
},
362362
},
363-
// Deployment before Secret (see statusEvents)
363+
// Wait events with same status sorted by Identifier (see pkg/testutil)
364364
{
365365
EventType: event.WaitType,
366366
WaitEvent: &testutil.ExpWaitEvent{
@@ -524,24 +524,24 @@ func TestApplier(t *testing.T) {
524524
Type: event.Started,
525525
},
526526
},
527-
// Apply Secrets before Deployments (see ordering.SortableMetas)
527+
// Wait events with same status sorted by Identifier (see pkg/testutil)
528528
{
529529
EventType: event.WaitType,
530530
WaitEvent: &testutil.ExpWaitEvent{
531531
GroupName: "wait-0",
532532
Status: event.ReconcilePending,
533-
Identifier: testutil.ToIdentifier(t, resources["secret"]),
533+
Identifier: testutil.ToIdentifier(t, resources["deployment"]),
534534
},
535535
},
536536
{
537537
EventType: event.WaitType,
538538
WaitEvent: &testutil.ExpWaitEvent{
539539
GroupName: "wait-0",
540540
Status: event.ReconcilePending,
541-
Identifier: testutil.ToIdentifier(t, resources["deployment"]),
541+
Identifier: testutil.ToIdentifier(t, resources["secret"]),
542542
},
543543
},
544-
// Wait Deployments before Secrets (see testutil.GroupedEventsByID)
544+
// Wait events with same status sorted by Identifier (see pkg/testutil)
545545
{
546546
EventType: event.WaitType,
547547
WaitEvent: &testutil.ExpWaitEvent{
@@ -730,7 +730,7 @@ func TestApplier(t *testing.T) {
730730
Type: event.Started,
731731
},
732732
},
733-
// Prune Deployments before Secrets (see ordering.SortableMetas)
733+
// Wait events with same status sorted by Identifier (see pkg/testutil)
734734
{
735735
EventType: event.WaitType,
736736
WaitEvent: &testutil.ExpWaitEvent{
@@ -747,7 +747,7 @@ func TestApplier(t *testing.T) {
747747
Identifier: testutil.ToIdentifier(t, resources["secret"]),
748748
},
749749
},
750-
// Wait Deployments before Secrets (see testutil.GroupedEventsByID)
750+
// Wait events with same status sorted by Identifier (see pkg/testutil)
751751
{
752752
EventType: event.WaitType,
753753
WaitEvent: &testutil.ExpWaitEvent{
@@ -1123,6 +1123,7 @@ func TestApplier(t *testing.T) {
11231123
Type: event.Started,
11241124
},
11251125
},
1126+
// Wait events sorted Pending > Successful (see pkg/testutil)
11261127
{
11271128
EventType: event.WaitType,
11281129
WaitEvent: &testutil.ExpWaitEvent{
@@ -1214,15 +1215,15 @@ func TestApplier(t *testing.T) {
12141215
Identifiers: object.ObjMetadataSet{
12151216
object.UnstructuredToObjMetadata(
12161217
testutil.Unstructured(t, resources["deployment"], JSONPathSetter{
1217-
"$.kind", "",
1218+
"$.metadata.name", "",
12181219
}),
12191220
),
12201221
},
12211222
Error: testutil.EqualErrorString(validation.NewError(
1222-
field.Required(field.NewPath("kind"), "kind is required"),
1223+
field.Required(field.NewPath("metadata", "name"), "name is required"),
12231224
object.UnstructuredToObjMetadata(
12241225
testutil.Unstructured(t, resources["deployment"], JSONPathSetter{
1225-
"$.kind", "",
1226+
"$.metadata.name", "",
12261227
}),
12271228
),
12281229
).Error()),
@@ -1234,15 +1235,15 @@ func TestApplier(t *testing.T) {
12341235
Identifiers: object.ObjMetadataSet{
12351236
object.UnstructuredToObjMetadata(
12361237
testutil.Unstructured(t, resources["deployment"], JSONPathSetter{
1237-
"$.metadata.name", "",
1238+
"$.kind", "",
12381239
}),
12391240
),
12401241
},
12411242
Error: testutil.EqualErrorString(validation.NewError(
1242-
field.Required(field.NewPath("metadata", "name"), "name is required"),
1243+
field.Required(field.NewPath("kind"), "kind is required"),
12431244
object.UnstructuredToObjMetadata(
12441245
testutil.Unstructured(t, resources["deployment"], JSONPathSetter{
1245-
"$.metadata.name", "",
1246+
"$.kind", "",
12461247
}),
12471248
),
12481249
).Error()),
@@ -1302,7 +1303,7 @@ func TestApplier(t *testing.T) {
13021303
Type: event.Started,
13031304
},
13041305
},
1305-
// Secret pending
1306+
// Wait events sorted Pending > Successful (see pkg/testutil)
13061307
{
13071308
EventType: event.WaitType,
13081309
WaitEvent: &testutil.ExpWaitEvent{
@@ -1311,7 +1312,6 @@ func TestApplier(t *testing.T) {
13111312
Identifier: testutil.ToIdentifier(t, resources["secret"]),
13121313
},
13131314
},
1314-
// Secret reconciled
13151315
{
13161316
EventType: event.WaitType,
13171317
WaitEvent: &testutil.ExpWaitEvent{
@@ -1802,6 +1802,7 @@ func TestApplierCancel(t *testing.T) {
18021802
Type: event.Started,
18031803
},
18041804
},
1805+
// Wait events sorted Pending > Successful (see pkg/testutil)
18051806
{
18061807
// Deployment reconcile pending.
18071808
EventType: event.WaitType,
@@ -1920,6 +1921,9 @@ func TestApplierCancel(t *testing.T) {
19201921
}
19211922
}
19221923

1924+
// sort to allow comparison of multiple wait events
1925+
testutil.SortExpEvents(receivedEvents)
1926+
19231927
// Validate the rest of the events
19241928
testutil.AssertEqual(t, tc.expectedEvents, receivedEvents,
19251929
"Actual events (%d) do not match expected events (%d)",

‎pkg/apply/destroyer_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ func TestDestroyerCancel(t *testing.T) {
259259
Type: event.Started,
260260
},
261261
},
262+
// Wait events sorted Pending > Successful (see pkg/testutil)
262263
{
263264
// Deployment reconcile pending.
264265
EventType: event.WaitType,
@@ -374,6 +375,9 @@ func TestDestroyerCancel(t *testing.T) {
374375
}
375376
}
376377

378+
// sort to allow comparison of multiple wait events
379+
testutil.SortExpEvents(receivedEvents)
380+
377381
// Validate the rest of the events
378382
testutil.AssertEqual(t, tc.expectedEvents, receivedEvents,
379383
"Actual events (%d) do not match expected events (%d)",

‎pkg/testutil/events.go‎

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -447,45 +447,41 @@ func (ape GroupedEventsByID) Less(i, j int) bool {
447447
return i < j
448448
}
449449
switch ape[i].EventType {
450+
case event.ValidationType:
451+
// Validation events are predictable ordered by input object set order.
450452
case event.ApplyType:
451-
if ape[i].ApplyEvent.GroupName == ape[j].ApplyEvent.GroupName &&
452-
ape[i].ApplyEvent.Status == ape[j].ApplyEvent.Status &&
453-
ape[i].ApplyEvent.Identifier.GroupKind == ape[j].ApplyEvent.Identifier.GroupKind {
454-
// apply events are predictably ordered by GroupKind, due to
455-
// ordering.SortableMetas.
456-
// So we only need to sort by namespace & name.
457-
return ape[i].ApplyEvent.Identifier.String() < ape[j].ApplyEvent.Identifier.String()
458-
}
453+
// Apply events are are predictably ordered by ordering.SortableMetas.
459454
case event.PruneType:
460-
if ape[i].PruneEvent.GroupName == ape[j].PruneEvent.GroupName &&
461-
ape[i].PruneEvent.Status == ape[j].PruneEvent.Status &&
462-
ape[i].PruneEvent.Identifier.GroupKind == ape[j].PruneEvent.Identifier.GroupKind {
463-
// prune events are predictably ordered by GroupKind, due to
464-
// ordering.SortableMetas (reversed).
465-
// So we only need to sort by namespace & name.
466-
return ape[i].PruneEvent.Identifier.String() < ape[j].PruneEvent.Identifier.String()
467-
}
455+
// Prune events are predictably ordered in reverse apply order.
468456
case event.DeleteType:
469-
if ape[i].DeleteEvent.GroupName == ape[j].DeleteEvent.GroupName &&
470-
ape[i].DeleteEvent.Status == ape[j].DeleteEvent.Status &&
471-
ape[i].DeleteEvent.Identifier.GroupKind == ape[j].DeleteEvent.Identifier.GroupKind {
472-
// delete events are predictably ordered by GroupKind, due to
473-
// ordering.SortableMetas (reversed).
474-
// So we only need to sort by namespace & name.
475-
return ape[i].DeleteEvent.Identifier.String() < ape[j].DeleteEvent.Identifier.String()
476-
}
457+
// Delete events are predictably ordered in reverse apply order.
477458
case event.WaitType:
478-
if ape[i].WaitEvent.GroupName == ape[j].WaitEvent.GroupName &&
479-
ape[i].WaitEvent.Status == ape[j].WaitEvent.Status &&
480-
ape[i].WaitEvent.Status == event.ReconcileSuccessful {
481-
// pending, skipped, and timeout operations are predictably ordered
482-
// using the order in WaitTask.Ids.
483-
// So we only need to sort Reconciled events, which occur in the
484-
// order the Waitask receives StatusEvents with Current/NotFound.
459+
// Wait events are unpredictably ordered, because the status may
460+
// reconcile before or after the WaitTask starts, and status event
461+
// order after starting is dependent on remote controller behavior.
462+
// So here we sort status groups explicitly:
463+
// Pending > Skipped > Successful > Failed > Timeout.
464+
// Each status group is then sorted by Identifier:
465+
// Group > Kind > Namespace > Name.
466+
// Note that the Pending status is always optional.
467+
if ape[i].WaitEvent.GroupName == ape[j].WaitEvent.GroupName {
468+
if ape[i].WaitEvent.Status != ape[j].WaitEvent.Status {
469+
return lessWaitStatus(ape[i].WaitEvent.Status, ape[j].WaitEvent.Status)
470+
}
485471
return ape[i].WaitEvent.Identifier.String() < ape[j].WaitEvent.Identifier.String()
486472
}
487-
case event.ValidationType:
488-
return ape[i].ValidationEvent.Identifiers.Hash() < ape[j].ValidationEvent.Identifiers.Hash()
489473
}
490474
return i < j
491475
}
476+
477+
var waitStatusWeight = map[event.WaitEventStatus]int{
478+
event.ReconcilePending: 0,
479+
event.ReconcileSkipped: 1,
480+
event.ReconcileSuccessful: 2,
481+
event.ReconcileFailed: 3,
482+
event.ReconcileTimeout: 4,
483+
}
484+
485+
func lessWaitStatus(x, y event.WaitEventStatus) bool {
486+
return waitStatusWeight[x] < waitStatusWeight[y]
487+
}

‎test/e2e/continue_on_error_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,25 +136,25 @@ func continueOnErrorTest(ctx context.Context, c client.Client, invConfig invconf
136136
},
137137
},
138138
{
139-
// CRD reconcile Skipped.
139+
// Pod1 reconcile Pending.
140140
EventType: event.WaitType,
141141
WaitEvent: &testutil.ExpWaitEvent{
142142
GroupName: "wait-0",
143-
Status: event.ReconcileSkipped,
144-
Identifier: object.UnstructuredToObjMetadata(invalidCrdObj),
143+
Status: event.ReconcilePending,
144+
Identifier: object.UnstructuredToObjMetadata(pod1Obj),
145145
},
146146
},
147147
{
148-
// Pod1 reconcile Pending.
148+
// CRD reconcile Skipped.
149149
EventType: event.WaitType,
150150
WaitEvent: &testutil.ExpWaitEvent{
151151
GroupName: "wait-0",
152-
Status: event.ReconcilePending,
153-
Identifier: object.UnstructuredToObjMetadata(pod1Obj),
152+
Status: event.ReconcileSkipped,
153+
Identifier: object.UnstructuredToObjMetadata(invalidCrdObj),
154154
},
155155
},
156156
{
157-
// Pod1 confirmed Current.
157+
// Pod1 reconcile Successful.
158158
EventType: event.WaitType,
159159
WaitEvent: &testutil.ExpWaitEvent{
160160
GroupName: "wait-0",

‎test/e2e/depends_on_test.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,10 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv
382382
},
383383
},
384384
}
385-
Expect(testutil.EventsToExpEvents(applierEvents)).To(testutil.Equal(expEvents))
385+
receivedEvents := testutil.EventsToExpEvents(applierEvents)
386+
// sort to handle objects reconciling in random order
387+
testutil.SortExpEvents(receivedEvents)
388+
Expect(receivedEvents).To(testutil.Equal(expEvents))
386389

387390
By("verify namespace1 created")
388391
e2eutil.AssertUnstructuredExists(ctx, c, namespace1Obj)
@@ -660,21 +663,21 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv
660663
},
661664
},
662665
{
663-
// Namespace2 reconcile Pending.
666+
// Namespace1 reconcile Pending.
664667
EventType: event.WaitType,
665668
WaitEvent: &testutil.ExpWaitEvent{
666669
GroupName: "wait-3",
667670
Status: event.ReconcilePending,
668-
Identifier: object.UnstructuredToObjMetadata(namespace2Obj),
671+
Identifier: object.UnstructuredToObjMetadata(namespace1Obj),
669672
},
670673
},
671674
{
672-
// Namespace1 reconcile Pending.
675+
// Namespace2 reconcile Pending.
673676
EventType: event.WaitType,
674677
WaitEvent: &testutil.ExpWaitEvent{
675678
GroupName: "wait-3",
676679
Status: event.ReconcilePending,
677-
Identifier: object.UnstructuredToObjMetadata(namespace1Obj),
680+
Identifier: object.UnstructuredToObjMetadata(namespace2Obj),
678681
},
679682
},
680683
{
@@ -723,7 +726,10 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv
723726
},
724727
},
725728
}
726-
Expect(testutil.EventsToExpEvents(destroyerEvents)).To(testutil.Equal(expEvents))
729+
receivedEvents = testutil.EventsToExpEvents(destroyerEvents)
730+
// sort to handle objects reconciling in random order
731+
testutil.SortExpEvents(receivedEvents)
732+
Expect(receivedEvents).To(testutil.Equal(expEvents))
727733

728734
By("verify pod1 deleted")
729735
e2eutil.AssertUnstructuredDoesNotExist(ctx, c, pod1Obj)

0 commit comments

Comments
 (0)