Support "remove" mode in ConfigReconciler.

This PR adds the support for 'remove' mode in ConfigReconciler. If the mode of a type is set to 'remove', all the propagated objects (i.e., objects created by HNC) of the type will be removed. However, source objects (i.e., objects created by users) of the type will not be removed.

Tested: GKE cluster; unit tests.

This PR contains changes introduced in kubernetes-retired#476.

Design doc: http://bit.ly/hnc-type-configuration
Issue: kubernetes-retired#411

incubator/hnc/pkg/forest/forest.go

@@ -27,6 +27,8 @@ var (
 type TypeSyncer interface {
 	// SyncNamespace syncs objects of a namespace for a specific type.
 	SyncNamespace(context.Context, logr.Logger, string) error
+	// SyncCluster syncs all objects of a cluster for a specific type.
+	SyncCluster(context.Context, logr.Logger) error
 	// Provides the GVK that is handled by the reconciler who implements the interface.
 	GetGVK() schema.GroupVersionKind
 	// SetMode sets the propagation mode of objects that are handled by the reconciler who implements the interface.

incubator/hnc/pkg/reconcilers/hnc_config.go

@@ -161,7 +161,6 @@ func (r *ConfigReconciler) syncObjectReconcilers(ctx context.Context, inst *api.
 			r.createObjectReconciler(gvk, t.Mode, inst)
 		}
 	}
-
 	return nil
 }
 

incubator/hnc/pkg/reconcilers/hnc_config_test.go

@@ -155,6 +155,44 @@ var _ = Describe("HNCConfiguration", func() {
 		Eventually(hasResourceQuota(ctx, barName, "foo-resource-quota")).Should(BeTrue())
 		Expect(resourceQuotaInheritedFrom(ctx, barName, "foo-resource-quota")).Should(Equal(fooName))
 	})
