From 518d0bcfd8b78c90fd0dacf9a500b2c8a49518e8 Mon Sep 17 00:00:00 2001 From: morvencao Date: Wed, 24 May 2023 12:38:23 +0000 Subject: [PATCH 1/4] disable grc in global hub. Signed-off-by: morvencao --- ...al-hub-operator.clusterserviceversion.yaml | 10 ++ operator/config/rbac/role.yaml | 10 ++ operator/main.go | 2 + operator/pkg/condition/condition.go | 14 ++ .../hubofhubs/globalhub_controller.go | 9 ++ .../controllers/hubofhubs/globalhub_mch.go | 153 ++++++++++++++++++ 6 files changed, 198 insertions(+) create mode 100644 operator/pkg/controllers/hubofhubs/globalhub_mch.go diff --git a/operator/bundle/manifests/multicluster-global-hub-operator.clusterserviceversion.yaml b/operator/bundle/manifests/multicluster-global-hub-operator.clusterserviceversion.yaml index 7ed860fb81..e6f7fed8ba 100644 --- a/operator/bundle/manifests/multicluster-global-hub-operator.clusterserviceversion.yaml +++ b/operator/bundle/manifests/multicluster-global-hub-operator.clusterserviceversion.yaml @@ -513,6 +513,16 @@ spec: - get - patch - update + - apiGroups: + - operator.open-cluster-management.io + resources: + - multiclusterhubs + verbs: + - get + - list + - patch + - update + - watch - apiGroups: - packages.operators.coreos.com resources: diff --git a/operator/config/rbac/role.yaml b/operator/config/rbac/role.yaml index 030143392b..5f02042d9a 100644 --- a/operator/config/rbac/role.yaml +++ b/operator/config/rbac/role.yaml @@ -296,6 +296,16 @@ rules: - get - patch - update +- apiGroups: + - operator.open-cluster-management.io + resources: + - multiclusterhubs + verbs: + - get + - list + - patch + - update + - watch - apiGroups: - packages.operators.coreos.com resources: diff --git a/operator/main.go b/operator/main.go index ead46acdc7..bebadde11d 100644 --- a/operator/main.go +++ b/operator/main.go @@ -27,6 +27,7 @@ import ( operatorsv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apis/operators/v1" "github.com/spf13/pflag" hypershiftdeploymentv1alpha1 "github.com/stolostron/hypershift-deployment-controller/api/v1alpha1" + mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" @@ -98,6 +99,7 @@ func init() { utilruntime.Must(policyv1.AddToScheme(scheme)) utilruntime.Must(applicationv1beta1.AddToScheme(scheme)) utilruntime.Must(admissionregistrationv1.AddToScheme(scheme)) + utilruntime.Must(mchv1.AddToScheme(scheme)) // +kubebuilder:scaffold:scheme } diff --git a/operator/pkg/condition/condition.go b/operator/pkg/condition/condition.go index 1b21252225..41214c887b 100644 --- a/operator/pkg/condition/condition.go +++ b/operator/pkg/condition/condition.go @@ -66,6 +66,13 @@ const ( CONDITION_MESSAGE_MANAGER_AVAILABLE = "Multicluster Global Hub Manager has been deployed" ) +// NOTE: the status of ManagerDeployed can only be True; otherwise there is no condition +const ( + CONDITION_TYPE_GRC_DISABLED = "GRCDISABLED" + CONDITION_REASON_GRC_DISABLED = "GRCDISABLED" + CONDITION_MESSAGE_GRC_DISABLED = "GRC has been disabled in MultiClusterHub" +) + // NOTE: the status of LeafHubDeployed can only be True; otherwise there is no condition const ( CONDITION_TYPE_LEAFHUB_DEPLOY = "LeafHubDeployed" @@ -112,6 +119,13 @@ func SetConditionManagerAvailable(ctx context.Context, c client.Client, mgh *ope CONDITION_REASON_MANAGER_AVAILABLE, CONDITION_MESSAGE_MANAGER_AVAILABLE) } +func SetConditionGRCDisabled(ctx context.Context, c client.Client, mgh *operatorv1alpha2.MulticlusterGlobalHub, + status metav1.ConditionStatus, +) error { + return SetCondition(ctx, c, mgh, CONDITION_TYPE_GRC_DISABLED, status, + CONDITION_REASON_GRC_DISABLED, CONDITION_MESSAGE_GRC_DISABLED) +} + func SetConditionLeafHubDeployed(ctx context.Context, c client.Client, mgh *operatorv1alpha2.MulticlusterGlobalHub, clusterName string, status metav1.ConditionStatus, ) error { diff --git a/operator/pkg/controllers/hubofhubs/globalhub_controller.go b/operator/pkg/controllers/hubofhubs/globalhub_controller.go index 7d6bebcc81..54bcb89748 100644 --- a/operator/pkg/controllers/hubofhubs/globalhub_controller.go +++ b/operator/pkg/controllers/hubofhubs/globalhub_controller.go @@ -98,6 +98,7 @@ type MulticlusterGlobalHubReconciler struct { // +kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=mutatingwebhookconfigurations,verbs=get;list;watch;create;update;delete // +kubebuilder:rbac:groups=addon.open-cluster-management.io,resources=clustermanagementaddons,verbs=create;delete;get;list;update;watch // +kubebuilder:rbac:groups=addon.open-cluster-management.io,resources=clustermanagementaddons/finalizers,verbs=update +// +kubebuilder:rbac:groups=operator.open-cluster-management.io,resources=multiclusterhubs,verbs=get;list;patch;update;watch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -136,6 +137,9 @@ func (r *MulticlusterGlobalHubReconciler) Reconcile(ctx context.Context, req ctr if err := r.pruneGlobalHubResources(ctx, mgh); err != nil { return ctrl.Result{}, fmt.Errorf("failed to prune Global Hub resources %v", err) } + if err := r.recoverMCH(ctx, mgh); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to recover MCH %v", err) + } return ctrl.Result{}, nil } @@ -220,6 +224,11 @@ func (r *MulticlusterGlobalHubReconciler) reconcileLargeScaleGlobalHub(ctx conte return err } + // disable grc component in mch instance + if err := r.reconcileMCH(ctx, mgh); err != nil { + return err + } + // reconcile manager if err := r.reconcileManager(ctx, mgh); err != nil { return err diff --git a/operator/pkg/controllers/hubofhubs/globalhub_mch.go b/operator/pkg/controllers/hubofhubs/globalhub_mch.go new file mode 100644 index 0000000000..74254a3961 --- /dev/null +++ b/operator/pkg/controllers/hubofhubs/globalhub_mch.go @@ -0,0 +1,153 @@ +package hubofhubs + +import ( + "context" + "fmt" + "strconv" + + mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "sigs.k8s.io/controller-runtime/pkg/client" + + operatorv1alpha2 "github.com/stolostron/multicluster-global-hub/operator/apis/v1alpha2" + "github.com/stolostron/multicluster-global-hub/operator/pkg/condition" +) + +const ( + GRCDisabledByMGHAnnotation = "operator.open-cluster-management.io/grc-disabled-by-mgh" +) + +func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, + mgh *operatorv1alpha2.MulticlusterGlobalHub, +) error { + mch, err := getMCH(ctx, r.Client) + if err != nil { + return fmt.Errorf("failed to get MCH instance. err = %v", err) + } + + grcDisabledByMGH := false + if mch.Spec.Overrides != nil { + found := false + for _, c := range mch.Spec.Overrides.Components { + if c.Name == "grc" { + if c.Enabled { + c.Enabled = false + grcDisabledByMGH = true + } + found = true + break + } + } + if !found { + mch.Spec.Overrides.Components = append(mch.Spec.Overrides.Components, + mchv1.ComponentConfig{ + Name: "grc", + Enabled: false, + }) + grcDisabledByMGH = true + } + } else { + mch.Spec.Overrides = &mchv1.Overrides{ + Components: []mchv1.ComponentConfig{ + { + Name: "grc", + Enabled: false, + }, + }, + } + grcDisabledByMGH = true + } + + if err := condition.SetConditionGRCDisabled(ctx, r.Client, mgh, + condition.CONDITION_STATUS_TRUE); err != nil { + return condition.FailToSetConditionError(condition.CONDITION_STATUS_TRUE, err) + } + + if grcDisabledByMGH { + // set the annotation to remember grc status + annotations := mgh.GetAnnotations() + annotations[GRCDisabledByMGHAnnotation] = strconv.FormatBool(grcDisabledByMGH) + mgh.SetAnnotations(annotations) + if err := r.Client.Update(ctx, mgh, &client.UpdateOptions{}); err != nil { + return fmt.Errorf("failed to annotation(%s) to mgh. err = %v", GRCDisabledByMGHAnnotation, err) + } + } + + return r.Client.Update(ctx, mch) +} + +func (r *MulticlusterGlobalHubReconciler) recoverMCH(ctx context.Context, + mgh *operatorv1alpha2.MulticlusterGlobalHub, +) error { + grcDisabledByMGHAnnotationVal := mgh.GetAnnotations()[GRCDisabledByMGHAnnotation] + if grcDisabledByMGHAnnotationVal == "" { + return nil + } + + grcDisabledByMGH, err := strconv.ParseBool(grcDisabledByMGHAnnotationVal) + if err != nil { + return fmt.Errorf("invalid annotation value type for %s from MGH instance. error = %v", + GRCDisabledByMGHAnnotation, err) + } + + if !grcDisabledByMGH { + return nil + } + + mch, err := getMCH(ctx, r.Client) + if err != nil { + return fmt.Errorf("failed to get MCH instance. err = %v", err) + } + + if mch.Spec.Overrides != nil { + found := false + for _, c := range mch.Spec.Overrides.Components { + if c.Name == "grc" { + if !c.Enabled { + c.Enabled = true + } + found = true + break + } + } + if !found { + mch.Spec.Overrides.Components = append(mch.Spec.Overrides.Components, + mchv1.ComponentConfig{ + Name: "grc", + Enabled: true, + }) + } + } else { + mch.Spec.Overrides = &mchv1.Overrides{ + Components: []mchv1.ComponentConfig{ + { + Name: "grc", + Enabled: true, + }, + }, + } + } + + return r.Client.Update(ctx, mch) +} + +func getMCH(ctx context.Context, k8sClient client.Client) (*mchv1.MultiClusterHub, error) { + mch := &mchv1.MultiClusterHubList{} + err := k8sClient.List(ctx, mch) + if errors.IsNotFound(err) { + return nil, nil + } + if meta.IsNoMatchError(err) { + return nil, nil + } + if err != nil { + return nil, err + } + + if len(mch.Items) == 0 { + return nil, err + } + + return &mch.Items[0], nil +} From 135e486c15a3378bc07ef0630af5acf334fb1ca8 Mon Sep 17 00:00:00 2001 From: morvencao Date: Thu, 25 May 2023 02:41:08 +0000 Subject: [PATCH 2/4] remove mch recovery. Signed-off-by: morvencao --- .../hubofhubs/globalhub_controller.go | 3 - .../controllers/hubofhubs/globalhub_mch.go | 86 +++---------------- 2 files changed, 13 insertions(+), 76 deletions(-) diff --git a/operator/pkg/controllers/hubofhubs/globalhub_controller.go b/operator/pkg/controllers/hubofhubs/globalhub_controller.go index 54bcb89748..19c291851c 100644 --- a/operator/pkg/controllers/hubofhubs/globalhub_controller.go +++ b/operator/pkg/controllers/hubofhubs/globalhub_controller.go @@ -137,9 +137,6 @@ func (r *MulticlusterGlobalHubReconciler) Reconcile(ctx context.Context, req ctr if err := r.pruneGlobalHubResources(ctx, mgh); err != nil { return ctrl.Result{}, fmt.Errorf("failed to prune Global Hub resources %v", err) } - if err := r.recoverMCH(ctx, mgh); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to recover MCH %v", err) - } return ctrl.Result{}, nil } diff --git a/operator/pkg/controllers/hubofhubs/globalhub_mch.go b/operator/pkg/controllers/hubofhubs/globalhub_mch.go index 74254a3961..77dfe28654 100644 --- a/operator/pkg/controllers/hubofhubs/globalhub_mch.go +++ b/operator/pkg/controllers/hubofhubs/globalhub_mch.go @@ -3,7 +3,6 @@ package hubofhubs import ( "context" "fmt" - "strconv" mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -14,10 +13,6 @@ import ( "github.com/stolostron/multicluster-global-hub/operator/pkg/condition" ) -const ( - GRCDisabledByMGHAnnotation = "operator.open-cluster-management.io/grc-disabled-by-mgh" -) - func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, mgh *operatorv1alpha2.MulticlusterGlobalHub, ) error { @@ -26,14 +21,14 @@ func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, return fmt.Errorf("failed to get MCH instance. err = %v", err) } - grcDisabledByMGH := false + // grcDisabledByMGH := false if mch.Spec.Overrides != nil { found := false for _, c := range mch.Spec.Overrides.Components { if c.Name == "grc" { if c.Enabled { c.Enabled = false - grcDisabledByMGH = true + // grcDisabledByMGH = true } found = true break @@ -45,7 +40,7 @@ func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, Name: "grc", Enabled: false, }) - grcDisabledByMGH = true + // grcDisabledByMGH = true } } else { mch.Spec.Overrides = &mchv1.Overrides{ @@ -56,7 +51,7 @@ func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, }, }, } - grcDisabledByMGH = true + // grcDisabledByMGH = true } if err := condition.SetConditionGRCDisabled(ctx, r.Client, mgh, @@ -64,70 +59,15 @@ func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, return condition.FailToSetConditionError(condition.CONDITION_STATUS_TRUE, err) } - if grcDisabledByMGH { - // set the annotation to remember grc status - annotations := mgh.GetAnnotations() - annotations[GRCDisabledByMGHAnnotation] = strconv.FormatBool(grcDisabledByMGH) - mgh.SetAnnotations(annotations) - if err := r.Client.Update(ctx, mgh, &client.UpdateOptions{}); err != nil { - return fmt.Errorf("failed to annotation(%s) to mgh. err = %v", GRCDisabledByMGHAnnotation, err) - } - } - - return r.Client.Update(ctx, mch) -} - -func (r *MulticlusterGlobalHubReconciler) recoverMCH(ctx context.Context, - mgh *operatorv1alpha2.MulticlusterGlobalHub, -) error { - grcDisabledByMGHAnnotationVal := mgh.GetAnnotations()[GRCDisabledByMGHAnnotation] - if grcDisabledByMGHAnnotationVal == "" { - return nil - } - - grcDisabledByMGH, err := strconv.ParseBool(grcDisabledByMGHAnnotationVal) - if err != nil { - return fmt.Errorf("invalid annotation value type for %s from MGH instance. error = %v", - GRCDisabledByMGHAnnotation, err) - } - - if !grcDisabledByMGH { - return nil - } - - mch, err := getMCH(ctx, r.Client) - if err != nil { - return fmt.Errorf("failed to get MCH instance. err = %v", err) - } - - if mch.Spec.Overrides != nil { - found := false - for _, c := range mch.Spec.Overrides.Components { - if c.Name == "grc" { - if !c.Enabled { - c.Enabled = true - } - found = true - break - } - } - if !found { - mch.Spec.Overrides.Components = append(mch.Spec.Overrides.Components, - mchv1.ComponentConfig{ - Name: "grc", - Enabled: true, - }) - } - } else { - mch.Spec.Overrides = &mchv1.Overrides{ - Components: []mchv1.ComponentConfig{ - { - Name: "grc", - Enabled: true, - }, - }, - } - } + // if grcDisabledByMGH { + // // set the annotation to remember grc status + // annotations := mgh.GetAnnotations() + // annotations[GRCDisabledByMGHAnnotation] = strconv.FormatBool(grcDisabledByMGH) + // mgh.SetAnnotations(annotations) + // if err := r.Client.Update(ctx, mgh, &client.UpdateOptions{}); err != nil { + // return fmt.Errorf("failed to annotation(%s) to mgh. err = %v", GRCDisabledByMGHAnnotation, err) + // } + // } return r.Client.Update(ctx, mch) } From 4f73abf4eb971d95474ea356ce9e7efade813b41 Mon Sep 17 00:00:00 2001 From: morvencao Date: Thu, 25 May 2023 03:05:53 +0000 Subject: [PATCH 3/4] add testing. Signed-off-by: morvencao --- operator/pkg/condition/condition.go | 4 +- .../controllers/hubofhubs/globalhub_mch.go | 54 +++++---- .../hubofhubs/globalhub_mch_test.go | 110 ++++++++++++++++++ .../controllers/hubofhubs/integration_test.go | 20 ++++ .../pkg/controllers/hubofhubs/suite_test.go | 3 + test/setup/hoh/components/mch.yaml | 35 ++++++ test/setup/hoh/hoh_setup.sh | 2 + 7 files changed, 204 insertions(+), 24 deletions(-) create mode 100644 operator/pkg/controllers/hubofhubs/globalhub_mch_test.go create mode 100644 test/setup/hoh/components/mch.yaml diff --git a/operator/pkg/condition/condition.go b/operator/pkg/condition/condition.go index 41214c887b..6070fa8303 100644 --- a/operator/pkg/condition/condition.go +++ b/operator/pkg/condition/condition.go @@ -68,8 +68,8 @@ const ( // NOTE: the status of ManagerDeployed can only be True; otherwise there is no condition const ( - CONDITION_TYPE_GRC_DISABLED = "GRCDISABLED" - CONDITION_REASON_GRC_DISABLED = "GRCDISABLED" + CONDITION_TYPE_GRC_DISABLED = "GRCDisabled" + CONDITION_REASON_GRC_DISABLED = "GRCDisabled" CONDITION_MESSAGE_GRC_DISABLED = "GRC has been disabled in MultiClusterHub" ) diff --git a/operator/pkg/controllers/hubofhubs/globalhub_mch.go b/operator/pkg/controllers/hubofhubs/globalhub_mch.go index 77dfe28654..3fef708394 100644 --- a/operator/pkg/controllers/hubofhubs/globalhub_mch.go +++ b/operator/pkg/controllers/hubofhubs/globalhub_mch.go @@ -21,14 +21,39 @@ func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, return fmt.Errorf("failed to get MCH instance. err = %v", err) } - // grcDisabledByMGH := false + mch, _ = disableGRCInMCH(mch) + + // if grcDisabledByMGH { + // // set the annotation to remember grc status + // annotations := mgh.GetAnnotations() + // annotations[GRCDisabledByMGHAnnotation] = strconv.FormatBool(grcDisabledByMGH) + // mgh.SetAnnotations(annotations) + // if err := r.Client.Update(ctx, mgh, &client.UpdateOptions{}); err != nil { + // return fmt.Errorf("failed to annotation(%s) to mgh. err = %v", GRCDisabledByMGHAnnotation, err) + // } + // } + + if err := r.Client.Update(ctx, mch); err != nil { + return fmt.Errorf("failed to update MCH instance. err = %v", err) + } + + if err := condition.SetConditionGRCDisabled(ctx, r.Client, mgh, + condition.CONDITION_STATUS_TRUE); err != nil { + return condition.FailToSetConditionError(condition.CONDITION_STATUS_TRUE, err) + } + + return nil +} + +func disableGRCInMCH(mch *mchv1.MultiClusterHub) (*mchv1.MultiClusterHub, bool) { + grcDisabledByMGH := false if mch.Spec.Overrides != nil { found := false - for _, c := range mch.Spec.Overrides.Components { + for i, c := range mch.Spec.Overrides.Components { if c.Name == "grc" { if c.Enabled { - c.Enabled = false - // grcDisabledByMGH = true + mch.Spec.Overrides.Components[i].Enabled = false + grcDisabledByMGH = true } found = true break @@ -40,7 +65,7 @@ func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, Name: "grc", Enabled: false, }) - // grcDisabledByMGH = true + grcDisabledByMGH = true } } else { mch.Spec.Overrides = &mchv1.Overrides{ @@ -51,25 +76,10 @@ func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, }, }, } - // grcDisabledByMGH = true + grcDisabledByMGH = true } - if err := condition.SetConditionGRCDisabled(ctx, r.Client, mgh, - condition.CONDITION_STATUS_TRUE); err != nil { - return condition.FailToSetConditionError(condition.CONDITION_STATUS_TRUE, err) - } - - // if grcDisabledByMGH { - // // set the annotation to remember grc status - // annotations := mgh.GetAnnotations() - // annotations[GRCDisabledByMGHAnnotation] = strconv.FormatBool(grcDisabledByMGH) - // mgh.SetAnnotations(annotations) - // if err := r.Client.Update(ctx, mgh, &client.UpdateOptions{}); err != nil { - // return fmt.Errorf("failed to annotation(%s) to mgh. err = %v", GRCDisabledByMGHAnnotation, err) - // } - // } - - return r.Client.Update(ctx, mch) + return mch, grcDisabledByMGH } func getMCH(ctx context.Context, k8sClient client.Client) (*mchv1.MultiClusterHub, error) { diff --git a/operator/pkg/controllers/hubofhubs/globalhub_mch_test.go b/operator/pkg/controllers/hubofhubs/globalhub_mch_test.go new file mode 100644 index 0000000000..ed257e6a7c --- /dev/null +++ b/operator/pkg/controllers/hubofhubs/globalhub_mch_test.go @@ -0,0 +1,110 @@ +package hubofhubs + +import ( + "testing" + + mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/stolostron/multicluster-global-hub/operator/pkg/config" +) + +func TestDisableGRCInMCH(t *testing.T) { + cases := []struct { + name string + mch *mchv1.MultiClusterHub + grcDisabled bool + }{ + { + name: "no need to disable grc", + mch: &mchv1.MultiClusterHub{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multiclusterhub", + Namespace: config.GetDefaultNamespace(), + }, + Spec: mchv1.MultiClusterHubSpec{ + Overrides: &mchv1.Overrides{ + Components: []mchv1.ComponentConfig{ + { + Name: "grc", + Enabled: false, + }, + }, + }, + }, + }, + grcDisabled: false, + }, + { + name: "disable grc explicitly", + mch: &mchv1.MultiClusterHub{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multiclusterhub", + Namespace: config.GetDefaultNamespace(), + }, + Spec: mchv1.MultiClusterHubSpec{ + Overrides: &mchv1.Overrides{ + Components: []mchv1.ComponentConfig{ + { + Name: "grc", + Enabled: true, + }, + }, + }, + }, + }, + grcDisabled: true, + }, + { + name: "disable grc by adding component explicitly", + mch: &mchv1.MultiClusterHub{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multiclusterhub", + Namespace: config.GetDefaultNamespace(), + }, + Spec: mchv1.MultiClusterHubSpec{ + Overrides: &mchv1.Overrides{ + Components: []mchv1.ComponentConfig{}, + }, + }, + }, + grcDisabled: true, + }, + { + name: "disable grc by adding overrides explicitly", + mch: &mchv1.MultiClusterHub{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multiclusterhub", + Namespace: config.GetDefaultNamespace(), + }, + Spec: mchv1.MultiClusterHubSpec{}, + }, + grcDisabled: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + newMCH, getGRCDisabled := disableGRCInMCH(tc.mch) + if newMCH.Spec.Overrides == nil { + t.Errorf("empty MCH component overrides") + } + foundGRC := false + for _, c := range newMCH.Spec.Overrides.Components { + if c.Name == "grc" { + foundGRC = true + if c.Enabled == true { + t.Errorf("GRC is not disabled in MCH") + } + break + } + } + if !foundGRC { + t.Errorf("GRC is not add to MCH component overrides") + } + if getGRCDisabled != tc.grcDisabled { + t.Errorf("unexpected grcDisabled, got: %v, want: %v", getGRCDisabled, tc.grcDisabled) + } + }) + } +} diff --git a/operator/pkg/controllers/hubofhubs/integration_test.go b/operator/pkg/controllers/hubofhubs/integration_test.go index 0fac2f5bf4..2320baf134 100644 --- a/operator/pkg/controllers/hubofhubs/integration_test.go +++ b/operator/pkg/controllers/hubofhubs/integration_test.go @@ -27,6 +27,7 @@ import ( "github.com/kylelemons/godebug/diff" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1" "gopkg.in/yaml.v2" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" @@ -291,6 +292,25 @@ var _ = Describe("MulticlusterGlobalHub controller", Ordered, func() { Type: corev1.SecretTypeOpaque, })).Should(Succeed()) + By("By creating a new MCH instance") + mch := &mchv1.MultiClusterHub{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multiclusterhub", + Namespace: config.GetDefaultNamespace(), + }, + Spec: mchv1.MultiClusterHubSpec{ + Overrides: &mchv1.Overrides{ + Components: []mchv1.ComponentConfig{ + { + Name: "grc", + Enabled: true, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, mch)).Should(Succeed()) + By("By creating a new MGH instance") mgh.SetNamespace(config.GetDefaultNamespace()) Expect(k8sClient.Create(ctx, mgh)).Should(Succeed()) diff --git a/operator/pkg/controllers/hubofhubs/suite_test.go b/operator/pkg/controllers/hubofhubs/suite_test.go index e48cc2c2a1..25a625741f 100644 --- a/operator/pkg/controllers/hubofhubs/suite_test.go +++ b/operator/pkg/controllers/hubofhubs/suite_test.go @@ -26,6 +26,7 @@ import ( routev1 "github.com/openshift/api/route/v1" operatorsv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apis/operators/v1" hypershiftdeploymentv1alpha1 "github.com/stolostron/hypershift-deployment-controller/api/v1alpha1" + mchv1 "github.com/stolostron/multiclusterhub-operator/api/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -130,6 +131,8 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) err = policyv1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) + err = mchv1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) //+kubebuilder:scaffold:scheme k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) diff --git a/test/setup/hoh/components/mch.yaml b/test/setup/hoh/components/mch.yaml new file mode 100644 index 0000000000..d660c4b0b0 --- /dev/null +++ b/test/setup/hoh/components/mch.yaml @@ -0,0 +1,35 @@ +apiVersion: operator.open-cluster-management.io/v1 +kind: MultiClusterHub +metadata: + name: multiclusterhub +spec: + availabilityConfig: High + enableClusterBackup: false + imagePullSecret: multiclusterhub-operator-pull-secret + ingress: + sslCiphers: + - ECDHE-ECDSA-AES256-GCM-SHA384 + - ECDHE-RSA-AES256-GCM-SHA384 + - ECDHE-ECDSA-AES128-GCM-SHA256 + - ECDHE-RSA-AES128-GCM-SHA256 + overrides: + components: + - enabled: true + name: console + - enabled: true + name: insights + - enabled: true + name: grc + - enabled: true + name: cluster-lifecycle + - enabled: true + name: volsync + - enabled: true + name: multicluster-engine + - enabled: true + name: search + - enabled: true + name: app-lifecycle + - enabled: false + name: cluster-backup + separateCertificateManagement: false diff --git a/test/setup/hoh/hoh_setup.sh b/test/setup/hoh/hoh_setup.sh index d616f57a86..7dd2faf62e 100755 --- a/test/setup/hoh/hoh_setup.sh +++ b/test/setup/hoh/hoh_setup.sh @@ -26,6 +26,8 @@ rootDir="$(cd "$(dirname "$0")/../.." ; pwd -P)" # create leader election configuration kubectl apply -f ${currentDir}/components/leader-election-configmap.yaml -n "$namespace" # install crds +kubectl apply -f ${rootDir}/pkg/testdata/crds/0000_01_operator.open-cluster-management.io_multiclusterhubs.crd.yaml +kubectl apply -f ${currentDir}/components/mch.yaml -n "$namespace" kubectl --context kind-hub1 apply -f ${rootDir}/pkg/testdata/crds/0000_01_operator.open-cluster-management.io_multiclusterhubs.crd.yaml kubectl --context kind-hub2 apply -f ${rootDir}/pkg/testdata/crds/0000_01_operator.open-cluster-management.io_multiclusterhubs.crd.yaml From f8584c734c2ff0bc82ab3530c9459c733d1f0106 Mon Sep 17 00:00:00 2001 From: morvencao Date: Thu, 25 May 2023 10:11:11 +0000 Subject: [PATCH 4/4] address comments. Signed-off-by: morvencao --- .../controllers/hubofhubs/globalhub_mch.go | 7 ++-- test/setup/hoh/components/mch.yaml | 35 ------------------- test/setup/hoh/hoh_setup.sh | 2 -- 3 files changed, 4 insertions(+), 40 deletions(-) delete mode 100644 test/setup/hoh/components/mch.yaml diff --git a/operator/pkg/controllers/hubofhubs/globalhub_mch.go b/operator/pkg/controllers/hubofhubs/globalhub_mch.go index 3fef708394..4755af3d69 100644 --- a/operator/pkg/controllers/hubofhubs/globalhub_mch.go +++ b/operator/pkg/controllers/hubofhubs/globalhub_mch.go @@ -16,9 +16,10 @@ import ( func (r *MulticlusterGlobalHubReconciler) reconcileMCH(ctx context.Context, mgh *operatorv1alpha2.MulticlusterGlobalHub, ) error { - mch, err := getMCH(ctx, r.Client) - if err != nil { - return fmt.Errorf("failed to get MCH instance. err = %v", err) + mch, _ := getMCH(ctx, r.Client) + // skip reconcile mch if it is not found + if mch == nil { + return nil } mch, _ = disableGRCInMCH(mch) diff --git a/test/setup/hoh/components/mch.yaml b/test/setup/hoh/components/mch.yaml deleted file mode 100644 index d660c4b0b0..0000000000 --- a/test/setup/hoh/components/mch.yaml +++ /dev/null @@ -1,35 +0,0 @@ -apiVersion: operator.open-cluster-management.io/v1 -kind: MultiClusterHub -metadata: - name: multiclusterhub -spec: - availabilityConfig: High - enableClusterBackup: false - imagePullSecret: multiclusterhub-operator-pull-secret - ingress: - sslCiphers: - - ECDHE-ECDSA-AES256-GCM-SHA384 - - ECDHE-RSA-AES256-GCM-SHA384 - - ECDHE-ECDSA-AES128-GCM-SHA256 - - ECDHE-RSA-AES128-GCM-SHA256 - overrides: - components: - - enabled: true - name: console - - enabled: true - name: insights - - enabled: true - name: grc - - enabled: true - name: cluster-lifecycle - - enabled: true - name: volsync - - enabled: true - name: multicluster-engine - - enabled: true - name: search - - enabled: true - name: app-lifecycle - - enabled: false - name: cluster-backup - separateCertificateManagement: false diff --git a/test/setup/hoh/hoh_setup.sh b/test/setup/hoh/hoh_setup.sh index 7dd2faf62e..d616f57a86 100755 --- a/test/setup/hoh/hoh_setup.sh +++ b/test/setup/hoh/hoh_setup.sh @@ -26,8 +26,6 @@ rootDir="$(cd "$(dirname "$0")/../.." ; pwd -P)" # create leader election configuration kubectl apply -f ${currentDir}/components/leader-election-configmap.yaml -n "$namespace" # install crds -kubectl apply -f ${rootDir}/pkg/testdata/crds/0000_01_operator.open-cluster-management.io_multiclusterhubs.crd.yaml -kubectl apply -f ${currentDir}/components/mch.yaml -n "$namespace" kubectl --context kind-hub1 apply -f ${rootDir}/pkg/testdata/crds/0000_01_operator.open-cluster-management.io_multiclusterhubs.crd.yaml kubectl --context kind-hub2 apply -f ${rootDir}/pkg/testdata/crds/0000_01_operator.open-cluster-management.io_multiclusterhubs.crd.yaml