Compute NumSourceObjects in HNCConfiguration status

This PR computes NumSourceObjects as a part of HNCConfiguration status. NumSourceObjects indicates the number of source objects of a specific type created by users.

Tested: unit tests, GKE cluster

Issue: kubernetes-retired#411

incubator/hnc/api/v1alpha1/hnc_config.go

@@ -78,6 +78,11 @@ type TypeSynchronizationStatus struct {
 	// +kubebuilder:validation:Minimum=0
 	// +optional
 	NumPropagatedObjects *int `json:"numPropagatedObjects,omitempty"`
+
+	// Tracks the number of objects that are created by users.
+	// +kubebuilder:validation:Minimum=0
+	// +optional
+	NumSourceObjects *int `json:"numSourceObjects,omitempty"`
 }
 
 // +kubebuilder:object:root=true

incubator/hnc/config/crd/bases/hnc.x-k8s.io_hncconfigurations.yaml

@@ -120,6 +120,11 @@ spec:
                       by HNC.
                     minimum: 0
                     type: integer
+                  numSourceObjects:
+                    description: Tracks the number of objects that are created by
+                      users.
+                    minimum: 0
+                    type: integer
                 type: object
               type: array
           type: object

incubator/hnc/pkg/forest/forest.go

@@ -38,10 +38,10 @@ type TypeSyncer interface {
 	GetNumPropagatedObjects() int
 }
 
-// NumPropagatedObjectsSyncer syncs the number of propagated objects. ConfigReconciler implements the
-// interface so that it can be called by an ObjectReconciler if the number of propagated objects is changed.
-type NumPropagatedObjectsSyncer interface {
-	SyncNumPropagatedObjects(logr.Logger)
+// NumObjectsSyncer syncs the number of propagated and source objects. ConfigReconciler implements the
+// interface so that it can be called by an ObjectReconciler if the number of propagated or source objects is changed.
+type NumObjectsSyncer interface {
+	SyncNumObjects(logr.Logger)
 }
 
 // Forest defines a forest of namespaces - that is, a set of trees. It includes methods to mutate
@@ -66,7 +66,7 @@ type Forest struct {
 
 	// ObjectsStatusSyncer is the ConfigReconciler that an object reconciler can call if the status of the HNCConfiguration
 	// object needs to be updated.
-	ObjectsStatusSyncer NumPropagatedObjectsSyncer
+	ObjectsStatusSyncer NumObjectsSyncer
 }
 
 func NewForest() *Forest {
@@ -142,6 +142,11 @@ func (f *Forest) GetNamespaceNames() []string {
 	return keys
 }
 
+// GetNamespaces returns the namespaces in the forest.
+func (f *Forest) GetNamespaces() namedNamespaces {
+	return f.namespaces
+}
+
 type namedNamespaces map[string]*Namespace
 
 // TODO Store source objects by GK in the forest - https://github.com/kubernetes-sigs/multi-tenancy/issues/281
@@ -382,6 +387,11 @@ func (ns *Namespace) GetOriginalObjects(gvk schema.GroupVersionKind) []*unstruct
 	return o
 }
 
+// GetNumOriginalObjects returns the total number of original objects of a specific GVK in the namespace.
+func (ns *Namespace) GetNumOriginalObjects(gvk schema.GroupVersionKind) int {
+	return len(ns.originalObjects[gvk])
+}
+
 // GetPropagatedObjects returns all original copies in the ancestors.
 func (ns *Namespace) GetPropagatedObjects(gvk schema.GroupVersionKind) []*unstructured.Unstructured {
 	o := []*unstructured.Unstructured{}

incubator/hnc/pkg/reconcilers/hnc_config.go

@@ -399,12 +399,22 @@ func (r *ConfigReconciler) setTypeStatuses(inst *api.HNCConfiguration) {
 			Mode:       ts.GetMode(), // may be different from the spec if it's implicit
 		}
 
-		// Only add counts if we're not ignoring this type
+		// Only add NumPropagatedObjects if we're not ignoring this type
 		if ts.GetMode() != api.Ignore {
 			numProp := ts.GetNumPropagatedObjects()
 			status.NumPropagatedObjects = &numProp
 		}
 
+		// Only add NumSourceObjects if we are propagating objects of this type.
+		if ts.GetMode() == api.Propagate {
+			numSrc := 0
+			nms := r.Forest.GetNamespaces()
+			for _, ns := range nms {
+				numSrc += ns.GetNumOriginalObjects(gvk)
+			}
+			status.NumSourceObjects = &numSrc
+		}
+
 		// Record the status
 		statuses = append(statuses, status)
 	}
@@ -447,15 +457,14 @@ func (r *ConfigReconciler) periodicTrigger() {
 		if r.shouldReconcile == false {
 			continue
 		}
-		r.enqueueSingleton(r.Log, "Syncing NumPropagatedObjects in the status")
+		r.enqueueSingleton(r.Log, "Syncing NumPropagatedObjects and/or NumSourceObjects in the status")
 		r.shouldReconcile = false
 	}
 }
 
