diff --git a/incubator/hnc/api/v1alpha1/hnc_config.go b/incubator/hnc/api/v1alpha1/hnc_config.go index fa1288885..3b1f1d677 100644 --- a/incubator/hnc/api/v1alpha1/hnc_config.go +++ b/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 diff --git a/incubator/hnc/config/crd/bases/hnc.x-k8s.io_hncconfigurations.yaml b/incubator/hnc/config/crd/bases/hnc.x-k8s.io_hncconfigurations.yaml index 2a607ead5..2ac140dfe 100644 --- a/incubator/hnc/config/crd/bases/hnc.x-k8s.io_hncconfigurations.yaml +++ b/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 diff --git a/incubator/hnc/pkg/reconcilers/hnc_config.go b/incubator/hnc/pkg/reconcilers/hnc_config.go index d4d3a0f54..e9197248d 100644 --- a/incubator/hnc/pkg/reconcilers/hnc_config.go +++ b/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 diff --git a/incubator/hnc/pkg/reconcilers/hnc_config_test.go b/incubator/hnc/pkg/reconcilers/hnc_config_test.go index f1eee37a8..02e57384c 100644 --- a/incubator/hnc/pkg/reconcilers/hnc_config_test.go +++ b/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{