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 {
@@ -382,6 +382,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

@@ -383,6 +383,11 @@ func (r *ConfigReconciler) validateSingletonName(ctx context.Context, nm string)
 // types in `propagate` and `remove` modes will be recorded. The Status.Types
 // is sorted in alphabetical order based on APIVersion and Kind.
 func (r *ConfigReconciler) setTypeStatuses(inst *api.HNCConfiguration) {
+	// We lock the forest here so that other reconcilers cannot modify the
+	// forest while we are reading from the forest.
+	r.Forest.Lock()
+	defer r.Forest.Unlock()
+
 	statuses := []api.TypeSynchronizationStatus{}
 	for _, ts := range r.Forest.GetTypeSyncers() {
 		// Don't output a status for any reconciler that isn't explicitly listed in the spec
@@ -399,12 +404,23 @@ 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.GetNamespaceNames()
+			for _, nm := range nms {
+				ns := r.Forest.Get(nm)
+				numSrc += ns.GetNumOriginalObjects(gvk)
+			}
+			status.NumSourceObjects = &numSrc
+		}
+
 		// Record the status
 		statuses = append(statuses, status)
 	}
@@ -447,15 +463,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")
@@ -323,6 +323,45 @@ var _ = Describe("HNCConfiguration", func() {
 		updateHNCConfigSpec(ctx, "v1", "LimitRange", api.Remove)
 
 		Eventually(getNumPropagatedObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(0))
+
+		// TODO: Delete objects created via makeObject after each test case.
+		deleteObject(ctx, "LimitRange", fooName, "foo-lr")
+	})
+
+	It("should set NumSourceObjects for a type in propagate mode", func() {
+		addToHNCConfig(ctx, "v1", "LimitRange", api.Propagate)
+		makeObject(ctx, "LimitRange", fooName, "foo-lr")
+
+		Eventually(getNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(1))
+
+		deleteObject(ctx, "LimitRange", fooName, "foo-lr")
+	})
+
+	// 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")
+
+		Eventually(getNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(1))
+
+		deleteObject(ctx, "LimitRange", fooName, "foo-lr")
+	})
+
+	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")
+
+		Eventually(getNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(1))
+
+		deleteObject(ctx, "LimitRange", fooName, "foo-lr")
+
+		Eventually(getNumSourceObjects(ctx, "v1", "LimitRange"), statusUpdateTime).Should(Equal(0))
+	})
+
+	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())
 	})
 })
 
@@ -338,7 +377,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 +542,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 {