Merge pull request #566 from Liujingfang1/master

Use empty object status list when the StatusPolicyNone is used

cmd/main.go

@@ -56,7 +56,7 @@ func main() {
 	initCmd := initcmd.NewCmdInit(f, ioStreams)
 	updateHelp(names, initCmd)
 	loader := manifestreader.NewManifestLoader(f)
-	invFactory := inventory.ClusterClientFactory{}
+	invFactory := inventory.ClusterClientFactory{StatusPolicy: inventory.StatusPolicyNone}
 	applyCmd := apply.Command(f, invFactory, loader, ioStreams)
 	updateHelp(names, applyCmd)
 	previewCmd := preview.Command(f, invFactory, loader, ioStreams)

pkg/apply/common_test.go

@@ -123,7 +123,7 @@ func newTestInventory(
 ) inventory.Client {
 	// Use an Client with a fakeInfoHelper to allow generating Info
 	// objects that use the FakeRESTClient as the UnstructuredClient.
-	invClient, err := inventory.ClusterClientFactory{}.NewClient(tf)
+	invClient, err := inventory.ClusterClientFactory{StatusPolicy: inventory.StatusPolicyAll}.NewClient(tf)
 	require.NoError(t, err)
 	return invClient
 }

pkg/inventory/inventory-client.go

@@ -105,7 +105,10 @@ func (cic *ClusterClient) Merge(localInv Info, objs object.ObjMetadataSet, dryRu
 	}
 	if clusterInv == nil {
 		// Wrap inventory object and store the inventory in it.
-		status := getObjStatus(nil, objs)
+		var status []actuation.ObjectStatus
+		if cic.statusPolicy == StatusPolicyAll {
+			status = getObjStatus(nil, objs)
+		}
 		inv := cic.InventoryFactoryFunc(invObj)
 		if err := inv.Store(objs, status); err != nil {
 			return nil, err
@@ -132,7 +135,10 @@ func (cic *ClusterClient) Merge(localInv Info, objs object.ObjMetadataSet, dryRu
 		}
 		pruneIds = clusterObjs.Diff(objs)
 		unionObjs := clusterObjs.Union(objs)
-		status := getObjStatus(pruneIds, unionObjs)
+		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 := cic.InventoryFactoryFunc(clusterInv)
@@ -203,6 +209,9 @@ func (cic *ClusterClient) Replace(localInv Info, objs object.ObjMetadataSet, sta
 // 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, error) {
+	if cic.statusPolicy == StatusPolicyNone {
+		status = nil
+	}
 	wrappedInv := cic.InventoryFactoryFunc(inv)
 	if err := wrappedInv.Store(objs, status); err != nil {
 		return nil, err

pkg/inventory/inventory-client_test.go

@@ -37,6 +37,12 @@ func podData(name string) map[string]string {
 	}
 }
 
+func podDataNoStatus(name string) map[string]string {
+	return map[string]string{
+		fmt.Sprintf("test-inventory-namespace_%s__Pod", name): "",
+	}
+}
+
 func TestGetClusterInventoryInfo(t *testing.T) {
 	tests := map[string]struct {
 		statusPolicy StatusPolicy
@@ -125,18 +131,20 @@ func TestMerge(t *testing.T) {
 		isError      bool
 	}{
 		"Nil local inventory object is error": {
-			localInv:    nil,
-			localObjs:   object.ObjMetadataSet{},
-			clusterObjs: object.ObjMetadataSet{},
-			pruneObjs:   object.ObjMetadataSet{},
-			isError:     true,
+			localInv:     nil,
+			localObjs:    object.ObjMetadataSet{},
+			clusterObjs:  object.ObjMetadataSet{},
+			pruneObjs:    object.ObjMetadataSet{},
+			isError:      true,
+			statusPolicy: StatusPolicyAll,
 		},
 		"Cluster and local inventories empty: no prune objects; no change": {
-			localInv:    copyInventory(),
-			localObjs:   object.ObjMetadataSet{},
-			clusterObjs: object.ObjMetadataSet{},
-			pruneObjs:   object.ObjMetadataSet{},
-			isError:     false,
+			localInv:     copyInventory(),
+			localObjs:    object.ObjMetadataSet{},
+			clusterObjs:  object.ObjMetadataSet{},
+			pruneObjs:    object.ObjMetadataSet{},
+			isError:      false,
+			statusPolicy: StatusPolicyAll,
 		},
 		"Cluster and local inventories same: no prune objects; no change": {
 			localInv: copyInventory(),
@@ -146,8 +154,9 @@ func TestMerge(t *testing.T) {
 			clusterObjs: object.ObjMetadataSet{
 				ignoreErrInfoToObjMeta(pod1Info),
 			},
-			pruneObjs: object.ObjMetadataSet{},
-			isError:   false,
+			pruneObjs:    object.ObjMetadataSet{},
+			isError:      false,
+			statusPolicy: StatusPolicyAll,
 		},
 		"Cluster two obj, local one: prune obj": {
 			localInv: copyInventory(),
@@ -161,7 +170,8 @@ func TestMerge(t *testing.T) {
 			pruneObjs: object.ObjMetadataSet{
 				ignoreErrInfoToObjMeta(pod3Info),
 			},
-			isError: false,
+			statusPolicy: StatusPolicyAll,
+			isError:      false,
 		},
 		"Cluster multiple objs, local multiple different objs: prune objs": {
 			localInv: copyInventory(),
@@ -176,7 +186,8 @@ func TestMerge(t *testing.T) {
 				ignoreErrInfoToObjMeta(pod1Info),
 				ignoreErrInfoToObjMeta(pod3Info),
 			},
-			isError: false,
+			statusPolicy: StatusPolicyAll,
+			isError:      false,
 		},
 	}
 
@@ -309,8 +320,9 @@ func TestReplace(t *testing.T) {
 			clusterObjs: object.ObjMetadataSet{
 				ignoreErrInfoToObjMeta(pod1Info),
 			},
-			objStatus: []actuation.ObjectStatus{podStatus(pod1Info)},
-			data:      podData("pod-1"),
+			objStatus:    []actuation.ObjectStatus{podStatus(pod1Info)},
+			data:         podData("pod-1"),
+			statusPolicy: StatusPolicyAll,
 		},
 		"Cluster two obj, local one": {
 			localObjs: object.ObjMetadataSet{
@@ -320,8 +332,9 @@ func TestReplace(t *testing.T) {
 				ignoreErrInfoToObjMeta(pod1Info),
 				ignoreErrInfoToObjMeta(pod3Info),
 			},
-			objStatus: []actuation.ObjectStatus{podStatus(pod1Info), podStatus(pod3Info)},
-			data:      podData("pod-1"),
+			objStatus:    []actuation.ObjectStatus{podStatus(pod1Info), podStatus(pod3Info)},
+			data:         podData("pod-1"),
+			statusPolicy: StatusPolicyAll,
 		},
 		"Cluster multiple objs, local multiple different objs": {
 			localObjs: object.ObjMetadataSet{
@@ -331,8 +344,21 @@ func TestReplace(t *testing.T) {
 				ignoreErrInfoToObjMeta(pod1Info),
 				ignoreErrInfoToObjMeta(pod2Info),
 				ignoreErrInfoToObjMeta(pod3Info)},
-			objStatus: []actuation.ObjectStatus{podStatus(pod2Info), podStatus(pod1Info), podStatus(pod3Info)},
-			data:      podData("pod-2"),
+			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,
 		},
 	}
 

pkg/inventory/inventorycm.go

@@ -107,22 +107,19 @@ func (icm *ConfigMap) Store(objMetas object.ObjMetadataSet, status []actuation.O
 // 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, err := buildObjMap(icm.objMetas, icm.objStatus)
-	if err != nil {
-		return nil, err
-	}
+	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(),
+	err := unstructured.SetNestedStringMap(invCopy.UnstructuredContent(),
 		objMap, "data")
 	if err != nil {
 		return nil, err
 	}
 	return invCopy, nil
 }
 
-func buildObjMap(objMetas object.ObjMetadataSet, objStatus []actuation.ObjectStatus) (map[string]string, error) {
+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 {
@@ -132,12 +129,11 @@ func buildObjMap(objMetas object.ObjMetadataSet, objStatus []actuation.ObjectSta
 		if status, found := objStatusMap[objMetadata]; found {
 			objMap[objMetadata.String()] = stringFrom(status)
 		} else {
-			// This should never happen since the objStatus have captured all the
-			// object status
-			return nil, fmt.Errorf("object status not found for %s", objMetadata)
+			// It's possible that the passed in status doesn't any object status
+			objMap[objMetadata.String()] = ""
 		}
 	}
-	return objMap, nil
+	return objMap
 }
 
 func stringFrom(status actuation.ObjectStatus) string {

pkg/inventory/inventorycm_test.go

@@ -54,21 +54,16 @@ func TestBuildObjMap(t *testing.T) {
 		},
 		"empty object status list": {
 			objSet:   object.ObjMetadataSet{ObjMetadataFromObjectReference(obj1), ObjMetadataFromObjectReference(obj2)},
-			hasError: true,
+			hasError: false,
+			expected: map[string]string{
+				"ns_na_group1_Kind": "",
+				"ns_na_group2_Kind": "",
+			},
 		},
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			actual, err := buildObjMap(tc.objSet, tc.objStatus)
-			if tc.hasError {
-				if err == nil {
-					t.Fatalf("expected erroe, but not happened")
-				}
-				return
-			}
-			if err != nil {
-				t.Fatal(err)
-			}
+			actual := buildObjMap(tc.objSet, tc.objStatus)
 			if diff := cmp.Diff(actual, tc.expected); diff != "" {
 				t.Errorf(diff)
 			}

test/e2e/e2e_test.go

@@ -345,11 +345,11 @@ func deleteNamespace(ctx context.Context, c client.Client, namespace *v1.Namespa
 }
 
 func newDefaultInvApplier() *apply.Applier {
-	return newApplierFromInvFactory(inventory.ClusterClientFactory{})
+	return newApplierFromInvFactory(inventory.ClusterClientFactory{StatusPolicy: inventory.StatusPolicyAll})
 }
 
 func newDefaultInvDestroyer() *apply.Destroyer {
-	return newDestroyerFromInvFactory(inventory.ClusterClientFactory{})
+	return newDestroyerFromInvFactory(inventory.ClusterClientFactory{StatusPolicy: inventory.StatusPolicyAll})
 }
 
 func defaultInvNotExistsFunc(ctx context.Context, c client.Client, name, namespace, id string) {