Handle multiple configurations of the same type

This PR handles multiple configurations for the same type in ConfigReconciler. If there are multiple configurations for the same type, only the first configuration will be followed, the rest will be ignored.

Tested:unit tests, GKE cluster

Issue: #411

incubator/hnc/api/v1alpha1/hnc_config.go

@@ -45,8 +45,9 @@ const (
 // HNCConfigurationCondition codes. *All* codes must also be documented in the
 // comment to HNCConfigurationCondition.Code.
 const (
-	CritSingletonNameInvalid       HNCConfigurationCode = "critSingletonNameInvalid"
-	ObjectReconcilerCreationFailed HNCConfigurationCode = "objectReconcilerCreationFailed"
+	CritSingletonNameInvalid         HNCConfigurationCode = "critSingletonNameInvalid"
+	ObjectReconcilerCreationFailed   HNCConfigurationCode = "objectReconcilerCreationFailed"
+	MultipleConfigurationsForOneType HNCConfigurationCode = "multipleConfigurationsForOneType"
 )
 
 // TypeSynchronizationSpec defines the desired synchronization state of a specific kind.
@@ -133,6 +134,9 @@ type HNCConfigurationCondition struct {
 	//
 	// - "objectReconcilerCreationFailed": an error exists when creating the object
 	// reconciler for the type specified in Msg.
+	//
+	// - "multipleConfigurationsForOneType": Multiple configurations exist for the type specified
+	// in the Msg. One type should only have one configuration.
 	Code HNCConfigurationCode `json:"code"`
 
 	// A human-readable description of the condition, if the `code` field is not

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

@@ -80,7 +80,9 @@ spec:
                       the specified singleton name is invalid. The name should be
                       the same as HNCConfigSingleton. \n - \"objectReconcilerCreationFailed\":
                       an error exists when creating the object reconciler for the
-                      type specified in Msg."
+                      type specified in Msg. \n - \"multipleConfigurationsForOneType\":
+                      Multiple configurations exist for the type specified in the
+                      Msg. One type should only have one configuration."
                     type: string
                   msg:
                     description: A human-readable description of the condition, if

incubator/hnc/pkg/reconcilers/hnc_config.go

@@ -44,13 +44,14 @@ type ConfigReconciler struct {
 	HierarchyConfigUpdates chan event.GenericEvent
 }
 
+type gvkSet map[schema.GroupVersionKind]bool
+
 // Reconcile sets up some basic variable and logs the Spec.
 // TODO: Updates the comment above when adding more logic to the Reconcile method.
 func (r *ConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	ctx := context.Background()
 
 	// Validate the singleton name.
-	// TODO: Add a validating admission controller to prevent the problem in the first place.
 	if err := r.validateSingletonName(ctx, req.NamespacedName.Name); err != nil {
 		r.Log.Error(err, "Singleton name validation failed")
 		return ctrl.Result{}, nil
@@ -222,7 +223,14 @@ func (r *ConfigReconciler) syncObjectReconcilers(ctx context.Context, inst *api.
 // syncActiveReconcilers syncs object reconcilers for types that are in the Spec. If an object reconciler exists, it sets
 // its mode according to the Spec; otherwise, it creates the object reconciler.
 func (r *ConfigReconciler) syncActiveReconcilers(ctx context.Context, inst *api.HNCConfiguration) error {
+	// exist keeps track of existing types in the `config` singleton.
+	exist := gvkSet{}
 	for _, t := range inst.Spec.Types {
+		// If there are multiple configurations of the same type, we will follow the first
+		// configuration and ignore the rest.
+		if r.ensureNoDuplicateTypeConfigurations(inst, t, exist) {
+			continue
+		}
 		gvk := schema.FromAPIVersionAndKind(t.APIVersion, t.Kind)
 		if ts := r.Forest.GetTypeSyncer(gvk); ts != nil {
 			if err := ts.SetMode(ctx, t.Mode, r.Log); err != nil {
@@ -235,6 +243,26 @@ func (r *ConfigReconciler) syncActiveReconcilers(ctx context.Context, inst *api.
 	return nil
 }
 
+// ensureNoDuplicateTypeConfigurations checks whether the configuration of a type already exists and sets
+// a condition in the status if the type exists. The method returns true if type configuration exists; otherwise, returns false.
+func (r *ConfigReconciler) ensureNoDuplicateTypeConfigurations(inst *api.HNCConfiguration, t api.TypeSynchronizationSpec,
+	mp gvkSet) bool {
+	gvk := schema.FromAPIVersionAndKind(t.APIVersion, t.Kind)
+	if !mp[gvk] {
+		mp[gvk] = true
+		return false
+	}
+	specMsg := fmt.Sprintf("APIVersion: %s, Kind: %s, Mode: %s", t.APIVersion, t.Kind, t.Mode)
+	r.Log.Info(fmt.Sprintf("Ignoring the configuration: %s", specMsg))
+	condition := api.HNCConfigurationCondition{
+		Code: api.MultipleConfigurationsForOneType,
+		Msg: fmt.Sprintf("Ignore the configuration: %s because the configuration of the type already exists; "+
+			"only the first configuration will be applied", specMsg),
+	}
+	inst.Status.Conditions = append(inst.Status.Conditions, condition)
+	return true
+}
+
 // syncRemovedReconcilers sets object reconcilers to "ignore" mode for types that are removed from the Spec.
 func (r *ConfigReconciler) syncRemovedReconcilers(ctx context.Context, inst *api.HNCConfiguration) error {
 	// If a type exists in the forest but not exists in the Spec, we will

incubator/hnc/pkg/reconcilers/hnc_config_test.go

@@ -2,6 +2,7 @@ package reconcilers_test
 
 import (
 	"context"
+	"fmt"
 	"strings"
 	"time"
 
@@ -116,6 +117,30 @@ var _ = Describe("HNCConfiguration", func() {
 		Eventually(hasHNCConfigurationConditionWithMsg(ctx, api.ObjectReconcilerCreationFailed, "/v2, Kind=LimitRange")).Should(BeFalse())
 	})
 
+	It("should set MultipleConfigurationsForOneType if there are multiple configurations for one type", func() {
+		// Add multiple configurations for a type.
+		addToHNCConfig(ctx, "v1", "Secret", api.Propagate)
+		addToHNCConfig(ctx, "v1", "Secret", api.Remove)
+
+		// The second configuration should be ignored.
+		Eventually(hasHNCConfigurationConditionWithMsg(ctx, api.MultipleConfigurationsForOneType,
+			fmt.Sprintf("APIVersion: v1, Kind: Secret, Mode: %s", api.Remove))).Should(BeTrue())
+	})
+
+	It("should unset MultipleConfigurationsForOneType if extra configurations are later removed", func() {
+		// Add multiple configurations for a type.
+		addToHNCConfig(ctx, "v1", "Secret", api.Propagate)
+		addToHNCConfig(ctx, "v1", "Secret", api.Remove)
+
+		Eventually(hasHNCConfigurationConditionWithMsg(ctx, api.MultipleConfigurationsForOneType,
+			fmt.Sprintf("APIVersion: v1, Kind: Secret, Mode: %s", api.Remove))).Should(BeTrue())
+
+		removeHNCConfigTypeWithMode(ctx, "v1", "Secret", api.Remove)
+
+		Eventually(hasHNCConfigurationConditionWithMsg(ctx, api.MultipleConfigurationsForOneType,
+			fmt.Sprintf("APIVersion: v1, Kind: Secret, Mode: %s", api.Remove))).Should(BeFalse())
+	})
+
 	It("should not propagate objects if the type is not in HNCConfiguration", func() {
 		setParent(ctx, barName, fooName)
 		makeObject(ctx, "ResourceQuota", fooName, "foo-resource-quota")
@@ -361,6 +386,25 @@ func removeHNCConfigType(ctx context.Context, apiVersion, kind string) {
 	}).Should(Succeed())
 }
 
+func removeHNCConfigTypeWithMode(ctx context.Context, apiVersion, kind string, mode api.SynchronizationMode) {
+	Eventually(func() error {
+		c := getHNCConfig(ctx)
+		i := 0
+		for ; i < len(c.Spec.Types); i++ {
+			if c.Spec.Types[i].APIVersion == apiVersion && c.Spec.Types[i].Kind == kind && c.Spec.Types[i].Mode == mode {
+				break
+			}
+		}
+		// The type does not exist. Nothing to remove.
+		if i == len(c.Spec.Types) {
+			return nil
+		}
+		c.Spec.Types[i] = c.Spec.Types[len(c.Spec.Types)-1]
+		c.Spec.Types = c.Spec.Types[:len(c.Spec.Types)-1]
+		return updateHNCConfig(ctx, c)
+	}).Should(Succeed())
+}
+
 func createCronTabCRD(ctx context.Context) {
 	crontab := v1beta1.CustomResourceDefinition{
 		TypeMeta: metav1.TypeMeta{