Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Install GH into a brownfield #1319

Merged
merged 3 commits into from
Jan 10, 2025
Merged

✨ Install GH into a brownfield #1319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
467567e
Install GH into a brownfield
clyang82 Jan 8, 2025
0311ac5
Increase code coverage
clyang82 Jan 9, 2025
d67700f
Rename to InstallAgentOnLocal
clyang82 Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions operator/api/operator/v1alpha4/multiclusterglobalhub_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,13 @@ type MulticlusterGlobalHubSpec struct {
// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:com.tectonic.ui:booleanSwitch"}
// +optional
EnableMetrics bool `json:"enableMetrics"`
// InstallAgentOnLocal determines whether deploy the Global Hub Agent on the local hub cluster or not.
// If set to true, the Global Hub Agent will be installed on the local hub cluster only.
// If set to false, the Global Hub Agent will not be installed on the local hub cluster.
// Currently, switching the value of this field is not supported after the Global Hub is installed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That means that value should be set when creating the instance. After creating, it won't support being updated.
If so, we should clarify the value is immutable and add validation for this logic

Copy link
Member

@yanmxa yanmxa Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no validation here, the desired and actual status will not be consistent after the user updates it

https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/#immutablility-after-first-modification

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just an initial version. Based on my understanding, we do not support switching the value right now. but we may support later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, Could you log a Jira issue(or GitHub issue), I'm concerned we might overlook it in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1321 log this github issue to trace. Thanks.

// +kubebuilder:default=false
// +optional
InstallAgentOnLocal bool `json:"InstallAgentOnLocal,omitempty"`
}

type AdvancedSpec struct {
Expand Down
8 changes: 8 additions & 0 deletions operator/bundle/manifests/operator.open-cluster-management.io_multiclusterglobalhubs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ spec:
retention: 18m
description: Spec specifies the desired state of multicluster global hub
properties:
InstallAgentOnLocal:
default: false
description: |-
InstallAgentOnLocal determines whether deploy the Global Hub Agent on the local hub cluster or not.
If set to true, the Global Hub Agent will be installed on the local hub cluster only.
If set to false, the Global Hub Agent will not be installed on the local hub cluster.
Currently, switching the value of this field is not supported after the Global Hub is installed.
type: boolean
advanced:
description: AdvancedSpec specifies the advanced configurations for
the multicluster global hub
Expand Down
8 changes: 8 additions & 0 deletions operator/config/crd/bases/operator.open-cluster-management.io_multiclusterglobalhubs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ spec:
retention: 18m
description: Spec specifies the desired state of multicluster global hub
properties:
InstallAgentOnLocal:
default: false
description: |-
InstallAgentOnLocal determines whether deploy the Global Hub Agent on the local hub cluster or not.
If set to true, the Global Hub Agent will be installed on the local hub cluster only.
If set to false, the Global Hub Agent will not be installed on the local hub cluster.
Currently, switching the value of this field is not supported after the Global Hub is installed.
type: boolean
advanced:
description: AdvancedSpec specifies the advanced configurations for
the multicluster global hub
Expand Down
22 changes: 19 additions & 3 deletions operator/pkg/controllers/agent/default_agent_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,24 @@ func (r *DefaultAgentController) Reconcile(ctx context.Context, req ctrl.Request
return ctrl.Result{}, nil
}

return ctrl.Result{}, r.reconcileAddonAndResources(ctx, cluster, clusterManagementAddOn)
if mgh.Spec.InstallAgentOnLocal {
// if installed agent on hub cluster, global hub is installed in a brownfield cluster
if cluster.GetName() == constants.LocalClusterName ||
cluster.Labels[constants.LocalClusterName] == "true" ||
deployMode == operatorconstants.GHAgentDeployModeDefault ||
deployMode == operatorconstants.GHAgentDeployModeHosted {
return ctrl.Result{}, r.reconcileAddonAndResources(ctx, cluster, clusterManagementAddOn)
}
} else {
// if not installed agent on hub cluster, global hub is installed in a greenfield cluster
if cluster.GetName() == constants.LocalClusterName ||
cluster.Labels[constants.LocalClusterName] == "true" {
return ctrl.Result{}, nil
}
return ctrl.Result{}, r.reconcileAddonAndResources(ctx, cluster, clusterManagementAddOn)
}

return ctrl.Result{}, nil
}