-// SyncNumPropagatedObjects will be called by object reconcilers to signal config
-// reconciler to reconcile when an object is reconciled successfully and the status of
-// the `config` object might need to be updated.
-func (r *ConfigReconciler) SyncNumPropagatedObjects(log logr.Logger) {
+// SyncNumObjects will be called by object reconcilers to signal config
+// reconciler to reconcile when the status of the `config` object might need to be updated.
+func (r *ConfigReconciler) SyncNumObjects(log logr.Logger) {
 	log.V(1).Info("Signalling config reconciler for reconciliation.")
 	r.shouldReconcile = true
 }

incubator/hnc/pkg/reconcilers/hnc_config_test.go

@@ -190,7 +190,7 @@ var _ = Describe("HNCConfiguration", func() {
 
 		// Change to ignore and wait for reconciler
 		updateHNCConfigSpec(ctx, "v1", "Secret", api.Ignore)
-		Eventually(typeHasStatus(ctx, "v1", "Secret")).Should(Equal(api.Ignore))
+		Eventually(typeStatusHasMode(ctx, "v1", "Secret")).Should(Equal(api.Ignore))
 
 		bazName := createNS(ctx, "baz")
 		setParent(ctx, bazName, fooName)
@@ -266,7 +266,7 @@ var _ = Describe("HNCConfiguration", func() {
 
 		// Remove from spec and wait for the reconciler to pick it up
 		removeHNCConfigType(ctx, "v1", "Secret")
-		Eventually(typeHasStatus(ctx, "v1", "Secret")).Should(Equal(testModeMisssing))
+		Eventually(typeStatusHasMode(ctx, "v1", "Secret")).Should(Equal(testModeMisssing))
 
 		// Give foo another secret.
 		makeObject(ctx, "Secret", fooName, "foo-sec-2")
@@ -324,6 +324,42 @@ var _ = Describe("HNCConfiguration", func() {
 
 		Eventually(getNumPropagatedObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(0))
 	})
+
+	It("should set NumSourceObjects for a type in propagate mode", func() {
+		addToHNCConfig(ctx, "v1", "LimitRange", api.Propagate)
+		makeObject(ctx, "LimitRange", fooName, "foo-lr")
+
+		// There should be 2 source objects of LimitRange: 1 added in this test case, the other added in the previous test case.
+		Eventually(getNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(2))
+	})
+
+	// If a mode is unset, it is treated as `propagate` by default, in which case we will also compute NumSourceObjects
+	It("should set NumSourceObjects for a type with unset mode", func() {
+		addToHNCConfig(ctx, "v1", "LimitRange", "")
+		makeObject(ctx, "LimitRange", fooName, "foo-lr")
+
+		// There should be 3 source objects of LimitRange: 1 added in this test case, the other 2 added in the previous two test cases.
+		Eventually(getNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(3))
+	})
+
+	It("should decrement NumSourceObjects correctly after deleting an object of a type in propagate mode", func() {
+		addToHNCConfig(ctx, "v1", "LimitRange", api.Propagate)
+		makeObject(ctx, "LimitRange", fooName, "foo-lr")
+
+		// There should be 4 source objects of LimitRange, 1 added in this teat case, the other 3 added by the previous three test cases.
+		Eventually(getNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(4))
+
+		deleteObject(ctx, "LimitRange", fooName, "foo-lr")
+
+		// There should be 3 source objects of LimitRange because we deleted the one added in this test case.
+		Eventually(getNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(3))
+	})
+
+	It("should not set NumSourceObjects for a type not in propagate mode", func() {
+		addToHNCConfig(ctx, "v1", "LimitRange", api.Remove)
+
+		Eventually(hasNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(BeFalse())
+	})
 })
 
 func typeSpecHasMode(ctx context.Context, apiVersion, kind string) func() api.SynchronizationMode {
@@ -338,7 +374,7 @@ func typeSpecHasMode(ctx context.Context, apiVersion, kind string) func() api.Sy
 	}
 }
 
-func typeHasStatus(ctx context.Context, apiVersion, kind string) func() api.SynchronizationMode {
+func typeStatusHasMode(ctx context.Context, apiVersion, kind string) func() api.SynchronizationMode {
 	return func() api.SynchronizationMode {
 		config := getHNCConfig(ctx)
 		for _, t := range config.Status.Types {
@@ -503,3 +539,35 @@ func getNumPropagatedObjects(ctx context.Context, apiVersion, kind string) func(
 		return -1, errors.New(fmt.Sprintf("apiversion %s, kind %s is not found in status", apiVersion, kind))
 	}
 }
+
+// hasNumSourceObjects returns true if NumSourceObjects is set (not nil) for a specific type and returns false
+// if NumSourceObjects is not set. It returns false and an error if the type does not exist in the status.
+func hasNumSourceObjects(ctx context.Context, apiVersion, kind string) func() (bool, error) {
+	return func() (bool, error) {
+		c := getHNCConfig(ctx)
+		for _, t := range c.Status.Types {
+			if t.APIVersion == apiVersion && t.Kind == kind {
+				return t.NumSourceObjects != nil, nil
+			}
+		}
+		return false, errors.New(fmt.Sprintf("apiversion %s, kind %s is not found in status", apiVersion, kind))
+	}
+}
+
+// getNumSourceObjects returns NumSourceObjects status for a given type. If NumSourceObjects is
+// not set or if type does not exist in status, it returns -1 and an error.
+func getNumSourceObjects(ctx context.Context, apiVersion, kind string) func() (int, error) {
+	return func() (int, error) {
+		c := getHNCConfig(ctx)
+		for _, t := range c.Status.Types {
+			if t.APIVersion == apiVersion && t.Kind == kind {
+				if t.NumSourceObjects != nil {
+					return *t.NumSourceObjects, nil
+				}
+				return -1, errors.New(fmt.Sprintf("NumSourceObjects field is not set for "+
+					"apiversion %s, kind %s", apiVersion, kind))
+			}
+		}
+		return -1, errors.New(fmt.Sprintf("apiversion %s, kind %s is not found in status", apiVersion, kind))
+	}
+}

incubator/hnc/pkg/reconcilers/object.go

@@ -392,6 +392,10 @@ func (r *ObjectReconciler) syncSource(ctx context.Context, log logr.Logger, src
 	// Update or create a copy of the source object in the forest
 	ns.SetOriginalObject(src.DeepCopy())
 
+	// Signal the config reconciler for reconciliation because it is possible that a source object is
+	// added to the apiserver.
+	r.Forest.ObjectsStatusSyncer.SyncNumObjects(log)
+
 	// Enqueue all the descendant copies
 	r.enqueueDescendants(ctx, log, src)
 }
@@ -590,6 +594,10 @@ func (r *ObjectReconciler) syncUnpropagatedSource(ctx context.Context, log logr.
 	gvk := inst.GroupVersionKind()
 	r.Forest.Get(nnm).DeleteOriginalObject(gvk, nm)
 
+	// Signal the config reconciler for reconciliation because it is possible that the source object is
+	// deleted on the apiserver.
+	r.Forest.ObjectsStatusSyncer.SyncNumObjects(log)
+
 	r.enqueueDescendants(ctx, log, inst)
 }
 
@@ -640,7 +648,7 @@ func (r *ObjectReconciler) recordPropagatedObject(log logr.Logger, namespace, na
 		Name:      name,
 	}
 	r.propagatedObjects[nnm] = true
-	r.Forest.ObjectsStatusSyncer.SyncNumPropagatedObjects(log)
+	r.Forest.ObjectsStatusSyncer.SyncNumObjects(log)
 }
 
 // recordRemovedObject records the fact that this (possibly) previously propagated object no longer
@@ -654,7 +662,7 @@ func (r *ObjectReconciler) recordRemovedObject(log logr.Logger, namespace, name
 		Name:      name,
 	}
 	delete(r.propagatedObjects, nnm)
-	r.Forest.ObjectsStatusSyncer.SyncNumPropagatedObjects(log)
+	r.Forest.ObjectsStatusSyncer.SyncNumObjects(log)
 }
 
 func (r *ObjectReconciler) SetupWithManager(mgr ctrl.Manager, maxReconciles int) error {