diff --git a/pkg/apply/applier_test.go b/pkg/apply/applier_test.go index 94103cb7..bb99acf7 100644 --- a/pkg/apply/applier_test.go +++ b/pkg/apply/applier_test.go @@ -343,13 +343,13 @@ func TestApplier(t *testing.T) { Type: event.Started, }, }, - // Secrets before Deployments (see pkg/ordering) + // Wait events with same status sorted by Identifier (see pkg/testutil) { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-0", Status: event.ReconcilePending, - Identifier: testutil.ToIdentifier(t, resources["secret"]), + Identifier: testutil.ToIdentifier(t, resources["deployment"]), }, }, { @@ -357,10 +357,10 @@ func TestApplier(t *testing.T) { WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-0", Status: event.ReconcilePending, - Identifier: testutil.ToIdentifier(t, resources["deployment"]), + Identifier: testutil.ToIdentifier(t, resources["secret"]), }, }, - // Deployment before Secret (see statusEvents) + // Wait events with same status sorted by Identifier (see pkg/testutil) { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ @@ -524,13 +524,13 @@ func TestApplier(t *testing.T) { Type: event.Started, }, }, - // Apply Secrets before Deployments (see ordering.SortableMetas) + // Wait events with same status sorted by Identifier (see pkg/testutil) { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-0", Status: event.ReconcilePending, - Identifier: testutil.ToIdentifier(t, resources["secret"]), + Identifier: testutil.ToIdentifier(t, resources["deployment"]), }, }, { @@ -538,10 +538,10 @@ func TestApplier(t *testing.T) { WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-0", Status: event.ReconcilePending, - Identifier: testutil.ToIdentifier(t, resources["deployment"]), + Identifier: testutil.ToIdentifier(t, resources["secret"]), }, }, - // Wait Deployments before Secrets (see testutil.GroupedEventsByID) + // Wait events with same status sorted by Identifier (see pkg/testutil) { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ @@ -730,7 +730,7 @@ func TestApplier(t *testing.T) { Type: event.Started, }, }, - // Prune Deployments before Secrets (see ordering.SortableMetas) + // Wait events with same status sorted by Identifier (see pkg/testutil) { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ @@ -747,7 +747,7 @@ func TestApplier(t *testing.T) { Identifier: testutil.ToIdentifier(t, resources["secret"]), }, }, - // Wait Deployments before Secrets (see testutil.GroupedEventsByID) + // Wait events with same status sorted by Identifier (see pkg/testutil) { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ @@ -1123,6 +1123,7 @@ func TestApplier(t *testing.T) { Type: event.Started, }, }, + // Wait events sorted Pending > Successful (see pkg/testutil) { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ @@ -1214,15 +1215,15 @@ func TestApplier(t *testing.T) { Identifiers: object.ObjMetadataSet{ object.UnstructuredToObjMetadata( testutil.Unstructured(t, resources["deployment"], JSONPathSetter{ - "$.kind", "", + "$.metadata.name", "", }), ), }, Error: testutil.EqualErrorString(validation.NewError( - field.Required(field.NewPath("kind"), "kind is required"), + field.Required(field.NewPath("metadata", "name"), "name is required"), object.UnstructuredToObjMetadata( testutil.Unstructured(t, resources["deployment"], JSONPathSetter{ - "$.kind", "", + "$.metadata.name", "", }), ), ).Error()), @@ -1234,15 +1235,15 @@ func TestApplier(t *testing.T) { Identifiers: object.ObjMetadataSet{ object.UnstructuredToObjMetadata( testutil.Unstructured(t, resources["deployment"], JSONPathSetter{ - "$.metadata.name", "", + "$.kind", "", }), ), }, Error: testutil.EqualErrorString(validation.NewError( - field.Required(field.NewPath("metadata", "name"), "name is required"), + field.Required(field.NewPath("kind"), "kind is required"), object.UnstructuredToObjMetadata( testutil.Unstructured(t, resources["deployment"], JSONPathSetter{ - "$.metadata.name", "", + "$.kind", "", }), ), ).Error()), @@ -1302,7 +1303,7 @@ func TestApplier(t *testing.T) { Type: event.Started, }, }, - // Secret pending + // Wait events sorted Pending > Successful (see pkg/testutil) { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ @@ -1311,7 +1312,6 @@ func TestApplier(t *testing.T) { Identifier: testutil.ToIdentifier(t, resources["secret"]), }, }, - // Secret reconciled { EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ @@ -1802,6 +1802,7 @@ func TestApplierCancel(t *testing.T) { Type: event.Started, }, }, + // Wait events sorted Pending > Successful (see pkg/testutil) { // Deployment reconcile pending. EventType: event.WaitType, @@ -1920,6 +1921,9 @@ func TestApplierCancel(t *testing.T) { } } + // sort to allow comparison of multiple wait events + testutil.SortExpEvents(receivedEvents) + // Validate the rest of the events testutil.AssertEqual(t, tc.expectedEvents, receivedEvents, "Actual events (%d) do not match expected events (%d)", diff --git a/pkg/apply/destroyer_test.go b/pkg/apply/destroyer_test.go index b191c92a..ee439fba 100644 --- a/pkg/apply/destroyer_test.go +++ b/pkg/apply/destroyer_test.go @@ -259,6 +259,7 @@ func TestDestroyerCancel(t *testing.T) { Type: event.Started, }, }, + // Wait events sorted Pending > Successful (see pkg/testutil) { // Deployment reconcile pending. EventType: event.WaitType, @@ -374,6 +375,9 @@ func TestDestroyerCancel(t *testing.T) { } } + // sort to allow comparison of multiple wait events + testutil.SortExpEvents(receivedEvents) + // Validate the rest of the events testutil.AssertEqual(t, tc.expectedEvents, receivedEvents, "Actual events (%d) do not match expected events (%d)", diff --git a/pkg/testutil/events.go b/pkg/testutil/events.go index 6222cfb2..cc3e034a 100644 --- a/pkg/testutil/events.go +++ b/pkg/testutil/events.go @@ -447,45 +447,41 @@ func (ape GroupedEventsByID) Less(i, j int) bool { return i < j } switch ape[i].EventType { + case event.ValidationType: + // Validation events are predictable ordered by input object set order. case event.ApplyType: - if ape[i].ApplyEvent.GroupName == ape[j].ApplyEvent.GroupName && - ape[i].ApplyEvent.Status == ape[j].ApplyEvent.Status && - ape[i].ApplyEvent.Identifier.GroupKind == ape[j].ApplyEvent.Identifier.GroupKind { - // apply events are predictably ordered by GroupKind, due to - // ordering.SortableMetas. - // So we only need to sort by namespace & name. - return ape[i].ApplyEvent.Identifier.String() < ape[j].ApplyEvent.Identifier.String() - } + // Apply events are are predictably ordered by ordering.SortableMetas. case event.PruneType: - if ape[i].PruneEvent.GroupName == ape[j].PruneEvent.GroupName && - ape[i].PruneEvent.Status == ape[j].PruneEvent.Status && - ape[i].PruneEvent.Identifier.GroupKind == ape[j].PruneEvent.Identifier.GroupKind { - // prune events are predictably ordered by GroupKind, due to - // ordering.SortableMetas (reversed). - // So we only need to sort by namespace & name. - return ape[i].PruneEvent.Identifier.String() < ape[j].PruneEvent.Identifier.String() - } + // Prune events are predictably ordered in reverse apply order. case event.DeleteType: - if ape[i].DeleteEvent.GroupName == ape[j].DeleteEvent.GroupName && - ape[i].DeleteEvent.Status == ape[j].DeleteEvent.Status && - ape[i].DeleteEvent.Identifier.GroupKind == ape[j].DeleteEvent.Identifier.GroupKind { - // delete events are predictably ordered by GroupKind, due to - // ordering.SortableMetas (reversed). - // So we only need to sort by namespace & name. - return ape[i].DeleteEvent.Identifier.String() < ape[j].DeleteEvent.Identifier.String() - } + // Delete events are predictably ordered in reverse apply order. case event.WaitType: - if ape[i].WaitEvent.GroupName == ape[j].WaitEvent.GroupName && - ape[i].WaitEvent.Status == ape[j].WaitEvent.Status && - ape[i].WaitEvent.Status == event.ReconcileSuccessful { - // pending, skipped, and timeout operations are predictably ordered - // using the order in WaitTask.Ids. - // So we only need to sort Reconciled events, which occur in the - // order the Waitask receives StatusEvents with Current/NotFound. + // Wait events are unpredictably ordered, because the status may + // reconcile before or after the WaitTask starts, and status event + // order after starting is dependent on remote controller behavior. + // So here we sort status groups explicitly: + // Pending > Skipped > Successful > Failed > Timeout. + // Each status group is then sorted by Identifier: + // Group > Kind > Namespace > Name. + // Note that the Pending status is always optional. + if ape[i].WaitEvent.GroupName == ape[j].WaitEvent.GroupName { + if ape[i].WaitEvent.Status != ape[j].WaitEvent.Status { + return lessWaitStatus(ape[i].WaitEvent.Status, ape[j].WaitEvent.Status) + } return ape[i].WaitEvent.Identifier.String() < ape[j].WaitEvent.Identifier.String() } - case event.ValidationType: - return ape[i].ValidationEvent.Identifiers.Hash() < ape[j].ValidationEvent.Identifiers.Hash() } return i < j } + +var waitStatusWeight = map[event.WaitEventStatus]int{ + event.ReconcilePending: 0, + event.ReconcileSkipped: 1, + event.ReconcileSuccessful: 2, + event.ReconcileFailed: 3, + event.ReconcileTimeout: 4, +} + +func lessWaitStatus(x, y event.WaitEventStatus) bool { + return waitStatusWeight[x] < waitStatusWeight[y] +} diff --git a/test/e2e/continue_on_error_test.go b/test/e2e/continue_on_error_test.go index 8d4aba16..c4d4889b 100644 --- a/test/e2e/continue_on_error_test.go +++ b/test/e2e/continue_on_error_test.go @@ -136,25 +136,25 @@ func continueOnErrorTest(ctx context.Context, c client.Client, invConfig invconf }, }, { - // CRD reconcile Skipped. + // Pod1 reconcile Pending. EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-0", - Status: event.ReconcileSkipped, - Identifier: object.UnstructuredToObjMetadata(invalidCrdObj), + Status: event.ReconcilePending, + Identifier: object.UnstructuredToObjMetadata(pod1Obj), }, }, { - // Pod1 reconcile Pending. + // CRD reconcile Skipped. EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-0", - Status: event.ReconcilePending, - Identifier: object.UnstructuredToObjMetadata(pod1Obj), + Status: event.ReconcileSkipped, + Identifier: object.UnstructuredToObjMetadata(invalidCrdObj), }, }, { - // Pod1 confirmed Current. + // Pod1 reconcile Successful. EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-0", diff --git a/test/e2e/depends_on_test.go b/test/e2e/depends_on_test.go index 6a15d3e2..cb2040aa 100644 --- a/test/e2e/depends_on_test.go +++ b/test/e2e/depends_on_test.go @@ -382,7 +382,10 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv }, }, } - Expect(testutil.EventsToExpEvents(applierEvents)).To(testutil.Equal(expEvents)) + receivedEvents := testutil.EventsToExpEvents(applierEvents) + // sort to handle objects reconciling in random order + testutil.SortExpEvents(receivedEvents) + Expect(receivedEvents).To(testutil.Equal(expEvents)) By("verify namespace1 created") e2eutil.AssertUnstructuredExists(ctx, c, namespace1Obj) @@ -660,21 +663,21 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv }, }, { - // Namespace2 reconcile Pending. + // Namespace1 reconcile Pending. EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-3", Status: event.ReconcilePending, - Identifier: object.UnstructuredToObjMetadata(namespace2Obj), + Identifier: object.UnstructuredToObjMetadata(namespace1Obj), }, }, { - // Namespace1 reconcile Pending. + // Namespace2 reconcile Pending. EventType: event.WaitType, WaitEvent: &testutil.ExpWaitEvent{ GroupName: "wait-3", Status: event.ReconcilePending, - Identifier: object.UnstructuredToObjMetadata(namespace1Obj), + Identifier: object.UnstructuredToObjMetadata(namespace2Obj), }, }, { @@ -723,7 +726,10 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv }, }, } - Expect(testutil.EventsToExpEvents(destroyerEvents)).To(testutil.Equal(expEvents)) + receivedEvents = testutil.EventsToExpEvents(destroyerEvents) + // sort to handle objects reconciling in random order + testutil.SortExpEvents(receivedEvents) + Expect(receivedEvents).To(testutil.Equal(expEvents)) By("verify pod1 deleted") e2eutil.AssertUnstructuredDoesNotExist(ctx, c, pod1Obj)