func (r *DefaultAgentController) deleteClusterManagementAddon(ctx context.Context) error {
Expand Down Expand Up @@ -457,6 +474,5 @@ func GetAllManagedHubNames(ctx context.Context, c client.Client) ([]string, erro

func filterManagedCluster(obj client.Object) bool {
return obj.GetLabels()["vendor"] != "OpenShift" ||
obj.GetLabels()["openshiftVersion"] == "3" ||
obj.GetName() == constants.LocalClusterName
obj.GetLabels()["openshiftVersion"] == "3"
}
56 changes: 55 additions & 1 deletion test/integration/operator/controllers/agent/cluster_default_addon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import (
clusterv1 "open-cluster-management.io/api/cluster/v1"
workv1 "open-cluster-management.io/api/work/v1"

globalhubv1alpha4 "github.com/stolostron/multicluster-global-hub/operator/api/operator/v1alpha4"
operatorconstants "github.com/stolostron/multicluster-global-hub/operator/pkg/constants"
"github.com/stolostron/multicluster-global-hub/pkg/constants"
)

// go test ./test/integration/operator/agent -ginkgo.focus "deploy default addon" -v
// go test ./test/integration/operator/controllers/agent -ginkgo.focus "deploy default addon" -v
var _ = Describe("deploy default addon", func() {
It("Should create agent when importing an bare OCP", func() {
clusterName := fmt.Sprintf("hub-%s", rand.String(6))
Expand Down Expand Up @@ -132,4 +133,57 @@ var _ = Describe("deploy default addon", func() {
// contains both the ACM and the Global Hub manifests
Expect(len(work.Spec.Workload.Manifests)).Should(Equal(17))
})

It("Should create agent for the local-cluster", func() {
By("set InstallAgentOnLocal to true")
mghLookupKey := types.NamespacedName{Namespace: "default", Name: MGHName}
existingMGH := &globalhubv1alpha4.MulticlusterGlobalHub{}
Eventually(func() bool {
err := runtimeClient.Get(ctx, mghLookupKey, existingMGH)
return err == nil
}, timeout, interval).Should(BeTrue())
existingMGH.Spec.InstallAgentOnLocal = true
Expect(runtimeClient.Update(ctx, existingMGH)).Should(Succeed())

clusterName := fmt.Sprintf("hub-%s", rand.String(6))
workName := fmt.Sprintf("addon-%s-deploy-0",
constants.GHManagedClusterAddonName)

By("By preparing an OCP Managed Clusters")
prepareCluster(clusterName,
map[string]string{"vendor": "OpenShift", "local-cluster": "true"},
map[string]string{},
[]clusterv1.ManagedClusterClaim{},
clusterAvailableCondition)

By("By checking the addon CR is is created in the cluster ns")
addon := &addonv1alpha1.ManagedClusterAddOn{}
Eventually(func() error {
return runtimeClient.Get(ctx, types.NamespacedName{
Name: constants.GHManagedClusterAddonName,
Namespace: clusterName,
}, addon)
}, timeout, interval).ShouldNot(HaveOccurred())

Expect(len(addon.GetAnnotations())).Should(Equal(0))

By("By checking the agent manifestworks are created for the local cluster")
work := &workv1.ManifestWork{}
Eventually(func() error {
return runtimeClient.Get(ctx, types.NamespacedName{
Name: workName,
Namespace: clusterName,
}, work)
}, timeout, interval).ShouldNot(HaveOccurred())

Expect(len(work.Spec.Workload.Manifests)).Should(Equal(8))

By("set InstallAgentOnLocal to false as a default value")
Eventually(func() bool {
err := runtimeClient.Get(ctx, mghLookupKey, existingMGH)
return err == nil
}, timeout, interval).Should(BeTrue())
existingMGH.Spec.InstallAgentOnLocal = false
Expect(runtimeClient.Update(ctx, existingMGH)).Should(Succeed())
})
})
2 changes: 1 addition & 1 deletion test/integration/operator/controllers/agent/cluster_hosted_addon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/stolostron/multicluster-global-hub/pkg/constants"
)

// go test ./test/integration/operator/agent -ginkgo.focus "deploy hosted addon" -v
// go test ./test/integration/operator/controllers/agent -ginkgo.focus "deploy hosted addon" -v
var _ = Describe("deploy hosted addon", func() {
It("Should create hosted addon in OCP", func() {
clusterName := fmt.Sprintf("hub-%s", rand.String(6)) // managed hub cluster -> enable local cluster
Expand Down
22 changes: 21 additions & 1 deletion test/integration/operator/controllers/agent/cluster_none_addon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/rand"
addonv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
clusterv1 "open-cluster-management.io/api/cluster/v1"
Expand Down Expand Up @@ -67,7 +68,7 @@ func prepareCluster(name string, labels, annotations map[string]string,
})).Should(Succeed())
}

// go test ./test/integration/operator/agent -ginkgo.focus "none addon" -v
// go test ./test/integration/operator/controllers/agent -ginkgo.focus "none addon" -v
var _ = Describe("none addon", func() {
It("Should not create addon in these cases", func() {
By("By preparing a non-OCP with deployMode label Managed Clusters")
Expand Down Expand Up @@ -146,4 +147,23 @@ var _ = Describe("none addon", func() {
return fmt.Errorf("check again %v", checkCount)
}, timeout, interval).ShouldNot(HaveOccurred())
})

It("Should not create agent for the local-cluster", func() {
clusterName := fmt.Sprintf("hub-%s", rand.String(6))
By("By preparing an OCP Managed Clusters")
prepareCluster(clusterName,
map[string]string{"vendor": "OpenShift", "local-cluster": "true"},
map[string]string{},
[]clusterv1.ManagedClusterClaim{},
clusterAvailableCondition)

By("By checking the addon CR is is created in the cluster ns")
addon := &addonv1alpha1.ManagedClusterAddOn{}
Eventually(func() error {
return runtimeClient.Get(ctx, types.NamespacedName{
Name: constants.GHManagedClusterAddonName,
Namespace: clusterName,
}, addon)
}, duration, interval).Should(HaveOccurred())
})
})
2 changes: 1 addition & 1 deletion test/integration/operator/controllers/agent/hosted_managedhub/mgh_hosted_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ var hostedMGH = globalhubv1alpha4.MulticlusterGlobalHub{
// hosted: put the acm agent cluster control plane into acm hub cluster
// global hub -> managed-hub(local-cluster)

// go test ./test/integration/operator/agent -ginkgo.focus "other addons in hosted mode test" -v
// go test ./test/integration/operator/controllers/agent -ginkgo.focus "other addons in hosted mode test" -v
var _ = Describe("other addons in hosted mode test", Ordered, func() {
var hostedAddonReconciler *agent.HostedAgentController
BeforeAll(func() {
Expand Down
Loading