diff --git a/commands/alpha/live/plan/command.go b/commands/alpha/live/plan/command.go index 17eef9b9a9..91dda2ab9e 100644 --- a/commands/alpha/live/plan/command.go +++ b/commands/alpha/live/plan/command.go @@ -178,8 +178,10 @@ func printText(plan *kptplanner.Plan, objs []*unstructured.Unstructured, ioStrea printEntry(" ", action, ioStreams) findAndPrintDiff(action.Original, action.Updated, ContentPrefix, ioStreams) case kptplanner.Skip: - // TODO: provide more information about why the resource was skipped. printEntryWithColor("=", print.YELLOW, action, ioStreams) + if action.SkipReason != "" { + printWithPrefix(action.SkipReason, ContentPrefix, ioStreams) + } case kptplanner.Error: printEntry("!", action, ioStreams) printWithPrefix(action.Error, ContentPrefix, ioStreams) diff --git a/commands/alpha/live/plan/command_test.go b/commands/alpha/live/plan/command_test.go new file mode 100644 index 0000000000..354326383b --- /dev/null +++ b/commands/alpha/live/plan/command_test.go @@ -0,0 +1,64 @@ +package plan + +import ( + "bytes" + "testing" + + kptplanner "github.com/kptdev/kpt/pkg/live/planner" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/cli-runtime/pkg/genericclioptions" +) + +func TestPrintTextSkip(t *testing.T) { + testCases := map[string]struct { + plan *kptplanner.Plan + expected string + }{ + "skipped resource with reason": { + plan: &kptplanner.Plan{ + Actions: []kptplanner.Action{ + { + Type: kptplanner.Skip, + Group: "apps", + Kind: "Deployment", + Name: "foo", + Namespace: "default", + SkipReason: "some skip reason", + }, + }, + }, + expected: "kpt will perform the following actions:\n\x1b[33m\t= apps/Deployment default/foo\n\x1b[0m\t\tsome skip reason\n\n", + }, + "skipped resource without reason": { + plan: &kptplanner.Plan{ + Actions: []kptplanner.Action{ + { + Type: kptplanner.Skip, + Group: "apps", + Kind: "Deployment", + Name: "foo", + Namespace: "default", + }, + }, + }, + expected: "kpt will perform the following actions:\n\x1b[33m\t= apps/Deployment default/foo\n\x1b[0m\n", + }, + } + + for tn := range testCases { + tc := testCases[tn] + t.Run(tn, func(t *testing.T) { + var buf bytes.Buffer + ioStreams := genericclioptions.IOStreams{ + Out: &buf, + ErrOut: &buf, + } + var objs []*unstructured.Unstructured + err := printText(tc.plan, objs, ioStreams) + assert.NoError(t, err) + + assert.Equal(t, tc.expected, buf.String()) + }) + } +} diff --git a/pkg/live/planner/cluster.go b/pkg/live/planner/cluster.go index e537c2b6aa..673d9b77aa 100644 --- a/pkg/live/planner/cluster.go +++ b/pkg/live/planner/cluster.go @@ -110,14 +110,15 @@ type Plan struct { } type Action struct { - Type ActionType - Group string - Kind string - Name string - Namespace string - Original *unstructured.Unstructured - Updated *unstructured.Unstructured - Error string + Type ActionType + Group string + Kind string + Name string + Namespace string + Original *unstructured.Unstructured + Updated *unstructured.Unstructured + Error string + SkipReason string } type Options struct { @@ -195,63 +196,65 @@ func (r *ClusterPlanner) dryRunForPlan( } func handleApplyEvent(e event.Event, a Action) Action { + if e.ApplyEvent.Status == event.ApplySkipped { + a.Type = Skip + if e.ApplyEvent.Error != nil { + a.SkipReason = e.ApplyEvent.Error.Error() + } + return a + } if e.ApplyEvent.Error != nil { a.Type = Error a.Error = e.ApplyEvent.Error.Error() - } else { - switch e.ApplyEvent.Status { - case event.ApplySkipped: - a.Type = Skip - case event.ApplySuccessful: - a.Updated = e.ApplyEvent.Resource - if a.Original != nil { - // TODO: Unclear if we should diff the full resources here. It doesn't work - // well with client-side apply as the managedFields property shows up as - // changes. It also means there is a race with controllers that might change - // the status of resources. - if reflect.DeepEqual(a.Original, a.Updated) { - a.Type = Unchanged - } else { - a.Type = Update - } + } else if e.ApplyEvent.Status == event.ApplySuccessful { + a.Updated = e.ApplyEvent.Resource + if a.Original != nil { + // TODO: Unclear if we should diff the full resources here. It doesn't work + // well with client-side apply as the managedFields property shows up as + // changes. It also means there is a race with controllers that might change + // the status of resources. + if reflect.DeepEqual(a.Original, a.Updated) { + a.Type = Unchanged } else { - a.Type = Create + a.Type = Update } + } else { + a.Type = Create } } return a } func handlePruneEvent(e event.Event, a Action) Action { + if e.PruneEvent.Status == event.PruneSkipped { + a.Type = Skip + if e.PruneEvent.Error != nil { + a.SkipReason = e.PruneEvent.Error.Error() + } + return a + } if e.PruneEvent.Error != nil { a.Type = Error a.Error = e.PruneEvent.Error.Error() - } else { - switch e.PruneEvent.Status { - case event.PruneSuccessful: - a.Type = Delete - // Lifecycle directives can cause resources to remain in the - // live state even if they would normally be pruned. - // TODO: Handle reason for skipped resources that has recently - // been added to the actuation library. - case event.PruneSkipped: - a.Type = Skip - } + } else if e.PruneEvent.Status == event.PruneSuccessful { + a.Type = Delete } return a } func handleDeleteEvent(e event.Event, a Action) Action { + if e.DeleteEvent.Status == event.DeleteSkipped { + a.Type = Skip + if e.DeleteEvent.Error != nil { + a.SkipReason = e.DeleteEvent.Error.Error() + } + return a + } if e.DeleteEvent.Error != nil { a.Type = Error a.Error = e.DeleteEvent.Error.Error() - } else { - switch e.DeleteEvent.Status { - case event.DeleteSuccessful: - a.Type = Delete - case event.DeleteSkipped: - a.Type = Skip - } + } else if e.DeleteEvent.Status == event.DeleteSuccessful { + a.Type = Delete } return a } diff --git a/pkg/live/planner/cluster_test.go b/pkg/live/planner/cluster_test.go index f72a43f035..5fbff89820 100644 --- a/pkg/live/planner/cluster_test.go +++ b/pkg/live/planner/cluster_test.go @@ -16,6 +16,7 @@ package planner import ( "context" + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -93,6 +94,258 @@ func TestClusterPlanner(t *testing.T) { }, }, }, + "skipped resource with reason": { + resources: []*unstructured.Unstructured{ + testutil.Unstructured(t, deploymentYAML), + }, + clusterResources: []*unstructured.Unstructured{}, + events: []event.Event{ + { + Type: event.InitType, + InitEvent: event.InitEvent{ + ActionGroups: event.ActionGroupList{ + { + Action: event.ApplyAction, + Name: "apply-1", + Identifiers: []object.ObjMetadata{ + testutil.ToIdentifier(t, deploymentYAML), + }, + }, + }, + }, + }, + { + Type: event.ApplyType, + ApplyEvent: event.ApplyEvent{ + GroupName: "apply-1", + Identifier: testutil.ToIdentifier(t, deploymentYAML), + Status: event.ApplySkipped, + Error: fmt.Errorf("some skip reason"), + }, + }, + }, + expectedPlan: &Plan{ + Actions: []Action{ + { + Type: Skip, + Name: "foo", + Namespace: "default", + Group: "apps", + Kind: "Deployment", + SkipReason: "some skip reason", + }, + }, + }, + }, + "skipped apply resource without reason": { + resources: []*unstructured.Unstructured{ + testutil.Unstructured(t, deploymentYAML), + }, + clusterResources: []*unstructured.Unstructured{}, + events: []event.Event{ + { + Type: event.InitType, + InitEvent: event.InitEvent{ + ActionGroups: event.ActionGroupList{ + { + Action: event.ApplyAction, + Name: "apply-1", + Identifiers: []object.ObjMetadata{ + testutil.ToIdentifier(t, deploymentYAML), + }, + }, + }, + }, + }, + { + Type: event.ApplyType, + ApplyEvent: event.ApplyEvent{ + GroupName: "apply-1", + Identifier: testutil.ToIdentifier(t, deploymentYAML), + Status: event.ApplySkipped, + }, + }, + }, + expectedPlan: &Plan{ + Actions: []Action{ + { + Type: Skip, + Name: "foo", + Namespace: "default", + Group: "apps", + Kind: "Deployment", + }, + }, + }, + }, + "skipped prune resource with reason": { + resources: []*unstructured.Unstructured{ + testutil.Unstructured(t, deploymentYAML), + }, + clusterResources: []*unstructured.Unstructured{}, + events: []event.Event{ + { + Type: event.InitType, + InitEvent: event.InitEvent{ + ActionGroups: event.ActionGroupList{ + { + Action: event.PruneAction, + Name: "prune-1", + Identifiers: []object.ObjMetadata{ + testutil.ToIdentifier(t, deploymentYAML), + }, + }, + }, + }, + }, + { + Type: event.PruneType, + PruneEvent: event.PruneEvent{ + GroupName: "prune-1", + Identifier: testutil.ToIdentifier(t, deploymentYAML), + Status: event.PruneSkipped, + Error: fmt.Errorf("some skip reason"), + }, + }, + }, + expectedPlan: &Plan{ + Actions: []Action{ + { + Type: Skip, + Name: "foo", + Namespace: "default", + Group: "apps", + Kind: "Deployment", + SkipReason: "some skip reason", + }, + }, + }, + }, + "skipped prune resource without reason": { + resources: []*unstructured.Unstructured{ + testutil.Unstructured(t, deploymentYAML), + }, + clusterResources: []*unstructured.Unstructured{}, + events: []event.Event{ + { + Type: event.InitType, + InitEvent: event.InitEvent{ + ActionGroups: event.ActionGroupList{ + { + Action: event.PruneAction, + Name: "prune-1", + Identifiers: []object.ObjMetadata{ + testutil.ToIdentifier(t, deploymentYAML), + }, + }, + }, + }, + }, + { + Type: event.PruneType, + PruneEvent: event.PruneEvent{ + GroupName: "prune-1", + Identifier: testutil.ToIdentifier(t, deploymentYAML), + Status: event.PruneSkipped, + }, + }, + }, + expectedPlan: &Plan{ + Actions: []Action{ + { + Type: Skip, + Name: "foo", + Namespace: "default", + Group: "apps", + Kind: "Deployment", + }, + }, + }, + }, + "skipped delete resource with reason": { + resources: []*unstructured.Unstructured{ + testutil.Unstructured(t, deploymentYAML), + }, + clusterResources: []*unstructured.Unstructured{}, + events: []event.Event{ + { + Type: event.InitType, + InitEvent: event.InitEvent{ + ActionGroups: event.ActionGroupList{ + { + Action: event.DeleteAction, + Name: "delete-1", + Identifiers: []object.ObjMetadata{ + testutil.ToIdentifier(t, deploymentYAML), + }, + }, + }, + }, + }, + { + Type: event.DeleteType, + DeleteEvent: event.DeleteEvent{ + GroupName: "delete-1", + Identifier: testutil.ToIdentifier(t, deploymentYAML), + Status: event.DeleteSkipped, + Error: fmt.Errorf("some skip reason"), + }, + }, + }, + expectedPlan: &Plan{ + Actions: []Action{ + { + Type: Skip, + Name: "foo", + Namespace: "default", + Group: "apps", + Kind: "Deployment", + SkipReason: "some skip reason", + }, + }, + }, + }, + "skipped delete resource without reason": { + resources: []*unstructured.Unstructured{ + testutil.Unstructured(t, deploymentYAML), + }, + clusterResources: []*unstructured.Unstructured{}, + events: []event.Event{ + { + Type: event.InitType, + InitEvent: event.InitEvent{ + ActionGroups: event.ActionGroupList{ + { + Action: event.DeleteAction, + Name: "delete-1", + Identifiers: []object.ObjMetadata{ + testutil.ToIdentifier(t, deploymentYAML), + }, + }, + }, + }, + }, + { + Type: event.DeleteType, + DeleteEvent: event.DeleteEvent{ + GroupName: "delete-1", + Identifier: testutil.ToIdentifier(t, deploymentYAML), + Status: event.DeleteSkipped, + }, + }, + }, + expectedPlan: &Plan{ + Actions: []Action{ + { + Type: Skip, + Name: "foo", + Namespace: "default", + Group: "apps", + Kind: "Deployment", + }, + }, + }, + }, } for tn := range testCases {