From c1037840004a2345a0b611ec182ac27da3b9dd8b Mon Sep 17 00:00:00 2001 From: Karl Isenberg Date: Wed, 2 Mar 2022 10:36:59 -0800 Subject: [PATCH] fix: Allow empty set of apply objects - Add e2e test to ensure the inventory is still correctly created and destroyed when the apply object set is empty. --- pkg/apply/applier_test.go | 48 +++++++++++++ pkg/apply/solver/solver.go | 15 ++-- pkg/apply/solver/solver_test.go | 68 ++++++++++++++++++ test/e2e/e2e_test.go | 4 ++ test/e2e/empty_set_test.go | 118 ++++++++++++++++++++++++++++++++ 5 files changed, 247 insertions(+), 6 deletions(-) create mode 100644 test/e2e/empty_set_test.go diff --git a/pkg/apply/applier_test.go b/pkg/apply/applier_test.go index 223b6c82..7feaeac1 100644 --- a/pkg/apply/applier_test.go +++ b/pkg/apply/applier_test.go @@ -640,6 +640,22 @@ func TestApplier(t *testing.T) { }, }, expectedStatusEvents: []testutil.ExpEvent{ + { + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + GroupName: "inventory-add-0", + Action: event.InventoryAction, + Type: event.Started, + }, + }, + { + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + GroupName: "inventory-add-0", + Action: event.InventoryAction, + Type: event.Finished, + }, + }, { EventType: event.StatusType, StatusEvent: &testutil.ExpStatusEvent{ @@ -937,6 +953,22 @@ func TestApplier(t *testing.T) { EventType: event.InitType, InitEvent: &testutil.ExpInitEvent{}, }, + { + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + GroupName: "inventory-add-0", + Action: event.InventoryAction, + Type: event.Started, + }, + }, + { + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + GroupName: "inventory-add-0", + Action: event.InventoryAction, + Type: event.Finished, + }, + }, { EventType: event.ActionGroupType, ActionGroupEvent: &testutil.ExpActionGroupEvent{ @@ -1060,6 +1092,22 @@ func TestApplier(t *testing.T) { EventType: event.InitType, InitEvent: &testutil.ExpInitEvent{}, }, + { + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + GroupName: "inventory-add-0", + Action: event.InventoryAction, + Type: event.Started, + }, + }, + { + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + GroupName: "inventory-add-0", + Action: event.InventoryAction, + Type: event.Finished, + }, + }, { EventType: event.ActionGroupType, ActionGroupEvent: &testutil.ExpActionGroupEvent{ diff --git a/pkg/apply/solver/solver.go b/pkg/apply/solver/solver.go index 92fc0a25..391a152f 100644 --- a/pkg/apply/solver/solver.go +++ b/pkg/apply/solver/solver.go @@ -154,12 +154,8 @@ func (t *TaskQueueBuilder) Build(taskContext *taskrunner.TaskContext, o Options) applyObjs = t.Collector.FilterInvalidObjects(applyObjs) pruneObjs = t.Collector.FilterInvalidObjects(pruneObjs) - if len(applyObjs) > 0 { - // Register actuation plan in the inventory - for _, id := range object.UnstructuredSetToObjMetadataSet(applyObjs) { - taskContext.InventoryManager().AddPendingApply(id) - } - + if !o.Destroy { + // InvAddTask creates the inventory and adds any objects being applied klog.V(2).Infof("adding inventory add task (%d objects)", len(applyObjs)) tasks = append(tasks, &task.InvAddTask{ TaskName: "inventory-add-0", @@ -168,6 +164,13 @@ func (t *TaskQueueBuilder) Build(taskContext *taskrunner.TaskContext, o Options) Objects: applyObjs, DryRun: o.DryRunStrategy, }) + } + + if len(applyObjs) > 0 { + // Register actuation plan in the inventory + for _, id := range object.UnstructuredSetToObjMetadataSet(applyObjs) { + taskContext.InventoryManager().AddPendingApply(id) + } // Filter idSetList down to just apply objects applySets := graph.HydrateSetList(idSetList, applyObjs) diff --git a/pkg/apply/solver/solver_test.go b/pkg/apply/solver/solver_test.go index 0118e6ca..ac02ed4a 100644 --- a/pkg/apply/solver/solver_test.go +++ b/pkg/apply/solver/solver_test.go @@ -143,6 +143,12 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { "no resources, no apply or wait tasks": { applyObjs: []*unstructured.Unstructured{}, expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.InvSetTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, @@ -850,6 +856,12 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { pruneObjs: []*unstructured.Unstructured{}, options: Options{Prune: true}, expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.InvSetTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, @@ -864,6 +876,12 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { }, options: Options{Prune: true}, expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ @@ -904,6 +922,12 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { }, options: Options{Prune: true}, expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ @@ -957,6 +981,12 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { options: Options{Prune: true}, // Opposite ordering when pruning/deleting expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ @@ -1022,6 +1052,12 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { PruneTimeout: 3 * time.Minute, }, expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ @@ -1068,6 +1104,13 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { }, // No wait task, since it is dry run expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + DryRun: common.DryRunServer, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ @@ -1115,6 +1158,12 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { options: Options{Prune: true}, // Opposite ordering when pruning/deleting. expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ @@ -1193,6 +1242,13 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { Prune: true, }, expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + DryRun: common.DryRunClient, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ @@ -1255,6 +1311,12 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { }, options: Options{Prune: true}, expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ @@ -1357,6 +1419,12 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { }, options: Options{Prune: true}, expectedTasks: []taskrunner.Task{ + &task.InvAddTask{ + TaskName: "inventory-add-0", + InvClient: &inventory.FakeClient{}, + InvInfo: invInfo, + Objects: object.UnstructuredSet{}, + }, &task.PruneTask{ TaskName: "prune-0", Objects: []*unstructured.Unstructured{ diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 8bf7e36f..35cdba5f 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -171,6 +171,10 @@ var _ = Describe("Applier", func() { dryRunTest(ctx, c, invConfig, inventoryName, namespace.GetName()) }) + It("EmptySet", func() { + emptySetTest(ctx, c, invConfig, inventoryName, namespace.GetName()) + }) + It("Deletion Prevention", func() { deletionPreventionTest(ctx, c, invConfig, inventoryName, namespace.GetName()) }) diff --git a/test/e2e/empty_set_test.go b/test/e2e/empty_set_test.go new file mode 100644 index 00000000..2808246a --- /dev/null +++ b/test/e2e/empty_set_test.go @@ -0,0 +1,118 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + "fmt" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/cli-utils/pkg/apply" + "sigs.k8s.io/cli-utils/pkg/apply/event" + "sigs.k8s.io/cli-utils/pkg/inventory" + "sigs.k8s.io/cli-utils/pkg/testutil" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +//nolint:dupl // expEvents similar to other tests +func emptySetTest(ctx context.Context, c client.Client, invConfig InventoryConfig, inventoryName, namespaceName string) { + By("Apply zero resources") + applier := invConfig.ApplierFactoryFunc() + + inventoryID := fmt.Sprintf("%s-%s", inventoryName, namespaceName) + inventoryInfo := invConfig.InvWrapperFunc(invConfig.FactoryFunc(inventoryName, namespaceName, inventoryID)) + + resources := []*unstructured.Unstructured{} + + applierEvents := runCollect(applier.Run(ctx, inventoryInfo, resources, apply.ApplierOptions{ + EmitStatusEvents: true, + })) + + expEvents := []testutil.ExpEvent{ + { + // InitTask + EventType: event.InitType, + InitEvent: &testutil.ExpInitEvent{}, + }, + { + // InvAddTask start + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + Action: event.InventoryAction, + GroupName: "inventory-add-0", + Type: event.Started, + }, + }, + { + // InvAddTask finished + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + Action: event.InventoryAction, + GroupName: "inventory-add-0", + Type: event.Finished, + }, + }, + { + // InvSetTask start + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + Action: event.InventoryAction, + GroupName: "inventory-set-0", + Type: event.Started, + }, + }, + { + // InvSetTask finished + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + Action: event.InventoryAction, + GroupName: "inventory-set-0", + Type: event.Finished, + }, + }, + } + Expect(testutil.EventsToExpEvents(applierEvents)).To(testutil.Equal(expEvents)) + + By("Verify inventory created") + invConfig.InvSizeVerifyFunc(ctx, c, inventoryName, namespaceName, inventoryID, 0) + + By("Destroy zero resources") + destroyer := invConfig.DestroyerFactoryFunc() + + options := apply.DestroyerOptions{InventoryPolicy: inventory.PolicyAdoptIfNoInventory} + destroyerEvents := runCollect(destroyer.Run(ctx, inventoryInfo, options)) + + expEvents = []testutil.ExpEvent{ + { + // InitTask + EventType: event.InitType, + InitEvent: &testutil.ExpInitEvent{}, + }, + { + // DeleteInvTask start + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + Action: event.InventoryAction, + GroupName: "delete-inventory-0", + Type: event.Started, + }, + }, + { + // DeleteInvTask finished + EventType: event.ActionGroupType, + ActionGroupEvent: &testutil.ExpActionGroupEvent{ + Action: event.InventoryAction, + GroupName: "delete-inventory-0", + Type: event.Finished, + }, + }, + } + + Expect(testutil.EventsToExpEvents(destroyerEvents)).To(testutil.Equal(expEvents)) + + By("Verify inventory deleted") + invConfig.InvNotExistsFunc(ctx, c, inventoryName, namespaceName, inventoryID) +}