From 6abffa8abe1234a8b09c11a0661f0638cd8df8dc Mon Sep 17 00:00:00 2001 From: Karl Isenberg Date: Wed, 19 Jan 2022 15:24:39 -0800 Subject: [PATCH] fix: Make CyclicDependencyError edges stable - Add sorting for Edges & Verticies - Improve tests that contain CyclicDependencyError --- pkg/apply/solver/solver_test.go | 127 +++++------- pkg/object/graph/graph.go | 36 +++- pkg/object/graph/graph_test.go | 330 +++++++++++++++++++++++++++++--- 3 files changed, 386 insertions(+), 107 deletions(-) diff --git a/pkg/apply/solver/solver_test.go b/pkg/apply/solver/solver_test.go index 925528f6..2eae9bd6 100644 --- a/pkg/apply/solver/solver_test.go +++ b/pkg/apply/solver/solver_test.go @@ -18,6 +18,8 @@ import ( "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/inventory" "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/object/graph" + "sigs.k8s.io/cli-utils/pkg/object/validation" "sigs.k8s.io/cli-utils/pkg/testutil" ) @@ -107,12 +109,11 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { applyObjs []*unstructured.Unstructured options Options expectedTasks []taskrunner.Task - isError bool + expectedError error }{ "no resources, no tasks": { applyObjs: []*unstructured.Unstructured{}, expectedTasks: []taskrunner.Task{}, - isError: false, }, "single resource, one apply task, one wait task": { applyObjs: []*unstructured.Unstructured{ @@ -134,7 +135,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "multiple resource with no timeout": { applyObjs: []*unstructured.Unstructured{ @@ -159,7 +159,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "multiple resources with reconcile timeout": { applyObjs: []*unstructured.Unstructured{ @@ -187,7 +186,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "multiple resources with reconcile timeout and dryrun": { applyObjs: []*unstructured.Unstructured{ @@ -208,7 +206,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { }, }, }, - isError: false, }, "multiple resources with reconcile timeout and server-dryrun": { applyObjs: []*unstructured.Unstructured{ @@ -229,7 +226,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { }, }, }, - isError: false, }, "multiple resources including CRD": { applyObjs: []*unstructured.Unstructured{ @@ -269,7 +265,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "no wait with CRDs if it is a dryrun": { applyObjs: []*unstructured.Unstructured{ @@ -296,7 +291,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { }, }, }, - isError: false, }, "resources in namespace creates multiple apply tasks": { applyObjs: []*unstructured.Unstructured{ @@ -336,7 +330,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "deployment depends on secret creates multiple tasks": { applyObjs: []*unstructured.Unstructured{ @@ -362,7 +355,8 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { &task.ApplyTask{ TaskName: "apply-1", Objects: []*unstructured.Unstructured{ - testutil.Unstructured(t, resources["deployment"]), + testutil.Unstructured(t, resources["deployment"], + testutil.AddDependsOn(t, testutil.ToIdentifier(t, resources["secret"]))), }, }, taskrunner.NewWaitTask( @@ -374,7 +368,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "cyclic dependency returns error": { applyObjs: []*unstructured.Unstructured{ @@ -384,7 +377,22 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { testutil.AddDependsOn(t, testutil.ToIdentifier(t, resources["deployment"]))), }, expectedTasks: []taskrunner.Task{}, - isError: true, + expectedError: validation.NewError( + graph.CyclicDependencyError{ + Edges: []graph.Edge{ + { + From: testutil.ToIdentifier(t, resources["secret"]), + To: testutil.ToIdentifier(t, resources["deployment"]), + }, + { + From: testutil.ToIdentifier(t, resources["deployment"]), + To: testutil.ToIdentifier(t, resources["secret"]), + }, + }, + }, + testutil.ToIdentifier(t, resources["secret"]), + testutil.ToIdentifier(t, resources["deployment"]), + ), }, } @@ -403,11 +411,11 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { []mutator.Interface{}, tc.options, ).Build() - if tc.isError { - assert.NotNil(t, err, "expected error, but received none") + if tc.expectedError != nil { + assert.EqualError(t, err, tc.expectedError.Error()) return } - assert.Nil(t, err, "unexpected error received") + assert.NoError(t, err) assert.Equal(t, len(tc.expectedTasks), len(tq.tasks)) for i, expTask := range tc.expectedTasks { actualTask := tq.tasks[i] @@ -417,18 +425,11 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) { switch expTsk := expTask.(type) { case *task.ApplyTask: actApplyTask := toApplyTask(t, actualTask) - assert.Equal(t, len(expTsk.Objects), len(actApplyTask.Objects)) - // Order is NOT important for objects stored within task. - verifyObjSets(t, expTsk.Objects, actApplyTask.Objects) + testutil.AssertEqual(t, expTsk.Objects, actApplyTask.Objects, "ApplyTask mismatch") case *taskrunner.WaitTask: actWaitTask := toWaitTask(t, actualTask) - assert.Equal(t, len(expTsk.Ids), len(actWaitTask.Ids)) - // Order is NOT important for ids stored within task. - if !expTsk.Ids.Equal(actWaitTask.Ids) { - t.Errorf("expected wait ids (%v), got (%v)", - expTsk.Ids, actWaitTask.Ids) - } - assert.Equal(t, taskrunner.AllCurrent, actWaitTask.Condition) + testutil.AssertEqual(t, expTsk.Ids, actWaitTask.Ids) + assert.Equal(t, taskrunner.AllCurrent, actWaitTask.Condition, "WaitTask mismatch") } } }) @@ -440,13 +441,12 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { pruneObjs []*unstructured.Unstructured options Options expectedTasks []taskrunner.Task - isError bool + expectedError error }{ "no resources, no tasks": { pruneObjs: []*unstructured.Unstructured{}, options: Options{Prune: true}, expectedTasks: []taskrunner.Task{}, - isError: false, }, "single resource, one prune task, one wait task": { pruneObjs: []*unstructured.Unstructured{ @@ -469,7 +469,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "multiple resources, one prune task, one wait task": { pruneObjs: []*unstructured.Unstructured{ @@ -495,7 +494,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "dependent resources, two prune tasks, two wait tasks": { pruneObjs: []*unstructured.Unstructured{ @@ -509,7 +507,8 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ - testutil.Unstructured(t, resources["pod"]), + testutil.Unstructured(t, resources["pod"], + testutil.AddDependsOn(t, testutil.ToIdentifier(t, resources["secret"]))), }, }, taskrunner.NewWaitTask( @@ -535,7 +534,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "single resource with prune timeout has wait task": { pruneObjs: []*unstructured.Unstructured{ @@ -562,7 +560,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "multiple resources with prune timeout and server-dryrun": { pruneObjs: []*unstructured.Unstructured{ @@ -584,7 +581,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { }, }, }, - isError: false, }, "multiple resources including CRD": { pruneObjs: []*unstructured.Unstructured{ @@ -626,7 +622,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "no wait with CRDs if it is a dryrun": { pruneObjs: []*unstructured.Unstructured{ @@ -654,7 +649,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { }, }, }, - isError: false, }, "resources in namespace creates multiple apply tasks": { pruneObjs: []*unstructured.Unstructured{ @@ -695,7 +689,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { testutil.NewFakeRESTMapper(), ), }, - isError: false, }, "cyclic dependency returns error": { pruneObjs: []*unstructured.Unstructured{ @@ -706,7 +699,22 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { }, options: Options{Prune: true}, expectedTasks: []taskrunner.Task{}, - isError: true, + expectedError: validation.NewError( + graph.CyclicDependencyError{ + Edges: []graph.Edge{ + { + From: testutil.ToIdentifier(t, resources["secret"]), + To: testutil.ToIdentifier(t, resources["deployment"]), + }, + { + From: testutil.ToIdentifier(t, resources["deployment"]), + To: testutil.ToIdentifier(t, resources["secret"]), + }, + }, + }, + testutil.ToIdentifier(t, resources["secret"]), + testutil.ToIdentifier(t, resources["deployment"]), + ), }, } @@ -721,11 +729,11 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { } emptyPruneFilters := []filter.ValidationFilter{} tq, err := tqb.AppendPruneWaitTasks(tc.pruneObjs, emptyPruneFilters, tc.options).Build() - if tc.isError { - assert.NotNil(t, err, "expected error, but received none") + if tc.expectedError != nil { + assert.EqualError(t, err, tc.expectedError.Error()) return } - assert.Nil(t, err, "unexpected error received") + assert.NoError(t, err) assert.Equal(t, len(tc.expectedTasks), len(tq.tasks)) for i, expTask := range tc.expectedTasks { actualTask := tq.tasks[i] @@ -735,16 +743,11 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { switch expTsk := expTask.(type) { case *task.PruneTask: actPruneTask := toPruneTask(t, actualTask) - assert.Equal(t, len(expTsk.Objects), len(actPruneTask.Objects)) - verifyObjSets(t, expTsk.Objects, actPruneTask.Objects) + testutil.AssertEqual(t, expTsk.Objects, actPruneTask.Objects, "PruneTask mismatch") case *taskrunner.WaitTask: actWaitTask := toWaitTask(t, actualTask) - assert.Equal(t, len(expTsk.Ids), len(actWaitTask.Ids)) - if !expTsk.Ids.Equal(actWaitTask.Ids) { - t.Errorf("expected wait ids (%v), got (%v)", - expTsk.Ids, actWaitTask.Ids) - } - assert.Equal(t, taskrunner.AllNotFound, actWaitTask.Condition) + testutil.AssertEqual(t, expTsk.Ids, actWaitTask.Ids) + assert.Equal(t, taskrunner.AllNotFound, actWaitTask.Condition, "WaitTask mismatch") // Validate the prune wait timeout. assert.Equal(t, tc.options.PruneTimeout, actualTask.(*taskrunner.WaitTask).Timeout) } @@ -753,32 +756,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) { } } -// verifyObjSets ensures the slice of expected objects is the same as -// the actual slice of objects. Order is NOT important. -func verifyObjSets(t *testing.T, expected []*unstructured.Unstructured, actual []*unstructured.Unstructured) { - if len(expected) != len(actual) { - t.Fatalf("expected set size (%d), got (%d)", len(expected), len(actual)) - } - for _, obj := range expected { - if !containsObj(actual, obj) { - t.Fatalf("expected object (%v) not in found", obj) - } - } -} - -// containsObj returns true if the passed object is within the passed -// slice of objects; false otherwise. -func containsObj(objs []*unstructured.Unstructured, obj *unstructured.Unstructured) bool { - ids := object.UnstructuredSetToObjMetadataSet(objs) - id := object.UnstructuredToObjMetadata(obj) - for _, i := range ids { - if i == id { - return true - } - } - return false -} - func toWaitTask(t *testing.T, task taskrunner.Task) *taskrunner.WaitTask { switch tsk := task.(type) { case *taskrunner.WaitTask: diff --git a/pkg/object/graph/graph.go b/pkg/object/graph/graph.go index d7deae82..8c9decf7 100644 --- a/pkg/object/graph/graph.go +++ b/pkg/object/graph/graph.go @@ -9,10 +9,12 @@ package graph import ( "bytes" "fmt" + "sort" "sigs.k8s.io/cli-utils/pkg/multierror" "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/cli-utils/pkg/object/validation" + "sigs.k8s.io/cli-utils/pkg/ordering" ) // Graph is contains a directed set of edges, implemented as @@ -45,7 +47,7 @@ func (g *Graph) AddVertex(v object.ObjMetadata) { } } -// GetVertices returns an unsorted set of unique vertices in the graph. +// GetVertices returns a sorted set of unique vertices in the graph. func (g *Graph) GetVertices() object.ObjMetadataSet { keys := make(object.ObjMetadataSet, len(g.edges)) i := 0 @@ -53,6 +55,7 @@ func (g *Graph) GetVertices() object.ObjMetadataSet { keys[i] = k i++ } + sort.Sort(ordering.SortableMetas(keys)) return keys } @@ -74,8 +77,7 @@ func (g *Graph) AddEdge(from object.ObjMetadata, to object.ObjMetadata) { } } -// GetEdges returns the slice of vertex pairs which are -// the directed edges of the graph. +// GetEdges returns a sorted slice of directed graph edges (vertex pairs). func (g *Graph) GetEdges() []Edge { edges := []Edge{} for from, toList := range g.edges { @@ -84,6 +86,7 @@ func (g *Graph) GetEdges() []Edge { edges = append(edges, edge) } } + sort.Sort(SortableEdges(edges)) return edges } @@ -164,3 +167,30 @@ func (cde CyclicDependencyError) Error() string { } return errorBuf.String() } + +// SortableEdges sorts a list of edges alphanumerically by From and then To. +type SortableEdges []Edge + +var _ sort.Interface = SortableEdges{} + +func (a SortableEdges) Len() int { return len(a) } +func (a SortableEdges) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a SortableEdges) Less(i, j int) bool { + if a[i].From != a[j].From { + return metaIsLessThan(a[i].From, a[j].From) + } + return metaIsLessThan(a[i].To, a[j].To) +} + +func metaIsLessThan(i, j object.ObjMetadata) bool { + if i.GroupKind.Group != j.GroupKind.Group { + return i.GroupKind.Group < j.GroupKind.Group + } + if i.GroupKind.Kind != j.GroupKind.Kind { + return i.GroupKind.Kind < j.GroupKind.Kind + } + if i.Namespace != j.Namespace { + return i.Namespace < j.Namespace + } + return i.Name < j.Name +} diff --git a/pkg/object/graph/graph_test.go b/pkg/object/graph/graph_test.go index c9e10ddb..94b5fc8f 100644 --- a/pkg/object/graph/graph_test.go +++ b/pkg/object/graph/graph_test.go @@ -6,10 +6,14 @@ package graph import ( + "sort" "testing" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/cli-utils/pkg/object" + "sigs.k8s.io/cli-utils/pkg/object/validation" + "sigs.k8s.io/cli-utils/pkg/testutil" ) var ( @@ -33,64 +37,89 @@ var ( func TestObjectGraphSort(t *testing.T) { testCases := map[string]struct { - vertices object.ObjMetadataSet - edges []Edge - expected []object.ObjMetadataSet - isError bool + vertices object.ObjMetadataSet + edges []Edge + expected []object.ObjMetadataSet + expectedError error }{ "one edge": { vertices: object.ObjMetadataSet{o1, o2}, edges: []Edge{e1}, expected: []object.ObjMetadataSet{{o2}, {o1}}, - isError: false, }, "two edges": { vertices: object.ObjMetadataSet{o1, o2, o3}, edges: []Edge{e1, e2}, expected: []object.ObjMetadataSet{{o3}, {o2}, {o1}}, - isError: false, }, "three edges": { vertices: object.ObjMetadataSet{o1, o2, o3}, edges: []Edge{e1, e3, e2}, expected: []object.ObjMetadataSet{{o3}, {o2}, {o1}}, - isError: false, }, "four edges": { vertices: object.ObjMetadataSet{o1, o2, o3, o4}, edges: []Edge{e1, e2, e4, e5}, expected: []object.ObjMetadataSet{{o4}, {o3}, {o2}, {o1}}, - isError: false, }, "five edges": { vertices: object.ObjMetadataSet{o1, o2, o3, o4}, edges: []Edge{e5, e1, e3, e2, e4}, expected: []object.ObjMetadataSet{{o4}, {o3}, {o2}, {o1}}, - isError: false, }, "no edges means all in the same first set": { vertices: object.ObjMetadataSet{o1, o2, o3, o4}, edges: []Edge{}, expected: []object.ObjMetadataSet{{o4, o3, o2, o1}}, - isError: false, }, "multiple objects in first set": { vertices: object.ObjMetadataSet{o1, o2, o3, o4, o5}, edges: []Edge{e1, e2, e5, e8}, expected: []object.ObjMetadataSet{{o5, o3}, {o4}, {o2}, {o1}}, - isError: false, }, "simple cycle in graph is an error": { vertices: object.ObjMetadataSet{o1, o2}, edges: []Edge{e1, e6}, expected: []object.ObjMetadataSet{}, - isError: true, + expectedError: validation.NewError( + CyclicDependencyError{ + Edges: []Edge{ + { + From: o1, + To: o2, + }, + { + From: o2, + To: o1, + }, + }, + }, + o1, o2, + ), }, "multi-edge cycle in graph is an error": { vertices: object.ObjMetadataSet{o1, o2, o3}, edges: []Edge{e1, e2, e7}, expected: []object.ObjMetadataSet{}, - isError: true, + expectedError: validation.NewError( + CyclicDependencyError{ + Edges: []Edge{ + { + From: o1, + To: o2, + }, + { + From: o2, + To: o3, + }, + { + From: o3, + To: o1, + }, + }, + }, + o1, o2, o3, + ), }, } @@ -104,23 +133,266 @@ func TestObjectGraphSort(t *testing.T) { g.AddEdge(edge.From, edge.To) } actual, err := g.Sort() - if err == nil && tc.isError { - t.Fatalf("expected error, but received none") - } - if err != nil && !tc.isError { - t.Errorf("unexpected error: %s", err) - } - if !tc.isError { - if len(actual) != len(tc.expected) { - t.Errorf("expected (%s), got (%s)", tc.expected, actual) - } - for i, actualSet := range actual { - expectedSet := tc.expected[i] - if !expectedSet.Equal(actualSet) { - t.Errorf("expected sorted objects (%s), got (%s)", tc.expected, actual) - } - } + if tc.expectedError != nil { + assert.EqualError(t, tc.expectedError, err.Error()) + return } + assert.NoError(t, err) + testutil.AssertEqual(t, tc.expected, actual) + }) + } +} + +func TestEdgeSort(t *testing.T) { + testCases := map[string]struct { + edges []Edge + expected []Edge + }{ + "one edge": { + edges: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + }, + expected: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + }, + }, + "two edges no change": { + edges: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + { + From: object.ObjMetadata{Name: "obj2"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + }, + expected: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + { + From: object.ObjMetadata{Name: "obj2"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + }, + }, + "two edges same from": { + edges: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + }, + expected: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + }, + }, + "two edges": { + edges: []Edge{ + { + From: object.ObjMetadata{Name: "obj2"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + }, + expected: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + { + From: object.ObjMetadata{Name: "obj2"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + }, + }, + "two edges by name": { + edges: []Edge{ + { + From: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + To: object.ObjMetadata{Name: "obj3", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + }, + { + From: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + To: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + }, + }, + expected: []Edge{ + { + From: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + To: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + }, + { + From: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + To: object.ObjMetadata{Name: "obj3", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + }, + }, + }, + "three edges": { + edges: []Edge{ + { + From: object.ObjMetadata{Name: "obj3"}, + To: object.ObjMetadata{Name: "obj4"}, + }, + { + From: object.ObjMetadata{Name: "obj2"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + }, + expected: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + { + From: object.ObjMetadata{Name: "obj2"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + { + From: object.ObjMetadata{Name: "obj3"}, + To: object.ObjMetadata{Name: "obj4"}, + }, + }, + }, + "two edges cycle": { + edges: []Edge{ + { + From: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + To: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + }, + { + From: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + To: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + }, + }, + expected: []Edge{ + { + From: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + To: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + }, + { + From: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + To: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}}, + }, + }, + }, + "three edges cycle": { + edges: []Edge{ + { + From: object.ObjMetadata{Name: "obj3"}, + To: object.ObjMetadata{Name: "obj1"}, + }, + { + From: object.ObjMetadata{Name: "obj2"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + }, + expected: []Edge{ + { + From: object.ObjMetadata{Name: "obj1"}, + To: object.ObjMetadata{Name: "obj2"}, + }, + { + From: object.ObjMetadata{Name: "obj2"}, + To: object.ObjMetadata{Name: "obj3"}, + }, + { + From: object.ObjMetadata{Name: "obj3"}, + To: object.ObjMetadata{Name: "obj1"}, + }, + }, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + sort.Sort(SortableEdges(tc.edges)) + assert.Equal(t, tc.expected, tc.edges) + }) + } +} + +func TestCyclicDependencyErrorString(t *testing.T) { + testCases := map[string]struct { + cycle CyclicDependencyError + expectedString string + }{ + "two object cycle": { + cycle: CyclicDependencyError{ + Edges: []Edge{ + { + From: o1, + To: o2, + }, + { + From: o2, + To: o1, + }, + }, + }, + expectedString: `cyclic dependency: +- /obj1 -> /obj2 +- /obj2 -> /obj1 +`, + }, + "three object cycle": { + cycle: CyclicDependencyError{ + Edges: []Edge{ + { + From: o1, + To: o2, + }, + { + From: o2, + To: o3, + }, + { + From: o3, + To: o1, + }, + }, + }, + expectedString: `cyclic dependency: +- /obj1 -> /obj2 +- /obj2 -> /obj3 +- /obj3 -> /obj1 +`, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + assert.Equal(t, tc.expectedString, tc.cycle.Error()) }) } }