From d26fb3acb9eb9fec5c187f2d7f91371f878f1009 Mon Sep 17 00:00:00 2001 From: Tesshu Flower Date: Mon, 1 Dec 2025 08:40:58 -0500 Subject: [PATCH] Allow setting installNamespace in addondeploymentconfig Signed-off-by: Tesshu Flower --- controllers/addoncontroller.go | 33 ++- controllers/addoncontroller_legacyolm_test.go | 28 ++- controllers/addoncontroller_test.go | 205 ++++++++++++++---- controllers/common.go | 11 +- controllers/manifesthelper.go | 1 + controllers/manifesthelper_helmdeploy.go | 33 ++- controllers/manifesthelper_operatordeploy.go | 12 +- 7 files changed, 264 insertions(+), 59 deletions(-) diff --git a/controllers/addoncontroller.go b/controllers/addoncontroller.go index c1761d18..1eb50205 100644 --- a/controllers/addoncontroller.go +++ b/controllers/addoncontroller.go @@ -1,6 +1,7 @@ package controllers import ( + "context" "embed" "strings" @@ -8,6 +9,7 @@ import ( operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" appsv1 "k8s.io/api/apps/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" @@ -18,6 +20,7 @@ import ( addonframeworkutils "open-cluster-management.io/addon-framework/pkg/utils" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned" + clusterv1client "open-cluster-management.io/api/client/cluster/clientset/versioned" clusterv1 "open-cluster-management.io/api/cluster/v1" workapiv1 "open-cluster-management.io/api/work/v1" @@ -124,7 +127,8 @@ var manifestFilesHelmDeployOpenShift = []string{ // Another agent with registration enabled. type volsyncAgent struct { - addonClient addonv1alpha1client.Interface + addonClient addonv1alpha1client.Interface + clusterClient clusterv1client.Interface } var _ agent.AgentAddon = &volsyncAgent{} @@ -182,6 +186,33 @@ func (h *volsyncAgent) GetAgentAddonOptions() agent.AgentAddonOptions { SupportedConfigGVRs: []schema.GroupVersionResource{ addonframeworkutils.AddOnDeploymentConfigGVR, }, + Registration: &agent.RegistrationOption{ + CSRConfigurations: func(cluster *clusterv1.ManagedCluster, + addon *addonapiv1alpha1.ManagedClusterAddOn) ([]addonapiv1alpha1.RegistrationConfig, error) { + // no-op func, we don't need any CSR configurations + return nil, nil + }, + // Set agent install namespace from addon deployment config if it exists or our our default + AgentInstallNamespace: getVolsyncInstallNamespaceFunc(h.addonClient, h.clusterClient), + }, + } +} + +func getVolsyncInstallNamespaceFunc( + addonClient addonv1alpha1client.Interface, + clusterClient clusterv1client.Interface, +) func(addon *addonapiv1alpha1.ManagedClusterAddOn) (string, error) { + return func(addon *addonapiv1alpha1.ManagedClusterAddOn) (string, error) { + // Lookup cluster + managedCluster, err := clusterClient.ClusterV1().ManagedClusters(). + Get(context.TODO(), addon.Namespace /* this is the mgd cluster name */, metav1.GetOptions{}) + if err != nil { + return "", err + } + + // ManifestHelper will get the install namespace (will be different for helm and operator deployments) + mh := getManifestHelper(embedFS, addonClient, managedCluster, addon) + return mh.getInstallNamespace() } } diff --git a/controllers/addoncontroller_legacyolm_test.go b/controllers/addoncontroller_legacyolm_test.go index c6ad1945..73f5d1f8 100644 --- a/controllers/addoncontroller_legacyolm_test.go +++ b/controllers/addoncontroller_legacyolm_test.go @@ -507,7 +507,7 @@ var _ = Describe("Addoncontroller - legacy OLM deployment tests", func() { } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) }) AfterEach(func() { cleanupAddonDeploymentConfig(addonDeploymentConfig, true) @@ -611,7 +611,7 @@ var _ = Describe("Addoncontroller - legacy OLM deployment tests", func() { }, } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -687,7 +687,7 @@ var _ = Describe("Addoncontroller - legacy OLM deployment tests", func() { }, } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -772,7 +772,7 @@ var _ = Describe("Addoncontroller - legacy OLM deployment tests", func() { }, } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -869,7 +869,7 @@ var _ = Describe("Addoncontroller - legacy OLM deployment tests", func() { }, } - defaultAddonDeploymentConfig = createAddonDeploymentConfig(defaultNodePlacement, "", "", nil, nil) + defaultAddonDeploymentConfig = createAddonDeploymentConfig(defaultNodePlacement, "", "", "", nil, nil) // Update the ClusterManagementAddOn before we create it to set a default deployment config clusterManagementAddon.Spec.SupportedConfigs[0].DefaultConfig = &addonv1alpha1.ConfigReferent{ @@ -1080,7 +1080,7 @@ var _ = Describe("Addoncontroller - legacy OLM deployment tests", func() { }, } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -1539,6 +1539,22 @@ var _ = Describe("Addon Status Update Tests", func() { Expect(statusCondition.Status).To(Equal(metav1.ConditionTrue)) Expect(statusCondition.Message).To(ContainSubstring("volsync add-on is available")) }) + + It("Should update the status with the install namespace", func() { + Eventually(func() bool { + err := testK8sClient.Get(testCtx, types.NamespacedName{ + Name: "volsync", + Namespace: testManagedClusterNamespace.GetName(), + }, mcAddon) + if err != nil { + return false + } + + return mcAddon.Status.Namespace != "" + }, timeout, interval).Should(BeTrue()) + + Expect(mcAddon.Status.Namespace).To(Equal("openshift-operators")) // hardcoded for olm deploy + }) }) }) }) diff --git a/controllers/addoncontroller_test.go b/controllers/addoncontroller_test.go index 025ef54f..26b48698 100644 --- a/controllers/addoncontroller_test.go +++ b/controllers/addoncontroller_test.go @@ -229,36 +229,8 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { Expect(len(totalMWManifests)).To(Equal(13)) } - // Get objects from our manifest that are not part of the helm chart - for _, m := range totalMWManifests { - obj, _, err := genericCodec.Decode(m.Raw, nil, nil) - Expect(err).NotTo(HaveOccurred()) - objKind := obj.GetObjectKind().GroupVersionKind().Kind - - switch objKind { - case "OperatorPolicy": - op, ok := obj.(*policyv1beta1.OperatorPolicy) - Expect(ok).To(BeTrue()) - operatorPolicyObj = op - case "ClusterRole": - cr, ok := obj.(*rbacv1.ClusterRole) - Expect(ok).To(BeTrue()) - if strings.Contains(cr.GetName(), "operatorpolicy-aggregate") { - // This is our operator policy clusterrole - operatorPolicyAggregateClusterRoleObj = cr - } else { - // This is a clusterrole from the helm chart - helmChartObjs = append(helmChartObjs, obj) - } - case "Namespace": - ns, ok := obj.(*corev1.Namespace) - Expect(ok).To(BeTrue()) - namespaceObj = ns - default: - // This object came from the helm chart - helmChartObjs = append(helmChartObjs, obj) - } - } + namespaceObj, operatorPolicyObj, operatorPolicyAggregateClusterRoleObj, helmChartObjs = + getObjectsFromVolSyncManifestWorks(manifestWorkList, genericCodec) Expect(namespaceObj).NotTo(BeNil()) @@ -378,6 +350,8 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { // ManagedClusterAddon.Spec.InstallNamespace is essentially deprecated and should not be used // See: https://github.com/open-cluster-management-io/ocm/issues/298 // volsync-addon-controller Code should ignore it + // Note it is possible to set the install namespace in the addondeploymentconfig, see tests + // below that test that. BeforeEach(func() { // Override to specifically set the ns in the spec - all the tests above in JustBeforeEach // should still be valid here @@ -560,6 +534,16 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { Expect(volsyncDeployment.Spec.Template.Spec.NodeSelector).To(BeNil()) }) + It("Should create the deployment and all resources in the default namespace of volsync-system", func() { + volsyncNamespace, _, _, helmChartObjs := getObjectsFromVolSyncManifestWorks(manifestWorkList, genericCodec) + Expect(volsyncNamespace).NotTo(BeNil()) + Expect(volsyncNamespace.GetName()).To(Equal("volsync-system")) + + helmutilstest.VerifyHelmRenderedVolSyncObjects(helmChartObjs, + "volsync-system", // All objects should be in the volsync-system namespace by default + true /* our defaults for this test are OpensShift */) + }) + Context("When the managedclusteraddon is updated later with a addondeploymentconfig", func() { var addonDeploymentConfig *addonv1alpha1.AddOnDeploymentConfig nodePlacement := &addonv1alpha1.NodePlacement{ @@ -580,7 +564,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { BeforeEach(func() { // Update addonDeploymentConfig with nodePlacment and specific rbacProxy and volsync imgs - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", customRbacProxyImage, customVolSyncImage, nil, nil) }) AfterEach(func() { @@ -680,7 +664,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { }, } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -754,7 +738,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { }, } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -837,7 +821,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { }, } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -916,7 +900,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nil, "", "", nil, customResourceRequirements) + addonDeploymentConfig = createAddonDeploymentConfig(nil, "", "", "", nil, customResourceRequirements) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -1299,7 +1283,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { logger.Info("setting customRegistries in addondeploymentconfig", "customRegistries", customRegistries) - addonDeploymentConfig = createAddonDeploymentConfig(nil, "", "", customRegistries, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nil, "", "", "", customRegistries, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -1385,7 +1369,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { logger.Info("setting customRegistries in addondeploymentconfig", "customRegistries", customRegistries) - addonDeploymentConfig = createAddonDeploymentConfig(nil, + addonDeploymentConfig = createAddonDeploymentConfig(nil, "", customVolSyncRbacProxyImage, customVolSyncImage, customRegistries, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig @@ -1454,6 +1438,81 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { }) }) + Context("When the addonDeploymentConfig sets the install namespace", func() { + var addonDeploymentConfig *addonv1alpha1.AddOnDeploymentConfig + var customInstallNamespace = "deploy-here-on-my-mgd-cluster" + + BeforeEach(func() { + addonDeploymentConfig = createAddonDeploymentConfig(nil, customInstallNamespace, "", "", nil, nil) + + // Update the managedclusteraddon before we create it to add the addondeploymentconfig + mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ + { + ConfigGroupResource: addonv1alpha1.ConfigGroupResource{ + Group: "addon.open-cluster-management.io", + Resource: "addondeploymentconfigs", + }, + ConfigReferent: addonv1alpha1.ConfigReferent{ + Name: addonDeploymentConfig.GetName(), + Namespace: addonDeploymentConfig.GetNamespace(), + }, + }, + } + }) + AfterEach(func() { + cleanupAddonDeploymentConfig(addonDeploymentConfig, true) + }) + + JustBeforeEach(func() { + // The controller that used to update the managedClusterAddOn status with the deploymentconfig + // has been moved to a common controller in the ocm hub - so simulate status update so our + // code can proceed + Eventually(func() error { + err := addDeploymentConfigStatusEntry(mcAddon, addonDeploymentConfig) + if err != nil { + // Reload the mcAddOn before we try again in case there was a conflict with updating + reloadErr := testK8sClient.Get(testCtx, client.ObjectKeyFromObject(mcAddon), mcAddon) + if reloadErr != nil { + return reloadErr + } + return err + } + return nil + }, timeout, interval).Should(Succeed()) + + // Now re-load the manifestworks, should get updated + Eventually(func() bool { + foundVsManifests := false + manifestWorkList, foundVsManifests = getVolSyncManifestWorks(testManagedCluster.GetName()) + if !foundVsManifests { + return false + } + + // Find the deployment in the manifestworks + var err error + volsyncDeployment, err = getVolSyncDeploymentFromManifestWorkList( + manifestWorkList, genericCodec) + if err != nil { + return false + } + + // If the deployment has the new install namespace set, then it's been updated properly + return volsyncDeployment.GetNamespace() == customInstallNamespace + }, timeout, interval).Should(BeTrue()) + }) + + It("Should create the deployment and all resources in the specified install namespace", func() { + logger.Info("ManifestWorkList", "manifestWorkList", manifestWorkList) + + volsyncNamespace, _, _, helmChartObjs := getObjectsFromVolSyncManifestWorks(manifestWorkList, genericCodec) + Expect(volsyncNamespace).NotTo(BeNil()) + Expect(volsyncNamespace.GetName()).To(Equal(customInstallNamespace)) + + helmutilstest.VerifyHelmRenderedVolSyncObjects(helmChartObjs, + customInstallNamespace, // All objects should be in the custom namespace by default + true /* our defaults for this test are OpenShift */) + }) + }) }) Context("When the volsync ClusterManagementAddOn has a default deployment config w/ node "+ @@ -1489,7 +1548,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { }, } - defaultAddonDeploymentConfig = createAddonDeploymentConfig(defaultNodePlacement, "", "", nil, nil) + defaultAddonDeploymentConfig = createAddonDeploymentConfig(defaultNodePlacement, "", "", "", nil, nil) // Update the ClusterManagementAddOn before we create it to set a default deployment config clusterManagementAddon.Spec.SupportedConfigs[0].DefaultConfig = &addonv1alpha1.ConfigReferent{ @@ -1677,7 +1736,7 @@ var _ = Describe("Addoncontroller - helm deployment tests", func() { }, } BeforeEach(func() { - addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", nil, nil) + addonDeploymentConfig = createAddonDeploymentConfig(nodePlacement, "", "", "", nil, nil) // Update the managedclusteraddon before we create it to add the addondeploymentconfig mcAddon.Spec.Configs = []addonv1alpha1.AddOnConfig{ @@ -2214,6 +2273,22 @@ var _ = Describe("Addon Status Update Tests", func() { Expect(statusCondition.Status).To(Equal(metav1.ConditionTrue)) Expect(statusCondition.Message).To(ContainSubstring("volsync add-on is available")) }) + + It("Should update the status with the install namespace", func() { + Eventually(func() bool { + err := testK8sClient.Get(testCtx, types.NamespacedName{ + Name: "volsync", + Namespace: testManagedClusterNamespace.GetName(), + }, mcAddon) + if err != nil { + return false + } + + return mcAddon.Status.Namespace != "" + }, timeout, interval).Should(BeTrue()) + + Expect(mcAddon.Status.Namespace).To(Equal("volsync-system")) // Default for helm deploy + }) }) }) }) @@ -2266,6 +2341,7 @@ func manifestWorkResourceStatusWithVolSyncDeploymentFeedBack( } func createAddonDeploymentConfig(nodePlacement *addonv1alpha1.NodePlacement, + installNamespace string, rbacProxyImage string, volSyncImage string, registries []addonv1alpha1.ImageMirror, @@ -2291,6 +2367,10 @@ func createAddonDeploymentConfig(nodePlacement *addonv1alpha1.NodePlacement, }, } + if installNamespace != "" { + customAddonDeploymentConfig.Spec.AgentInstallNamespace = installNamespace + } + // Set rbac proxy image as env var override if rbacProxyImage != "" { customAddonDeploymentConfig.Spec.CustomizedVariables = append(customAddonDeploymentConfig.Spec.CustomizedVariables, @@ -2399,6 +2479,53 @@ func getVolSyncManifestWorks(namespace string) ([]*workv1.ManifestWork, bool) { return manifestWorkList, totalVSManifests >= 13 } +func getObjectsFromVolSyncManifestWorks(manifestWorkList []*workv1.ManifestWork, + genericCodec runtime.Decoder, +) (*corev1.Namespace, *policyv1beta1.OperatorPolicy, *rbacv1.ClusterRole, []runtime.Object) { + + totalMWManifests := []workv1.Manifest{} + for i := range manifestWorkList { + totalMWManifests = append(totalMWManifests, manifestWorkList[i].Spec.Workload.Manifests...) + } + + var namespaceObj *corev1.Namespace + var operatorPolicyObj *policyv1beta1.OperatorPolicy + var operatorPolicyAggregateClusterRoleObj *rbacv1.ClusterRole + var helmChartObjs []runtime.Object + + for _, m := range totalMWManifests { + obj, _, err := genericCodec.Decode(m.Raw, nil, nil) + Expect(err).NotTo(HaveOccurred()) + objKind := obj.GetObjectKind().GroupVersionKind().Kind + + switch objKind { + case "OperatorPolicy": + op, ok := obj.(*policyv1beta1.OperatorPolicy) + Expect(ok).To(BeTrue()) + operatorPolicyObj = op + case "ClusterRole": + cr, ok := obj.(*rbacv1.ClusterRole) + Expect(ok).To(BeTrue()) + if strings.Contains(cr.GetName(), "operatorpolicy-aggregate") { + // This is our operator policy clusterrole + operatorPolicyAggregateClusterRoleObj = cr + } else { + // This is a clusterrole from the helm chart + helmChartObjs = append(helmChartObjs, obj) + } + case "Namespace": + ns, ok := obj.(*corev1.Namespace) + Expect(ok).To(BeTrue()) + namespaceObj = ns + default: + // This object came from the helm chart + helmChartObjs = append(helmChartObjs, obj) + } + } + + return namespaceObj, operatorPolicyObj, operatorPolicyAggregateClusterRoleObj, helmChartObjs +} + // Will return err if the volsync deployment is not found in the manifestworklist func getVolSyncDeploymentFromManifestWorkList(manifestWorkList []*workv1.ManifestWork, decoder runtime.Decoder) (*appsv1.Deployment, error) { diff --git a/controllers/common.go b/controllers/common.go index 3e2ace3e..abc12ee9 100644 --- a/controllers/common.go +++ b/controllers/common.go @@ -18,6 +18,7 @@ import ( "open-cluster-management.io/addon-framework/pkg/addonmanager" addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1" addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned" + clusterv1client "open-cluster-management.io/api/client/cluster/clientset/versioned" clusterv1 "open-cluster-management.io/api/cluster/v1" ) @@ -27,11 +28,19 @@ func StartControllers(ctx context.Context, config *rest.Config) error { return err } + clusterClient, err := clusterv1client.NewForConfig(config) + if err != nil { + return err + } + mgr, err := addonmanager.New(config) if err != nil { return err } - err = mgr.AddAgent(&volsyncAgent{addOnClient}) + err = mgr.AddAgent(&volsyncAgent{ + addOnClient, + clusterClient, + }) if err != nil { return err } diff --git a/controllers/manifesthelper.go b/controllers/manifesthelper.go index fc06c205..c1704934 100644 --- a/controllers/manifesthelper.go +++ b/controllers/manifesthelper.go @@ -20,6 +20,7 @@ import ( type manifestHelper interface { loadManifests() ([]runtime.Object, error) subHealthCheck(fieldResults []agent.FieldResult) error + getInstallNamespace() (string, error) } func getManifestHelper(embedFS embed.FS, addonClient addonv1alpha1client.Interface, diff --git a/controllers/manifesthelper_helmdeploy.go b/controllers/manifesthelper_helmdeploy.go index ece55c3d..b5ef9c06 100644 --- a/controllers/manifesthelper_helmdeploy.go +++ b/controllers/manifesthelper_helmdeploy.go @@ -22,7 +22,12 @@ type manifestHelperHelmDeploy struct { var _ manifestHelper = &manifestHelperHelmDeploy{} func (mh *manifestHelperHelmDeploy) loadManifests() ([]runtime.Object, error) { - values, err := mh.getValuesForManifest() + installNamespace, err := mh.getInstallNamespace() + if err != nil { + return nil, err + } + + values, err := mh.getValuesForManifest(installNamespace) if err != nil { return nil, err } @@ -39,7 +44,7 @@ func (mh *manifestHelperHelmDeploy) loadManifests() ([]runtime.Object, error) { } // Now load manifest objects rendered from our volsync helm chart - helmObjects, err := mh.loadManifestsFromHelmRepo(values) + helmObjects, err := mh.loadManifestsFromHelmRepo(installNamespace, values) if err != nil { return nil, err } @@ -95,9 +100,8 @@ func (mh *manifestHelperHelmDeploy) subHealthCheck(fieldResults []agent.FieldRes } // Now need to load and render the helm charts into objects -func (mh *manifestHelperHelmDeploy) loadManifestsFromHelmRepo(values addonfactory.Values) ([]runtime.Object, error) { - installNamespace := mh.getInstallNamespace() - +func (mh *manifestHelperHelmDeploy) loadManifestsFromHelmRepo(installNamespace string, + values addonfactory.Values) ([]runtime.Object, error) { // Chart will include default values from the values.yaml chart, err := helmutils.GetEmbeddedChart(mh.getChartKey()) if err != nil { @@ -111,7 +115,7 @@ func (mh *manifestHelperHelmDeploy) loadManifestsFromHelmRepo(values addonfactor } //nolint:funlen -func (mh *manifestHelperHelmDeploy) getValuesForManifest() (addonfactory.Values, error) { +func (mh *manifestHelperHelmDeploy) getValuesForManifest(installNamespace string) (addonfactory.Values, error) { manifestConfig := struct { // OpenShift target cluster parameters - this is the same as the subscription name // only used for cleaning up old OLM based install of VolSync (olm sub/csv will be removed @@ -128,7 +132,7 @@ func (mh *manifestHelperHelmDeploy) getValuesForManifest() (addonfactory.Values, // These are our default values OperatorName: operatorName, ManagedClusterName: mh.cluster.GetName(), - InstallNamespace: mh.getInstallNamespace(), + InstallNamespace: installNamespace, ImagePullSecrets: mh.getImagePullSecrets(), } @@ -184,8 +188,19 @@ func (mh *manifestHelperHelmDeploy) getValuesForManifest() (addonfactory.Values, return mergedValuesFinal, nil } -func (mh *manifestHelperHelmDeploy) getInstallNamespace() string { - return DefaultHelmInstallNamespace +func (mh *manifestHelperHelmDeploy) getInstallNamespace() (string, error) { + ns, err := addonframeworkutils.AgentInstallNamespaceFromDeploymentConfigFunc( + addonframeworkutils.NewAddOnDeploymentConfigGetter(mh.addonClient))(mh.addon) + if err != nil { + return "", err + } + + // AgentInstallNamespaceFromDeploymentConfigFunc will return an empty string if no addon deployment config exists + // or no ns specified + if ns == "" { + return DefaultHelmInstallNamespace, nil + } + return ns, nil } // Image pull secrets - we will set the "open-cluster-management-image-pull-credentials" secret copied from hub diff --git a/controllers/manifesthelper_operatordeploy.go b/controllers/manifesthelper_operatordeploy.go index 8983fcbc..59d829c2 100644 --- a/controllers/manifesthelper_operatordeploy.go +++ b/controllers/manifesthelper_operatordeploy.go @@ -58,6 +58,11 @@ func (mh *manifestHelperOperatorDeploy) subHealthCheck(fieldResults []agent.Fiel } func (mh *manifestHelperOperatorDeploy) getValuesForManifest() (addonfactory.Values, error) { + installNamespace, err := mh.getInstallNamespace() + if err != nil { + return nil, err // should never happen + } + manifestConfig := struct { OperatorInstallNamespace string @@ -70,7 +75,7 @@ func (mh *manifestHelperOperatorDeploy) getValuesForManifest() (addonfactory.Val Channel string StartingCSV string }{ - OperatorInstallNamespace: mh.getOperatorInstallNamespace(), + OperatorInstallNamespace: installNamespace, OperatorName: operatorName, CatalogSource: mh.getCatalogSource(), @@ -97,9 +102,10 @@ func (mh *manifestHelperOperatorDeploy) getValuesForManifest() (addonfactory.Val return mergedValues, nil } -func (mh *manifestHelperOperatorDeploy) getOperatorInstallNamespace() string { +// Returns error to conform to ManifestHelper interface +func (mh *manifestHelperOperatorDeploy) getInstallNamespace() (string, error) { // The only namespace supported is openshift-operators, so ignore whatever is in the spec - return globalOperatorInstallNamespace + return globalOperatorInstallNamespace, nil } func (mh *manifestHelperOperatorDeploy) getCatalogSource() string {