feat: rewrite inventory client with basic CRUD

This change rewrites the inventory client interface using simple CRUD
methods. Prior to this change the inventory client consisted of several
tightly coupled interfaces which made the client more complicated and
less flexible.

The old client also performed a Get of the inventory many times
throughout the applier/destroyer processes, including before every
update. This approach discarded optimistic concurrency protection since
the resourceVersion was never persisted. The new approach preserves an
internal interface representation of the inventory so that the
resourceVersion can be preserved across updates.

The new client interface is simpler to implement and is intended to
support implementations for sharded inventories, which may consist of
multiple underlying KRM objects. The prior interfaces made assumptions
of a single KRM object in several places, which made it incompatible
with a sharded inventory.

Author: sdowell

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.ObjectRefs()
+
 	printData := printer.PrintData{
 		Identifiers: object.ObjMetadataSet{},
 		InvNameMap:  make(map[object.ObjMetadata]string),
@@ -179,7 +181,7 @@ 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()
+			printData.InvNameMap[obj] = invInfo.(inventory.SingleObjectInfo).Name()
 			// append to identifiers
 			printData.Identifiers = append(printData.Identifiers, obj)
 		}
@@ -201,11 +203,16 @@ 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 {
+		identifiersMap[inv.Info().(inventory.SingleObjectInfo).Name()] = inv.ObjectRefs()
+	}
+
 	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 {

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",
 				},

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,

pkg/apis/actuation/zz_generated.deepcopy.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,19 +53,19 @@ 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 {
 		return nil, nil, err
 	}
 	// Add the inventory annotation to the resources being applied.
 	for _, localObj := range localObjs {
-		inventory.AddInventoryIDAnnotation(localObj, localInv)
+		inventory.AddInventoryIDAnnotation(localObj, inv.Info().ID())
 	}
-	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().ID() != invInfo.ID() {
+			handleError(eventChannel, fmt.Errorf("expected inventory object to have inventory-id %q but got %q",
+				invInfo.ID(), inv.Info().ID()))
+			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))