diff --git a/cmd/status/cmdstatus.go b/cmd/status/cmdstatus.go index 0ad87a8e..7c2e3009 100644 --- a/cmd/status/cmdstatus.go +++ b/cmd/status/cmdstatus.go @@ -152,7 +152,7 @@ func (r *Runner) preRunE(*cobra.Command, []string) error { // and get info from the cluster based on the local info // wrap it to be a map mapping from string to objectMetadataSet func (r *Runner) loadInvFromDisk(cmd *cobra.Command, args []string) (*printer.PrintData, error) { - inv, err := r.loader.GetInvInfo(cmd, args) + invInfo, err := r.loader.GetInvInfo(cmd, args) if err != nil { return nil, err } @@ -164,11 +164,13 @@ func (r *Runner) loadInvFromDisk(cmd *cobra.Command, args []string) (*printer.Pr // Based on the inventory template manifest we look up the inventory // from the live state using the inventory client. - identifiers, err := invClient.GetClusterObjs(cmd.Context(), inv) + inv, err := invClient.Get(cmd.Context(), invInfo, inventory.GetOptions{}) if err != nil { return nil, err } + identifiers := inv.GetObjectRefs() + printData := printer.PrintData{ Identifiers: object.ObjMetadataSet{}, InvNameMap: make(map[object.ObjMetadata]string), @@ -179,7 +181,9 @@ func (r *Runner) loadInvFromDisk(cmd *cobra.Command, args []string) (*printer.Pr // check if the object is under one of the targeted namespaces if _, ok := r.namespaceSet[obj.Namespace]; ok || len(r.namespaceSet) == 0 { // add to the map for future reference - printData.InvNameMap[obj] = inv.Name() + // Cast to SingleObjectInfo is safe because this always uses an inventory client + // with a singleton inventory object + printData.InvNameMap[obj] = invInfo.(*inventory.SingleObjectInfo).GetName() // append to identifiers printData.Identifiers = append(printData.Identifiers, obj) } @@ -201,11 +205,18 @@ func (r *Runner) listInvFromCluster() (*printer.PrintData, error) { StatusSet: r.statusSet, } - identifiersMap, err := invClient.ListClusterInventoryObjs(r.ctx) + inventories, err := invClient.List(r.ctx, inventory.ListOptions{}) if err != nil { return nil, err } + identifiersMap := make(map[string]object.ObjMetadataSet) + for _, inv := range inventories { + // Cast to SingleObjectInfo is safe because this always uses an inventory client + // with a singleton inventory object + identifiersMap[inv.Info().(*inventory.SingleObjectInfo).GetName()] = inv.GetObjectRefs() + } + for invName, identifiers := range identifiersMap { // Check if there are targeted inventory names and include the current inventory name if _, ok := r.inventoryNameSet[invName]; !ok && len(r.inventoryNameSet) != 0 { diff --git a/cmd/status/cmdstatus_test.go b/cmd/status/cmdstatus_test.go index cf0f196c..ef6352f9 100644 --- a/cmd/status/cmdstatus_test.go +++ b/cmd/status/cmdstatus_test.go @@ -31,8 +31,8 @@ kind: ConfigMap apiVersion: v1 metadata: labels: - cli-utils.sigs.k8s.io/inventory-id: test - name: foo + cli-utils.sigs.k8s.io/inventory-id: inventory-id + name: inventory-name namespace: default ` depObject = object.ObjMetadata{ @@ -121,8 +121,8 @@ func TestCommand(t *testing.T) { }, }, expectedOutput: ` -foo/deployment.apps/default/foo is InProgress: inProgress -foo/statefulset.apps/default/bar is Current: current +inventory-name/deployment.apps/default/foo is InProgress: inProgress +inventory-name/statefulset.apps/default/bar is Current: current `, }, "wait for all current": { @@ -168,10 +168,10 @@ foo/statefulset.apps/default/bar is Current: current }, }, expectedOutput: ` -foo/deployment.apps/default/foo is InProgress: inProgress -foo/statefulset.apps/default/bar is InProgress: inProgress -foo/statefulset.apps/default/bar is Current: current -foo/deployment.apps/default/foo is Current: current +inventory-name/deployment.apps/default/foo is InProgress: inProgress +inventory-name/statefulset.apps/default/bar is InProgress: inProgress +inventory-name/statefulset.apps/default/bar is Current: current +inventory-name/deployment.apps/default/foo is Current: current `, }, "wait for all deleted": { @@ -201,8 +201,8 @@ foo/deployment.apps/default/foo is Current: current }, }, expectedOutput: ` -foo/statefulset.apps/default/bar is NotFound: notFound -foo/deployment.apps/default/foo is NotFound: notFound +inventory-name/statefulset.apps/default/bar is NotFound: notFound +inventory-name/deployment.apps/default/foo is NotFound: notFound `, }, "forever with timeout": { @@ -233,8 +233,8 @@ foo/deployment.apps/default/foo is NotFound: notFound }, }, expectedOutput: ` -foo/statefulset.apps/default/bar is InProgress: inProgress -foo/deployment.apps/default/foo is InProgress: inProgress +inventory-name/statefulset.apps/default/bar is InProgress: inProgress +inventory-name/deployment.apps/default/foo is InProgress: inProgress `, }, } @@ -283,7 +283,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "foo", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "InProgress", "message": "inProgress", }, @@ -294,7 +294,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "bar", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "Current", "message": "current", }, @@ -350,7 +350,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "foo", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "InProgress", "message": "inProgress", }, @@ -361,7 +361,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "bar", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "InProgress", "message": "inProgress", }, @@ -372,7 +372,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "bar", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "Current", "message": "current", }, @@ -383,7 +383,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "foo", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "Current", "message": "current", }, @@ -423,7 +423,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "bar", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "NotFound", "message": "notFound", }, @@ -434,7 +434,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "foo", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "NotFound", "message": "notFound", }, @@ -475,7 +475,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "bar", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "InProgress", "message": "inProgress", }, @@ -486,7 +486,7 @@ foo/deployment.apps/default/foo is InProgress: inProgress "name": "foo", "timestamp": "", "type": "status", - "inventory-name": "foo", + "inventory-name": "inventory-name", "status": "InProgress", "message": "inProgress", }, diff --git a/pkg/apis/actuation/types.go b/pkg/apis/actuation/types.go index 0f1dd642..63cf3d6c 100644 --- a/pkg/apis/actuation/types.go +++ b/pkg/apis/actuation/types.go @@ -4,31 +4,9 @@ package actuation import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) -// Inventory represents the inventory object in memory. -// Inventory is currently only used for in-memory storage and not serialized to -// disk or to the API server. -type Inventory struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` - - Spec InventorySpec `json:"spec,omitempty"` - Status InventoryStatus `json:"status,omitempty"` -} - -// InventorySpec is the specification of the desired/expected inventory state. -type InventorySpec struct { - Objects []ObjectReference `json:"objects,omitempty"` -} - -// InventoryStatus is the status of the current/last-known inventory state. -type InventoryStatus struct { - Objects []ObjectStatus `json:"objects,omitempty"` -} - // ObjectReference is a reference to a KRM resource by name and kind. // // Kubernetes only stores one API Version for each Kind at any given time, diff --git a/pkg/apis/actuation/zz_generated.deepcopy.go b/pkg/apis/actuation/zz_generated.deepcopy.go index 1045f12c..35d40082 100644 --- a/pkg/apis/actuation/zz_generated.deepcopy.go +++ b/pkg/apis/actuation/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated // Copyright 2023 The Kubernetes Authors. // SPDX-License-Identifier: Apache-2.0 @@ -7,68 +8,6 @@ package actuation -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Inventory) DeepCopyInto(out *Inventory) { - *out = *in - out.TypeMeta = in.TypeMeta - in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - in.Spec.DeepCopyInto(&out.Spec) - in.Status.DeepCopyInto(&out.Status) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Inventory. -func (in *Inventory) DeepCopy() *Inventory { - if in == nil { - return nil - } - out := new(Inventory) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *InventorySpec) DeepCopyInto(out *InventorySpec) { - *out = *in - if in.Objects != nil { - in, out := &in.Objects, &out.Objects - *out = make([]ObjectReference, len(*in)) - copy(*out, *in) - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InventorySpec. -func (in *InventorySpec) DeepCopy() *InventorySpec { - if in == nil { - return nil - } - out := new(InventorySpec) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *InventoryStatus) DeepCopyInto(out *InventoryStatus) { - *out = *in - if in.Objects != nil { - in, out := &in.Objects, &out.Objects - *out = make([]ObjectStatus, len(*in)) - copy(*out, *in) - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InventoryStatus. -func (in *InventoryStatus) DeepCopy() *InventoryStatus { - if in == nil { - return nil - } - out := new(InventoryStatus) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ObjectReference) DeepCopyInto(out *ObjectReference) { *out = *in diff --git a/pkg/apply/applier.go b/pkg/apply/applier.go index b0c99b0a..c78a6012 100644 --- a/pkg/apply/applier.go +++ b/pkg/apply/applier.go @@ -8,6 +8,7 @@ import ( "fmt" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -52,9 +53,9 @@ type Applier struct { // prepareObjects returns the set of objects to apply and to prune or // an error if one occurred. -func (a *Applier) prepareObjects(ctx context.Context, localInv inventory.Info, localObjs object.UnstructuredSet, +func (a *Applier) prepareObjects(ctx context.Context, inv inventory.Inventory, localObjs object.UnstructuredSet, o ApplierOptions) (object.UnstructuredSet, object.UnstructuredSet, error) { - if localInv == nil { + if inv == nil { return nil, nil, fmt.Errorf("the local inventory can't be nil") } if err := inventory.ValidateNoInventory(localObjs); err != nil { @@ -62,9 +63,9 @@ func (a *Applier) prepareObjects(ctx context.Context, localInv inventory.Info, l } // Add the inventory annotation to the resources being applied. for _, localObj := range localObjs { - inventory.AddInventoryIDAnnotation(localObj, localInv) + inventory.AddInventoryIDAnnotation(localObj, inv.Info().GetID()) } - pruneObjs, err := a.pruner.GetPruneObjs(ctx, localInv, localObjs, prune.Options{ + pruneObjs, err := a.pruner.GetPruneObjs(ctx, inv, localObjs, prune.Options{ DryRunStrategy: o.DryRunStrategy, }) if err != nil { @@ -96,8 +97,22 @@ func (a *Applier) Run(ctx context.Context, invInfo inventory.Info, objects objec } validator.Validate(objects) + inv, err := a.invClient.Get(ctx, invInfo, inventory.GetOptions{}) + if apierrors.IsNotFound(err) { + inv, err = a.invClient.NewInventory(invInfo) + } + if err != nil { + handleError(eventChannel, err) + return + } + if inv.Info().GetID() != invInfo.GetID() { + handleError(eventChannel, fmt.Errorf("expected inventory object to have inventory-id %q but got %q", + invInfo.GetID(), inv.Info().GetID())) + return + } + // Decide which objects to apply and which to prune - applyObjs, pruneObjs, err := a.prepareObjects(ctx, invInfo, objects, options) + applyObjs, pruneObjs, err := a.prepareObjects(ctx, inv, objects, options) if err != nil { handleError(eventChannel, err) return @@ -154,6 +169,7 @@ func (a *Applier) Run(ctx context.Context, invInfo inventory.Info, objects objec OpenAPIGetter: a.openAPIGetter, InfoHelper: a.infoHelper, Mapper: a.mapper, + Inventory: inv, InvClient: a.invClient, Collector: vCollector, ApplyFilters: applyFilters, @@ -175,7 +191,6 @@ func (a *Applier) Run(ctx context.Context, invInfo inventory.Info, objects objec taskQueue := taskBuilder. WithApplyObjects(applyObjs). WithPruneObjects(pruneObjs). - WithInventory(invInfo). Build(taskContext, opts) klog.V(4).Infof("validation errors: %d", len(vCollector.Errors)) @@ -303,7 +318,7 @@ func localNamespaces(localInv inventory.Info, localObjs []object.ObjMetadata) se namespaces.Insert(obj.Namespace) } } - invNamespace := localInv.Namespace() + invNamespace := localInv.GetNamespace() if invNamespace != "" { namespaces.Insert(invNamespace) } diff --git a/pkg/apply/applier_test.go b/pkg/apply/applier_test.go index 824a4741..6f9b1429 100644 --- a/pkg/apply/applier_test.go +++ b/pkg/apply/applier_test.go @@ -11,6 +11,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubectl/pkg/scheme" "sigs.k8s.io/cli-utils/pkg/apis/actuation" @@ -50,16 +52,6 @@ metadata: type: Opaque spec: foo: bar -`, - "inventory": ` -apiVersion: v1 -kind: ConfigMap -metadata: - name: test-inventory-obj - namespace: test-namespace - labels: - cli-utils.sigs.k8s.io/inventory-id: test-app-label -data: {} `, "obj1": ` apiVersion: v1 @@ -93,7 +85,7 @@ func TestApplier(t *testing.T) { // resources input to applier resources object.UnstructuredSet // inventory input to applier - invInfo inventoryInfo + invObj *unstructured.Unstructured // objects in the cluster clusterObjs object.UnstructuredSet // options input to applier.Run @@ -114,11 +106,13 @@ func TestApplier(t *testing.T) { resources: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, - invInfo: inventoryInfo{ - name: "abc-123", - namespace: "default", - id: "test", - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "default", + }), + nil, + ), clusterObjs: object.UnstructuredSet{}, options: ApplierOptions{ NoPrune: true, @@ -221,11 +215,13 @@ func TestApplier(t *testing.T) { testutil.Unstructured(t, resources["deployment"]), testutil.Unstructured(t, resources["secret"]), }, - invInfo: inventoryInfo{ - name: "inv-123", - namespace: "default", - id: "test", - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "default", + }), + nil, + ), clusterObjs: object.UnstructuredSet{}, options: ApplierOptions{ ReconcileTimeout: time.Minute, @@ -410,16 +406,17 @@ func TestApplier(t *testing.T) { testutil.Unstructured(t, resources["deployment"]), testutil.Unstructured(t, resources["secret"]), }, - invInfo: inventoryInfo{ - name: "inv-123", - namespace: "default", - id: "test", - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "default", + }), + object.ObjMetadataSet{ object.UnstructuredToObjMetadata( testutil.Unstructured(t, resources["deployment"]), ), }, - }, + ), clusterObjs: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, @@ -588,11 +585,12 @@ func TestApplier(t *testing.T) { "apply no resources and prune all existing": { namespace: "default", resources: object.UnstructuredSet{}, - invInfo: inventoryInfo{ - name: "inv-123", - namespace: "default", - id: "test", - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "inv-123", + Namespace: "default", + }), + object.ObjMetadataSet{ object.UnstructuredToObjMetadata( testutil.Unstructured(t, resources["deployment"]), ), @@ -600,7 +598,7 @@ func TestApplier(t *testing.T) { testutil.Unstructured(t, resources["secret"]), ), }, - }, + ), clusterObjs: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"], testutil.AddOwningInv(t, "test")), testutil.Unstructured(t, resources["secret"], testutil.AddOwningInv(t, "test")), @@ -796,11 +794,13 @@ func TestApplier(t *testing.T) { resources: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, - invInfo: inventoryInfo{ - name: "abc-123", - namespace: "default", - id: "test", - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "default", + }), + nil, + ), clusterObjs: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"], testutil.AddOwningInv(t, "unmatched")), }, @@ -910,16 +910,17 @@ func TestApplier(t *testing.T) { "resources belonging to a different inventory should not be pruned": { namespace: "default", resources: object.UnstructuredSet{}, - invInfo: inventoryInfo{ - name: "abc-123", - namespace: "default", - id: "test", - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "default", + }), + object.ObjMetadataSet{ object.UnstructuredToObjMetadata( testutil.Unstructured(t, resources["deployment"]), ), }, - }, + ), clusterObjs: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"], testutil.AddOwningInv(t, "unmatched")), }, @@ -1022,16 +1023,17 @@ func TestApplier(t *testing.T) { "prune with inventory object annotation matched": { namespace: "default", resources: object.UnstructuredSet{}, - invInfo: inventoryInfo{ - name: "abc-123", - namespace: "default", - id: "test", - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "default", + }), + object.ObjMetadataSet{ object.UnstructuredToObjMetadata( testutil.Unstructured(t, resources["deployment"]), ), }, - }, + ), clusterObjs: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"], testutil.AddOwningInv(t, "test")), }, @@ -1178,11 +1180,13 @@ func TestApplier(t *testing.T) { }), testutil.Unstructured(t, resources["secret"]), }, - invInfo: inventoryInfo{ - name: "inv-123", - namespace: "default", - id: "test", - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "inv-123", + Namespace: "default", + }), + nil, + ), clusterObjs: object.UnstructuredSet{}, options: ApplierOptions{ ReconcileTimeout: time.Minute, @@ -1358,11 +1362,13 @@ func TestApplier(t *testing.T) { }), testutil.Unstructured(t, resources["secret"]), }, - invInfo: inventoryInfo{ - name: "inv-123", - namespace: "default", - id: "test", - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "inv-123", + Namespace: "default", + }), + nil, + ), clusterObjs: object.UnstructuredSet{}, options: ApplierOptions{ ReconcileTimeout: time.Minute, @@ -1416,7 +1422,7 @@ func TestApplier(t *testing.T) { } applier := newTestApplier(t, - tc.invInfo, + tc.invObj, validObjs, tc.clusterObjs, statusWatcher, @@ -1431,7 +1437,7 @@ func TestApplier(t *testing.T) { testCtx, testCancel := context.WithTimeout(context.Background(), testTimeout) defer testCancel() // cleanup - invInfo, err := tc.invInfo.toWrapped() + invInfo, err := inventory.ConfigMapToInventoryInfo(tc.invObj) require.NoError(t, err) eventChannel := applier.Run(runCtx, invInfo, tc.resources, tc.options) @@ -1513,7 +1519,7 @@ func TestApplierCancel(t *testing.T) { // resources input to applier resources object.UnstructuredSet // inventory input to applier - invInfo inventoryInfo + invObj *unstructured.Unstructured // objects in the cluster clusterObjs object.UnstructuredSet // options input to applier.Run @@ -1538,11 +1544,13 @@ func TestApplierCancel(t *testing.T) { resources: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, - invInfo: inventoryInfo{ - name: "abc-123", - namespace: "test", - id: "test", - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "test", + }), + nil, + ), clusterObjs: object.UnstructuredSet{}, options: ApplierOptions{ // EmitStatusEvents required to test event output @@ -1696,11 +1704,13 @@ func TestApplierCancel(t *testing.T) { resources: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, - invInfo: inventoryInfo{ - name: "abc-123", - namespace: "test", - id: "test", - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "test", + }), + nil, + ), clusterObjs: object.UnstructuredSet{}, options: ApplierOptions{ // EmitStatusEvents required to test event output @@ -1860,7 +1870,7 @@ func TestApplierCancel(t *testing.T) { statusWatcher := newFakeWatcher(tc.statusEvents) applier := newTestApplier(t, - tc.invInfo, + tc.invObj, tc.resources, tc.clusterObjs, statusWatcher, @@ -1874,7 +1884,7 @@ func TestApplierCancel(t *testing.T) { testCtx, testCancel := context.WithTimeout(context.Background(), tc.testTimeout) defer testCancel() // cleanup - invInfo, err := tc.invInfo.toWrapped() + invInfo, err := inventory.ConfigMapToInventoryInfo(tc.invObj) require.NoError(t, err) eventChannel := applier.Run(runCtx, invInfo, tc.resources, tc.options) @@ -1952,9 +1962,13 @@ func TestReadAndPrepareObjectsNilInv(t *testing.T) { } func TestReadAndPrepareObjects(t *testing.T) { - inventoryObj := testutil.Unstructured(t, resources["inventory"]) - invInfo, err := inventory.ConfigMapToInventoryInfo(inventoryObj) - require.NoError(t, err) + inventoryObj := newInventoryObj( + inventory.NewSingleObjectInfo("test-app-label", types.NamespacedName{ + Name: "test-inventory-obj", + Namespace: "test-namespace", + }), + nil, + ) obj1 := testutil.Unstructured(t, resources["obj1"]) obj2 := testutil.Unstructured(t, resources["obj2"]) @@ -1964,7 +1978,7 @@ func TestReadAndPrepareObjects(t *testing.T) { // objects in the cluster clusterObjs object.UnstructuredSet // invInfo input to applier - invInfo inventoryInfo + invObj *unstructured.Unstructured // resources input to applier resources object.UnstructuredSet // expected objects to apply @@ -1975,70 +1989,78 @@ func TestReadAndPrepareObjects(t *testing.T) { isError bool }{ "objects include inventory": { - invInfo: inventoryInfo{ - name: invInfo.Name(), - namespace: invInfo.Namespace(), - id: invInfo.ID(), - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test-app-label", types.NamespacedName{ + Name: "test-inventory-obj", + Namespace: "test-namespace", + }), + nil, + ), resources: object.UnstructuredSet{inventoryObj}, isError: true, }, "empty inventory, empty objects, apply none, prune none": { - invInfo: inventoryInfo{ - name: invInfo.Name(), - namespace: invInfo.Namespace(), - id: invInfo.ID(), - }, + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test-app-label", types.NamespacedName{ + Name: "test-inventory-obj", + Namespace: "test-namespace", + }), + nil, + ), }, "one in inventory, empty objects, prune one": { clusterObjs: object.UnstructuredSet{obj1}, - invInfo: inventoryInfo{ - name: invInfo.Name(), - namespace: invInfo.Namespace(), - id: invInfo.ID(), - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test-app-label", types.NamespacedName{ + Name: "test-inventory-obj", + Namespace: "test-namespace", + }), + object.ObjMetadataSet{ object.UnstructuredToObjMetadata(obj1), }, - }, + ), pruneObjs: object.UnstructuredSet{obj1}, }, "all in inventory, apply all": { - invInfo: inventoryInfo{ - name: invInfo.Name(), - namespace: invInfo.Namespace(), - id: invInfo.ID(), - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test-app-label", types.NamespacedName{ + Name: "test-inventory-obj", + Namespace: "test-namespace", + }), + object.ObjMetadataSet{ object.UnstructuredToObjMetadata(obj1), object.UnstructuredToObjMetadata(clusterScopedObj), }, - }, + ), resources: object.UnstructuredSet{obj1, clusterScopedObj}, applyObjs: object.UnstructuredSet{obj1, clusterScopedObj}, }, "disjoint set, apply new, prune old": { clusterObjs: object.UnstructuredSet{obj2}, - invInfo: inventoryInfo{ - name: invInfo.Name(), - namespace: invInfo.Namespace(), - id: invInfo.ID(), - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test-app-label", types.NamespacedName{ + Name: "test-inventory-obj", + Namespace: "test-namespace", + }), + object.ObjMetadataSet{ object.UnstructuredToObjMetadata(obj2), }, - }, + ), resources: object.UnstructuredSet{obj1, clusterScopedObj}, applyObjs: object.UnstructuredSet{obj1, clusterScopedObj}, pruneObjs: object.UnstructuredSet{obj2}, }, "most in inventory, apply all": { clusterObjs: object.UnstructuredSet{obj2}, - invInfo: inventoryInfo{ - name: invInfo.Name(), - namespace: invInfo.Namespace(), - id: invInfo.ID(), - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test-app-label", types.NamespacedName{ + Name: "test-inventory-obj", + Namespace: "test-namespace", + }), + object.ObjMetadataSet{ object.UnstructuredToObjMetadata(obj2), }, - }, + ), resources: object.UnstructuredSet{obj1, obj2, clusterScopedObj}, applyObjs: object.UnstructuredSet{obj1, obj2, clusterScopedObj}, pruneObjs: object.UnstructuredSet{}, @@ -2048,16 +2070,16 @@ func TestReadAndPrepareObjects(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { applier := newTestApplier(t, - tc.invInfo, + tc.invObj, tc.resources, tc.clusterObjs, // no events needed for prepareObjects watcher.BlindStatusWatcher{}, ) - invInfoObj, err := tc.invInfo.toWrapped() + inv, err := inventory.ConfigMapToInventoryObj(tc.invObj) require.NoError(t, err) - applyObjs, pruneObjs, err := applier.prepareObjects(t.Context(), invInfoObj, tc.resources, ApplierOptions{}) + applyObjs, pruneObjs, err := applier.prepareObjects(t.Context(), inv, tc.resources, ApplierOptions{}) if tc.isError { assert.Error(t, err) return diff --git a/pkg/apply/common_test.go b/pkg/apply/common_test.go index 8ab9b8ec..6730405f 100644 --- a/pkg/apply/common_test.go +++ b/pkg/apply/common_test.go @@ -35,16 +35,9 @@ import ( "sigs.k8s.io/cli-utils/pkg/object" ) -type inventoryInfo struct { - name string - namespace string - id string - set object.ObjMetadataSet -} - -func (i inventoryInfo) toUnstructured() *unstructured.Unstructured { +func newInventoryObj(info *inventory.SingleObjectInfo, set object.ObjMetadataSet) *unstructured.Unstructured { invMap := make(map[string]interface{}) - for _, objMeta := range i.set { + for _, objMeta := range set { invMap[objMeta.String()] = "" } @@ -53,10 +46,10 @@ func (i inventoryInfo) toUnstructured() *unstructured.Unstructured { "apiVersion": "v1", "kind": "ConfigMap", "metadata": map[string]interface{}{ - "name": i.name, - "namespace": i.namespace, + "name": info.GetName(), + "namespace": info.GetNamespace(), "labels": map[string]interface{}{ - common.InventoryLabel: i.id, + common.InventoryLabel: info.GetID().String(), }, }, "data": invMap, @@ -64,18 +57,14 @@ func (i inventoryInfo) toUnstructured() *unstructured.Unstructured { } } -func (i inventoryInfo) toWrapped() (inventory.Info, error) { - return inventory.ConfigMapToInventoryInfo(i.toUnstructured()) -} - func newTestApplier( t *testing.T, - invInfo inventoryInfo, + invObj *unstructured.Unstructured, resources object.UnstructuredSet, clusterObjs object.UnstructuredSet, statusWatcher watcher.StatusWatcher, ) *Applier { - tf := newTestFactory(t, invInfo, resources, clusterObjs) + tf := newTestFactory(t, invObj, resources, clusterObjs) defer tf.Cleanup() infoHelper := &fakeInfoHelper{ @@ -100,11 +89,11 @@ func newTestApplier( func newTestDestroyer( t *testing.T, - invInfo inventoryInfo, + invObj *unstructured.Unstructured, clusterObjs object.UnstructuredSet, statusWatcher watcher.StatusWatcher, ) *Destroyer { - tf := newTestFactory(t, invInfo, object.UnstructuredSet{}, clusterObjs) + tf := newTestFactory(t, invObj, object.UnstructuredSet{}, clusterObjs) defer tf.Cleanup() invClient := newTestInventory(t, tf) @@ -132,11 +121,11 @@ func newTestInventory( func newTestFactory( t *testing.T, - invInfo inventoryInfo, + invObj *unstructured.Unstructured, resourceSet object.UnstructuredSet, clusterObjs object.UnstructuredSet, ) *cmdtesting.TestFactory { - tf := cmdtesting.NewTestFactory().WithNamespace(invInfo.namespace) + tf := cmdtesting.NewTestFactory().WithNamespace(invObj.GetNamespace()) mapper, err := tf.ToRESTMapper() require.NoError(t, err) @@ -170,7 +159,7 @@ func newTestFactory( } tf.UnstructuredClient = newFakeRESTClient(t, handlers) - tf.FakeDynamicClient = fakeDynamicClient(t, mapper, invInfo, objs...) + tf.FakeDynamicClient = fakeDynamicClient(t, mapper, invObj, objs...) return tf } @@ -283,9 +272,9 @@ func (g *genericHandler) handle(t *testing.T, req *http.Request) (*http.Response return nil, false, nil } -func newInventoryReactor(invInfo inventoryInfo) *inventoryReactor { +func newInventoryReactor(invObj *unstructured.Unstructured) *inventoryReactor { return &inventoryReactor{ - inventoryObj: invInfo.toUnstructured(), + inventoryObj: invObj, } } @@ -430,10 +419,10 @@ func (f *fakeInfoHelper) getClient(gv schema.GroupVersion) (resource.RESTClient, } // fakeDynamicClient returns a fake dynamic client. -func fakeDynamicClient(t *testing.T, mapper meta.RESTMapper, invInfo inventoryInfo, objs ...resourceInfo) *dynamicfake.FakeDynamicClient { +func fakeDynamicClient(t *testing.T, mapper meta.RESTMapper, invObj *unstructured.Unstructured, objs ...resourceInfo) *dynamicfake.FakeDynamicClient { fakeClient := dynamicfake.NewSimpleDynamicClient(scheme.Scheme) - invReactor := newInventoryReactor(invInfo) + invReactor := newInventoryReactor(invObj) invReactor.updateFakeDynamicClient(fakeClient) for i := range objs { diff --git a/pkg/apply/destroyer.go b/pkg/apply/destroyer.go index ae3c5f73..60e5ac03 100644 --- a/pkg/apply/destroyer.go +++ b/pkg/apply/destroyer.go @@ -8,6 +8,7 @@ import ( "fmt" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/discovery" @@ -79,10 +80,24 @@ func (d *Destroyer) Run(ctx context.Context, invInfo inventory.Info, options Des setDestroyerDefaults(&options) go func() { defer close(eventChannel) + inv, err := d.invClient.Get(ctx, invInfo, inventory.GetOptions{}) + if apierrors.IsNotFound(err) { + inv, err = d.invClient.NewInventory(invInfo) + } + if err != nil { + handleError(eventChannel, err) + return + } + if inv.Info().GetID() != invInfo.GetID() { + handleError(eventChannel, fmt.Errorf("expected inventory object to have inventory-id %q but got %q", + invInfo.GetID(), inv.Info().GetID())) + return + } + // Retrieve the objects to be deleted from the cluster. Second parameter is empty // because no local objects returns all inventory objects for deletion. emptyLocalObjs := object.UnstructuredSet{} - deleteObjs, err := d.pruner.GetPruneObjs(ctx, invInfo, emptyLocalObjs, prune.Options{ + deleteObjs, err := d.pruner.GetPruneObjs(ctx, inv, emptyLocalObjs, prune.Options{ DryRunStrategy: options.DryRunStrategy, }) if err != nil { @@ -123,6 +138,7 @@ func (d *Destroyer) Run(ctx context.Context, invInfo inventory.Info, options Des InfoHelper: d.infoHelper, Mapper: d.mapper, InvClient: d.invClient, + Inventory: inv, Collector: vCollector, PruneFilters: deleteFilters, } @@ -138,7 +154,6 @@ func (d *Destroyer) Run(ctx context.Context, invInfo inventory.Info, options Des // Build the ordered set of tasks to execute. taskQueue := taskBuilder. WithPruneObjects(deleteObjs). - WithInventory(invInfo). Build(taskContext, opts) klog.V(4).Infof("validation errors: %d", len(vCollector.Errors)) diff --git a/pkg/apply/destroyer_test.go b/pkg/apply/destroyer_test.go index 6712d2b0..4338f397 100644 --- a/pkg/apply/destroyer_test.go +++ b/pkg/apply/destroyer_test.go @@ -11,6 +11,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/inventory" pollevent "sigs.k8s.io/cli-utils/pkg/kstatus/polling/event" @@ -22,7 +24,7 @@ import ( func TestDestroyerCancel(t *testing.T) { testCases := map[string]struct { // inventory input to destroyer - invInfo inventoryInfo + invObj *unstructured.Unstructured // objects in the cluster clusterObjs object.UnstructuredSet // options input to destroyer.Run @@ -44,14 +46,15 @@ func TestDestroyerCancel(t *testing.T) { expectRunTimeout: true, runTimeout: 2 * time.Second, testTimeout: 30 * time.Second, - invInfo: inventoryInfo{ - name: "abc-123", - namespace: "test", - id: "test", - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "test", + }), + object.ObjMetadataSet{ testutil.ToIdentifier(t, resources["deployment"]), }, - }, + ), clusterObjs: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"], testutil.AddOwningInv(t, "test")), }, @@ -167,14 +170,15 @@ func TestDestroyerCancel(t *testing.T) { expectRunTimeout: false, runTimeout: 10 * time.Second, testTimeout: 30 * time.Second, - invInfo: inventoryInfo{ - name: "abc-123", - namespace: "test", - id: "test", - set: object.ObjMetadataSet{ + invObj: newInventoryObj( + inventory.NewSingleObjectInfo("test", types.NamespacedName{ + Name: "abc-123", + Namespace: "test", + }), + object.ObjMetadataSet{ testutil.ToIdentifier(t, resources["deployment"]), }, - }, + ), clusterObjs: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"], testutil.AddOwningInv(t, "test")), }, @@ -314,15 +318,13 @@ func TestDestroyerCancel(t *testing.T) { t.Run(tn, func(t *testing.T) { statusWatcher := newFakeWatcher(tc.statusEvents) - invInfo, err := tc.invInfo.toWrapped() - require.NoError(t, err) - cm, err := inventory.InvInfoToConfigMap(invInfo) + invInfo, err := inventory.ConfigMapToInventoryInfo(tc.invObj) require.NoError(t, err) destroyer := newTestDestroyer(t, - tc.invInfo, + tc.invObj, // Add the inventory to the cluster (to allow deletion) - append(tc.clusterObjs, cm), + append(tc.clusterObjs, tc.invObj), statusWatcher, ) diff --git a/pkg/apply/prune/prune.go b/pkg/apply/prune/prune.go index cd8a085d..45af0ae3 100644 --- a/pkg/apply/prune/prune.go +++ b/pkg/apply/prune/prune.go @@ -235,17 +235,13 @@ func (p *Pruner) removeInventoryAnnotation(ctx context.Context, obj *unstructure // if one occurs. func (p *Pruner) GetPruneObjs( ctx context.Context, - inv inventory.Info, + inv inventory.Inventory, objs object.UnstructuredSet, opts Options, ) (object.UnstructuredSet, error) { ids := object.UnstructuredSetToObjMetadataSet(objs) - invIDs, err := p.InvClient.GetClusterObjs(ctx, inv) - if err != nil { - return nil, err - } // only return objects that were in the inventory but not in the object set - ids = invIDs.Diff(ids) + ids = inv.GetObjectRefs().Diff(ids) objs = object.UnstructuredSet{} for _, id := range ids { pruneObj, err := p.getObject(ctx, id) diff --git a/pkg/apply/prune/prune_test.go b/pkg/apply/prune/prune_test.go index b62fcb62..458a65a1 100644 --- a/pkg/apply/prune/prune_test.go +++ b/pkg/apply/prune/prune_test.go @@ -122,21 +122,14 @@ metadata: // Returns a inventory object with the inventory set from // the passed "children". -func createInventoryInfo(children ...*unstructured.Unstructured) (inventory.Info, error) { +func newInventory(children ...*unstructured.Unstructured) (inventory.Inventory, error) { inventoryObjCopy := inventoryObj.DeepCopy() wrappedInv, err := inventory.ConfigMapToInventoryObj(inventoryObjCopy) if err != nil { return nil, err } - objs := object.UnstructuredSetToObjMetadataSet(children) - if err := wrappedInv.Store(objs, nil); err != nil { - return nil, err - } - obj, err := wrappedInv.GetObject() - if err != nil { - return nil, err - } - return inventory.ConfigMapToInventoryInfo(obj) + wrappedInv.SetObjectRefs(object.UnstructuredSetToObjMetadataSet(children)) + return wrappedInv, nil } // podDeletionPrevention object contains the "on-remove:keep" lifecycle directive. @@ -862,7 +855,7 @@ func TestGetPruneObjs(t *testing.T) { Mapper: testrestmapper.TestOnlyStaticRESTMapper(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...), } - currentInventory, err := createInventoryInfo(tc.prevInventory...) + currentInventory, err := newInventory(tc.prevInventory...) require.NoError(t, err) actualObjs, err := po.GetPruneObjs(t.Context(), currentInventory, tc.localObjs, Options{}) if err != nil { diff --git a/pkg/apply/solver/solver.go b/pkg/apply/solver/solver.go index 2c26ee09..132c1dc1 100644 --- a/pkg/apply/solver/solver.go +++ b/pkg/apply/solver/solver.go @@ -43,7 +43,8 @@ type TaskQueueBuilder struct { OpenAPIGetter discovery.OpenAPISchemaInterface InfoHelper info.Helper Mapper meta.RESTMapper - InvClient inventory.Client + Inventory inventory.Inventory + InvClient inventory.WriteClient // Collector is used to collect validation errors and invalid objects. // Invalid objects will be filtered and not be injected into tasks. Collector *validation.Collector @@ -56,7 +57,6 @@ type TaskQueueBuilder struct { pruneCounter int waitCounter int - invInfo inventory.Info applyObjs object.UnstructuredSet pruneObjs object.UnstructuredSet } @@ -100,12 +100,6 @@ type Options struct { InventoryPolicy inventory.Policy } -// WithInventory sets the inventory info and returns the builder for chaining. -func (t *TaskQueueBuilder) WithInventory(inv inventory.Info) *TaskQueueBuilder { - t.invInfo = inv - return t -} - // WithApplyObjects sets the apply objects and returns the builder for chaining. func (t *TaskQueueBuilder) WithApplyObjects(applyObjs object.UnstructuredSet) *TaskQueueBuilder { t.applyObjs = applyObjs @@ -162,7 +156,7 @@ func (t *TaskQueueBuilder) Build(taskContext *taskrunner.TaskContext, o Options) InvClient: t.InvClient, DynamicClient: t.DynamicClient, Mapper: t.Mapper, - InvInfo: t.invInfo, + Inventory: t.Inventory, Objects: applyObjs, DryRun: o.DryRunStrategy, }) @@ -213,7 +207,6 @@ func (t *TaskQueueBuilder) Build(taskContext *taskrunner.TaskContext, o Options) } } - prevInvIDs, _ := t.InvClient.GetClusterObjs(taskContext.Context(), t.invInfo) klog.V(2).Infoln("adding delete/update inventory task") var taskName string if o.Destroy { @@ -222,12 +215,11 @@ func (t *TaskQueueBuilder) Build(taskContext *taskrunner.TaskContext, o Options) taskName = "inventory-set-0" } tasks = append(tasks, &task.DeleteOrUpdateInvTask{ - TaskName: taskName, - InvClient: t.InvClient, - InvInfo: t.invInfo, - PrevInventory: prevInvIDs, - DryRun: o.DryRunStrategy, - Destroy: o.Destroy, + TaskName: taskName, + Inventory: t.Inventory, + InvClient: t.InvClient, + DryRun: o.DryRunStrategy, + Destroy: o.Destroy, }) return &TaskQueue{tasks: tasks} diff --git a/pkg/apply/solver/solver_test.go b/pkg/apply/solver/solver_test.go index 39cfb727..b0ebadbf 100644 --- a/pkg/apply/solver/solver_test.go +++ b/pkg/apply/solver/solver_test.go @@ -10,7 +10,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/cli-utils/pkg/apis/actuation" "sigs.k8s.io/cli-utils/pkg/apply/prune" @@ -117,7 +116,8 @@ func newInvObject(name, namespace, inventoryID string) *unstructured.Unstructure common.InventoryLabel: inventoryID, }, }, - "data": map[string]string{}, + // data must be map[string]interface{} for generic Unstructured methods + "data": map[string]interface{}{}, }, } } @@ -131,9 +131,7 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { inventoryInfoComparer(), ) - invInfo, err := inventory.ConfigMapToInventoryInfo(newInvObject( - "abc-123", "default", "test")) - require.NoError(t, err) + uObj := newInvObject("abc-123", "default", "test") testCases := map[string]struct { applyObjs []*unstructured.Unstructured @@ -148,14 +146,11 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.DeleteOrUpdateInvTask{ - TaskName: "inventory-set-0", - InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{}, + TaskName: "inventory-set-0", + InvClient: &inventory.FakeClient{}, }, }, }, @@ -167,7 +162,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, @@ -188,10 +182,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["deployment"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -214,7 +204,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), testutil.Unstructured(t, resources["secret"]), @@ -239,11 +228,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["deployment"]), - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -277,7 +261,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["secret"]), testutil.Unstructured(t, resources["deployment"]), @@ -303,11 +286,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["secret"]), - testutil.ToIdentifier(t, resources["deployment"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -343,7 +321,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), testutil.Unstructured(t, resources["secret"]), @@ -361,12 +338,7 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["deployment"]), - testutil.ToIdentifier(t, resources["secret"]), - }, - DryRun: common.DryRunClient, + DryRun: common.DryRunClient, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -402,7 +374,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["pod"]), testutil.Unstructured(t, resources["default-pod"]), @@ -420,12 +391,7 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["pod"]), - testutil.ToIdentifier(t, resources["default-pod"]), - }, - DryRun: common.DryRunServer, + DryRun: common.DryRunServer, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -457,7 +423,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["crontab1"]), testutil.Unstructured(t, resources["crd"]), @@ -497,12 +462,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["crontab1"]), - testutil.ToIdentifier(t, resources["crd"]), - testutil.ToIdentifier(t, resources["crontab2"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -546,7 +505,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["crontab1"]), testutil.Unstructured(t, resources["crd"]), @@ -572,13 +530,7 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["crontab1"]), - testutil.ToIdentifier(t, resources["crd"]), - testutil.ToIdentifier(t, resources["crontab2"]), - }, - DryRun: common.DryRunClient, + DryRun: common.DryRunClient, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -618,7 +570,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["namespace"]), testutil.Unstructured(t, resources["pod"]), @@ -658,12 +609,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["namespace"]), - testutil.ToIdentifier(t, resources["pod"]), - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -703,7 +648,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"], testutil.AddDependsOn(t, testutil.ToIdentifier(t, resources["secret"]))), @@ -742,11 +686,6 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["deployment"]), - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -798,6 +737,7 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { mapper := testutil.NewFakeRESTMapper() + inventoryObj := inventory.NewUnstructuredInventory(uObj) // inject mapper for equality comparison for _, t := range tc.expectedTasks { switch typedTask := t.(type) { @@ -807,6 +747,9 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { typedTask.Mapper = mapper case *task.InvAddTask: typedTask.Mapper = mapper + typedTask.Inventory = inventoryObj + case *task.DeleteOrUpdateInvTask: + typedTask.Inventory = inventoryObj } } @@ -816,12 +759,12 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { tqb := TaskQueueBuilder{ Pruner: pruner, Mapper: mapper, + Inventory: inventoryObj, InvClient: fakeInvClient, Collector: vCollector, } taskContext := taskrunner.NewTaskContext(t.Context(), nil, nil) - tq := tqb.WithInventory(invInfo). - WithApplyObjects(tc.applyObjs). + tq := tqb.WithApplyObjects(tc.applyObjs). Build(taskContext, tc.options) err := vCollector.ToError() if tc.expectedError != nil { @@ -831,7 +774,7 @@ func TestTaskQueueBuilder_ApplyBuild(t *testing.T) { assert.NoError(t, err) asserter.Equal(t, tc.expectedTasks, tq.tasks) - actualStatus := taskContext.InventoryManager().Inventory().Status.Objects + actualStatus := taskContext.InventoryManager().Inventory().ObjectStatuses testutil.AssertEqual(t, tc.expectedStatus, actualStatus) }) } @@ -846,9 +789,7 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { inventoryInfoComparer(), ) - invInfo, err := inventory.ConfigMapToInventoryInfo(newInvObject( - "abc-123", "default", "test")) - require.NoError(t, err) + uObj := newInvObject("abc-123", "default", "test") testCases := map[string]struct { pruneObjs []*unstructured.Unstructured @@ -864,14 +805,11 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.DeleteOrUpdateInvTask{ - TaskName: "inventory-set-0", - InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{}, + TaskName: "inventory-set-0", + InvClient: &inventory.FakeClient{}, }, }, }, @@ -884,7 +822,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.PruneTask{ @@ -903,10 +840,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["default-pod"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -930,7 +863,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.PruneTask{ @@ -951,11 +883,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["default-pod"]), - testutil.ToIdentifier(t, resources["pod"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -989,7 +916,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.PruneTask{ @@ -1022,11 +948,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["pod"]), - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1060,7 +981,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.PruneTask{ @@ -1080,10 +1000,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["pod"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1112,7 +1028,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, DryRun: common.DryRunServer, }, @@ -1127,12 +1042,7 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["pod"]), - testutil.ToIdentifier(t, resources["default-pod"]), - }, - DryRun: common.DryRunServer, + DryRun: common.DryRunServer, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1166,7 +1076,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.PruneTask{ @@ -1200,12 +1109,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["crontab1"]), - testutil.ToIdentifier(t, resources["crd"]), - testutil.ToIdentifier(t, resources["crontab2"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1250,7 +1153,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, DryRun: common.DryRunClient, }, @@ -1272,13 +1174,7 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["crontab1"]), - testutil.ToIdentifier(t, resources["crd"]), - testutil.ToIdentifier(t, resources["crontab2"]), - }, - DryRun: common.DryRunClient, + DryRun: common.DryRunClient, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1319,7 +1215,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.PruneTask{ @@ -1353,12 +1248,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["namespace"]), - testutil.ToIdentifier(t, resources["pod"]), - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1427,7 +1316,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{}, }, &task.PruneTask{ @@ -1447,10 +1335,6 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["pod"]), - }, }, }, expectedError: validation.NewError( @@ -1475,6 +1359,7 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { mapper := testutil.NewFakeRESTMapper() + inventoryObj := inventory.NewUnstructuredInventory(uObj) // inject mapper & pruner for equality comparison for _, t := range tc.expectedTasks { switch typedTask := t.(type) { @@ -1484,6 +1369,9 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { typedTask.Mapper = mapper case *task.InvAddTask: typedTask.Mapper = mapper + typedTask.Inventory = inventoryObj + case *task.DeleteOrUpdateInvTask: + typedTask.Inventory = inventoryObj } } @@ -1494,11 +1382,11 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { Pruner: pruner, Mapper: mapper, InvClient: fakeInvClient, + Inventory: inventoryObj, Collector: vCollector, } taskContext := taskrunner.NewTaskContext(t.Context(), nil, nil) - tq := tqb.WithInventory(invInfo). - WithPruneObjects(tc.pruneObjs). + tq := tqb.WithPruneObjects(tc.pruneObjs). Build(taskContext, tc.options) err := vCollector.ToError() if tc.expectedError != nil { @@ -1508,7 +1396,7 @@ func TestTaskQueueBuilder_PruneBuild(t *testing.T) { assert.NoError(t, err) asserter.Equal(t, tc.expectedTasks, tq.tasks) - actualStatus := taskContext.InventoryManager().Inventory().Status.Objects + actualStatus := taskContext.InventoryManager().Inventory().ObjectStatuses testutil.AssertEqual(t, tc.expectedStatus, actualStatus) }) } @@ -1523,9 +1411,7 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { inventoryInfoComparer(), ) - invInfo, err := inventory.ConfigMapToInventoryInfo(newInvObject( - "abc-123", "default", "test")) - require.NoError(t, err) + uObj := newInvObject("abc-123", "default", "test") testCases := map[string]struct { inventoryIDs object.ObjMetadataSet @@ -1551,7 +1437,6 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, @@ -1585,10 +1470,6 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1625,7 +1506,6 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, @@ -1646,10 +1526,6 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1684,7 +1560,6 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"], testutil.AddDependsOn(t, testutil.ToIdentifier(t, resources["secret"]))), @@ -1720,10 +1595,6 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1765,7 +1636,6 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { &task.InvAddTask{ TaskName: "inventory-add-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, Objects: object.UnstructuredSet{ testutil.Unstructured(t, resources["deployment"]), }, @@ -1800,10 +1670,6 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { &task.DeleteOrUpdateInvTask{ TaskName: "inventory-set-0", InvClient: &inventory.FakeClient{}, - InvInfo: invInfo, - PrevInventory: object.ObjMetadataSet{ - testutil.ToIdentifier(t, resources["secret"]), - }, }, }, expectedStatus: []actuation.ObjectStatus{ @@ -1830,6 +1696,7 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { mapper := testutil.NewFakeRESTMapper() + inventoryObj := inventory.NewUnstructuredInventory(uObj) // inject mapper & pruner for equality comparison for _, t := range tc.expectedTasks { switch typedTask := t.(type) { @@ -1841,6 +1708,9 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { typedTask.Mapper = mapper case *task.InvAddTask: typedTask.Mapper = mapper + typedTask.Inventory = inventoryObj + case *task.DeleteOrUpdateInvTask: + typedTask.Inventory = inventoryObj } } @@ -1850,11 +1720,11 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { Pruner: pruner, Mapper: mapper, InvClient: fakeInvClient, + Inventory: inventoryObj, Collector: vCollector, } taskContext := taskrunner.NewTaskContext(t.Context(), nil, nil) - tq := tqb.WithInventory(invInfo). - WithApplyObjects(tc.applyObjs). + tq := tqb.WithApplyObjects(tc.applyObjs). WithPruneObjects(tc.pruneObjs). Build(taskContext, tc.options) @@ -1867,7 +1737,7 @@ func TestTaskQueueBuilder_ApplyPruneBuild(t *testing.T) { asserter.Equal(t, tc.expectedTasks, tq.tasks) - actualStatus := taskContext.InventoryManager().Inventory().Status.Objects + actualStatus := taskContext.InventoryManager().Inventory().ObjectStatuses testutil.AssertEqual(t, tc.expectedStatus, actualStatus) }) } @@ -1906,9 +1776,7 @@ func fakeClientComparer() cmp.Option { // inventoryInfoComparer allows comparison of inventory.Info, ignoring impl. func inventoryInfoComparer() cmp.Option { return cmp.Comparer(func(x, y inventory.Info) bool { - return x.ID() == y.ID() && - x.Name() == y.Name() && - x.Namespace() == y.Namespace() && - x.Strategy() == y.Strategy() + return x.GetID() == y.GetID() && + x.GetNamespace() == y.GetNamespace() }) } diff --git a/pkg/apply/task/delete_inv_task_test.go b/pkg/apply/task/delete_inv_task_test.go index a2ea8eb9..643dd1e9 100644 --- a/pkg/apply/task/delete_inv_task_test.go +++ b/pkg/apply/task/delete_inv_task_test.go @@ -81,7 +81,7 @@ func TestDeleteInvTask(t *testing.T) { } for name, tc := range testCases { t.Run(name, func(t *testing.T) { - client := inventory.NewFakeClient(object.ObjMetadataSet{}) + client := inventory.NewFakeClient(tc.prevInventory) client.Err = tc.err eventChannel := make(chan event.Event) resourceCache := cache.NewResourceCacheMap() @@ -105,12 +105,11 @@ func TestDeleteInvTask(t *testing.T) { } task := DeleteOrUpdateInvTask{ - TaskName: taskName, - InvClient: client, - InvInfo: nil, - DryRun: common.DryRunNone, - PrevInventory: tc.prevInventory, - Destroy: true, + TaskName: taskName, + InvClient: client, + DryRun: common.DryRunNone, + Inventory: client.Inv, + Destroy: true, } if taskName != task.Name() { t.Errorf("expected task name (%s), got (%s)", taskName, task.Name()) @@ -121,15 +120,17 @@ func TestDeleteInvTask(t *testing.T) { if tc.err != result.Err { t.Errorf("running DeleteOrUpdateInvTask expected error (%s), got (%s)", tc.err, result.Err) } - } else { - if result.Err != nil { - t.Errorf("unexpected error running DeleteOrUpdateInvTask: %s", result.Err) - } + return + } + if result.Err != nil { + t.Errorf("unexpected error running DeleteOrUpdateInvTask: %s", result.Err) + } + actual, _ := client.Get(context.TODO(), nil, inventory.GetOptions{}) + if len(tc.expectedObjs) > 0 { + testutil.AssertEqual(t, tc.expectedObjs, actual.GetObjectRefs(), + "Actual cluster objects (%d) do not match expected cluster objects (%d)", + len(actual.GetObjectRefs()), len(tc.expectedObjs)) } - actual, _ := client.GetClusterObjs(context.Background(), nil) - testutil.AssertEqual(t, tc.expectedObjs, actual, - "Actual cluster objects (%d) do not match expected cluster objects (%d)", - len(actual), len(tc.expectedObjs)) }) } } diff --git a/pkg/apply/task/inv_add_task.go b/pkg/apply/task/inv_add_task.go index 9f562477..c765eb7c 100644 --- a/pkg/apply/task/inv_add_task.go +++ b/pkg/apply/task/inv_add_task.go @@ -14,6 +14,7 @@ import ( "k8s.io/client-go/dynamic" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/util" + "sigs.k8s.io/cli-utils/pkg/apis/actuation" "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/apply/taskrunner" "sigs.k8s.io/cli-utils/pkg/common" @@ -30,10 +31,10 @@ var ( // before the actual object is applied. type InvAddTask struct { TaskName string - InvClient inventory.Client + Inventory inventory.Inventory + InvClient inventory.WriteClient DynamicClient dynamic.Interface Mapper meta.RESTMapper - InvInfo inventory.Info Objects object.UnstructuredSet DryRun common.DryRunStrategy } @@ -60,8 +61,9 @@ func (i *InvAddTask) Start(taskContext *taskrunner.TaskContext) { return } // If the inventory is namespaced, ensure the namespace exists - if i.InvInfo.Namespace() != "" { - if invNamespace := inventoryNamespaceInSet(i.InvInfo, i.Objects); invNamespace != nil { + invInfo := i.Inventory.Info() + if invInfo.GetNamespace() != "" { + if invNamespace := inventoryNamespaceInSet(invInfo, i.Objects); invNamespace != nil { if err := i.createNamespace(taskContext.Context(), invNamespace, i.DryRun); err != nil { err = fmt.Errorf("failed to create inventory namespace: %w", err) i.sendTaskResult(taskContext, err) @@ -71,7 +73,18 @@ func (i *InvAddTask) Start(taskContext *taskrunner.TaskContext) { } klog.V(4).Infof("merging %d local objects into inventory", len(i.Objects)) currentObjs := object.UnstructuredSetToObjMetadataSet(i.Objects) - _, err := i.InvClient.Merge(taskContext.Context(), i.InvInfo, currentObjs, i.DryRun) + inventoryObjs := i.Inventory.GetObjectRefs() + unionObjs := inventoryObjs.Union(currentObjs) + pruneObjs := inventoryObjs.Diff(currentObjs) + + i.Inventory.SetObjectRefs(unionObjs) + i.Inventory.SetObjectStatuses(i.getObjStatus(pruneObjs, unionObjs)) + + var err error + if !i.DryRun.ClientOrServerDryRun() { + err = i.InvClient.CreateOrUpdate(taskContext.Context(), i.Inventory, inventory.UpdateOptions{}) + } + i.sendTaskResult(taskContext, err) }() } @@ -89,12 +102,12 @@ func inventoryNamespaceInSet(inv inventory.Info, objs object.UnstructuredSet) *u if inv == nil { return nil } - invNamespace := inv.Namespace() + invNamespace := inv.GetNamespace() for _, obj := range objs { gvk := obj.GetObjectKind().GroupVersionKind() if gvk == namespaceGVKv1 && obj.GetName() == invNamespace { - inventory.AddInventoryIDAnnotation(obj, inv) + inventory.AddInventoryIDAnnotation(obj, inv.GetID()) return obj } } @@ -136,3 +149,27 @@ func (i *InvAddTask) sendTaskResult(taskContext *taskrunner.TaskContext, err err Err: err, } } + +// getObjStatus returns the list of object status +// at the beginning of an apply process. +func (i *InvAddTask) getObjStatus(pruneIDs, unionIDs []object.ObjMetadata) []actuation.ObjectStatus { + var status []actuation.ObjectStatus + pruneMap := make(map[object.ObjMetadata]bool) + for _, obj := range pruneIDs { + pruneMap[obj] = true + } + for _, obj := range unionIDs { + strategy := actuation.ActuationStrategyApply + if isPruneObj, ok := pruneMap[obj]; ok && isPruneObj { + strategy = actuation.ActuationStrategyDelete + } + status = append(status, + actuation.ObjectStatus{ + ObjectReference: inventory.ObjectReferenceFromObjMetadata(obj), + Strategy: strategy, + Actuation: actuation.ActuationPending, + Reconcile: actuation.ReconcilePending, + }) + } + return status +} diff --git a/pkg/apply/task/inv_add_task_test.go b/pkg/apply/task/inv_add_task_test.go index 4564f95a..ce2797bb 100644 --- a/pkg/apply/task/inv_add_task_test.go +++ b/pkg/apply/task/inv_add_task_test.go @@ -7,10 +7,12 @@ import ( "testing" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" clienttesting "k8s.io/client-go/testing" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" + "sigs.k8s.io/cli-utils/pkg/apis/actuation" "sigs.k8s.io/cli-utils/pkg/apply/cache" "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/apply/taskrunner" @@ -19,15 +21,13 @@ import ( "sigs.k8s.io/cli-utils/pkg/object" ) -var namespace = "test-namespace" - var inventoryObj = &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", "metadata": map[string]interface{}{ - "name": "test-inventory-obj", - "namespace": namespace, + "name": inventory.TestInventoryName, + "namespace": inventory.TestInventoryNamespace, "labels": map[string]interface{}{ common.InventoryLabel: "test-app-label", }, @@ -41,7 +41,7 @@ var obj1 = &unstructured.Unstructured{ "kind": "Pod", "metadata": map[string]interface{}{ "name": "obj1", - "namespace": namespace, + "namespace": inventory.TestInventoryNamespace, }, }, } @@ -52,7 +52,7 @@ var obj2 = &unstructured.Unstructured{ "kind": "Job", "metadata": map[string]interface{}{ "name": "obj2", - "namespace": namespace, + "namespace": inventory.TestInventoryNamespace, }, }, } @@ -73,7 +73,7 @@ var nsObj = &unstructured.Unstructured{ "apiVersion": "v1", "kind": "Namespace", "metadata": map[string]interface{}{ - "name": namespace, + "name": inventory.TestInventoryNamespace, }, }, } @@ -81,8 +81,6 @@ var nsObj = &unstructured.Unstructured{ const taskName = "test-inventory-task" func TestInvAddTask(t *testing.T) { - localInv, err := inventory.ConfigMapToInventoryInfo(inventoryObj) - require.NoError(t, err) id1 := object.UnstructuredToObjMetadata(obj1) id2 := object.UnstructuredToObjMetadata(obj2) id3 := object.UnstructuredToObjMetadata(obj3) @@ -92,6 +90,7 @@ func TestInvAddTask(t *testing.T) { initialObjs object.ObjMetadataSet applyObjs []*unstructured.Unstructured expectedObjs object.ObjMetadataSet + expectedObjStatuses []actuation.ObjectStatus reactorError error expectCreateNamespace bool }{ @@ -104,26 +103,78 @@ func TestInvAddTask(t *testing.T) { initialObjs: object.ObjMetadataSet{}, applyObjs: []*unstructured.Unstructured{obj1}, expectedObjs: object.ObjMetadataSet{id1}, + expectedObjStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(id1), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationPending, + Reconcile: actuation.ReconcilePending, + }, + }, }, "one initial inventory, no apply object; one merged inventory": { initialObjs: object.ObjMetadataSet{id2}, applyObjs: []*unstructured.Unstructured{}, expectedObjs: object.ObjMetadataSet{id2}, + expectedObjStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(id2), + Strategy: actuation.ActuationStrategyDelete, + Actuation: actuation.ActuationPending, + Reconcile: actuation.ReconcilePending, + }, + }, }, "one initial inventory, one apply object; one merged inventory": { initialObjs: object.ObjMetadataSet{id3}, applyObjs: []*unstructured.Unstructured{obj3}, expectedObjs: object.ObjMetadataSet{id3}, + expectedObjStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(id3), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationPending, + Reconcile: actuation.ReconcilePending, + }, + }, }, "three initial inventory, two same objects; three merged inventory": { initialObjs: object.ObjMetadataSet{id1, id2, id3}, applyObjs: []*unstructured.Unstructured{obj2, obj3}, expectedObjs: object.ObjMetadataSet{id1, id2, id3}, + expectedObjStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(id1), + Strategy: actuation.ActuationStrategyDelete, + Actuation: actuation.ActuationPending, + Reconcile: actuation.ReconcilePending, + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(id2), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationPending, + Reconcile: actuation.ReconcilePending, + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(id3), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationPending, + Reconcile: actuation.ReconcilePending, + }, + }, }, "namespace of inventory inside inventory": { - initialObjs: object.ObjMetadataSet{}, - applyObjs: []*unstructured.Unstructured{nsObj}, - expectedObjs: object.ObjMetadataSet{idNs}, + initialObjs: object.ObjMetadataSet{}, + applyObjs: []*unstructured.Unstructured{nsObj}, + expectedObjs: object.ObjMetadataSet{idNs}, + expectedObjStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(idNs), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationPending, + Reconcile: actuation.ReconcilePending, + }, + }, expectCreateNamespace: true, }, } @@ -134,7 +185,7 @@ func TestInvAddTask(t *testing.T) { eventChannel := make(chan event.Event) resourceCache := cache.NewResourceCacheMap() taskContext := taskrunner.NewTaskContext(t.Context(), eventChannel, resourceCache) - tf := cmdtesting.NewTestFactory().WithNamespace(namespace) + tf := cmdtesting.NewTestFactory().WithNamespace(inventory.TestInventoryNamespace) defer tf.Cleanup() createdNamespace := false @@ -151,7 +202,7 @@ func TestInvAddTask(t *testing.T) { task := InvAddTask{ TaskName: taskName, InvClient: client, - InvInfo: localInv, + Inventory: client.Inv, Objects: tc.applyObjs, DynamicClient: tf.FakeDynamicClient, Mapper: mapper, @@ -168,10 +219,14 @@ func TestInvAddTask(t *testing.T) { if result.Err != nil { t.Errorf("unexpected error running InvAddTask: %s", result.Err) } - actual, _ := client.GetClusterObjs(t.Context(), nil) - if !tc.expectedObjs.Equal(actual) { + // argument doesn't matter for fake client, it always returns cached obj + actual, _ := client.Get(t.Context(), nil, inventory.GetOptions{}) + if !tc.expectedObjs.Equal(actual.GetObjectRefs()) { t.Errorf("expected merged inventory (%s), got (%s)", tc.expectedObjs, actual) } + if !equality.Semantic.DeepEqual(tc.expectedObjStatuses, actual.GetObjectStatuses()) { + t.Errorf("expected object statuses (%v), got (%v)", tc.expectedObjStatuses, actual.GetObjectStatuses()) + } if createdNamespace != tc.expectCreateNamespace { t.Errorf("expected create namespace %v, got %v", tc.expectCreateNamespace, createdNamespace) } @@ -182,7 +237,7 @@ func TestInvAddTask(t *testing.T) { func TestInventoryNamespaceInSet(t *testing.T) { localInv, err := inventory.ConfigMapToInventoryInfo(inventoryObj) require.NoError(t, err) - inventoryNamespace := createNamespace(namespace) + inventoryNamespace := createNamespace(inventory.TestInventoryNamespace) tests := map[string]struct { inv inventory.Info diff --git a/pkg/apply/task/inv_set_task.go b/pkg/apply/task/inv_set_task.go index 0323ab5f..c15fcc4f 100644 --- a/pkg/apply/task/inv_set_task.go +++ b/pkg/apply/task/inv_set_task.go @@ -18,11 +18,10 @@ import ( // DeleteOrUpdateInvTask encapsulates structures necessary to set the // inventory references at the end of the apply/prune. type DeleteOrUpdateInvTask struct { - TaskName string - InvClient inventory.Client - InvInfo inventory.Info - PrevInventory object.ObjMetadataSet - DryRun common.DryRunStrategy + TaskName string + Inventory inventory.Inventory + InvClient inventory.WriteClient + DryRun common.DryRunStrategy // if Destroy is set, the inventory will be deleted if all objects were successfully pruned Destroy bool } @@ -97,12 +96,13 @@ func (i *DeleteOrUpdateInvTask) updateInventory(taskContext *taskrunner.TaskCont klog.V(4).Infof("set inventory %d successful applies", len(appliedObjs)) invObjs = invObjs.Union(appliedObjs) + prevInventory := i.Inventory.GetObjectRefs() // If an object failed to apply and was previously stored in the inventory, // then keep it in the inventory so it can be applied/pruned next time. // This will remove new resources that failed to apply from the inventory, // because even tho they were added by InvAddTask, the PrevInventory // represents the inventory before the pipeline has run. - applyFailures := i.PrevInventory.Intersection(im.FailedApplies()) + applyFailures := prevInventory.Intersection(im.FailedApplies()) klog.V(4).Infof("keep in inventory %d failed applies", len(applyFailures)) invObjs = invObjs.Union(applyFailures) @@ -111,7 +111,7 @@ func (i *DeleteOrUpdateInvTask) updateInventory(taskContext *taskrunner.TaskCont // It's likely that all the skipped applies are already in the inventory, // because the apply filters all currently depend on cluster state, // but we're doing the intersection anyway just to be sure. - applySkips := i.PrevInventory.Intersection(im.SkippedApplies()) + applySkips := prevInventory.Intersection(im.SkippedApplies()) klog.V(4).Infof("keep in inventory %d skipped applies", len(applySkips)) invObjs = invObjs.Union(applySkips) @@ -120,7 +120,7 @@ func (i *DeleteOrUpdateInvTask) updateInventory(taskContext *taskrunner.TaskCont // It's likely that all the delete failures are already in the inventory, // because the set of resources to prune comes from the inventory, // but we're doing the intersection anyway just to be sure. - pruneFailures := i.PrevInventory.Intersection(im.FailedDeletes()) + pruneFailures := prevInventory.Intersection(im.FailedDeletes()) klog.V(4).Infof("set inventory %d failed prunes", len(pruneFailures)) invObjs = invObjs.Union(pruneFailures) @@ -129,19 +129,19 @@ func (i *DeleteOrUpdateInvTask) updateInventory(taskContext *taskrunner.TaskCont // It's likely that all the skipped deletes are already in the inventory, // because the set of resources to prune comes from the inventory, // but we're doing the intersection anyway just to be sure. - pruneSkips := i.PrevInventory.Intersection(im.SkippedDeletes()) + pruneSkips := prevInventory.Intersection(im.SkippedDeletes()) klog.V(4).Infof("keep in inventory %d skipped prunes", len(pruneSkips)) invObjs = invObjs.Union(pruneSkips) // If an object failed to reconcile and was previously stored in the inventory, // then keep it in the inventory so it can be waited on next time. - reconcileFailures := i.PrevInventory.Intersection(im.FailedReconciles()) + reconcileFailures := prevInventory.Intersection(im.FailedReconciles()) klog.V(4).Infof("set inventory %d failed reconciles", len(reconcileFailures)) invObjs = invObjs.Union(reconcileFailures) // If an object timed out reconciling and was previously stored in the inventory, // then keep it in the inventory so it can be waited on next time. - reconcileTimeouts := i.PrevInventory.Intersection(im.TimeoutReconciles()) + reconcileTimeouts := prevInventory.Intersection(im.TimeoutReconciles()) klog.V(4).Infof("keep in inventory %d timeout reconciles", len(reconcileTimeouts)) invObjs = invObjs.Union(reconcileTimeouts) @@ -152,24 +152,38 @@ func (i *DeleteOrUpdateInvTask) updateInventory(taskContext *taskrunner.TaskCont // If an object is invalid and was previously stored in the inventory, // then keep it in the inventory so it can be applied/pruned next time. - invalidObjects := i.PrevInventory.Intersection(taskContext.InvalidObjects()) + invalidObjects := prevInventory.Intersection(taskContext.InvalidObjects()) klog.V(4).Infof("keep in inventory %d invalid objects", len(invalidObjects)) invObjs = invObjs.Union(invalidObjects) klog.V(4).Infof("get the apply status for %d objects", len(invObjs)) - objStatus := taskContext.InventoryManager().Inventory().Status.Objects + objStatus := taskContext.InventoryManager().Inventory().ObjectStatuses klog.V(4).Infof("set inventory %d total objects", len(invObjs)) - err := i.InvClient.Replace(taskContext.Context(), i.InvInfo, invObjs, objStatus, i.DryRun) + // Exit before updating the inventory, but after logging the above changes + if i.DryRun.ClientOrServerDryRun() { + klog.V(4).Infoln("dry-run replace inventory object: not applied") + return nil + } + + i.Inventory.SetObjectRefs(invObjs) + i.Inventory.SetObjectStatuses(objStatus) + if err := i.InvClient.CreateOrUpdate(taskContext.Context(), i.Inventory, inventory.UpdateOptions{}); err != nil { + return err + } klog.V(2).Infof("inventory set task completing (name: %q)", i.TaskName) - return err + return nil } // deleteInventory deletes the inventory object from the cluster. func (i *DeleteOrUpdateInvTask) deleteInventory(ctx context.Context) error { klog.V(2).Infof("delete inventory task starting (name: %q)", i.Name()) - err := i.InvClient.DeleteInventoryObj(ctx, i.InvInfo, i.DryRun) + if i.DryRun.ClientOrServerDryRun() { + klog.V(4).Infoln("dry-run delete inventory object: not deleted") + return nil + } + err := i.InvClient.Delete(ctx, i.Inventory.Info(), inventory.DeleteOptions{}) // Not found is not error, since this means it was already deleted. if apierrors.IsNotFound(err) { err = nil diff --git a/pkg/apply/task/inv_set_task_test.go b/pkg/apply/task/inv_set_task_test.go index 9f6b9020..dfc3993e 100644 --- a/pkg/apply/task/inv_set_task_test.go +++ b/pkg/apply/task/inv_set_task_test.go @@ -191,17 +191,16 @@ func TestInvSetTask(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - client := inventory.NewFakeClient(object.ObjMetadataSet{}) + client := inventory.NewFakeClient(tc.prevInventory) eventChannel := make(chan event.Event) resourceCache := cache.NewResourceCacheMap() taskContext := taskrunner.NewTaskContext(t.Context(), eventChannel, resourceCache) task := DeleteOrUpdateInvTask{ - TaskName: taskName, - InvClient: client, - InvInfo: nil, - PrevInventory: tc.prevInventory, - Destroy: false, + TaskName: taskName, + InvClient: client, + Inventory: client.Inv, + Destroy: false, } im := taskContext.InventoryManager() for _, applyObj := range tc.appliedObjs { @@ -246,10 +245,10 @@ func TestInvSetTask(t *testing.T) { if result.Err != nil { t.Errorf("unexpected error running InvAddTask: %s", result.Err) } - actual, _ := client.GetClusterObjs(t.Context(), nil) - testutil.AssertEqual(t, tc.expectedObjs, actual, + actual, _ := client.Get(t.Context(), nil, inventory.GetOptions{}) + testutil.AssertEqual(t, tc.expectedObjs, actual.GetObjectRefs(), "Actual cluster objects (%d) do not match expected cluster objects (%d)", - len(actual), len(tc.expectedObjs)) + len(actual.GetObjectRefs()), len(tc.expectedObjs)) }) } } diff --git a/pkg/apply/taskrunner/task_test.go b/pkg/apply/taskrunner/task_test.go index 45c4646a..21867b46 100644 --- a/pkg/apply/taskrunner/task_test.go +++ b/pkg/apply/taskrunner/task_test.go @@ -216,41 +216,39 @@ loop: "Actual events (%d) do not match expected events (%d)", len(receivedEvents), len(expectedEvents)) - expectedInventory := actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment1.GetUID(), - Generation: testDeployment1.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment2.GetUID(), - Generation: testDeployment2.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment3ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationFailed, - Reconcile: actuation.ReconcileSkipped, - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment4ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSkipped, - Reconcile: actuation.ReconcileSkipped, - }, + expectedInventory := inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment1.GetUID(), + Generation: testDeployment1.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment2.GetUID(), + Generation: testDeployment2.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment3ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationFailed, + Reconcile: actuation.ReconcileSkipped, + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment4ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSkipped, + Reconcile: actuation.ReconcileSkipped, }, }, } - testutil.AssertEqual(t, &expectedInventory, taskContext.InventoryManager().Inventory()) + testutil.AssertEqual(t, expectedInventory, taskContext.InventoryManager().Inventory()) } func TestWaitTask_Timeout(t *testing.T) { @@ -388,41 +386,39 @@ loop: "Actual events (%d) do not match expected events (%d)", len(receivedEvents), len(expectedEvents)) - expectedInventory := actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment1.GetUID(), - Generation: testDeployment1.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileTimeout, - UID: testDeployment2.GetUID(), - Generation: testDeployment2.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment3ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationFailed, - Reconcile: actuation.ReconcileSkipped, - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment4ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSkipped, - Reconcile: actuation.ReconcileSkipped, - }, + expectedInventory := inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment1.GetUID(), + Generation: testDeployment1.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileTimeout, + UID: testDeployment2.GetUID(), + Generation: testDeployment2.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment3ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationFailed, + Reconcile: actuation.ReconcileSkipped, + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment4ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSkipped, + Reconcile: actuation.ReconcileSkipped, }, }, } - testutil.AssertEqual(t, &expectedInventory, taskContext.InventoryManager().Inventory()) + testutil.AssertEqual(t, expectedInventory, taskContext.InventoryManager().Inventory()) } func TestWaitTask_StartAndComplete(t *testing.T) { @@ -493,21 +489,19 @@ loop: "Actual events (%d) do not match expected events (%d)", len(receivedEvents), len(expectedEvents)) - expectedInventory := actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeploymentID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment.GetUID(), - Generation: testDeployment.GetGeneration(), - }, + expectedInventory := inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeploymentID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment.GetUID(), + Generation: testDeployment.GetGeneration(), }, }, } - testutil.AssertEqual(t, &expectedInventory, taskContext.InventoryManager().Inventory()) + testutil.AssertEqual(t, expectedInventory, taskContext.InventoryManager().Inventory()) } func TestWaitTask_Cancel(t *testing.T) { @@ -580,21 +574,19 @@ loop: "Actual events (%d) do not match expected events (%d)", len(receivedEvents), len(expectedEvents)) - expectedInventory := actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeploymentID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcilePending, - UID: testDeployment.GetUID(), - Generation: testDeployment.GetGeneration(), - }, + expectedInventory := inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeploymentID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcilePending, + UID: testDeployment.GetUID(), + Generation: testDeployment.GetGeneration(), }, }, } - testutil.AssertEqual(t, &expectedInventory, taskContext.InventoryManager().Inventory()) + testutil.AssertEqual(t, expectedInventory, taskContext.InventoryManager().Inventory()) } func TestWaitTask_SingleTaskResult(t *testing.T) { @@ -688,21 +680,19 @@ loop: } assert.Equal(t, expectedResults, receivedResults) - expectedInventory := actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeploymentID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment.GetUID(), - Generation: testDeployment.GetGeneration(), - }, + expectedInventory := inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeploymentID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment.GetUID(), + Generation: testDeployment.GetGeneration(), }, }, } - testutil.AssertEqual(t, &expectedInventory, taskContext.InventoryManager().Inventory()) + testutil.AssertEqual(t, expectedInventory, taskContext.InventoryManager().Inventory()) } func TestWaitTask_Failed(t *testing.T) { @@ -723,7 +713,7 @@ func TestWaitTask_Failed(t *testing.T) { eventsFunc func(*cache.ResourceCacheMap, *WaitTask, *TaskContext) waitTimeout time.Duration expectedEvents []event.Event - expectedInventory *actuation.Inventory + expectedInventory inventory.InventoryContents }{ "continue on failed if others InProgress": { configureTaskContextFunc: func(taskContext *TaskContext) { @@ -791,25 +781,23 @@ func TestWaitTask_Failed(t *testing.T) { }, }, }, - expectedInventory: &actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileFailed, - UID: testDeployment1.GetUID(), - Generation: testDeployment1.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment2.GetUID(), - Generation: testDeployment2.GetGeneration(), - }, + expectedInventory: inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileFailed, + UID: testDeployment1.GetUID(), + Generation: testDeployment1.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment2.GetUID(), + Generation: testDeployment2.GetGeneration(), }, }, }, @@ -874,25 +862,23 @@ func TestWaitTask_Failed(t *testing.T) { }, }, }, - expectedInventory: &actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileFailed, - UID: testDeployment1.GetUID(), - Generation: testDeployment1.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment2.GetUID(), - Generation: testDeployment2.GetGeneration(), - }, + expectedInventory: inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileFailed, + UID: testDeployment1.GetUID(), + Generation: testDeployment1.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment2.GetUID(), + Generation: testDeployment2.GetGeneration(), }, }, }, @@ -972,25 +958,23 @@ func TestWaitTask_Failed(t *testing.T) { }, }, }, - expectedInventory: &actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment1.GetUID(), - Generation: testDeployment1.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment2.GetUID(), - Generation: testDeployment2.GetGeneration(), - }, + expectedInventory: inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment1.GetUID(), + Generation: testDeployment1.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment2.GetUID(), + Generation: testDeployment2.GetGeneration(), }, }, }, @@ -1079,25 +1063,23 @@ func TestWaitTask_Failed(t *testing.T) { }, }, }, - expectedInventory: &actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileTimeout, - UID: testDeployment1.GetUID(), - Generation: testDeployment1.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment2.GetUID(), - Generation: testDeployment2.GetGeneration(), - }, + expectedInventory: inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileTimeout, + UID: testDeployment1.GetUID(), + Generation: testDeployment1.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment2.GetUID(), + Generation: testDeployment2.GetGeneration(), }, }, }, @@ -1176,7 +1158,7 @@ func TestWaitTask_UIDChanged(t *testing.T) { eventsFunc func(*cache.ResourceCacheMap, *WaitTask, *TaskContext) waitTimeout time.Duration expectedEvents []event.Event - expectedInventory *actuation.Inventory + expectedInventory inventory.InventoryContents }{ "UID changed after apply means reconcile failure": { condition: AllCurrent, @@ -1240,27 +1222,25 @@ func TestWaitTask_UIDChanged(t *testing.T) { }, }, }, - expectedInventory: &actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - // UID change causes failure after apply - Reconcile: actuation.ReconcileFailed, - // Recorded UID should be from the applied object, not the new replacement - UID: testDeployment1.GetUID(), - Generation: testDeployment1.GetGeneration(), - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment2.GetUID(), - Generation: testDeployment2.GetGeneration(), - }, + expectedInventory: inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + // UID change causes failure after apply + Reconcile: actuation.ReconcileFailed, + // Recorded UID should be from the applied object, not the new replacement + UID: testDeployment1.GetUID(), + Generation: testDeployment1.GetGeneration(), + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), + Strategy: actuation.ActuationStrategyApply, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment2.GetUID(), + Generation: testDeployment2.GetGeneration(), }, }, }, @@ -1327,27 +1307,25 @@ func TestWaitTask_UIDChanged(t *testing.T) { }, }, }, - expectedInventory: &actuation.Inventory{ - Status: actuation.InventoryStatus{ - Objects: []actuation.ObjectStatus{ - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), - Strategy: actuation.ActuationStrategyDelete, - Actuation: actuation.ActuationSucceeded, - // UID change causes success after delete - Reconcile: actuation.ReconcileSucceeded, - // Recorded UID should be from the deleted object, not the new replacement - UID: testDeployment1.GetUID(), - // Deleted generation is unknown - }, - { - ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), - Strategy: actuation.ActuationStrategyDelete, - Actuation: actuation.ActuationSucceeded, - Reconcile: actuation.ReconcileSucceeded, - UID: testDeployment2.GetUID(), - // Deleted generation is unknown - }, + expectedInventory: inventory.InventoryContents{ + ObjectStatuses: []actuation.ObjectStatus{ + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment1ID), + Strategy: actuation.ActuationStrategyDelete, + Actuation: actuation.ActuationSucceeded, + // UID change causes success after delete + Reconcile: actuation.ReconcileSucceeded, + // Recorded UID should be from the deleted object, not the new replacement + UID: testDeployment1.GetUID(), + // Deleted generation is unknown + }, + { + ObjectReference: inventory.ObjectReferenceFromObjMetadata(testDeployment2ID), + Strategy: actuation.ActuationStrategyDelete, + Actuation: actuation.ActuationSucceeded, + Reconcile: actuation.ReconcileSucceeded, + UID: testDeployment2.GetUID(), + // Deleted generation is unknown }, }, }, diff --git a/pkg/inventory/configmap-client-factory.go b/pkg/inventory/configmap-client-factory.go index df3e4fd6..54a1250e 100644 --- a/pkg/inventory/configmap-client-factory.go +++ b/pkg/inventory/configmap-client-factory.go @@ -23,5 +23,5 @@ type ConfigMapClientFactory struct { } func (ccf ConfigMapClientFactory) NewClient(factory cmdutil.Factory) (Client, error) { - return NewClient(factory, ConfigMapToInventoryObj, InvInfoToConfigMap, ccf.StatusPolicy, ConfigMapGVK) + return NewUnstructuredClient(factory, configMapToInventory, inventoryToConfigMap(ccf.StatusPolicy), ConfigMapGVK, StatusPolicyNone) } diff --git a/pkg/inventory/fake-inventory-client.go b/pkg/inventory/fake-inventory-client.go index 8056d039..96e9ca4d 100644 --- a/pkg/inventory/fake-inventory-client.go +++ b/pkg/inventory/fake-inventory-client.go @@ -6,23 +6,28 @@ package inventory import ( "context" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" cmdutil "k8s.io/kubectl/pkg/cmd/util" - "sigs.k8s.io/cli-utils/pkg/apis/actuation" - "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/object" ) +const ( + // TestInventoryName is the name of the fake inventory used for unit tests + TestInventoryName = "test-inventory" + // TestInventoryNamespace is the namespace of the fake inventory used for unit tests + TestInventoryNamespace = "test-namespace" +) + // FakeClient is a testing implementation of the Client interface. type FakeClient struct { - Objs object.ObjMetadataSet - Status []actuation.ObjectStatus - Err error + Inv Inventory + Err error } var ( _ Client = &FakeClient{} _ ClientFactory = FakeClientFactory{} + + _ Inventory = &FakeInventory{} ) type FakeClientFactory object.ObjMetadataSet @@ -32,58 +37,58 @@ func (f FakeClientFactory) NewClient(cmdutil.Factory) (Client, error) { } // NewFakeClient returns a FakeClient. -func NewFakeClient(initObjs object.ObjMetadataSet) *FakeClient { +func NewFakeClient(objs object.ObjMetadataSet) *FakeClient { return &FakeClient{ - Objs: initObjs, - Err: nil, + Inv: &FakeInventory{ + InventoryID: TestInventoryName, + InventoryNamespace: TestInventoryNamespace, + InventoryContents: InventoryContents{ + ObjectRefs: objs, + }, + }, + Err: nil, } } -// GetClusterObjs returns currently stored set of objects. -func (fic *FakeClient) GetClusterObjs(context.Context, Info) (object.ObjMetadataSet, error) { - if fic.Err != nil { - return object.ObjMetadataSet{}, fic.Err +// NewInventory returns a new empty inventory object +func (fic *FakeClient) NewInventory(id Info) (Inventory, error) { + inv := &FakeInventory{ + InventoryID: id.GetID(), + InventoryNamespace: id.GetNamespace(), } - return fic.Objs, nil + return inv, nil } -// Merge stores the passed objects with the current stored cluster inventory -// objects. Returns the set difference of the current set of objects minus -// the passed set of objects, or an error if one is set up. -func (fic *FakeClient) Merge(_ context.Context, _ Info, objs object.ObjMetadataSet, _ common.DryRunStrategy) (object.ObjMetadataSet, error) { +// Get returns currently stored inventory. +func (fic *FakeClient) Get(ctx context.Context, id Info, opts GetOptions) (Inventory, error) { if fic.Err != nil { - return object.ObjMetadataSet{}, fic.Err + return nil, fic.Err } - diffObjs := fic.Objs.Diff(objs) - fic.Objs = fic.Objs.Union(objs) - return diffObjs, nil + return fic.Inv, nil } -// Replace the stored cluster inventory objs with the passed obj, or an +// CreateOrUpdate the stored cluster inventory objs with the passed obj, or an // error if one is set up. -func (fic *FakeClient) Replace(_ context.Context, _ Info, objs object.ObjMetadataSet, status []actuation.ObjectStatus, - _ common.DryRunStrategy) error { +func (fic *FakeClient) CreateOrUpdate(ctx context.Context, inv Inventory, opts UpdateOptions) error { if fic.Err != nil { return fic.Err } - fic.Objs = objs - fic.Status = status + fic.Inv = inv return nil } -// DeleteInventoryObj returns an error if one is forced; does nothing otherwise. -func (fic *FakeClient) DeleteInventoryObj(context.Context, Info, common.DryRunStrategy) error { +// Delete returns an error if one is forced; does nothing otherwise. +func (fic *FakeClient) Delete(ctx context.Context, id Info, opts DeleteOptions) error { if fic.Err != nil { return fic.Err } return nil } -func (fic *FakeClient) ApplyInventoryNamespace(*unstructured.Unstructured, common.DryRunStrategy) error { - if fic.Err != nil { - return fic.Err - } - return nil +// List the in-cluster inventory +// Performs a simple in-place update on the ConfigMap +func (fic *FakeClient) List(ctx context.Context, opts ListOptions) ([]Inventory, error) { + return nil, nil } // SetError forces an error on the subsequent client call if it returns an error. @@ -96,14 +101,20 @@ func (fic *FakeClient) ClearError() { fic.Err = nil } -func (fic *FakeClient) GetClusterInventoryInfo(Info) (*unstructured.Unstructured, error) { - return nil, nil +type FakeInventory struct { + InventoryContents + InventoryID ID + InventoryNamespace string +} + +func (fi *FakeInventory) GetID() ID { + return fi.InventoryID } -func (fic *FakeClient) GetClusterInventoryObjs(_ Info) (object.UnstructuredSet, error) { - return object.UnstructuredSet{}, nil +func (fi *FakeInventory) GetNamespace() string { + return fi.InventoryNamespace } -func (fic *FakeClient) ListClusterInventoryObjs(_ context.Context) (map[string]object.ObjMetadataSet, error) { - return map[string]object.ObjMetadataSet{}, nil +func (fi *FakeInventory) Info() Info { + return NewSimpleInfo(fi.InventoryID, fi.InventoryNamespace) } diff --git a/pkg/inventory/inventory-client.go b/pkg/inventory/inventory-client.go index 20386052..15e07d3b 100644 --- a/pkg/inventory/inventory-client.go +++ b/pkg/inventory/inventory-client.go @@ -8,12 +8,12 @@ import ( "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/discovery" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" cmdutil "k8s.io/kubectl/pkg/cmd/util" "sigs.k8s.io/cli-utils/pkg/apis/actuation" @@ -24,455 +24,420 @@ import ( // Client expresses an interface for interacting with // objects which store references to objects (inventory objects). type Client interface { - // GetClusterObjs returns the set of previously applied objects as ObjMetadata, - // or an error if one occurred. This set of previously applied object references - // is stored in the inventory objects living in the cluster. - GetClusterObjs(ctx context.Context, inv Info) (object.ObjMetadataSet, error) - // Merge applies the union of the passed objects with the currently - // stored objects in the inventory object. Returns the set of - // objects which are not in the passed objects (objects to be pruned). - // Otherwise, returns an error if one happened. - Merge(ctx context.Context, inv Info, objs object.ObjMetadataSet, dryRun common.DryRunStrategy) (object.ObjMetadataSet, error) - // Replace replaces the set of objects stored in the inventory - // object with the passed set of objects, or an error if one occurs. - Replace(ctx context.Context, inv Info, objs object.ObjMetadataSet, status []actuation.ObjectStatus, dryRun common.DryRunStrategy) error - // DeleteInventoryObj deletes the passed inventory object from the APIServer. - DeleteInventoryObj(ctx context.Context, inv Info, dryRun common.DryRunStrategy) error - // ListClusterInventoryObjs returns a map mapping from inventory name to a list of cluster inventory objects - ListClusterInventoryObjs(ctx context.Context) (map[string]object.ObjMetadataSet, error) + ReadClient + WriteClient + Factory } -// ClusterClient is a concrete implementation of the -// Client interface. -type ClusterClient struct { - dc dynamic.Interface - discoveryClient discovery.CachedDiscoveryInterface - mapper meta.RESTMapper - InventoryFactoryFunc StorageFactoryFunc - invToUnstructuredFunc ToUnstructuredFunc - statusPolicy StatusPolicy - gvk schema.GroupVersionKind -} - -var _ Client = &ClusterClient{} +// ID is a unique identifier for an inventory object. +// It's used by the inventory client to get/update/delete the object. +// For example, it could be a name or a label depending on the inventory client +// implementation. +type ID string -// NewClient returns a concrete implementation of the -// Client interface or an error. -func NewClient(factory cmdutil.Factory, - invFunc StorageFactoryFunc, - invToUnstructuredFunc ToUnstructuredFunc, - statusPolicy StatusPolicy, - gvk schema.GroupVersionKind, -) (*ClusterClient, error) { - dc, err := factory.DynamicClient() - if err != nil { - return nil, err - } - mapper, err := factory.ToRESTMapper() - if err != nil { - return nil, err - } - discoveryClinet, err := factory.ToDiscoveryClient() - if err != nil { - return nil, err - } - clusterClient := ClusterClient{ - dc: dc, - discoveryClient: discoveryClinet, - mapper: mapper, - InventoryFactoryFunc: invFunc, - invToUnstructuredFunc: invToUnstructuredFunc, - statusPolicy: statusPolicy, - gvk: gvk, - } - return &clusterClient, nil +// String implements the fmt.Stringer interface for ID +func (id ID) String() string { + return string(id) } -// Merge stores the union of the passed objects with the objects currently -// stored in the cluster inventory object. Retrieves and caches the cluster -// inventory object. Returns the set differrence of the cluster inventory -// objects and the currently applied objects. This is the set of objects -// to prune. Creates the initial cluster inventory object storing the passed -// objects if an inventory object does not exist. Returns an error if one -// occurred. -func (cic *ClusterClient) Merge(ctx context.Context, localInv Info, objs object.ObjMetadataSet, dryRun common.DryRunStrategy) (object.ObjMetadataSet, error) { - pruneIDs := object.ObjMetadataSet{} - invObj, err := cic.invToUnstructuredFunc(localInv) - if err != nil { - return pruneIDs, err - } - clusterInv, err := cic.getClusterInventoryInfo(ctx, localInv) - if err != nil { - return pruneIDs, err - } +// Info provides the minimal information for the applier +// to create, look up and update an inventory. +// The inventory object can be any type, the Provider in the applier +// needs to know how to create, look up and update it based +// on the Info. +type Info interface { + // GetNamespace of the inventory object. + // It should be the value of the field .metadata.namespace. + GetNamespace() string + + // GetID of the inventory object. + // The inventory client uses this to determine how to get/update the object(s) + // from the cluster. + GetID() ID +} - // Inventory does not exist on the cluster. - if clusterInv == nil { - // Wrap inventory object and store the inventory in it. - var status []actuation.ObjectStatus - if cic.statusPolicy == StatusPolicyAll { - status = getObjStatus(nil, objs) - } - inv, err := cic.InventoryFactoryFunc(invObj) - if err != nil { - return nil, err - } - if err := inv.Store(objs, status); err != nil { - return nil, err - } - klog.V(4).Infof("creating initial inventory object with %d objects", len(objs)) +// SimpleInfo is the simplest implementation of the Info interface +type SimpleInfo struct { + // id of the inventory object + id ID + // namespace of the inventory object + namespace string +} - if dryRun.ClientOrServerDryRun() { - klog.V(4).Infof("dry-run create inventory object: not created") - return nil, nil - } +// GetID returns the ID of the inventory object +func (i *SimpleInfo) GetID() ID { + return i.id +} - err = inv.Apply(ctx, cic.dc, cic.mapper, cic.statusPolicy) - return nil, err - } +// GetNamespace returns the namespace of the inventory object +func (i *SimpleInfo) GetNamespace() string { + return i.namespace +} - // Update existing cluster inventory with merged union of objects - clusterObjs, err := cic.GetClusterObjs(ctx, localInv) - if err != nil { - return pruneIDs, err - } - pruneIDs = clusterObjs.Diff(objs) - unionObjs := clusterObjs.Union(objs) - var status []actuation.ObjectStatus - if cic.statusPolicy == StatusPolicyAll { - status = getObjStatus(pruneIDs, unionObjs) - } - klog.V(4).Infof("num objects to prune: %d", len(pruneIDs)) - klog.V(4).Infof("num merged objects to store in inventory: %d", len(unionObjs)) - wrappedInv, err := cic.InventoryFactoryFunc(clusterInv) - if err != nil { - return pruneIDs, err - } - if err = wrappedInv.Store(unionObjs, status); err != nil { - return pruneIDs, err +// NewSimpleInfo constructs a new SimpleInfo +func NewSimpleInfo(id ID, namespace string) *SimpleInfo { + return &SimpleInfo{ + id: id, + namespace: namespace, } +} - // Update not required when all objects in inventory are the same and - // status does not need to be updated. If status is stored, always update the - // inventory to store the latest status. - if objs.Equal(clusterObjs) && cic.statusPolicy == StatusPolicyNone { - return pruneIDs, nil - } +// SingleObjectInfo implements the Info interface and also adds a Name field. +// This assumes the underlying inventory is a singleton object with a single +// name. Used by UnstructuredInventory, which assumes a single object. +type SingleObjectInfo struct { + SimpleInfo + // name of the singleton inventory object + name string +} - if dryRun.ClientOrServerDryRun() { - klog.V(4).Infof("dry-run create inventory object: not created") - return pruneIDs, nil - } - err = wrappedInv.Apply(ctx, cic.dc, cic.mapper, cic.statusPolicy) - return pruneIDs, err +// GetName returns the name of the inventory object +func (i *SingleObjectInfo) GetName() string { + return i.name } -// Replace stores the passed objects in the cluster inventory object, or -// an error if one occurred. -func (cic *ClusterClient) Replace(ctx context.Context, localInv Info, objs object.ObjMetadataSet, status []actuation.ObjectStatus, - dryRun common.DryRunStrategy) error { - // Skip entire function for dry-run. - if dryRun.ClientOrServerDryRun() { - klog.V(4).Infoln("dry-run replace inventory object: not applied") - return nil - } - clusterInv, err := cic.getClusterInventoryInfo(ctx, localInv) - if err != nil { - return fmt.Errorf("failed to read inventory from cluster: %w", err) +// NewSingleObjectInfo constructs a new SingleObjectInfo +func NewSingleObjectInfo(id ID, nn types.NamespacedName) *SingleObjectInfo { + return &SingleObjectInfo{ + SimpleInfo: SimpleInfo{ + id: id, + namespace: nn.Namespace, + }, + name: nn.Name, } +} - clusterObjs, err := cic.GetClusterObjs(ctx, localInv) - if err != nil { - return fmt.Errorf("failed to read inventory objects from cluster: %w", err) - } +type Factory interface { + // NewInventory returns an empty initialized inventory object. + // This is used in the case that there is no existing object on the cluster. + NewInventory(Info) (Inventory, error) +} - clusterInv, wrappedInv, err := cic.replaceInventory(clusterInv, objs, status) - if err != nil { - return err - } +// Inventory is an internal representation of the inventory object that is used +// to track which objects have been applied by the applier. The interface exists +// so different underlying resource types can be used to implement the inventory. +type Inventory interface { + Info() Info + // GetObjectRefs returns the list of object references tracked in the inventory + GetObjectRefs() object.ObjMetadataSet + // GetObjectStatuses returns the list of statuses for each object reference tracked in the inventory + GetObjectStatuses() []actuation.ObjectStatus + // SetObjectRefs updates the local cache of object references tracked in the inventory. + // This will be persisted to the cluster when the Inventory is passed to CreateOrUpdate. + SetObjectRefs(object.ObjMetadataSet) + // SetObjectStatuses updates the local cache of object statuses tracked in the inventory. + // This will be persisted to the cluster when the Inventory is passed to CreateOrUpdate. + SetObjectStatuses([]actuation.ObjectStatus) +} - // Update not required when all objects in inventory are the same and - // status does not need to be updated. If status is stored, always update the - // inventory to store the latest status. - if objs.Equal(clusterObjs) && cic.statusPolicy == StatusPolicyNone { - return nil - } +type ReadClient interface { + // Get an inventory object from the cluster. + Get(ctx context.Context, inv Info, opts GetOptions) (Inventory, error) + // List the inventory objects from the cluster. This is used by the CLI and + // not used by the applier/destroyer. + List(ctx context.Context, opts ListOptions) ([]Inventory, error) +} - klog.V(4).Infof("replace cluster inventory: %s/%s", clusterInv.GetNamespace(), clusterInv.GetName()) - klog.V(4).Infof("replace cluster inventory %d objects", len(objs)) +type WriteClient interface { + // CreateOrUpdate an inventory object on the cluster. Always updates spec, + // and will update the status if StatusPolicyAll is used. + CreateOrUpdate(ctx context.Context, inv Inventory, opts UpdateOptions) error + // Delete an inventory object on the cluster. + Delete(ctx context.Context, inv Info, opts DeleteOptions) error +} - if err := wrappedInv.ApplyWithPrune(ctx, cic.dc, cic.mapper, cic.statusPolicy, objs); err != nil { - return fmt.Errorf("failed to write updated inventory to cluster: %w", err) +// UpdateOptions are used to configure the behavior of the CreateOrUpdate method. +// These options are set by the applier and should be honored by the inventory +// client implementation. +type UpdateOptions struct{} + +// GetOptions are used to configure the behavior of the Get method. +// These options are set by the applier and should be honored by the inventory +// client implementation. +type GetOptions struct{} + +// ListOptions are used to configure the behavior of the List method. +// These options are set by the applier and should be honored by the inventory +// client implementation. +type ListOptions struct{} + +// DeleteOptions are used to configure the behavior of the Delete method. +// These options are set by the applier and should be honored by the inventory +// client implementation. +type DeleteOptions struct{} + +var _ Client = &UnstructuredClient{} + +// NewUnstructuredInventory constructs a new UnstructuredInventory object +// from a raw unstructured object. +func NewUnstructuredInventory(uObj *unstructured.Unstructured) *UnstructuredInventory { + return &UnstructuredInventory{ + SingleObjectInfo: SingleObjectInfo{ + SimpleInfo: SimpleInfo{ + id: ID(uObj.GetLabels()[common.InventoryLabel]), + namespace: uObj.GetNamespace(), + }, + name: uObj.GetName(), + }, } +} - return nil +// UnstructuredInventory implements Inventory while also tracking the actual +// KRM object from the cluster. This enables the client to update the +// same object and utilize resourceVersion checks. +type UnstructuredInventory struct { + SingleObjectInfo + InventoryContents } -// replaceInventory stores the passed objects into the passed inventory object. -func (cic *ClusterClient) replaceInventory(inv *unstructured.Unstructured, objs object.ObjMetadataSet, - status []actuation.ObjectStatus) (*unstructured.Unstructured, Storage, error) { - if cic.statusPolicy == StatusPolicyNone { - status = nil - } - wrappedInv, err := cic.InventoryFactoryFunc(inv) - if err != nil { - return nil, nil, err - } - if err := wrappedInv.Store(objs, status); err != nil { - return nil, nil, err - } - clusterInv, err := wrappedInv.GetObject() - if err != nil { - return nil, nil, err - } +// Info returns the metadata associated with this UnstructuredInventory +func (ui *UnstructuredInventory) Info() Info { + return &ui.SingleObjectInfo +} + +var _ Inventory = &UnstructuredInventory{} - return clusterInv, wrappedInv, nil +// InventoryContents is a boilerplate struct that contains the basic methods +// to implement Inventory. Can be extended for different inventory implementations. +// nolint:revive +type InventoryContents struct { + // ObjectRefs and ObjectStatuses are in memory representations of the inventory which are + // read and manipulated by the applier. + ObjectRefs object.ObjMetadataSet + ObjectStatuses []actuation.ObjectStatus } -// DeleteInventoryObj deletes the inventory object from the cluster. -func (cic *ClusterClient) DeleteInventoryObj(ctx context.Context, localInv Info, dryRun common.DryRunStrategy) error { - if localInv == nil { - return fmt.Errorf("retrieving cluster inventory object with nil local inventory") - } - switch localInv.Strategy() { - case NameStrategy: - obj, err := cic.invToUnstructuredFunc(localInv) - if err != nil { - return err - } - return cic.deleteInventoryObjByName(ctx, obj, dryRun) - case LabelStrategy: - return cic.deleteInventoryObjsByLabel(ctx, localInv, dryRun) - default: - panic(fmt.Errorf("unknown inventory strategy: %s", localInv.Strategy())) - } +// GetObjectRefs returns the list of object references tracked in the inventory +func (inv *InventoryContents) GetObjectRefs() object.ObjMetadataSet { + return inv.ObjectRefs } -func (cic *ClusterClient) deleteInventoryObjsByLabel(ctx context.Context, inv Info, dryRun common.DryRunStrategy) error { - clusterInvObjs, err := cic.getClusterInventoryObjsByLabel(ctx, inv) - if err != nil { - return err - } - for _, invObj := range clusterInvObjs { - if err := cic.deleteInventoryObjByName(ctx, invObj, dryRun); err != nil { - return err - } - } - return nil +// GetObjectStatuses returns the list of statuses for each object reference tracked in the inventory +func (inv *InventoryContents) GetObjectStatuses() []actuation.ObjectStatus { + return inv.ObjectStatuses } -// GetClusterObjs returns the objects stored in the cluster inventory object, or -// an error if one occurred. -func (cic *ClusterClient) GetClusterObjs(ctx context.Context, localInv Info) (object.ObjMetadataSet, error) { - var objs object.ObjMetadataSet - clusterInv, err := cic.getClusterInventoryInfo(ctx, localInv) - if err != nil { - return objs, fmt.Errorf("failed to read inventory from cluster: %w", err) - } - // First time; no inventory obj yet. - if clusterInv == nil { - return objs, nil - } - wrapped, err := cic.InventoryFactoryFunc(clusterInv) - if err != nil { - return objs, err - } - return wrapped.Load() +// SetObjectRefs updates the local cache of object references tracked in the inventory. +// This will be persisted to the cluster when the Inventory is passed to CreateOrUpdate. +func (inv *InventoryContents) SetObjectRefs(objs object.ObjMetadataSet) { + inv.ObjectRefs = objs } -// getClusterInventoryObj returns a pointer to the cluster inventory object, or -// an error if one occurred. Returns the cached cluster inventory object if it -// has been previously retrieved. Uses the ResourceBuilder to retrieve the -// inventory object in the cluster, using the namespace, group resource, and -// inventory label. Merges multiple inventory objects into one if it retrieves -// more than one (this should be very rare). -// -// TODO(seans3): Remove the special case code to merge multiple cluster inventory -// objects once we've determined that this case is no longer possible. -func (cic *ClusterClient) getClusterInventoryInfo(ctx context.Context, inv Info) (*unstructured.Unstructured, error) { - clusterInvObjects, err := cic.getClusterInventoryObjs(ctx, inv) - if err != nil { - return nil, fmt.Errorf("failed to read inventory objects from cluster: %w", err) - } +// SetObjectStatuses updates the local cache of object statuses tracked in the inventory. +// This will be persisted to the cluster when the Inventory is passed to CreateOrUpdate. +func (inv *InventoryContents) SetObjectStatuses(statuses []actuation.ObjectStatus) { + inv.ObjectStatuses = statuses +} - var clusterInv *unstructured.Unstructured - if len(clusterInvObjects) == 1 { - clusterInv = clusterInvObjects[0] - } else if l := len(clusterInvObjects); l > 1 { - return nil, fmt.Errorf("found %d inventory objects with inventory id %s", l, inv.ID()) - } - return clusterInv, nil +// FromUnstructuredFunc is used by UnstructuredClient to convert an unstructured +// object, usually fetched from the cluster, to an Inventory object for use by +// the applier/destroyer. +type FromUnstructuredFunc func(fromObj *unstructured.Unstructured) (*UnstructuredInventory, error) + +// ToUnstructuredFunc is used by UnstructuredClient to take an unstructured object, +// usually fetched from the cluster, and update it using the UnstructuredInventory. +// The function should return an updated unstructured object which will then be +// applied to the cluster by UnstructuredClient. +type ToUnstructuredFunc func(fromObj *unstructured.Unstructured, toInv *UnstructuredInventory) (*unstructured.Unstructured, error) + +// UnstructuredClient implements the inventory client interface for a single unstructured object +type UnstructuredClient struct { + client dynamic.NamespaceableResourceInterface + fromUnstructured FromUnstructuredFunc + toUnstructured ToUnstructuredFunc + gvk schema.GroupVersionKind + statusPolicy StatusPolicy } -func (cic *ClusterClient) getClusterInventoryObjsByLabel(ctx context.Context, inv Info) (object.UnstructuredSet, error) { - localInv, err := cic.invToUnstructuredFunc(inv) - if err != nil { - return nil, err - } - if localInv == nil { - return nil, fmt.Errorf("retrieving cluster inventory object with nil local inventory") - } - localObj := object.UnstructuredToObjMetadata(localInv) - mapping, err := cic.getMapping(localInv) +// NewUnstructuredClient constructs an instance of UnstructuredClient. +// This can be used as an inventory client for any arbitrary GVK, assuming it +// is a singleton object. +func NewUnstructuredClient(factory cmdutil.Factory, + from FromUnstructuredFunc, + to ToUnstructuredFunc, + gvk schema.GroupVersionKind, + statusPolicy StatusPolicy) (*UnstructuredClient, error) { + dc, err := factory.DynamicClient() if err != nil { return nil, err } - groupResource := mapping.Resource.GroupResource().String() - namespace := localObj.Namespace - label, err := retrieveInventoryLabel(localInv) + mapper, err := factory.ToRESTMapper() if err != nil { return nil, err } - labelSelector := fmt.Sprintf("%s=%s", common.InventoryLabel, label) - klog.V(4).Infof("inventory object fetch by label (group: %q, namespace: %q, selector: %q)", groupResource, namespace, labelSelector) - - uList, err := cic.dc.Resource(mapping.Resource).Namespace(namespace).List(ctx, metav1.ListOptions{ - LabelSelector: labelSelector, - }) + mapping, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version) if err != nil { return nil, err } - var invList []*unstructured.Unstructured - for i := range uList.Items { - invList = append(invList, &uList.Items[i]) + unstructuredClient := &UnstructuredClient{ + client: dc.Resource(mapping.Resource), + fromUnstructured: from, + toUnstructured: to, + gvk: gvk, + statusPolicy: statusPolicy, } - return invList, nil + return unstructuredClient, nil } -func (cic *ClusterClient) getClusterInventoryObjsByName(ctx context.Context, inv Info) (object.UnstructuredSet, error) { - localInv, err := cic.invToUnstructuredFunc(inv) - if err != nil { - return nil, err - } - if localInv == nil { - return nil, fmt.Errorf("retrieving cluster inventory object with nil local inventory") - } - - mapping, err := cic.getMapping(localInv) - if err != nil { - return nil, err +// NewInventory returns a new, empty Inventory object +func (cic *UnstructuredClient) NewInventory(inv Info) (Inventory, error) { + soi, ok := inv.(*SingleObjectInfo) + if !ok { + return nil, fmt.Errorf("expected SingleObjectInfo but got %T", inv) } + return cic.fromUnstructured(cic.newUnstructuredObject(soi)) +} - klog.V(4).Infof("inventory object fetch by name (namespace: %q, name: %q)", inv.Namespace(), inv.Name()) - clusterInv, err := cic.dc.Resource(mapping.Resource).Namespace(inv.Namespace()). - Get(ctx, inv.Name(), metav1.GetOptions{}) - if err != nil && !apierrors.IsNotFound(err) { - return nil, err - } - if apierrors.IsNotFound(err) { - return object.UnstructuredSet{}, nil - } - if inv.ID() != "" { - if inventoryID, err := retrieveInventoryLabel(clusterInv); err != nil { - return nil, err - } else if inv.ID() != inventoryID { - return nil, fmt.Errorf("inventory-id of inventory object %s/%s in cluster doesn't match provided id %q", - inv.Namespace(), inv.Name(), inv.ID()) - } - } - return object.UnstructuredSet{clusterInv}, nil +// newInventory is used internally to return a typed UnstructuredInventory +func (cic *UnstructuredClient) newUnstructuredObject(invInfo *SingleObjectInfo) *unstructured.Unstructured { + obj := &unstructured.Unstructured{} + obj.SetName(invInfo.GetName()) + obj.SetNamespace(invInfo.GetNamespace()) + obj.SetGroupVersionKind(cic.gvk) + obj.SetLabels(map[string]string{common.InventoryLabel: invInfo.GetID().String()}) + return obj } -func (cic *ClusterClient) getClusterInventoryObjs(ctx context.Context, inv Info) (object.UnstructuredSet, error) { +// Get the in-cluster inventory +func (cic *UnstructuredClient) Get(ctx context.Context, inv Info, _ GetOptions) (Inventory, error) { if inv == nil { - return nil, fmt.Errorf("inventoryInfo must be specified") + return nil, fmt.Errorf("inventory Info is nil") } - - var clusterInvObjects object.UnstructuredSet - var err error - switch inv.Strategy() { - case NameStrategy: - clusterInvObjects, err = cic.getClusterInventoryObjsByName(ctx, inv) - case LabelStrategy: - clusterInvObjects, err = cic.getClusterInventoryObjsByLabel(ctx, inv) - default: - panic(fmt.Errorf("unknown inventory strategy: %s", inv.Strategy())) + soi, ok := inv.(*SingleObjectInfo) + if !ok { + return nil, fmt.Errorf("expected SingleObjectInfo but got %T", inv) } - return clusterInvObjects, err -} - -func (cic *ClusterClient) ListClusterInventoryObjs(ctx context.Context) (map[string]object.ObjMetadataSet, error) { - // Define the mapping - mapping, err := cic.mapper.RESTMapping(cic.gvk.GroupKind(), cic.gvk.Version) + obj, err := cic.client.Namespace(soi.GetNamespace()).Get(ctx, soi.GetName(), metav1.GetOptions{}) if err != nil { return nil, err } + return cic.fromUnstructured(obj) +} - // retrieve the list from the cluster - clusterInvs, err := cic.dc.Resource(mapping.Resource).List(ctx, metav1.ListOptions{}) - if err != nil && !apierrors.IsNotFound(err) { +// List the in-cluster inventory +// Used by the CLI commands +func (cic *UnstructuredClient) List(ctx context.Context, _ ListOptions) ([]Inventory, error) { + objs, err := cic.client.List(ctx, metav1.ListOptions{}) + if err != nil { return nil, err } - if apierrors.IsNotFound(err) { - return map[string]object.ObjMetadataSet{}, nil - } - - identifiers := make(map[string]object.ObjMetadataSet) - - for i, inv := range clusterInvs.Items { - invName := inv.GetName() - identifiers[invName] = object.ObjMetadataSet{} - invObj, err := cic.InventoryFactoryFunc(&clusterInvs.Items[i]) - if err != nil { - return nil, err - } - wrappedInvObjSlice, err := invObj.Load() + var inventories []Inventory + for _, obj := range objs.Items { + uInv, err := cic.fromUnstructured(&obj) if err != nil { return nil, err } - identifiers[invName] = append(identifiers[invName], wrappedInvObjSlice...) + inventories = append(inventories, uInv) } - - return identifiers, nil + return inventories, nil } -// deleteInventoryObjByName deletes the passed inventory object from the APIServer, or -// an error if one occurs. -func (cic *ClusterClient) deleteInventoryObjByName(ctx context.Context, obj *unstructured.Unstructured, dryRun common.DryRunStrategy) error { - if obj == nil { - return fmt.Errorf("attempting delete a nil inventory object") - } - if dryRun.ClientOrServerDryRun() { - klog.V(4).Infof("dry-run delete inventory object: not deleted") +// CreateOrUpdate the in-cluster inventory +// Updates the unstructured object, or creates it if it doesn't exist +func (cic *UnstructuredClient) CreateOrUpdate(ctx context.Context, inv Inventory, opts UpdateOptions) error { + ui, ok := inv.(*UnstructuredInventory) + if !ok { + return fmt.Errorf("expected UnstructuredInventory") + } + if ui == nil { + return fmt.Errorf("inventory is nil") + } + // Attempt to retry on a resource conflict error to avoid needing to retry the + // entire Apply/Destroy when there's a transient conflict. + attempt := 0 + var obj *unstructured.Unstructured + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + defer func() { + attempt++ + }() + create := false + var err error + obj, err = cic.client.Namespace(ui.GetNamespace()).Get(ctx, ui.GetName(), metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + create = true + obj = cic.newUnstructuredObject(&ui.SingleObjectInfo) + } else if err != nil { + return err + } + uObj, err := cic.toUnstructured(obj.DeepCopy(), ui) + if err != nil { + return err + } + if create { + klog.V(4).Infof("[attempt %d] creating inventory object %s/%s/%s", + attempt, cic.gvk, uObj.GetNamespace(), uObj.GetName()) + obj, err = cic.client.Namespace(uObj.GetNamespace()).Create(ctx, uObj, metav1.CreateOptions{}) + if err != nil { + return err + } + } else { + klog.V(4).Infof("[attempt %d] updating inventory object %s/%s/%s", + attempt, cic.gvk, uObj.GetNamespace(), uObj.GetName()) + obj, err = cic.client.Namespace(uObj.GetNamespace()).Update(ctx, uObj, metav1.UpdateOptions{}) + if err != nil { + return err + } + } return nil - } - - mapping, err := cic.getMapping(obj) + }) if err != nil { return err } - - klog.V(4).Infof("deleting inventory object: %s/%s", obj.GetNamespace(), obj.GetName()) - return cic.dc.Resource(mapping.Resource).Namespace(obj.GetNamespace()). - Delete(ctx, obj.GetName(), metav1.DeleteOptions{}) -} - -// getMapping returns the RESTMapping for the provided resource. -func (cic *ClusterClient) getMapping(obj *unstructured.Unstructured) (*meta.RESTMapping, error) { - return cic.mapper.RESTMapping(obj.GroupVersionKind().GroupKind(), obj.GroupVersionKind().Version) + if cic.statusPolicy != StatusPolicyAll { + return nil + } + attempt = 0 + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + defer func() { + attempt++ + }() + if attempt > 0 { // reuse object from spec update/create on first attempt + obj, err = cic.client.Namespace(ui.GetNamespace()).Get(ctx, ui.GetName(), metav1.GetOptions{}) + if err != nil { + return err + } + } + uObj, err := cic.toUnstructured(obj.DeepCopy(), ui) + if err != nil { + return err + } + // Update observedGeneration, if it exists + // If using a custom inventory resource, make sure the observedGeneration + // defaults to 0. That will ensure the observedGeneration is updated here, and + // the kstatus computes as InProgress after the object is created. + _, ok, err = unstructured.NestedInt64(uObj.Object, "status", "observedGeneration") + if err != nil { + return err + } + if ok { + err = unstructured.SetNestedField(uObj.Object, uObj.GetGeneration(), "status", "observedGeneration") + if err != nil { + return err + } + } + klog.V(4).Infof("[attempt %d] updating status of inventory object %s/%s/%s", + attempt, cic.gvk, uObj.GetNamespace(), uObj.GetName()) + _, err = cic.client.Namespace(uObj.GetNamespace()).UpdateStatus(ctx, uObj, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("updateStatus: %w", err) + } + return nil + }) } -// getObjStatus returns the list of object status -// at the beginning of an apply process. -func getObjStatus(pruneIDs, unionIDs []object.ObjMetadata) []actuation.ObjectStatus { - status := []actuation.ObjectStatus{} - for _, obj := range unionIDs { - status = append(status, - actuation.ObjectStatus{ - ObjectReference: ObjectReferenceFromObjMetadata(obj), - Strategy: actuation.ActuationStrategyApply, - Actuation: actuation.ActuationPending, - Reconcile: actuation.ReconcilePending, - }) +// Delete the in-cluster inventory +// Performs a simple deletion of the unstructured object +func (cic *UnstructuredClient) Delete(ctx context.Context, inv Info, _ DeleteOptions) error { + if inv == nil { + return fmt.Errorf("inventory Info is nil") + } + soi, ok := inv.(*SingleObjectInfo) + if !ok { + return fmt.Errorf("expected SingleObjectInfo but got %T", inv) } - for _, obj := range pruneIDs { - status = append(status, - actuation.ObjectStatus{ - ObjectReference: ObjectReferenceFromObjMetadata(obj), - Strategy: actuation.ActuationStrategyDelete, - Actuation: actuation.ActuationPending, - Reconcile: actuation.ReconcilePending, - }) + err := cic.client.Namespace(soi.GetNamespace()).Delete(ctx, soi.GetName(), metav1.DeleteOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("delete: %w", err) } - return status + return nil } diff --git a/pkg/inventory/inventory-client_test.go b/pkg/inventory/inventory-client_test.go index bdf150d6..e329c727 100644 --- a/pkg/inventory/inventory-client_test.go +++ b/pkg/inventory/inventory-client_test.go @@ -4,18 +4,15 @@ package inventory import ( - "fmt" + "context" "testing" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/resource" clienttesting "k8s.io/client-go/testing" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" "sigs.k8s.io/cli-utils/pkg/apis/actuation" - "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/object" ) @@ -28,19 +25,7 @@ func podStatus(info *resource.Info) actuation.ObjectStatus { } } -func podData(name string) map[string]string { - return map[string]string{ - fmt.Sprintf("test-inventory-namespace_%s__Pod", name): "{\"actuation\":\"Succeeded\",\"reconcile\":\"Succeeded\",\"strategy\":\"Apply\"}", - } -} - -func podDataNoStatus(name string) map[string]string { - return map[string]string{ - fmt.Sprintf("test-inventory-namespace_%s__Pod", name): "", - } -} - -func TestGetClusterInventoryInfo(t *testing.T) { +func TestGet(t *testing.T) { localInv, err := ConfigMapToInventoryInfo(inventoryObj) require.NoError(t, err) tests := map[string]struct { @@ -83,316 +68,146 @@ func TestGetClusterInventoryInfo(t *testing.T) { }, } - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() - for name, tc := range tests { t.Run(name, func(t *testing.T) { - invClient, err := NewClient(tf, - ConfigMapToInventoryObj, InvInfoToConfigMap, tc.statusPolicy, ConfigMapGVK) + tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) + defer tf.Cleanup() + tf.FakeDynamicClient.PrependReactor("get", "configmaps", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + inv := NewUnstructuredInventory(emptyInventoryObject()) + inv.SetObjectRefs(tc.localObjs) + inv.SetObjectStatuses(tc.objStatus) + cm, _ := inventoryToConfigMap(tc.statusPolicy)(emptyInventoryObject(), inv) + return true, cm, nil + }) + invClient, err := ConfigMapClientFactory{StatusPolicy: tc.statusPolicy}.NewClient(tf) require.NoError(t, err) - if tc.inv != nil { - wrapped, err := storeObjsInInventory(tc.inv, tc.localObjs, tc.objStatus) - require.NoError(t, err) - err = wrapped.Apply(t.Context(), invClient.dc, invClient.mapper, tc.statusPolicy) - require.NoError(t, err) - } - clusterInv, err := invClient.getClusterInventoryInfo(t.Context(), tc.inv) + clusterInv, err := invClient.Get(t.Context(), tc.inv, GetOptions{}) if tc.isError { require.Error(t, err) return } require.NoError(t, err) - require.NotNil(t, clusterInv) - wrapped, err := ConfigMapToInventoryObj(clusterInv) - require.NoError(t, err) - clusterObjs, err := wrapped.Load() - require.NoError(t, err) - if !tc.localObjs.Equal(clusterObjs) { - t.Fatalf("expected cluster objs (%v), got (%v)", tc.localObjs, clusterObjs) + if clusterInv != nil { + if !tc.localObjs.Equal(clusterInv.GetObjectRefs()) { + t.Fatalf("expected cluster objs (%v), got (%v)", tc.localObjs, clusterInv.GetObjectRefs()) + } } }) } } -func TestMerge(t *testing.T) { +func TestCreateOrUpdate(t *testing.T) { tests := map[string]struct { - statusPolicy StatusPolicy - localInv Info - localObjs object.ObjMetadataSet - clusterObjs object.ObjMetadataSet - pruneObjs object.ObjMetadataSet - isError bool + inventory *UnstructuredInventory + createObjs object.ObjMetadataSet + updateObjs object.ObjMetadataSet + isError bool }{ "Nil local inventory object is error": { - localInv: nil, - localObjs: object.ObjMetadataSet{}, - clusterObjs: object.ObjMetadataSet{}, - pruneObjs: object.ObjMetadataSet{}, - isError: true, - statusPolicy: StatusPolicyAll, + inventory: nil, + createObjs: object.ObjMetadataSet{}, + isError: true, }, - "Cluster and local inventories empty: no prune objects; no change": { - localInv: copyInventory(t), - localObjs: object.ObjMetadataSet{}, - clusterObjs: object.ObjMetadataSet{}, - pruneObjs: object.ObjMetadataSet{}, - isError: false, - statusPolicy: StatusPolicyAll, + "Create and update inventory with empty object set": { + inventory: NewUnstructuredInventory(emptyInventoryObject()), + createObjs: object.ObjMetadataSet{}, + updateObjs: object.ObjMetadataSet{}, + isError: false, }, - "Cluster and local inventories same: no prune objects; no change": { - localInv: copyInventory(t), - localObjs: object.ObjMetadataSet{ + "Create and Update inventory with identical object set": { + inventory: NewUnstructuredInventory(emptyInventoryObject()), + createObjs: object.ObjMetadataSet{ ignoreErrInfoToObjMeta(pod1Info), }, - clusterObjs: object.ObjMetadataSet{ + updateObjs: object.ObjMetadataSet{ ignoreErrInfoToObjMeta(pod1Info), }, - pruneObjs: object.ObjMetadataSet{}, - isError: false, - statusPolicy: StatusPolicyAll, + isError: false, }, - "Cluster two obj, local one: prune obj": { - localInv: copyInventory(t), - localObjs: object.ObjMetadataSet{ + "Create and Update inventory with expanding object set": { + inventory: NewUnstructuredInventory(emptyInventoryObject()), + createObjs: object.ObjMetadataSet{ ignoreErrInfoToObjMeta(pod1Info), }, - clusterObjs: object.ObjMetadataSet{ + updateObjs: object.ObjMetadataSet{ ignoreErrInfoToObjMeta(pod1Info), ignoreErrInfoToObjMeta(pod3Info), }, - pruneObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod3Info), - }, - statusPolicy: StatusPolicyAll, - isError: false, + isError: false, }, - "Cluster multiple objs, local multiple different objs: prune objs": { - localInv: copyInventory(t), - localObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod2Info), - }, - clusterObjs: object.ObjMetadataSet{ + "Create and Update inventory with shrinking object set": { + inventory: NewUnstructuredInventory(emptyInventoryObject()), + createObjs: object.ObjMetadataSet{ ignoreErrInfoToObjMeta(pod1Info), ignoreErrInfoToObjMeta(pod2Info), - ignoreErrInfoToObjMeta(pod3Info)}, - pruneObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod1Info), ignoreErrInfoToObjMeta(pod3Info), }, - statusPolicy: StatusPolicyAll, - isError: false, - }, - } - - for name, tc := range tests { - for i := range common.Strategies { - drs := common.Strategies[i] - t.Run(name, func(t *testing.T) { - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() - - tf.FakeDynamicClient.PrependReactor("list", "configmaps", toReactionFunc(tc.clusterObjs)) - // Create the local inventory object storing "tc.localObjs" - invClient, err := NewClient(tf, - ConfigMapToInventoryObj, InvInfoToConfigMap, tc.statusPolicy, ConfigMapGVK) - require.NoError(t, err) - - // Call "Merge" to create the union of clusterObjs and localObjs. - pruneObjs, err := invClient.Merge(t.Context(), tc.localInv, tc.localObjs, drs) - if tc.isError { - if err == nil { - t.Fatalf("expected error but received none") - } - return - } - if !tc.isError && err != nil { - t.Fatalf("unexpected error: %s", err) - } - if !tc.pruneObjs.Equal(pruneObjs) { - t.Errorf("expected (%v) prune objs; got (%v)", tc.pruneObjs, pruneObjs) - } - }) - } - } -} - -func TestReplace(t *testing.T) { - tests := map[string]struct { - statusPolicy StatusPolicy - localObjs object.ObjMetadataSet - clusterObjs object.ObjMetadataSet - objStatus []actuation.ObjectStatus - data map[string]string - }{ - "Cluster and local inventories empty": { - localObjs: object.ObjMetadataSet{}, - clusterObjs: object.ObjMetadataSet{}, - data: map[string]string{}, - }, - "Cluster and local inventories same": { - localObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod1Info), - }, - clusterObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod1Info), - }, - objStatus: []actuation.ObjectStatus{podStatus(pod1Info)}, - data: podData("pod-1"), - statusPolicy: StatusPolicyAll, - }, - "Cluster two obj, local one": { - localObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod1Info), - }, - clusterObjs: object.ObjMetadataSet{ + updateObjs: object.ObjMetadataSet{ ignoreErrInfoToObjMeta(pod1Info), ignoreErrInfoToObjMeta(pod3Info), }, - objStatus: []actuation.ObjectStatus{podStatus(pod1Info), podStatus(pod3Info)}, - data: podData("pod-1"), - statusPolicy: StatusPolicyAll, - }, - "Cluster multiple objs, local multiple different objs": { - localObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod2Info), - }, - clusterObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod1Info), - ignoreErrInfoToObjMeta(pod2Info), - ignoreErrInfoToObjMeta(pod3Info)}, - objStatus: []actuation.ObjectStatus{podStatus(pod2Info), podStatus(pod1Info), podStatus(pod3Info)}, - data: podData("pod-2"), - statusPolicy: StatusPolicyAll, - }, - "Cluster multiple objs, local multiple different objs with StatusPolicyNone": { - localObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod2Info), - }, - clusterObjs: object.ObjMetadataSet{ - ignoreErrInfoToObjMeta(pod1Info), - ignoreErrInfoToObjMeta(pod2Info), - ignoreErrInfoToObjMeta(pod3Info)}, - objStatus: []actuation.ObjectStatus{podStatus(pod2Info), podStatus(pod1Info), podStatus(pod3Info)}, - data: podDataNoStatus("pod-2"), - statusPolicy: StatusPolicyNone, + isError: false, }, } - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() - - // Client and server dry-run do not throw errors. - invClient, err := NewClient(tf, - ConfigMapToInventoryObj, InvInfoToConfigMap, StatusPolicyAll, ConfigMapGVK) - require.NoError(t, err) - err = invClient.Replace(t.Context(), copyInventory(t), object.ObjMetadataSet{}, nil, common.DryRunClient) - if err != nil { - t.Fatalf("unexpected error received: %s", err) - } - err = invClient.Replace(t.Context(), copyInventory(t), object.ObjMetadataSet{}, nil, common.DryRunServer) - if err != nil { - t.Fatalf("unexpected error received: %s", err) - } - for name, tc := range tests { t.Run(name, func(t *testing.T) { - // Create inventory client, and store the cluster objs in the inventory object. - invClient, err := NewClient(tf, - ConfigMapToInventoryObj, InvInfoToConfigMap, tc.statusPolicy, ConfigMapGVK) - require.NoError(t, err) - wrappedInv, err := invClient.InventoryFactoryFunc(inventoryObj) + tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) + defer tf.Cleanup() + + var updateCalls int + var createCalls int + tf.FakeDynamicClient.PrependReactor("update", "configmaps", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + updateCalls++ + return false, nil, nil + }) + tf.FakeDynamicClient.PrependReactor("create", "configmaps", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + createCalls++ + return false, nil, nil + }) + + // Create the local inventory object storing "tc.localObjs" + invClient, err := ConfigMapClientFactory{StatusPolicy: StatusPolicyAll}.NewClient(tf) require.NoError(t, err) - if err := wrappedInv.Store(tc.clusterObjs, tc.objStatus); err != nil { - t.Fatalf("unexpected error storing inventory objects: %s", err) - } - inv, err := wrappedInv.GetObject() - if err != nil { - t.Fatalf("unexpected error storing inventory objects: %s", err) + + inventory := tc.inventory + if inventory != nil { + inventory.SetObjectRefs(tc.createObjs) } - // Call replaceInventory with the new set of "localObjs" - inv, _, err = invClient.replaceInventory(inv, tc.localObjs, tc.objStatus) - if err != nil { - t.Fatalf("unexpected error received: %s", err) + // Call Update an initial time should create the object + err = invClient.CreateOrUpdate(context.TODO(), inventory, UpdateOptions{}) + if tc.isError { + require.Error(t, err) + return } - wrappedInv, err = invClient.InventoryFactoryFunc(inv) require.NoError(t, err) - // Validate that the stored objects are now the "localObjs". - actualObjs, err := wrappedInv.Load() - if err != nil { - t.Fatalf("unexpected error received: %s", err) - } - if !tc.localObjs.Equal(actualObjs) { - t.Errorf("expected objects (%s), got (%s)", tc.localObjs, actualObjs) + if updateCalls != 0 || createCalls != 1 { // Update should fail, causing create + t.Fatalf("expected 0 update but got %d and 1 create but got %d", updateCalls, createCalls) } - data, _, err := unstructured.NestedStringMap(inv.Object, "data") - if err != nil { - t.Fatalf("unexpected error received: %s", err) - } - if diff := cmp.Diff(data, tc.data); diff != "" { - t.Fatal(diff) + inv, err := invClient.Get(context.TODO(), tc.inventory.Info(), GetOptions{}) + require.NoError(t, err) + if !tc.createObjs.Equal(inv.GetObjectRefs()) { + t.Fatalf("expected %v to equal %v", tc.createObjs, inv.GetObjectRefs()) } - }) - } -} -func TestGetClusterObjs(t *testing.T) { - tests := map[string]struct { - statusPolicy StatusPolicy - localInv Info - clusterObjs object.ObjMetadataSet - isError bool - }{ - "Nil cluster inventory is error": { - localInv: nil, - clusterObjs: object.ObjMetadataSet{}, - isError: true, - }, - "No cluster objs": { - localInv: copyInventory(t), - clusterObjs: object.ObjMetadataSet{}, - isError: false, - }, - "Single cluster obj": { - localInv: copyInventory(t), - clusterObjs: object.ObjMetadataSet{ignoreErrInfoToObjMeta(pod1Info)}, - isError: false, - }, - "Multiple cluster objs": { - localInv: copyInventory(t), - clusterObjs: object.ObjMetadataSet{ignoreErrInfoToObjMeta(pod1Info), ignoreErrInfoToObjMeta(pod3Info)}, - isError: false, - }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() - tf.FakeDynamicClient.PrependReactor("list", "configmaps", toReactionFunc(tc.clusterObjs)) - - invClient, err := NewClient(tf, - ConfigMapToInventoryObj, InvInfoToConfigMap, tc.statusPolicy, ConfigMapGVK) + inventory.SetObjectRefs(tc.updateObjs) + // Call Update a second time should update the existing object + err = invClient.CreateOrUpdate(context.TODO(), inventory, UpdateOptions{}) require.NoError(t, err) - clusterObjs, err := invClient.GetClusterObjs(t.Context(), tc.localInv) - if tc.isError { - if err == nil { - t.Fatalf("expected error but received none") - } - return + if updateCalls != 1 || createCalls != 1 { // Update should succeed, create not called again + t.Fatalf("expected 1 update but got %d and 1 create but got %d", updateCalls, createCalls) } - if !tc.isError && err != nil { - t.Fatalf("unexpected error received: %s", err) - } - if !tc.clusterObjs.Equal(clusterObjs) { - t.Errorf("expected (%v) cluster inventory objs; got (%v)", tc.clusterObjs, clusterObjs) + inv, err = invClient.Get(context.TODO(), tc.inventory.Info(), GetOptions{}) + require.NoError(t, err) + if !tc.updateObjs.Equal(inv.GetObjectRefs()) { + t.Fatalf("expected %v to equal %v", tc.updateObjs, inv.GetObjectRefs()) } }) } } -func TestDeleteInventoryObj(t *testing.T) { +func TestDelete(t *testing.T) { localInv, err := ConfigMapToInventoryInfo(inventoryObj) require.NoError(t, err) tests := map[string]struct { @@ -400,12 +215,12 @@ func TestDeleteInventoryObj(t *testing.T) { inv Info localObjs object.ObjMetadataSet objStatus []actuation.ObjectStatus - wantError bool + wantErr bool }{ "Nil local inventory object is an error": { inv: nil, localObjs: object.ObjMetadataSet{}, - wantError: true, + wantErr: true, }, "Empty local inventory object": { inv: localInv, @@ -433,32 +248,19 @@ func TestDeleteInventoryObj(t *testing.T) { } for name, tc := range tests { - for i := range common.Strategies { - drs := common.Strategies[i] - t.Run(name, func(t *testing.T) { - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() + t.Run(name, func(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) + defer tf.Cleanup() - invClient, err := NewClient(tf, - ConfigMapToInventoryObj, InvInfoToConfigMap, tc.statusPolicy, ConfigMapGVK) + invClient, err := ConfigMapClientFactory{StatusPolicy: StatusPolicyAll}.NewClient(tf) + require.NoError(t, err) + err = invClient.Delete(context.TODO(), tc.inv, DeleteOptions{}) + if tc.wantErr { + require.Error(t, err) + } else { require.NoError(t, err) - var obj *unstructured.Unstructured - if tc.inv != nil { - wrapped, err := storeObjsInInventory(tc.inv, tc.localObjs, tc.objStatus) - require.NoError(t, err) - err = wrapped.Apply(t.Context(), invClient.dc, invClient.mapper, tc.statusPolicy) - require.NoError(t, err) - obj, err = wrapped.GetObject() - require.NoError(t, err) - } - err = invClient.deleteInventoryObjByName(t.Context(), obj, drs) - if tc.wantError { - require.Error(t, err) - } else { - require.NoError(t, err) - } - }) - } + } + }) } } @@ -466,32 +268,3 @@ func ignoreErrInfoToObjMeta(info *resource.Info) object.ObjMetadata { objMeta, _ := object.InfoToObjMeta(info) return objMeta } - -func toReactionFunc(objs object.ObjMetadataSet) clienttesting.ReactionFunc { - return func(action clienttesting.Action) (bool, runtime.Object, error) { - u := copyInventoryInfo() - err := unstructured.SetNestedStringMap(u.Object, objs.ToStringMap(), "data") - if err != nil { - return true, nil, err - } - list := &unstructured.UnstructuredList{} - list.Items = []unstructured.Unstructured{*u} - return true, list, err - } -} - -func storeObjsInInventory(info Info, objs object.ObjMetadataSet, status []actuation.ObjectStatus) (Storage, error) { - cm, err := InvInfoToConfigMap(info) - if err != nil { - return nil, err - } - wrapped, err := ConfigMapToInventoryObj(cm) - if err != nil { - return nil, err - } - err = wrapped.Store(objs, status) - if err != nil { - return nil, err - } - return wrapped, err -} diff --git a/pkg/inventory/inventory-info.go b/pkg/inventory/inventory-info.go deleted file mode 100644 index 085becc3..00000000 --- a/pkg/inventory/inventory-info.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2020 The Kubernetes Authors. -// SPDX-License-Identifier: Apache-2.0 - -package inventory - -type Strategy string - -const ( - NameStrategy Strategy = "name" - LabelStrategy Strategy = "label" -) - -// Info provides the minimal information for the applier -// to create, look up and update an inventory. -// The inventory object can be any type, the Provider in the applier -// needs to know how to create, look up and update it based -// on the Info. -type Info interface { - // Namespace of the inventory object. - // It should be the value of the field .metadata.namespace. - Namespace() string - - // Name of the inventory object. - // It should be the value of the field .metadata.name. - Name() string - - // ID of the inventory object. It is optional. - // The Provider contained in the applier should know - // if the Id is necessary and how to use it for pruning objects. - ID() string - - Strategy() Strategy -} diff --git a/pkg/inventory/inventory_test.go b/pkg/inventory/inventory_test.go index b30776a3..22c2a059 100644 --- a/pkg/inventory/inventory_test.go +++ b/pkg/inventory/inventory_test.go @@ -7,7 +7,6 @@ import ( "regexp" "testing" - "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/cli-runtime/pkg/resource" @@ -140,7 +139,7 @@ func TestFindInventoryObj(t *testing.T) { name: "", }, "Only inventory object is true": { - infos: []*unstructured.Unstructured{copyInventoryInfo()}, + infos: []*unstructured.Unstructured{emptyInventoryObject()}, exists: true, name: inventoryObjName, }, @@ -155,7 +154,7 @@ func TestFindInventoryObj(t *testing.T) { name: "", }, "Inventory object with multiple others is true": { - infos: []*unstructured.Unstructured{pod1, pod2, copyInventoryInfo(), pod3}, + infos: []*unstructured.Unstructured{pod1, pod2, emptyInventoryObject(), pod3}, exists: true, name: inventoryObjName, }, @@ -321,20 +320,20 @@ func TestAddSuffixToName(t *testing.T) { }, // Empty suffix should return error. { - obj: copyInventoryInfo(), + obj: emptyInventoryObject(), suffix: "", expected: "", isError: true, }, // Empty suffix should return error. { - obj: copyInventoryInfo(), + obj: emptyInventoryObject(), suffix: " \t", expected: "", isError: true, }, { - obj: copyInventoryInfo(), + obj: emptyInventoryObject(), suffix: "hashsuffix", expected: inventoryObjName + "-hashsuffix", isError: false, @@ -411,13 +410,6 @@ func TestLegacyInventoryName(t *testing.T) { } } -func copyInventoryInfo() *unstructured.Unstructured { +func emptyInventoryObject() *unstructured.Unstructured { return inventoryObj.DeepCopy() } - -func copyInventory(t *testing.T) Info { - u := inventoryObj.DeepCopy() - invInfoObj, err := ConfigMapToInventoryInfo(u) - require.NoError(t, err) - return invInfoObj -} diff --git a/pkg/inventory/inventorycm.go b/pkg/inventory/inventorycm.go index 74b70762..3e6a90b4 100644 --- a/pkg/inventory/inventorycm.go +++ b/pkg/inventory/inventorycm.go @@ -9,19 +9,12 @@ package inventory import ( - "context" "encoding/json" "fmt" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" - "k8s.io/klog/v2" "sigs.k8s.io/cli-utils/pkg/apis/actuation" - "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/object" ) @@ -34,182 +27,75 @@ var ConfigMapGVK = schema.GroupVersionKind{ // ConfigMapToInventoryObj takes a passed ConfigMap (as a resource.Info), // wraps it with the ConfigMap and upcasts the wrapper as // an the Inventory interface. -func ConfigMapToInventoryObj(inv *unstructured.Unstructured) (Storage, error) { - return &ConfigMap{inv: inv}, nil +func ConfigMapToInventoryObj(uObj *unstructured.Unstructured) (Inventory, error) { + return configMapToInventory(uObj) } // ConfigMapToInventoryInfo takes a passed ConfigMap (as a resource.Info), // wraps it with the ConfigMap and upcasts the wrapper as // an the Info interface. -func ConfigMapToInventoryInfo(inv *unstructured.Unstructured) (Info, error) { - return &ConfigMap{inv: inv}, nil +func ConfigMapToInventoryInfo(uObj *unstructured.Unstructured) (Info, error) { + inv := NewUnstructuredInventory(uObj) + return inv.Info(), nil } -func InvInfoToConfigMap(inv Info) (*unstructured.Unstructured, error) { - icm, ok := inv.(*ConfigMap) - if ok { - return icm.inv, nil +// buildDataMap converts the inventory to the storage format to be used in a ConfigMap +func buildDataMap(objMetas object.ObjMetadataSet, objStatus []actuation.ObjectStatus) map[string]string { + objMap := map[string]string{} + objStatusMap := map[object.ObjMetadata]actuation.ObjectStatus{} + for _, status := range objStatus { + objStatusMap[ObjMetadataFromObjectReference(status.ObjectReference)] = status } - return nil, fmt.Errorf("failed to convert to ConfigMap") -} - -// ConfigMap wraps a ConfigMap resource and implements -// the Inventory interface. This wrapper loads and stores the -// object metadata (inventory) to and from the wrapped ConfigMap. -type ConfigMap struct { - inv *unstructured.Unstructured - objMetas object.ObjMetadataSet - objStatus []actuation.ObjectStatus -} - -var _ Info = &ConfigMap{} -var _ Storage = &ConfigMap{} - -func (icm *ConfigMap) Name() string { - return icm.inv.GetName() -} - -func (icm *ConfigMap) Namespace() string { - return icm.inv.GetNamespace() -} - -func (icm *ConfigMap) ID() string { - // Empty string if not set. - return icm.inv.GetLabels()[common.InventoryLabel] -} - -func (icm *ConfigMap) Strategy() Strategy { - return LabelStrategy + for _, objMetadata := range objMetas { + if status, found := objStatusMap[objMetadata]; found { + objMap[objMetadata.String()] = stringFrom(status) + } else { + // It's possible that the passed in status doesn't any object status + objMap[objMetadata.String()] = "" + } + } + return objMap } -func (icm *ConfigMap) UnstructuredInventory() *unstructured.Unstructured { - return icm.inv -} +var _ FromUnstructuredFunc = configMapToInventory -// Load is an Inventory interface function returning the set of -// object metadata from the wrapped ConfigMap, or an error. -func (icm *ConfigMap) Load() (object.ObjMetadataSet, error) { - objs := object.ObjMetadataSet{} - objMap, exists, err := unstructured.NestedStringMap(icm.inv.Object, "data") +func configMapToInventory(configMap *unstructured.Unstructured) (*UnstructuredInventory, error) { + inv := NewUnstructuredInventory(configMap) + objMap, exists, err := unstructured.NestedStringMap(configMap.Object, "data") if err != nil { err := fmt.Errorf("error retrieving object metadata from inventory object") - return objs, err + return nil, err } if exists { for objStr := range objMap { obj, err := object.ParseObjMetadata(objStr) if err != nil { - return objs, err + return nil, err } - objs = append(objs, obj) + inv.ObjectRefs = append(inv.ObjectRefs, obj) } } - return objs, nil + return inv, nil } -// Store is an Inventory interface function implemented to store -// the object metadata in the wrapped ConfigMap. Actual storing -// happens in "GetObject". -func (icm *ConfigMap) Store(objMetas object.ObjMetadataSet, status []actuation.ObjectStatus) error { - icm.objMetas = objMetas - icm.objStatus = status - return nil -} - -// GetObject returns the wrapped object (ConfigMap) as a resource.Info -// or an error if one occurs. -func (icm *ConfigMap) GetObject() (*unstructured.Unstructured, error) { - // Create the objMap of all the resources, and compute the hash. - objMap := buildObjMap(icm.objMetas, icm.objStatus) - // Create the inventory object by copying the template. - invCopy := icm.inv.DeepCopy() - // Adds the inventory map to the ConfigMap "data" section. - err := unstructured.SetNestedStringMap(invCopy.UnstructuredContent(), - objMap, "data") - if err != nil { - return nil, err - } - return invCopy, nil -} - -// Apply is an Storage interface function implemented to apply the inventory -// object. StatusPolicy is not needed since ConfigMaps do not have a status subresource. -func (icm *ConfigMap) Apply(ctx context.Context, dc dynamic.Interface, mapper meta.RESTMapper, _ StatusPolicy) error { - invInfo, namespacedClient, err := icm.getNamespacedClient(dc, mapper) - if err != nil { - return err - } - - // Get cluster object, if exsists. - clusterObj, err := namespacedClient.Get(ctx, invInfo.GetName(), metav1.GetOptions{}) - if err != nil && !apierrors.IsNotFound(err) { - return err - } - - // Create cluster inventory object, if it does not exist on cluster. - if clusterObj == nil { - klog.V(4).Infof("creating inventory object: %s/%s", invInfo.GetNamespace(), invInfo.GetName()) - _, err = namespacedClient.Create(ctx, invInfo, metav1.CreateOptions{}) - return err - } - - // Update the cluster inventory object instead. - klog.V(4).Infof("updating inventory object: %s/%s", invInfo.GetNamespace(), invInfo.GetName()) - _, err = namespacedClient.Update(ctx, invInfo, metav1.UpdateOptions{}) - return err -} - -// ApplyWithPrune is a Storage interface function implemented to apply the inventory object with a list of objects -// to be pruned. StatusPolicy is not needed since ConfigMaps do not have a status subresource. -func (icm *ConfigMap) ApplyWithPrune(ctx context.Context, dc dynamic.Interface, mapper meta.RESTMapper, _ StatusPolicy, _ object.ObjMetadataSet) error { - invInfo, namespacedClient, err := icm.getNamespacedClient(dc, mapper) - if err != nil { - return err - } - - // Update the cluster inventory object. - klog.V(4).Infof("updating inventory object: %s/%s", invInfo.GetNamespace(), invInfo.GetName()) - _, err = namespacedClient.Update(ctx, invInfo, metav1.UpdateOptions{}) - return err -} - -// getNamespacedClient is a helper function for Apply and ApplyWithPrune that creates a namespaced client for interacting with the live -// cluster, as well as returning the ConfigMap object as a wrapped resource.Info object. -func (icm *ConfigMap) getNamespacedClient(dc dynamic.Interface, mapper meta.RESTMapper) (*unstructured.Unstructured, dynamic.ResourceInterface, error) { - invInfo, err := icm.GetObject() - if err != nil { - return nil, nil, err - } - if invInfo == nil { - return nil, nil, fmt.Errorf("attempting to create a nil inventory object") - } - - mapping, err := mapper.RESTMapping(invInfo.GroupVersionKind().GroupKind(), invInfo.GroupVersionKind().Version) - if err != nil { - return nil, nil, err - } - - // Create client to interact with cluster. - namespacedClient := dc.Resource(mapping.Resource).Namespace(invInfo.GetNamespace()) - - return invInfo, namespacedClient, nil -} - -func buildObjMap(objMetas object.ObjMetadataSet, objStatus []actuation.ObjectStatus) map[string]string { - objMap := map[string]string{} - objStatusMap := map[object.ObjMetadata]actuation.ObjectStatus{} - for _, status := range objStatus { - objStatusMap[ObjMetadataFromObjectReference(status.ObjectReference)] = status - } - for _, objMetadata := range objMetas { - if status, found := objStatusMap[objMetadata]; found { - objMap[objMetadata.String()] = stringFrom(status) +// ConfigMap does not have an actual status, so the object statuses are persisted +// as values in the ConfigMap key/value pairs. +func inventoryToConfigMap(statusPolicy StatusPolicy) ToUnstructuredFunc { + return func(uObj *unstructured.Unstructured, inv *UnstructuredInventory) (*unstructured.Unstructured, error) { + var dataMap map[string]string + if statusPolicy == StatusPolicyAll { + dataMap = buildDataMap(inv.GetObjectRefs(), inv.GetObjectStatuses()) } else { - // It's possible that the passed in status doesn't any object status - objMap[objMetadata.String()] = "" + dataMap = buildDataMap(inv.GetObjectRefs(), nil) } + // Adds the inventory map to the ConfigMap "data" section. + err := unstructured.SetNestedStringMap(uObj.UnstructuredContent(), + dataMap, "data") + if err != nil { + return nil, err + } + return uObj, err } - return objMap } func stringFrom(status actuation.ObjectStatus) string { diff --git a/pkg/inventory/inventorycm_test.go b/pkg/inventory/inventorycm_test.go index 70222042..46196a77 100644 --- a/pkg/inventory/inventorycm_test.go +++ b/pkg/inventory/inventorycm_test.go @@ -63,7 +63,7 @@ func TestBuildObjMap(t *testing.T) { } for name, tc := range tests { t.Run(name, func(t *testing.T) { - actual := buildObjMap(tc.objSet, tc.objStatus) + actual := buildDataMap(tc.objSet, tc.objStatus) if diff := cmp.Diff(actual, tc.expected); diff != "" { t.Error(diff) } diff --git a/pkg/inventory/manager.go b/pkg/inventory/manager.go index e9f18563..9c75aa73 100644 --- a/pkg/inventory/manager.go +++ b/pkg/inventory/manager.go @@ -14,18 +14,18 @@ import ( // Manager wraps an Inventory with convenience methods that use ObjMetadata. type Manager struct { - inventory *actuation.Inventory + inventory InventoryContents } // NewManager returns a new manager instance. func NewManager() *Manager { return &Manager{ - inventory: &actuation.Inventory{}, + inventory: InventoryContents{}, } } // Inventory returns the in-memory version of the managed inventory. -func (tc *Manager) Inventory() *actuation.Inventory { +func (tc *Manager) Inventory() InventoryContents { return tc.inventory } @@ -33,9 +33,9 @@ func (tc *Manager) Inventory() *actuation.Inventory { // The returned status is a pointer and can be updated in-place for efficiency. func (tc *Manager) ObjectStatus(id object.ObjMetadata) (*actuation.ObjectStatus, bool) { ref := ObjectReferenceFromObjMetadata(id) - for i, objStatus := range tc.inventory.Status.Objects { + for i, objStatus := range tc.inventory.ObjectStatuses { if objStatus.ObjectReference == ref { - return &(tc.inventory.Status.Objects[i]), true + return &(tc.inventory.ObjectStatuses[i]), true } } return nil, false @@ -45,7 +45,7 @@ func (tc *Manager) ObjectStatus(id object.ObjMetadata) (*actuation.ObjectStatus, // specified actuation strategy and status. func (tc *Manager) ObjectsWithActuationStatus(strategy actuation.ActuationStrategy, status actuation.ActuationStatus) object.ObjMetadataSet { var ids object.ObjMetadataSet - for _, objStatus := range tc.inventory.Status.Objects { + for _, objStatus := range tc.inventory.ObjectStatuses { if objStatus.Strategy == strategy && objStatus.Actuation == status { ids = append(ids, ObjMetadataFromObjectReference(objStatus.ObjectReference)) } @@ -57,7 +57,7 @@ func (tc *Manager) ObjectsWithActuationStatus(strategy actuation.ActuationStrate // specified reconcile status, regardless of actuation strategy. func (tc *Manager) ObjectsWithReconcileStatus(status actuation.ReconcileStatus) object.ObjMetadataSet { var ids object.ObjMetadataSet - for _, objStatus := range tc.inventory.Status.Objects { + for _, objStatus := range tc.inventory.ObjectStatuses { if objStatus.Reconcile == status { ids = append(ids, ObjMetadataFromObjectReference(objStatus.ObjectReference)) } @@ -67,13 +67,13 @@ func (tc *Manager) ObjectsWithReconcileStatus(status actuation.ReconcileStatus) // SetObjectStatus updates or adds an ObjectStatus record to the inventory. func (tc *Manager) SetObjectStatus(newObjStatus actuation.ObjectStatus) { - for i, oldObjStatus := range tc.inventory.Status.Objects { + for i, oldObjStatus := range tc.inventory.ObjectStatuses { if oldObjStatus.ObjectReference == newObjStatus.ObjectReference { - tc.inventory.Status.Objects[i] = newObjStatus + tc.inventory.ObjectStatuses[i] = newObjStatus return } } - tc.inventory.Status.Objects = append(tc.inventory.Status.Objects, newObjStatus) + tc.inventory.ObjectStatuses = append(tc.inventory.ObjectStatuses, newObjStatus) } // IsSuccessfulApply returns true if the object apply was successful @@ -119,7 +119,7 @@ func (tc *Manager) AppliedResourceUID(id object.ObjMetadata) (types.UID, bool) { // successfully applied resources. func (tc *Manager) AppliedResourceUIDs() sets.String { // nolint:staticcheck uids := sets.NewString() - for _, objStatus := range tc.inventory.Status.Objects { + for _, objStatus := range tc.inventory.ObjectStatuses { if objStatus.Strategy == actuation.ActuationStrategyApply && objStatus.Actuation == actuation.ActuationSucceeded { if objStatus.UID != "" { diff --git a/pkg/inventory/policy.go b/pkg/inventory/policy.go index 0f906a56..cae3d02a 100644 --- a/pkg/inventory/policy.go +++ b/pkg/inventory/policy.go @@ -89,7 +89,7 @@ func IDMatch(inv Info, obj *unstructured.Unstructured) IDMatchStatus { if !found { return Empty } - if value == inv.ID() { + if value == inv.GetID().String() { return Match } return NoMatch @@ -141,11 +141,11 @@ func CanPrune(inv Info, obj *unstructured.Unstructured, policy Policy) (bool, er } } -func AddInventoryIDAnnotation(obj *unstructured.Unstructured, inv Info) { +func AddInventoryIDAnnotation(obj *unstructured.Unstructured, inventoryID ID) { annotations := obj.GetAnnotations() if annotations == nil { annotations = make(map[string]string) } - annotations[OwningInventoryKey] = inv.ID() + annotations[OwningInventoryKey] = inventoryID.String() obj.SetAnnotations(annotations) } diff --git a/pkg/inventory/policy_test.go b/pkg/inventory/policy_test.go index 72196d99..63beb866 100644 --- a/pkg/inventory/policy_test.go +++ b/pkg/inventory/policy_test.go @@ -12,26 +12,6 @@ import ( "sigs.k8s.io/cli-utils/pkg/testutil" ) -type fakeInventoryInfo struct { - id string -} - -func (i *fakeInventoryInfo) Name() string { - return "" -} - -func (i *fakeInventoryInfo) Namespace() string { - return "" -} - -func (i *fakeInventoryInfo) ID() string { - return i.id -} - -func (i *fakeInventoryInfo) Strategy() Strategy { - return NameStrategy -} - func testObjectWithAnnotation(key, val string) *unstructured.Unstructured { obj := &unstructured.Unstructured{ Object: map[string]interface{}{ @@ -61,19 +41,19 @@ func TestInventoryIDMatch(t *testing.T) { { name: "empty", obj: testObjectWithAnnotation("", ""), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, expected: Empty, }, { name: "matched", obj: testObjectWithAnnotation(OwningInventoryKey, "matched"), - inv: &fakeInventoryInfo{id: "matched"}, + inv: &SimpleInfo{id: "matched"}, expected: Match, }, { name: "unmatched", obj: testObjectWithAnnotation(OwningInventoryKey, "unmatched"), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, expected: NoMatch, }, } @@ -97,21 +77,21 @@ func TestCanApply(t *testing.T) { { name: "empty with AdoptIfNoInventory", obj: testObjectWithAnnotation("", ""), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyAdoptIfNoInventory, canApply: true, }, { name: "empty with AdoptAll", obj: testObjectWithAnnotation("", ""), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyAdoptAll, canApply: true, }, { name: "empty with InventoryPolicyMustMatch", obj: testObjectWithAnnotation("", ""), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyMustMatch, canApply: false, expectedError: &PolicyPreventedActuationError{ @@ -123,28 +103,28 @@ func TestCanApply(t *testing.T) { { name: "matched with InventoryPolicyMustMatch", obj: testObjectWithAnnotation(OwningInventoryKey, "matched"), - inv: &fakeInventoryInfo{id: "matched"}, + inv: &SimpleInfo{id: "matched"}, policy: PolicyMustMatch, canApply: true, }, { name: "matched with AdoptIfNoInventory", obj: testObjectWithAnnotation(OwningInventoryKey, "matched"), - inv: &fakeInventoryInfo{id: "matched"}, + inv: &SimpleInfo{id: "matched"}, policy: PolicyAdoptIfNoInventory, canApply: true, }, { name: "matched with AloptAll", obj: testObjectWithAnnotation(OwningInventoryKey, "matched"), - inv: &fakeInventoryInfo{id: "matched"}, + inv: &SimpleInfo{id: "matched"}, policy: PolicyAdoptAll, canApply: true, }, { name: "unmatched with InventoryPolicyMustMatch", obj: testObjectWithAnnotation(OwningInventoryKey, "unmatched"), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyMustMatch, canApply: false, expectedError: &PolicyPreventedActuationError{ @@ -156,7 +136,7 @@ func TestCanApply(t *testing.T) { { name: "unmatched with AdoptIfNoInventory", obj: testObjectWithAnnotation(OwningInventoryKey, "unmatched"), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyAdoptIfNoInventory, canApply: false, expectedError: &PolicyPreventedActuationError{ @@ -168,7 +148,7 @@ func TestCanApply(t *testing.T) { { name: "unmatched with AdoptAll", obj: testObjectWithAnnotation(OwningInventoryKey, "unmatched"), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyAdoptAll, canApply: true, }, @@ -194,21 +174,21 @@ func TestCanPrune(t *testing.T) { { name: "empty with AdoptIfNoInventory", obj: testObjectWithAnnotation("", ""), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyAdoptIfNoInventory, canPrune: true, }, { name: "empty with AdoptAll", obj: testObjectWithAnnotation("", ""), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyAdoptAll, canPrune: true, }, { name: "empty with PolicyMustMatch", obj: testObjectWithAnnotation("", ""), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyMustMatch, canPrune: false, expectedError: &PolicyPreventedActuationError{ @@ -220,28 +200,28 @@ func TestCanPrune(t *testing.T) { { name: "matched with PolicyMustMatch", obj: testObjectWithAnnotation(OwningInventoryKey, "matched"), - inv: &fakeInventoryInfo{id: "matched"}, + inv: &SimpleInfo{id: "matched"}, policy: PolicyMustMatch, canPrune: true, }, { name: "matched with AdoptIfNoInventory", obj: testObjectWithAnnotation(OwningInventoryKey, "matched"), - inv: &fakeInventoryInfo{id: "matched"}, + inv: &SimpleInfo{id: "matched"}, policy: PolicyAdoptIfNoInventory, canPrune: true, }, { name: "matched with AloptAll", obj: testObjectWithAnnotation(OwningInventoryKey, "matched"), - inv: &fakeInventoryInfo{id: "matched"}, + inv: &SimpleInfo{id: "matched"}, policy: PolicyAdoptAll, canPrune: true, }, { name: "unmatched with PolicyMustMatch", obj: testObjectWithAnnotation(OwningInventoryKey, "unmatched"), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyMustMatch, canPrune: false, expectedError: &PolicyPreventedActuationError{ @@ -253,7 +233,7 @@ func TestCanPrune(t *testing.T) { { name: "unmatched with AdoptIfNoInventory", obj: testObjectWithAnnotation(OwningInventoryKey, "unmatched"), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyAdoptIfNoInventory, canPrune: false, expectedError: &PolicyPreventedActuationError{ @@ -265,7 +245,7 @@ func TestCanPrune(t *testing.T) { { name: "unmatched with AdoptAll", obj: testObjectWithAnnotation(OwningInventoryKey, "unmatched"), - inv: &fakeInventoryInfo{id: "random-id"}, + inv: &SimpleInfo{id: "random-id"}, policy: PolicyAdoptAll, canPrune: true, }, diff --git a/pkg/inventory/storage.go b/pkg/inventory/storage.go index ebfcf7ca..736d6b32 100644 --- a/pkg/inventory/storage.go +++ b/pkg/inventory/storage.go @@ -12,15 +12,11 @@ package inventory import ( - "context" "fmt" "strings" - "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/dynamic" "k8s.io/klog/v2" - "sigs.k8s.io/cli-utils/pkg/apis/actuation" "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/object" ) @@ -28,35 +24,6 @@ import ( // The default inventory name stored in the inventory template. const legacyInvName = "inventory" -// Storage describes methods necessary for an object which -// can persist the object metadata for pruning and other group -// operations. -type Storage interface { - // Load retrieves the set of object metadata from the inventory object - Load() (object.ObjMetadataSet, error) - // Store the set of object metadata in the inventory object. This will - // replace the metadata, spec and status. - Store(objs object.ObjMetadataSet, status []actuation.ObjectStatus) error - // GetObject returns the object that stores the inventory - GetObject() (*unstructured.Unstructured, error) - // Apply applies the inventory object. This utility function is used - // in InventoryClient.Merge and merges the metadata, spec and status. - Apply(context.Context, dynamic.Interface, meta.RESTMapper, StatusPolicy) error - // ApplyWithPrune applies the inventory object with a set of pruneIDs of - // objects to be pruned (object.ObjMetadataSet). This function is used in - // InventoryClient.Replace. pruneIDs are required for enabling custom logic - // handling of multiple ResourceGroup inventories. - ApplyWithPrune(context.Context, dynamic.Interface, meta.RESTMapper, StatusPolicy, object.ObjMetadataSet) error -} - -// StorageFactoryFunc creates the object which implements the Inventory -// interface from the passed info object. -type StorageFactoryFunc func(*unstructured.Unstructured) (Storage, error) - -// ToUnstructuredFunc returns the unstructured object for the -// given Info. -type ToUnstructuredFunc func(Info) (*unstructured.Unstructured, error) - // FindInventoryObj returns the "Inventory" object (ConfigMap with // inventory label) if it exists, or nil if it does not exist. func FindInventoryObj(objs object.UnstructuredSet) *unstructured.Unstructured { diff --git a/test/e2e/customprovider/provider.go b/test/e2e/customprovider/provider.go index 80395386..9bdc2bcc 100644 --- a/test/e2e/customprovider/provider.go +++ b/test/e2e/customprovider/provider.go @@ -4,19 +4,11 @@ package customprovider import ( - "context" - "fmt" "strings" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" "k8s.io/kubectl/pkg/cmd/util" - "sigs.k8s.io/cli-utils/pkg/apis/actuation" - "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/inventory" "sigs.k8s.io/cli-utils/pkg/object" ) @@ -112,86 +104,12 @@ type CustomClientFactory struct { } func (CustomClientFactory) NewClient(factory util.Factory) (inventory.Client, error) { - return inventory.NewClient(factory, - WrapInventoryObj, invToUnstructuredFunc, inventory.StatusPolicyAll, inventory.ConfigMapGVK) + return inventory.NewUnstructuredClient(factory, fromUnstructured, toUnstructured, InventoryGVK, inventory.StatusPolicyAll) } -func invToUnstructuredFunc(inv inventory.Info) (*unstructured.Unstructured, error) { - switch invInfo := inv.(type) { - case *InventoryCustomType: - return invInfo.inv, nil - default: - return nil, fmt.Errorf("wrong type: want %T but got %T", InventoryCustomType{}, inv) - } -} - -func WrapInventoryObj(obj *unstructured.Unstructured) (inventory.Storage, error) { - return &InventoryCustomType{inv: obj}, nil -} - -func WrapInventoryInfoObj(obj *unstructured.Unstructured) inventory.Info { - return &InventoryCustomType{inv: obj} -} - -var _ inventory.Storage = &InventoryCustomType{} -var _ inventory.Info = &InventoryCustomType{} - -type InventoryCustomType struct { - inv *unstructured.Unstructured -} - -func (i InventoryCustomType) Namespace() string { - return i.inv.GetNamespace() -} - -func (i InventoryCustomType) Name() string { - return i.inv.GetName() -} - -func (i InventoryCustomType) Strategy() inventory.Strategy { - return inventory.NameStrategy -} - -func (i InventoryCustomType) ID() string { - labels := i.inv.GetLabels() - id, found := labels[common.InventoryLabel] - if !found { - return "" - } - return id -} - -func (i InventoryCustomType) Load() (object.ObjMetadataSet, error) { - var inv object.ObjMetadataSet - s, found, err := unstructured.NestedSlice(i.inv.Object, "spec", "objects") - if err != nil { - return inv, err - } - if !found { - return inv, nil - } - for _, item := range s { - m := item.(map[string]interface{}) - namespace, _, _ := unstructured.NestedString(m, "namespace") - name, _, _ := unstructured.NestedString(m, "name") - group, _, _ := unstructured.NestedString(m, "group") - kind, _, _ := unstructured.NestedString(m, "kind") - id := object.ObjMetadata{ - Namespace: namespace, - Name: name, - GroupKind: schema.GroupKind{ - Group: group, - Kind: kind, - }, - } - inv = append(inv, id) - } - return inv, nil -} - -func (i InventoryCustomType) Store(objs object.ObjMetadataSet, status []actuation.ObjectStatus) error { +func toUnstructured(uObj *unstructured.Unstructured, inv *inventory.UnstructuredInventory) (*unstructured.Unstructured, error) { var specObjs []interface{} - for _, obj := range objs { + for _, obj := range inv.ObjectRefs { specObjs = append(specObjs, map[string]interface{}{ "group": obj.GroupKind.Group, "kind": obj.GroupKind.Kind, @@ -200,7 +118,7 @@ func (i InventoryCustomType) Store(objs object.ObjMetadataSet, status []actuatio }) } var statusObjs []interface{} - for _, objStatus := range status { + for _, objStatus := range inv.ObjectStatuses { statusObjs = append(statusObjs, map[string]interface{}{ "group": objStatus.Group, "kind": objStatus.Kind, @@ -212,95 +130,52 @@ func (i InventoryCustomType) Store(objs object.ObjMetadataSet, status []actuatio }) } if len(specObjs) > 0 { - err := unstructured.SetNestedSlice(i.inv.Object, specObjs, "spec", "objects") + err := unstructured.SetNestedSlice(uObj.Object, specObjs, "spec", "objects") if err != nil { - return err + return nil, err } } else { - unstructured.RemoveNestedField(i.inv.Object, "spec") + unstructured.RemoveNestedField(uObj.Object, "spec") } if len(statusObjs) > 0 { - err := unstructured.SetNestedSlice(i.inv.Object, statusObjs, "status", "objects") + err := unstructured.SetNestedSlice(uObj.Object, statusObjs, "status", "objects") if err != nil { - return err + return nil, err } } else { - unstructured.RemoveNestedField(i.inv.Object, "status") + unstructured.RemoveNestedField(uObj.Object, "status") } - return nil + return uObj, nil } -func (i InventoryCustomType) GetObject() (*unstructured.Unstructured, error) { - return i.inv, nil -} - -// Apply is an Inventory interface function implemented to apply the inventory -// object. -func (i InventoryCustomType) Apply(ctx context.Context, dc dynamic.Interface, mapper meta.RESTMapper, _ inventory.StatusPolicy) error { - invInfo, namespacedClient, err := i.getNamespacedClient(dc, mapper) +func fromUnstructured(obj *unstructured.Unstructured) (*inventory.UnstructuredInventory, error) { + inv := inventory.NewUnstructuredInventory(obj) + s, found, err := unstructured.NestedSlice(obj.Object, "spec", "objects") if err != nil { - return err - } - - // Get cluster object, if exsists. - clusterObj, err := namespacedClient.Get(ctx, invInfo.GetName(), metav1.GetOptions{}) - if err != nil && !apierrors.IsNotFound(err) { - return err + return nil, err } - - var appliedObj *unstructured.Unstructured - - if clusterObj == nil { - // Create cluster inventory object, if it does not exist on cluster. - appliedObj, err = namespacedClient.Create(ctx, invInfo, metav1.CreateOptions{}) - } else { - // Update the cluster inventory object instead. - appliedObj, err = namespacedClient.Update(ctx, invInfo, metav1.UpdateOptions{}) - } - if err != nil { - return err - } - - // Update status. - invInfo.SetResourceVersion(appliedObj.GetResourceVersion()) - _, err = namespacedClient.UpdateStatus(ctx, invInfo, metav1.UpdateOptions{}) - return err -} - -func (i InventoryCustomType) ApplyWithPrune(ctx context.Context, dc dynamic.Interface, mapper meta.RESTMapper, _ inventory.StatusPolicy, _ object.ObjMetadataSet) error { - invInfo, namespacedClient, err := i.getNamespacedClient(dc, mapper) - if err != nil { - return err + if !found { + return inv, nil } - - // Update the cluster inventory object. - appliedObj, err := namespacedClient.Update(ctx, invInfo, metav1.UpdateOptions{}) - if err != nil { - return err + for _, item := range s { + m := item.(map[string]interface{}) + namespace, _, _ := unstructured.NestedString(m, "namespace") + name, _, _ := unstructured.NestedString(m, "name") + group, _, _ := unstructured.NestedString(m, "group") + kind, _, _ := unstructured.NestedString(m, "kind") + id := object.ObjMetadata{ + Namespace: namespace, + Name: name, + GroupKind: schema.GroupKind{ + Group: group, + Kind: kind, + }, + } + inv.ObjectRefs = append(inv.ObjectRefs, id) } - - // Update status. - invInfo.SetResourceVersion(appliedObj.GetResourceVersion()) - _, err = namespacedClient.UpdateStatus(ctx, invInfo, metav1.UpdateOptions{}) - return err + return inv, nil } -func (i InventoryCustomType) getNamespacedClient(dc dynamic.Interface, mapper meta.RESTMapper) (*unstructured.Unstructured, dynamic.ResourceInterface, error) { - invInfo, err := i.GetObject() - if err != nil { - return nil, nil, err - } - if invInfo == nil { - return nil, nil, fmt.Errorf("attempting to create a nil inventory object") - } - - mapping, err := mapper.RESTMapping(invInfo.GroupVersionKind().GroupKind(), invInfo.GroupVersionKind().Version) - if err != nil { - return nil, nil, err - } - - // Create client to interact with cluster. - namespacedClient := dc.Resource(mapping.Resource).Namespace(invInfo.GetNamespace()) - - return invInfo, namespacedClient, nil +func WrapInventoryInfoObj(obj *unstructured.Unstructured) inventory.Info { + return inventory.NewUnstructuredInventory(obj).Info() } diff --git a/test/e2e/deletion_prevention_test.go b/test/e2e/deletion_prevention_test.go index f15c7021..f4ee8253 100644 --- a/test/e2e/deletion_prevention_test.go +++ b/test/e2e/deletion_prevention_test.go @@ -38,15 +38,15 @@ func deletionPreventionTest(ctx context.Context, c client.Client, invConfig invc By("Verify deployment created") obj := e2eutil.AssertUnstructuredExists(ctx, c, e2eutil.WithNamespace(e2eutil.ManifestToUnstructured(deployment1), namespaceName)) - Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.ID())) + Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.GetID().String())) By("Verify pod1 created") obj = e2eutil.AssertUnstructuredExists(ctx, c, e2eutil.WithNamespace(e2eutil.ManifestToUnstructured(pod1), namespaceName)) - Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.ID())) + Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.GetID().String())) By("Verify pod2 created") obj = e2eutil.AssertUnstructuredExists(ctx, c, e2eutil.WithNamespace(e2eutil.ManifestToUnstructured(pod2), namespaceName)) - Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.ID())) + Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.GetID().String())) By("Verify the inventory size is 3") invConfig.InvSizeVerifyFunc(ctx, c, inventoryName, namespaceName, inventoryID, 3, 3) @@ -63,15 +63,15 @@ func deletionPreventionTest(ctx context.Context, c client.Client, invConfig invc By("Verify deployment still exists and has the config.k8s.io/owning-inventory annotation") obj = e2eutil.AssertUnstructuredExists(ctx, c, e2eutil.WithNamespace(e2eutil.ManifestToUnstructured(deployment1), namespaceName)) - Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.ID())) + Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.GetID().String())) By("Verify pod1 still exits and has the config.k8s.io/owning-inventory annotation") obj = e2eutil.AssertUnstructuredExists(ctx, c, e2eutil.WithNamespace(e2eutil.ManifestToUnstructured(pod1), namespaceName)) - Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.ID())) + Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.GetID().String())) By("Verify pod2 still exits and has the config.k8s.io/owning-inventory annotation") obj = e2eutil.AssertUnstructuredExists(ctx, c, e2eutil.WithNamespace(e2eutil.ManifestToUnstructured(pod2), namespaceName)) - Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.ID())) + Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.GetID().String())) By("Verify the inventory size is still 3") invConfig.InvSizeVerifyFunc(ctx, c, inventoryName, namespaceName, inventoryID, 3, 3) @@ -87,7 +87,7 @@ func deletionPreventionTest(ctx context.Context, c client.Client, invConfig invc By("Verify deployment still exists and has the config.k8s.io/owning-inventory annotation") obj = e2eutil.AssertUnstructuredExists(ctx, c, e2eutil.WithNamespace(e2eutil.ManifestToUnstructured(deployment1), namespaceName)) - Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.ID())) + Expect(obj.GetAnnotations()[inventory.OwningInventoryKey]).To(Equal(inventoryInfo.GetID().String())) By("Verify pod1 still exits and does not have the config.k8s.io/owning-inventory annotation") obj = e2eutil.AssertUnstructuredExists(ctx, c, e2eutil.WithNamespace(e2eutil.ManifestToUnstructured(pod1), namespaceName)) diff --git a/test/e2e/invconfig/configmap.go b/test/e2e/invconfig/configmap.go index edbbc593..517811bd 100644 --- a/test/e2e/invconfig/configmap.go +++ b/test/e2e/invconfig/configmap.go @@ -19,7 +19,6 @@ import ( func NewConfigMapTypeInvConfig(cfg *rest.Config) InventoryConfig { return InventoryConfig{ ClientConfig: cfg, - Strategy: inventory.LabelStrategy, FactoryFunc: cmInventoryManifest, InvWrapperFunc: func(obj *unstructured.Unstructured) inventory.Info { info, err := inventory.ConfigMapToInventoryInfo(obj) diff --git a/test/e2e/invconfig/custom.go b/test/e2e/invconfig/custom.go index c0deb0dd..79400ba4 100644 --- a/test/e2e/invconfig/custom.go +++ b/test/e2e/invconfig/custom.go @@ -16,7 +16,6 @@ import ( "k8s.io/client-go/rest" "sigs.k8s.io/cli-utils/pkg/apply" "sigs.k8s.io/cli-utils/pkg/common" - "sigs.k8s.io/cli-utils/pkg/inventory" "sigs.k8s.io/cli-utils/test/e2e/customprovider" "sigs.k8s.io/cli-utils/test/e2e/e2eutil" "sigs.k8s.io/controller-runtime/pkg/client" @@ -25,7 +24,6 @@ import ( func NewCustomTypeInvConfig(cfg *rest.Config) InventoryConfig { return InventoryConfig{ ClientConfig: cfg, - Strategy: inventory.NameStrategy, FactoryFunc: customInventoryManifest, InvWrapperFunc: customprovider.WrapInventoryInfoObj, ApplierFactoryFunc: newCustomInvApplierFactory(cfg), diff --git a/test/e2e/invconfig/invconfig.go b/test/e2e/invconfig/invconfig.go index e929e3a8..609d17a7 100644 --- a/test/e2e/invconfig/invconfig.go +++ b/test/e2e/invconfig/invconfig.go @@ -5,7 +5,6 @@ package invconfig import ( "context" - "fmt" "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -14,7 +13,6 @@ import ( "k8s.io/kubectl/pkg/cmd/util" "sigs.k8s.io/cli-utils/pkg/apply" "sigs.k8s.io/cli-utils/pkg/inventory" - "sigs.k8s.io/cli-utils/test/e2e/e2eutil" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -28,7 +26,6 @@ type invNotExistsFunc func(ctx context.Context, c client.Client, name, namespace type InventoryConfig struct { ClientConfig *rest.Config - Strategy inventory.Strategy FactoryFunc inventoryFactoryFunc InvWrapperFunc invWrapperFunc ApplierFactoryFunc applierFactoryFunc @@ -39,14 +36,7 @@ type InventoryConfig struct { } func CreateInventoryInfo(invConfig InventoryConfig, inventoryName, namespaceName, inventoryID string) inventory.Info { - switch invConfig.Strategy { - case inventory.NameStrategy: - return invConfig.InvWrapperFunc(invConfig.FactoryFunc(inventoryName, namespaceName, e2eutil.RandomString("inventory-"))) - case inventory.LabelStrategy: - return invConfig.InvWrapperFunc(invConfig.FactoryFunc(e2eutil.RandomString("inventory-"), namespaceName, inventoryID)) - default: - panic(fmt.Errorf("unknown inventory strategy %q", invConfig.Strategy)) - } + return invConfig.InvWrapperFunc(invConfig.FactoryFunc(inventoryName, namespaceName, inventoryID)) } func newFactory(cfg *rest.Config) util.Factory { diff --git a/test/e2e/name_inv_strategy_test.go b/test/e2e/name_inv_strategy_test.go index abcae389..bab2a440 100644 --- a/test/e2e/name_inv_strategy_test.go +++ b/test/e2e/name_inv_strategy_test.go @@ -47,7 +47,5 @@ func applyWithExistingInvTest(ctx context.Context, c client.Client, invConfig in By("Verify that we get the correct error") Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring( - fmt.Sprintf("inventory-id of inventory object %s/%s in cluster doesn't match provided id", - namespaceName, inventoryName))) + Expect(err.Error()).To(ContainSubstring("expected inventory object to have inventory-id")) }