fluxcd
/
cli-utils
mirror of https://github.com/fluxcd/cli-utils.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Expose cyclic dependency error

This commit is contained in:
Sean Sullivan 2021-08-23 09:42:00 -07:00
parent 7f8ec2b3a4
commit 769f5bd735
7 changed files with 144 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
pkg/apply/applier.go
View File

@ -173,12 +173,15 @@ func (a *Applier) Run(ctx context.Context, invInfo inventory.InventoryInfo, obje
},
}
// Build the task queue by appending tasks in the proper order.
taskQueue := taskBuilder.
taskQueue, err := taskBuilder.
AppendInvAddTask(invInfo, applyObjs, options.DryRunStrategy).
AppendApplyWaitTasks(invInfo, applyObjs, opts).
AppendPruneWaitTasks(pruneObjs, pruneFilters, opts).
AppendInvSetTask(invInfo, options.DryRunStrategy).
Build()
if err != nil {
handleError(eventChannel, err)
}
// Send event to inform the caller about the resources that
// will be applied/pruned.
eventChannel <- event.Event{

5
pkg/apply/destroyer.go
View File

@ -131,10 +131,13 @@ func (d *Destroyer) Run(inv inventory.InventoryInfo, options DestroyerOptions) <
},
}
// Build the ordered set of tasks to execute.
taskQueue := taskBuilder.
taskQueue, err := taskBuilder.
AppendPruneWaitTasks(deleteObjs, deleteFilters, opts).
AppendDeleteInvTask(inv, options.DryRunStrategy).
Build()
if err != nil {
handleError(eventChannel, err)
}
// Send event to inform the caller about the resources that
// will be pruned.
eventChannel <- event.Event{

20
pkg/apply/solver/solver.go
View File

@ -54,6 +54,7 @@ type TaskQueueBuilder struct {
waitCounter int
pruneCounter int
tasks []taskrunner.Task
err error
}
type TaskQueue struct {
@ -92,10 +93,15 @@ type Options struct {
}
// Build returns the queue of tasks that have been created.
func (t *TaskQueueBuilder) Build() *TaskQueue {
// TODO(seans): Now that we're reporting errors, we probably
// want to move away from the Builder patter for the TaskBuilder.
func (t *TaskQueueBuilder) Build() (*TaskQueue, error) {
if t.err != nil {
return nil, t.err
}
return &TaskQueue{
tasks: t.tasks,
}
}, nil
}
// AppendInvAddTask appends an inventory add task to the task queue.
@ -211,7 +217,10 @@ func (t *TaskQueueBuilder) AppendApplyWaitTasks(inv inventory.InventoryInfo,
applyObjs []*unstructured.Unstructured, o Options) *TaskQueueBuilder {
// Use the "depends-on" annotation to create a graph, ands sort the
// objects to apply into sets using a topological sort.
applySets := graph.SortObjs(applyObjs)
applySets, err := graph.SortObjs(applyObjs)
if err != nil {
t.err = err
}
addWaitTask, waitTimeout := waitTaskTimeout(o.DryRunStrategy.ClientOrServerDryRun(),
len(applySets), o.ReconcileTimeout)
for _, applySet := range applySets {
@ -232,7 +241,10 @@ func (t *TaskQueueBuilder) AppendPruneWaitTasks(pruneObjs []*unstructured.Unstru
if o.Prune {
// Use the "depends-on" annotation to create a graph, ands sort the
// objects to prune into sets using a (reverse) topological sort.
pruneSets := graph.ReverseSortObjs(pruneObjs)
pruneSets, err := graph.ReverseSortObjs(pruneObjs)
if err != nil {
t.err = err
}
addWaitTask, waitTimeout := waitTaskTimeout(o.DryRunStrategy.ClientOrServerDryRun(),
len(pruneSets), o.ReconcileTimeout)
for _, pruneSet := range pruneSets {

56
pkg/apply/solver/solver_test.go
View File

@ -121,10 +121,12 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
applyObjs []*unstructured.Unstructured
options Options
expectedTasks []taskrunner.Task
isError bool
}{
"no resources, no tasks": {
applyObjs: []*unstructured.Unstructured{},
expectedTasks: []taskrunner.Task{},
isError: false,
},
"single resource, one apply task": {
applyObjs: []*unstructured.Unstructured{
@ -138,6 +140,7 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
},
},
},
isError: false,
},
"multiple resources with reconcile timeout": {
applyObjs: []*unstructured.Unstructured{
@ -164,6 +167,7 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
taskrunner.AllCurrent, 1*time.Second,
testutil.NewFakeRESTMapper()),
},
isError: false,
},
"multiple resources with reconcile timeout and dryrun": {
applyObjs: []*unstructured.Unstructured{
@ -184,6 +188,7 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
},
},
},
isError: false,
},
"multiple resources with reconcile timeout and server-dryrun": {
applyObjs: []*unstructured.Unstructured{
@ -204,6 +209,7 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
},
},
},
isError: false,
},
"multiple resources including CRD": {
applyObjs: []*unstructured.Unstructured{
@ -241,6 +247,7 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
taskrunner.AllCurrent, 1*time.Second,
testutil.NewFakeRESTMapper()),
},
isError: false,
},
"no wait with CRDs if it is a dryrun": {
applyObjs: []*unstructured.Unstructured{
@ -267,6 +274,7 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
},
},
},
isError: false,
},
"resources in namespace creates multiple apply tasks": {
applyObjs: []*unstructured.Unstructured{
@ -304,6 +312,7 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
taskrunner.AllCurrent, 1*time.Second,
testutil.NewFakeRESTMapper()),
},
isError: false,
},
"deployment depends on secret creates multiple tasks": {
applyObjs: []*unstructured.Unstructured{
@ -339,6 +348,17 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
taskrunner.AllCurrent, 1*time.Second,
testutil.NewFakeRESTMapper()),
},
isError: false,
},
"cyclic dependency returns error": {
applyObjs: []*unstructured.Unstructured{
testutil.Unstructured(t, resources["deployment"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["secret"]))),
testutil.Unstructured(t, resources["secret"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["deployment"]))),
},
expectedTasks: []taskrunner.Task{},
isError: true,
},
}
@ -351,8 +371,12 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
Mapper: testutil.NewFakeRESTMapper(),
InvClient: fakeInvClient,
}
tq := tqb.AppendApplyWaitTasks(localInv, tc.applyObjs, tc.options).Build()
tq, err := tqb.AppendApplyWaitTasks(localInv, tc.applyObjs, tc.options).Build()
if tc.isError {
assert.NotNil(t, err, "expected error, but received none")
return
}
assert.Nil(t, err, "unexpected error received")
assert.Equal(t, len(tc.expectedTasks), len(tq.tasks))
for i, expTask := range tc.expectedTasks {
actualTask := tq.tasks[i]
@ -385,11 +409,13 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
pruneObjs []*unstructured.Unstructured
options Options
expectedTasks []taskrunner.Task
isError bool
}{
"no resources, no tasks": {
pruneObjs: []*unstructured.Unstructured{},
options: Options{Prune: true},
expectedTasks: []taskrunner.Task{},
isError: false,
},
"single resource, one prune task": {
pruneObjs: []*unstructured.Unstructured{
@ -404,6 +430,7 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
},
},
},
isError: false,
},
"multiple resources, one prune task": {
pruneObjs: []*unstructured.Unstructured{
@ -420,6 +447,7 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
},
},
},
isError: false,
},
"dependent resources, two prune tasks, two wait tasks": {
pruneObjs: []*unstructured.Unstructured{
@ -457,6 +485,7 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
taskrunner.AllCurrent, 1*time.Second,
testutil.NewFakeRESTMapper()),
},
isError: false,
},
"multiple resources with prune timeout and server-dryrun": {
pruneObjs: []*unstructured.Unstructured{
@ -478,6 +507,7 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
},
},
},
isError: false,
},
"multiple resources including CRD": {
pruneObjs: []*unstructured.Unstructured{
@ -517,6 +547,7 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
taskrunner.AllCurrent, 1*time.Second,
testutil.NewFakeRESTMapper()),
},
isError: false,
},
"no wait with CRDs if it is a dryrun": {
pruneObjs: []*unstructured.Unstructured{
@ -544,6 +575,7 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
},
},
},
isError: false,
},
"resources in namespace creates multiple apply tasks": {
pruneObjs: []*unstructured.Unstructured{
@ -582,6 +614,18 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
taskrunner.AllCurrent, 1*time.Second,
testutil.NewFakeRESTMapper()),
},
isError: false,
},
"cyclic dependency returns error": {
pruneObjs: []*unstructured.Unstructured{
testutil.Unstructured(t, resources["deployment"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["secret"]))),
testutil.Unstructured(t, resources["secret"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["deployment"]))),
},
options: Options{Prune: true},
expectedTasks: []taskrunner.Task{},
isError: true,
},
}
@ -595,8 +639,12 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
InvClient: fakeInvClient,
}
emptyPruneFilters := []filter.ValidationFilter{}
tq := tqb.AppendPruneWaitTasks(tc.pruneObjs, emptyPruneFilters, tc.options).Build()
tq, err := tqb.AppendPruneWaitTasks(tc.pruneObjs, emptyPruneFilters, tc.options).Build()
if tc.isError {
assert.NotNil(t, err, "expected error, but received none")
return
}
assert.Nil(t, err, "unexpected error received")
assert.Equal(t, len(tc.expectedTasks), len(tq.tasks))
for i, expTask := range tc.expectedTasks {
actualTask := tq.tasks[i]

17
pkg/object/graph/depends.go
View File

@ -16,9 +16,9 @@ import (
// Each of the objects in an apply set is applied together. The order of
// the returned applied sets is a topological ordering of the sets to apply.
// Returns an single empty apply set if there are no objects to apply.
func SortObjs(objs []*unstructured.Unstructured) [][]*unstructured.Unstructured {
func SortObjs(objs []*unstructured.Unstructured) ([][]*unstructured.Unstructured, error) {
if len(objs) == 0 {
return [][]*unstructured.Unstructured{}
return [][]*unstructured.Unstructured{}, nil
}
// Create the graph, and build a map of object metadata to the object (Unstructured).
g := New()
@ -35,7 +35,7 @@ func SortObjs(objs []*unstructured.Unstructured) [][]*unstructured.Unstructured
objSets := [][]*unstructured.Unstructured{}
sortedObjSets, err := g.Sort()
if err != nil {
return objSets
return [][]*unstructured.Unstructured{}, err
}
// Map the object metadata back to the sorted sets of unstructured objects.
for _, objSet := range sortedObjSets {
@ -49,18 +49,21 @@ func SortObjs(objs []*unstructured.Unstructured) [][]*unstructured.Unstructured
}
objSets = append(objSets, currentSet)
}
return objSets
return objSets, nil
}
// ReverseSortObjs is the same as SortObjs but using reverse ordering.
func ReverseSortObjs(objs []*unstructured.Unstructured) [][]*unstructured.Unstructured {
func ReverseSortObjs(objs []*unstructured.Unstructured) ([][]*unstructured.Unstructured, error) {
// Sorted objects using normal ordering.
s := SortObjs(objs)
s, err := SortObjs(objs)
if err != nil {
return [][]*unstructured.Unstructured{}, err
}
// Reverse the ordering of the object sets using swaps.
for i, j := 0, len(s)-1; i < j; i, j = i+1, j-1 {
s[i], s[j] = s[j], s[i]
}
return s
return s, nil
}
// addExplicitEdges updates the graph with edges from objects

37
pkg/object/graph/depends_test.go
View File

@ -6,6 +6,7 @@ package graph
import (
"testing"
"github.com/stretchr/testify/assert"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/cli-utils/pkg/object"
"sigs.k8s.io/cli-utils/pkg/testutil"
@ -235,11 +236,38 @@ func TestSortObjs(t *testing.T) {
},
isError: false,
},
"two objects depends on each other is cyclic dependency": {
objs: []*unstructured.Unstructured{
testutil.Unstructured(t, resources["deployment"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["secret"]))),
testutil.Unstructured(t, resources["secret"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["deployment"]))),
},
expected: [][]*unstructured.Unstructured{},
isError: true,
},
"three objects in cyclic dependency": {
objs: []*unstructured.Unstructured{
testutil.Unstructured(t, resources["deployment"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["secret"]))),
testutil.Unstructured(t, resources["secret"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["pod"]))),
testutil.Unstructured(t, resources["pod"],
testutil.AddDependsOn(t, testutil.Unstructured(t, resources["deployment"]))),
},
expected: [][]*unstructured.Unstructured{},
isError: true,
},
}
for tn, tc := range testCases {
t.Run(tn, func(t *testing.T) {
actual := SortObjs(tc.objs)
actual, err := SortObjs(tc.objs)
if tc.isError {
assert.NotNil(t, err, "expected error, but received none")
return
}
assert.Nil(t, err, "unexpected error received")
verifyObjSets(t, tc.expected, actual)
})
}
@ -328,7 +356,12 @@ func TestReverseSortObjs(t *testing.T) {
for tn, tc := range testCases {
t.Run(tn, func(t *testing.T) {
actual := ReverseSortObjs(tc.objs)
actual, err := ReverseSortObjs(tc.objs)
if tc.isError {
assert.NotNil(t, err, "expected error, but received none")
return
}
assert.Nil(t, err, "unexpected error received")
verifyObjSets(t, tc.expected, actual)
})
}

25
pkg/object/graph/graph.go
View File

@ -7,6 +7,7 @@
package graph
import (
"bytes"
"fmt"
"sigs.k8s.io/cli-utils/pkg/object"
@ -129,9 +130,12 @@ func (g *Graph) Sort() ([][]object.ObjMetadata, error) {
leafVertices = append(leafVertices, v)
}
}
// No leaf vertices means cycle in the directed graph.
// No leaf vertices means cycle in the directed graph,
// where remaining edges define the cycle.
if len(leafVertices) == 0 {
return sorted, fmt.Errorf("cycle in directed graph")
return [][]object.ObjMetadata{}, CyclicDependencyError{
Edges: g.GetEdges(),
}
}
// Remove all edges to leaf vertices.
for _, v := range leafVertices {
@ -141,3 +145,20 @@ func (g *Graph) Sort() ([][]object.ObjMetadata, error) {
}
return sorted, nil
}
// CyclicDependencyError when directed acyclic graph contains a cycle.
// The cycle makes it impossible to topological sort.
type CyclicDependencyError struct {
Edges []Edge
}
func (cde CyclicDependencyError) Error() string {
var errorBuf bytes.Buffer
errorBuf.WriteString("cyclic dependency")
for _, edge := range cde.Edges {
from := fmt.Sprintf("%s/%s", edge.From.Namespace, edge.From.Name)
to := fmt.Sprintf("%s/%s", edge.To.Namespace, edge.To.Name)
errorBuf.WriteString(fmt.Sprintf("\n\t%s -> %s", from, to))
}
return errorBuf.String()
}