address comments

k8sdeps/transformer/inventory/inventory.go

@@ -29,7 +29,7 @@ import (
 	"sigs.k8s.io/kustomize/pkg/types"
 )
 
-// inventoryTransformer compute the ConfigMap used in prune
+// inventoryTransformer compute the inventory object used in prune
 type inventoryTransformer struct {
 	append      bool
 	cmName      string
@@ -50,25 +50,28 @@ func NewInventoryTransformer(p *types.Inventory, namespace string, append bool)
 	}
 }
 
-// Transform generates an inventory ConfigMap based on the input ResMap.
-// this tranformer doesn't change existing resources -
+// Transform generates an inventory object based on the input ResMap.
+// this transformer doesn't change existing resources -
 // it just visits resources and accumulates information to make a new ConfigMap.
 // The prune ConfigMap is used to support the pruning command in the client side tool,
 // which is proposed in https://github.com/kubernetes/enhancements/pull/810
+// The inventory data is written to annotation since
+//   1. The key in data field is constrained and couldn't include arbitrary letters
+//   2. The annotation can be put into any kind of objects
 func (o *inventoryTransformer) Transform(m resmap.ResMap) error {
 	invty := inventory.NewInventory()
 	var keys []string
 	for _, r := range m {
 		ns, _ := r.GetFieldValue("metadata.namespace")
-		item := resid.New(r.GetGvk(), ns, r.GetName())
+		item := resid.NewItemId(r.GetGvk(), ns, r.GetName())
 		var refs []resid.ItemId
 
 		for _, refid := range r.GetRefBy() {
 			ref := m[refid]
 			ns, _ := ref.GetFieldValue("metadata.namespace")
-			refs = append(refs, resid.New(ref.GetGvk(), ns, ref.GetName()))
+			refs = append(refs, resid.NewItemId(ref.GetGvk(), ns, ref.GetName()))
 		}
-		invty.Current[item.String()] = refs
+		invty.Current[item] = refs
 		keys = append(keys, item.String())
 	}
 	h, err := hash.SortArrayAndComputeHash(keys)

pkg/inventory/inventory.go

@@ -22,9 +22,20 @@ import (
 	"sigs.k8s.io/kustomize/pkg/resid"
 )
 
-// A Refs is a map from an Item string to a list of Items
-// that it is referred
-type Refs map[string][]resid.ItemId
+//Refs is a reference map.  Each key is the id
+//of a k8s resource, and each value is a list of
+//object ids that refer back to the object in the
+//key.
+
+//For example, the key could correspond to a
+//ConfigMap, and the list of values might include
+//several different Deployments that get data from
+//that ConfigMap (and thus refer to it).
+
+//References are important in inventory management
+//because one may not delete an object before all
+//objects referencing it have been removed.
+type Refs map[resid.ItemId][]resid.ItemId
 
 func NewRefs() Refs {
 	return Refs{}
@@ -43,24 +54,36 @@ func (rf Refs) Merge(b Refs) Refs {
 	return rf
 }
 
-// RemoveIfContains removes the reference relationship
+// removeIfContains removes the reference relationship
 //  a --> b
 // from the Refs if it exists
-func (rf Refs) RemoveIfContains(a string, b resid.ItemId) {
+func (rf Refs) RemoveIfContains(a, b resid.ItemId) {
 	refs, ok := rf[a]
 	if !ok {
 		return
 	}
 	for i, ref := range refs {
-		if ref.String() == b.String() {
+		if ref.Equals(b) {
 			rf[a] = append(refs[:i], refs[i+1:]...)
 			break
 		}
 	}
 }
 
-// An Inventory contains current refs
-// and previous refs
+//Inventory is a an object intended for
+//serialization into the annotations of a so-called
+//apply-root object (a ConfigMap, an Application,
+//etc.) living in the cluster.  This apply-root
+//object is written as part of an apply operation as
+//a means to record overall cluster state changes.
+
+//At the end of a successful apply, the "current"
+//field in Inventory will be a map whose keys all
+//correspond to an object in the cluster, and
+//"previous" will be the previous such set (an empty
+//set on the first apply).
+
+//An Inventory allows the Prune method to work.
 type Inventory struct {
 	Current  Refs `json:"current,omitempty"`
 	Previous Refs `json:"previous,omitempty"`
@@ -88,43 +111,10 @@ func (a *Inventory) UpdateCurrent(curref Refs) *Inventory {
 	return a
 }
 
-// Prune returns a list of Items that can be pruned
-// as well as updates the Inventory
-func (a *Inventory) Prune() []resid.ItemId {
+func (a *Inventory) removeNewlyOrphanedItemsFromPrevious() []resid.ItemId {
 	var results []resid.ItemId
-	curref := a.Current
-
-	// Remove references that are already in Current refs
-	for item, refs := range curref {
-		for _, ref := range refs {
-			a.Previous.RemoveIfContains(item, ref)
-		}
-	}
-	// Remove items that are already in Current refs
 	for item, refs := range a.Previous {
-		if len(refs) == 0 {
-			if _, ok := curref[item]; ok {
-				delete(a.Previous, item)
-			}
-		}
-	}
-
-	// Remove items from the Previous refs
-	// that are not referred by others
-	for item, refs := range a.Previous {
-		if _, ok := curref[item]; ok {
-			continue
-		}
-		if len(refs) == 0 {
-			results = append(results, resid.FromString(item))
-			delete(a.Previous, item)
-		}
-	}
-
-	// Remove items from the Previous refs
-	// that are referred only by to be deleted items
-	for item, refs := range a.Previous {
-		if _, ok := curref[item]; ok {
+		if _, ok := a.Current[item]; ok {
 			delete(a.Previous, item)
 			continue
 		}
@@ -132,13 +122,13 @@ func (a *Inventory) Prune() []resid.ItemId {
 		var newRefs []resid.ItemId
 		toDelete := true
 		for _, ref := range refs {
-			if _, ok := curref[ref.String()]; ok {
+			if _, ok := a.Current[ref]; ok {
 				toDelete = false
 				newRefs = append(newRefs, ref)
 			}
 		}
 		if toDelete {
-			results = append(results, resid.FromString(item))
+			results = append(results, item)
 			delete(a.Previous, item)
 		} else {
 			a.Previous[item] = newRefs
@@ -147,14 +137,98 @@ func (a *Inventory) Prune() []resid.ItemId {
 	return results
 }
 
+func (a *Inventory) removeOrphanedItemsFromPreviousThatAreNotInCurrent() []resid.ItemId {
+	var results []resid.ItemId
+	for item, refs := range a.Previous {
+		if _, ok := a.Current[item]; ok {
+			continue
+		}
+		if len(refs) == 0 {
+			results = append(results, item)
+			delete(a.Previous, item)
+		}
+	}
+	return results
+}
+
+func (a *Inventory) removeOrphanedItemsFromPreviousThatAreInCurrent() {
+	//Remove references from Previous that are already in Current refs
+	for item, refs := range a.Current {
+		for _, ref := range refs {
+			a.Previous.RemoveIfContains(item, ref)
+		}
+	}
+	//Remove items from Previous that are already in Current refs
+	for item, refs := range a.Previous {
+		if len(refs) == 0 {
+			if _, ok := a.Current[item]; ok {
+				delete(a.Previous, item)
+			}
+		}
+	}
+}
+
+// Prune computes the diff of Current refs and Previous refs
+// and returns a list of Items that can be pruned.
+// An item that can be pruned shows up only in Previous refs.
+// Prune also updates the Previous refs with those items removed
+func (a *Inventory) Prune() []resid.ItemId {
+	a.removeOrphanedItemsFromPreviousThatAreInCurrent()
+
+	// These are candidates for deletion from the cluster.
+	removable1 := a.removeOrphanedItemsFromPreviousThatAreNotInCurrent()
+	removable2 := a.removeNewlyOrphanedItemsFromPrevious()
+	return append(removable1, removable2...)
+}
+
+// inventory is the internal type used for serialization
+type inventory struct {
+	Current  map[string][]resid.ItemId `json:"current,omitempty"`
+	Previous map[string][]resid.ItemId `json:"previous,omitempty"`
+}
+
+func (a *Inventory) toInternalType() inventory {
+	prev := map[string][]resid.ItemId{}
+	curr := map[string][]resid.ItemId{}
+	for id, refs := range a.Current {
+		curr[id.String()] = refs
+	}
+	for id, refs := range a.Previous {
+		prev[id.String()] = refs
+	}
+	return inventory{
+		Current:  curr,
+		Previous: prev,
+	}
+}
+
+func (a *Inventory) fromInternalType(i *inventory) {
+	for s, refs := range i.Previous {
+		a.Previous[resid.FromString(s)] = refs
+	}
+	for s, refs := range i.Current {
+		a.Current[resid.FromString(s)] = refs
+	}
+}
+
 func (a *Inventory) marshal() ([]byte, error) {
-	return json.Marshal(a)
+	return json.Marshal(a.toInternalType())
 }
 
 func (a *Inventory) unMarshal(data []byte) error {
-	return json.Unmarshal(data, a)
+	inv := &inventory{
+		Current:  map[string][]resid.ItemId{},
+		Previous: map[string][]resid.ItemId{},
+	}
+	err := json.Unmarshal(data, inv)
+	if err != nil {
+		return err
+	}
+	a.fromInternalType(inv)
+	return nil
 }
 
+// UpdateAnnotations update the annotation map
 func (a *Inventory) UpdateAnnotations(annot map[string]string) error {
 	data, err := a.marshal()
 	if err != nil {
@@ -164,6 +238,7 @@ func (a *Inventory) UpdateAnnotations(annot map[string]string) error {
 	return nil
 }
 
+// LoadFromAnnotation loads the Inventory date from the annotation map
 func (a *Inventory) LoadFromAnnotation(annot map[string]string) error {
 	value, ok := annot[InventoryAnnotation]
 	if ok {

pkg/inventory/inventory_test.go

@@ -27,12 +27,12 @@ func makeRefs() (Refs, Refs) {
 	b := resid.FromString("G2_V2_K2|ns2|nm2")
 	c := resid.FromString("G3_V3_K3|ns3|nm3")
 	current := NewRefs()
-	current[a.String()] = []resid.ItemId{b, c}
-	current[b.String()] = []resid.ItemId{}
-	current[c.String()] = []resid.ItemId{}
+	current[a] = []resid.ItemId{b, c}
+	current[b] = []resid.ItemId{}
+	current[c] = []resid.ItemId{}
 	new := NewRefs()
-	new[a.String()] = []resid.ItemId{b}
-	new[b.String()] = []resid.ItemId{}
+	new[a] = []resid.ItemId{b}
+	new[b] = []resid.ItemId{}
 	return current, new
 }
 

pkg/resid/itemid.go

@@ -53,7 +53,11 @@ func (i ItemId) String() string {
 		[]string{i.Gvk.String(), ns, nm}, separator)
 }
 
-func New(g gvk.Gvk, ns, nm string) ItemId {
+func (i ItemId) Equals(b ItemId) bool {
+	return i.String() == b.String()
+}
+
+func NewItemId(g gvk.Gvk, ns, nm string) ItemId {
 	return ItemId{
 		Gvk:       g,
 		Namespace: ns,