+
+	It("should remove propagated objects if the mode of a type is changed from propagate to remove", func() {
+		addToHNCConfig(ctx, "v1", "Secret", api.Propagate)
+		setParent(ctx, barName, fooName)
+		makeSecret(ctx, fooName, "foo-sec")
+
+		Eventually(hasSecret(ctx, fooName, "foo-sec")).Should(BeTrue())
+		// "foo-sec" should be propagated from foo to bar.
+		Eventually(hasSecret(ctx, barName, "foo-sec")).Should(BeTrue())
+		Expect(secretInheritedFrom(ctx, barName, "foo-sec")).Should(Equal(fooName))
+
+		updateHNCConfigSpec(ctx, "v1", "v1", "Secret", "Secret", api.Propagate, api.Remove)
+
+		// Foo should still have "foo-sec" because it is a source object, not propagated one.
+		// Therefore, we do not remove it.
+		Eventually(hasSecret(ctx, fooName, "foo-sec")).Should(BeTrue())
+		// "foo-sec" should be removed from bar.
+		Eventually(hasSecret(ctx, barName, "foo-sec")).Should(BeFalse())
+	})
+
+	It("should propagate objects if the mode of a type is changed from remove to propagate", func() {
+		addToHNCConfig(ctx, "v1", "ResourceQuota", api.Remove)
+		setParent(ctx, barName, fooName)
+		makeResourceQuota(ctx, fooName, "foo-resource-quota")
+
+		// Foo should have "foo-resource-quota" because it is a source object, which will not be removed.
+		Eventually(hasResourceQuota(ctx, fooName, "foo-resource-quota")).Should(BeTrue())
+		// Sleep to give "foo-resource-quota" a chance to propagate from foo to bar, if it could.
+		time.Sleep(sleepTime)
+		// "foo-resource-quota" should not be propagated from foo to bar.
+		Expect(hasResourceQuota(ctx, barName, "foo-resource-quota")()).Should(BeFalse())
+
+		updateHNCConfigSpec(ctx, "v1", "v1", "ResourceQuota", "ResourceQuota", api.Remove, api.Propagate)
+
+		// "foo-resource-quota" should be propagated from foo to bar.
+		Eventually(hasResourceQuota(ctx, barName, "foo-resource-quota")).Should(BeTrue())
+		Expect(resourceQuotaInheritedFrom(ctx, barName, "foo-resource-quota")).Should(Equal(fooName))
+	})
 })
 
 func hasTypeWithMode(apiVersion, kind string, mode api.SynchronizationMode, config *api.HNCConfiguration) func() bool {

incubator/hnc/pkg/reconcilers/object.go

@@ -95,6 +95,22 @@ func (r *ObjectReconciler) SyncNamespace(ctx context.Context, log logr.Logger, n
 	return nil
 }
 
+// SyncCluster can be called manually by the ConfigReconciler when the mode of a type is changed.
+// It enqueues all the current objects in all namespaces.
+func (r *ObjectReconciler) SyncCluster(ctx context.Context, log logr.Logger) error {
+	log = log.WithValues("gvk", r.GVK)
+
+	keys := r.Forest.GetNamespaceNames()
+	for _, ns := range keys {
+		// Enqueue all the current objects in the namespace.
+		if err := r.enqueueLocalObjects(ctx, log, ns); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // GetGVK provides GVK that is handled by this reconciler.
 func (r *ObjectReconciler) GetGVK() schema.GroupVersionKind {
 	return r.GVK
@@ -111,11 +127,10 @@ func (r *ObjectReconciler) SetMode(ctx context.Context, mode api.Synchronization
 	}
 	r.Log.Info("Changing mode of the object reconciler", "old", oldMode, "new", newMode)
 	r.Mode = newMode
-	// Currently, there are only two modes: 'ignore' and 'propagate'. If we change from "ignore" mode to 'propagate',
-	// we need to update objects in the cluster.
-	// TODO: We might need to revisit when we want to call SyncCluster if we support more modes in future.
-	if oldMode == api.Ignore {
-		err := r.syncCluster(ctx, r.Log)
+	// If the new mode is "propagate" or "remove", we need to update objects in the cluster
+	// (e.g., propagate or remove existing objects).
+	if newMode == api.Propagate || newMode == api.Remove {
+		err := r.enqueueAllObjects(ctx, r.Log)
 		if err != nil {
 			return err
 		}
@@ -129,10 +144,7 @@ func (r *ObjectReconciler) SetMode(ctx context.Context, mode api.Synchronization
 // treated as api.Ignore.
 func (r *ObjectReconciler) getValidateMode(mode api.SynchronizationMode, log logr.Logger) api.SynchronizationMode {
 	switch mode {
-	// TODO: Add api.Remove here. We currently do not support 'remove' mode. A
-	// 'remove' mode will be treated as 'ignore'. We will add api.Remove to the first
-	// case when we start to support the mode.
-	case api.Propagate, api.Ignore:
+	case api.Propagate, api.Ignore, api.Remove:
 		return mode
 	case "":
 		log.Info("Unset mode; using 'propagate'")
@@ -143,8 +155,8 @@ func (r *ObjectReconciler) getValidateMode(mode api.SynchronizationMode, log log
 	}
 }
 
-// SyncCluster enqueues all the current objects in all namespaces.
-func (r *ObjectReconciler) syncCluster(ctx context.Context, log logr.Logger) error {
+// enqueueAllObjects enqueues all the current objects in all namespaces.
+func (r *ObjectReconciler) enqueueAllObjects(ctx context.Context, log logr.Logger) error {
 	keys := r.Forest.GetNamespaceNames()
 	for _, ns := range keys {
 		// Enqueue all the current objects in the namespace.
@@ -153,7 +165,6 @@ func (r *ObjectReconciler) syncCluster(ctx context.Context, log logr.Logger) err
 			return err
 		}
 	}
-
 	return nil
 }
 
@@ -505,8 +516,7 @@ func (r *ObjectReconciler) syncDeletedSource(ctx context.Context, log logr.Logge
 }
 
 // exclude returns true if the object shouldn't be handled by the HNC, and in some non-obvious cases
-// sets a condition on the namespace. Eventually, this may be user-configurable, but right now it's
-// used for Service Account token secrets and to decide object propagation based on finalizer field.
+// sets a condition on the namespace.
 func (r *ObjectReconciler) exclude(log logr.Logger, inst *unstructured.Unstructured) bool {
 	// Object with nonempty finalizer list is not propagated
 	if len(inst.GetFinalizers()) != 0 {
@@ -515,6 +525,9 @@ func (r *ObjectReconciler) exclude(log logr.Logger, inst *unstructured.Unstructu
 	}
 
 	switch {
+	case r.Mode == api.Remove:
+		return true
+
 	case r.GVK.Group == "" && r.GVK.Kind == "Secret":
 		// These are reaped by a builtin K8s controller so there's no point copying them.
 		// More to the point, SA tokens really aren't supposed to be copied between