diff --git a/controllers/gitopsservice_controller.go b/controllers/gitopsservice_controller.go index b643cad0529..9ffdcc4e426 100644 --- a/controllers/gitopsservice_controller.go +++ b/controllers/gitopsservice_controller.go @@ -248,86 +248,114 @@ func (r *ReconcileGitopsService) Reconcile(ctx context.Context, request reconcil return reconcile.Result{}, err } - // Create namespace if it doesn't already exist - namespaceRef := newRestrictedNamespace(namespace) - err = r.Client.Get(ctx, types.NamespacedName{Name: namespace}, namespaceRef) - if err != nil { - if errors.IsNotFound(err) { - reqLogger.Info("Creating a new Namespace", "Name", namespace) - ensureInfraNodeSelectorAnnotation(namespaceRef, instance.Spec.RunOnInfra) - err = r.Client.Create(ctx, namespaceRef) - if err != nil { + gitopsserviceNamespacedName := types.NamespacedName{ + Name: serviceName, + Namespace: namespace, + } + + if !r.DisableDefaultInstall { + // Create namespace if it doesn't already exist (only when default install is enabled) + namespaceRef := newRestrictedNamespace(namespace) + err = r.Client.Get(ctx, types.NamespacedName{Name: namespace}, namespaceRef) + if err != nil { + if errors.IsNotFound(err) { + reqLogger.Info("Creating a new Namespace", "Name", namespace) + ensureInfraNodeSelectorAnnotation(namespaceRef, instance.Spec.RunOnInfra) + err = r.Client.Create(ctx, namespaceRef) + if err != nil { + return reconcile.Result{}, err + } + } else { return reconcile.Result{}, err } } else { - return reconcile.Result{}, err - } - } else { - if ensureNamespaceMetadata(namespaceRef, instance.Spec.RunOnInfra) { - err = r.Client.Update(context.TODO(), namespaceRef) - if err != nil { - return reconcile.Result{}, err + if ensureNamespaceMetadata(namespaceRef, instance.Spec.RunOnInfra) { + err = r.Client.Update(context.TODO(), namespaceRef) + if err != nil { + return reconcile.Result{}, err + } } } - } - - gitopsserviceNamespacedName := types.NamespacedName{ - Name: serviceName, - Namespace: namespace, - } - r.cleanKAMResources(ctx, reqLogger) + r.cleanKAMResources(ctx, reqLogger) - if !r.DisableDefaultInstall { - // Create/reconcile the default Argo CD instance, unless default install is disabled + // Create/reconcile the default Argo CD instance if result, err := r.reconcileDefaultArgoCDInstance(instance, reqLogger); err != nil { return result, fmt.Errorf("unable to reconcile default Argo CD instance: %v", err) } - } else { - // If installation of default Argo CD instance is disabled, make sure it doesn't exist, - // deleting it if necessary - if err := r.ensureDefaultArgoCDInstanceDoesntExist(); err != nil { - return reconcile.Result{}, fmt.Errorf("unable to ensure non-existence of default Argo CD instance: %v", err) + + // Reconcile backend service + if result, err := r.reconcileBackend(gitopsserviceNamespacedName, instance, reqLogger); err != nil { + return result, err } - } - if result, err := r.reconcileBackend(gitopsserviceNamespacedName, instance, reqLogger); err != nil { - return result, err - } + // Reconcile console plugin (only when default install is enabled) + dynamicPluginStartOCPVersion := os.Getenv(dynamicPluginStartOCPVersionEnv) + if dynamicPluginStartOCPVersion == "" { + dynamicPluginStartOCPVersion = common.DefaultDynamicPluginStartOCPVersion + } - dynamicPluginStartOCPVersion := os.Getenv(dynamicPluginStartOCPVersionEnv) - if dynamicPluginStartOCPVersion == "" { - dynamicPluginStartOCPVersion = common.DefaultDynamicPluginStartOCPVersion - } + OCPVersion, err := util.GetClusterVersion(r.Client) + if err != nil { + log.Printf("Unable to get cluster version: %v", err) + return reconcile.Result{}, nil + } - OCPVersion, err := util.GetClusterVersion(r.Client) - if err != nil { - log.Printf("Unable to get cluster version: %v", err) - return reconcile.Result{}, nil - } + v1, err := version.NewVersion(OCPVersion) + if err != nil { + log.Printf("Unable to retrieve current OCP version: %v", err) + return reconcile.Result{}, nil + } + realVersion := v1.Segments() + if len(realVersion) < 2 { + log.Printf("OCP version has fewer than 2 segments, skipping plugin reconciliation") + return reconcile.Result{}, nil + } + realMajorVersion := realVersion[0] + realMinorVersion := realVersion[1] - v1, err := version.NewVersion(OCPVersion) - if err != nil { - log.Printf("Unable to retrieve current OCP version: %v", err) - return reconcile.Result{}, nil - } - realVersion := v1.Segments() - realMajorVersion := realVersion[0] - realMinorVersion := realVersion[1] + v2, err := version.NewVersion(dynamicPluginStartOCPVersion) + if err != nil { + return reconcile.Result{}, nil + } + startVersion := v2.Segments() + if len(startVersion) < 2 { + log.Printf("DYNAMIC_PLUGIN_START_OCP_VERSION has fewer than 2 segments, skipping plugin reconciliation") + return reconcile.Result{}, nil + } + startMajorVersion := startVersion[0] + startMinorVersion := startVersion[1] - v2, err := version.NewVersion(dynamicPluginStartOCPVersion) - if err != nil { + if realMajorVersion >= startMajorVersion && (realMajorVersion > startMajorVersion || realMinorVersion >= startMinorVersion) { + // Reconcile plugin only if OCP version supports it + return r.reconcilePlugin(instance, request) + } return reconcile.Result{}, nil - } - startVersion := v2.Segments() - startMajorVersion := startVersion[0] - startMinorVersion := startVersion[1] + } else { + // If installation of default Argo CD instance is disabled, make sure it doesn't exist, + // deleting it if necessary + if err := r.ensureDefaultArgoCDInstanceDoesntExist(); err != nil { + return reconcile.Result{}, fmt.Errorf("unable to ensure non-existence of default Argo CD instance: %v", err) + } - if realMajorVersion < startMajorVersion || (realMajorVersion == startMajorVersion && realMinorVersion < startMinorVersion) { - // Skip plugin reconciliation if real OCP version is less than dynamic plugin start OCP version + // When default install is disabled, only reconcile backend if namespace exists and is not being deleted + namespaceRef := newRestrictedNamespace(namespace) + err := r.Client.Get(ctx, types.NamespacedName{Name: namespace}, namespaceRef) + if err == nil { + // Check if namespace is being deleted + if namespaceRef.DeletionTimestamp != nil { + // Namespace is being deleted, skip backend reconciliation + return reconcile.Result{}, nil + } + // Namespace exists and is not terminating, reconcile backend + if result, err := r.reconcileBackend(gitopsserviceNamespacedName, instance, reqLogger); err != nil { + return result, err + } + } else if !errors.IsNotFound(err) { + return reconcile.Result{}, err + } + // If namespace doesn't exist, skip backend reconciliation and plugin reconciliation return reconcile.Result{}, nil - } else { - return r.reconcilePlugin(instance, request) } } @@ -404,6 +432,13 @@ func (r *ReconcileGitopsService) ensureDefaultArgoCDInstanceDoesntExist() error return err } + // Also delete the namespace when DISABLE_DEFAULT_ARGOCD_INSTANCE is true + if err := r.Client.Delete(context.TODO(), argocdNS); err != nil { + if !errors.IsNotFound(err) { + return fmt.Errorf("failed to delete openshift-gitops namespace: %w", err) + } + } + return nil } diff --git a/controllers/gitopsservice_controller_test.go b/controllers/gitopsservice_controller_test.go index 65860dfeff7..b08e9d601fb 100644 --- a/controllers/gitopsservice_controller_test.go +++ b/controllers/gitopsservice_controller_test.go @@ -154,21 +154,25 @@ func TestReconcileDisableDefault(t *testing.T) { argoCD := &argoapp.ArgoCD{} - // ArgoCD instance SHOULD NOT created (in openshift-gitops namespace) + // ArgoCD instance SHOULD NOT be created (in openshift-gitops namespace) if err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: common.ArgoCDInstanceName, Namespace: serviceNamespace}, argoCD); err == nil || !errors.IsNotFound(err) { t.Fatalf("ArgoCD instance should not exist in namespace, error: %v", err) } - // openshift-gitops namespace SHOULD be created + // openshift-gitops namespace SHOULD NOT be created when DISABLE_DEFAULT_ARGOCD_INSTANCE is true err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceNamespace}, &corev1.Namespace{}) - assertNoError(t, err) + if err == nil || !errors.IsNotFound(err) { + t.Fatalf("Namespace should not exist when DISABLE_DEFAULT_ARGOCD_INSTANCE is true, error: %v", err) + } - // backend Deployment SHOULD be created + // backend Deployment SHOULD NOT be created (no namespace to deploy into) deploy := &appsv1.Deployment{} err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceName, Namespace: serviceNamespace}, deploy) - assertNoError(t, err) + if err == nil || !errors.IsNotFound(err) { + t.Fatalf("Backend deployment should not exist when namespace doesn't exist, error: %v", err) + } } @@ -208,14 +212,11 @@ func TestReconcileDisableDefault_DeleteIfAlreadyExists(t *testing.T) { t.Fatalf("ArgoCD instance should not exist in namespace, error: %v", err) } - // openshift-gitops namespace SHOULD still exist + // openshift-gitops namespace SHOULD be deleted when DISABLE_DEFAULT_ARGOCD_INSTANCE is enabled err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceNamespace}, &corev1.Namespace{}) - assertNoError(t, err) - - // backend Deployment SHOULD still exist - deploy := &appsv1.Deployment{} - err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceName, Namespace: serviceNamespace}, deploy) - assertNoError(t, err) + if err == nil || !errors.IsNotFound(err) { + t.Fatalf("Namespace should be deleted when DISABLE_DEFAULT_ARGOCD_INSTANCE is enabled, error: %v", err) + } } diff --git a/docs/Migration_Guide.md b/docs/Migration_Guide.md index cf01762a7ed..f573d9a89c3 100644 --- a/docs/Migration_Guide.md +++ b/docs/Migration_Guide.md @@ -6,7 +6,7 @@ For understanding the differences between [Argo CD Community Operator](https://g **Note**: Installing GitOps operator will create a namespace with the name `openshift-gitops` and an Argo CD instance in the same namespace. This instance can be used for managing your OpenShift cluster configuration. It is enabled with Dex OpenShift connector by default which allows users to log in with their OpenShift credentials. -The default Argo CD instance in the `openshift-gitops` namespace can be deleted by adding an environmental variable `DISABLE_DEFAULT_ARGOCD_INSTANCE` with the value `true` in the Subscription resource. +The default Argo CD instance and the `openshift-gitops` namespace can be deleted by adding an environmental variable `DISABLE_DEFAULT_ARGOCD_INSTANCE` with the value `true` in the Subscription resource. This will completely remove the default installation. To disable the default instance, edit the Subscription and add the following: @@ -65,7 +65,7 @@ Post migration the above environment variables has to be copied to GitOps operat **Note**: GitOps operator supports the below additional environment variables -`DISABLE_DEFAULT_ARGOCD_INSTANCE`: Disables the installation of default instance in openshift-gitops namespace. +`DISABLE_DEFAULT_ARGOCD_INSTANCE`: Disables the installation of default instance in openshift-gitops namespace. This prevents the creation of the `openshift-gitops` namespace and ArgoCD instance. If they already exist, they will be deleted. `ARGOCD_CLUSTER_CONFIG_NAMESPACES`: Argo CD is granted permissions to manage specific cluster-scoped resources which include platform operators, optional OLM operators, user management, etc. Argo CD is not granted cluster-admin. You can find the complete diff --git a/docs/OpenShift GitOps Usage Guide.md b/docs/OpenShift GitOps Usage Guide.md index 0d8876a9922..82340daf39f 100644 --- a/docs/OpenShift GitOps Usage Guide.md +++ b/docs/OpenShift GitOps Usage Guide.md @@ -121,7 +121,7 @@ When installing the OpenShift GitOps operator to ROSA/OSD, cluster administrator To disable the default ‘ready-to-use’ installation of Argo CD: as an admin, update the existing Subscription Object for Gitops Operator and add `DISABLE_DEFAULT_ARGOCD_INSTANCE = true` to the spec. -**Warning**: setting this option to true will cause the existing Argo CD install in the *openshift-gitops* namespace to be deleted. Argo CD instances in other namespaces should not be affected. +**Warning**: setting this option to true will cause the existing Argo CD instance **and the `openshift-gitops` namespace** to be deleted. This completely removes the default installation. Argo CD instances in other namespaces should not be affected. On OpenShift Console, go to @@ -222,7 +222,7 @@ Updating the following environment variables in the existing Subscription Object DISABLE_DEFAULT_ARGOCD_INSTANCE false - When set to `true`, will disable the default 'ready-to-use' installation of Argo CD in `openshift-gitops` namespace. + When set to `true`, will disable the default 'ready-to-use' installation of Argo CD in `openshift-gitops` namespace. This prevents the creation of the `openshift-gitops` namespace and ArgoCD instance. If the namespace already exists, it will be deleted along with the ArgoCD instance. SERVER_CLUSTER_ROLE diff --git a/hack/non-olm-install/README.md b/hack/non-olm-install/README.md index dfd0e458543..8460170f223 100644 --- a/hack/non-olm-install/README.md +++ b/hack/non-olm-install/README.md @@ -81,7 +81,7 @@ The following environment variables can be set to configure various options for | ----------- | ----------- |------------- | | **ARGOCD_CLUSTER_CONFIG_NAMESPACES** |OpenShift GitOps instances in the identified namespaces are granted limited additional permissions to manage specific cluster-scoped resources, which include platform operators, optional OLM operators, user management, etc.Multiple namespaces can be specified via a comma delimited list. | openshift-gitops | | **CONTROLLER_CLUSTER_ROLE** | This environment variable enables administrators to configure a common cluster role to use across all managed namespaces in the role bindings the operator creates for the Argo CD application controller. | None | -| **DISABLE_DEFAULT_ARGOCD_INSTANCE** | When set to `true`, this will disable the default 'ready-to-use' installation of Argo CD in the `openshift-gitops` namespace. |false | +| **DISABLE_DEFAULT_ARGOCD_INSTANCE** | When set to `true`, this will disable the default 'ready-to-use' installation of Argo CD in the `openshift-gitops` namespace. This prevents the creation of the `openshift-gitops` namespace and ArgoCD instance. If they already exist, they will be deleted. |false | | **SERVER_CLUSTER_ROLE** |This environment variable enables administrators to configure a common cluster role to use across all of the managed namespaces in the role bindings the operator creates for the Argo CD server. | None | | **WATCH_NAMESPACE** | namespaces in which Argo applications can be created | None | | **ENABLE_CONVERSION_WEBHOOK** | This environment variable enables conversion webhook to convert v1alpha1 ArgoCD resources to v1beta1 | true | diff --git a/test/nondefaulte2e/gitops_service_nondefault_test.go b/test/nondefaulte2e/gitops_service_nondefault_test.go index 71f6c3f7f8d..a82019a85d6 100644 --- a/test/nondefaulte2e/gitops_service_nondefault_test.go +++ b/test/nondefaulte2e/gitops_service_nondefault_test.go @@ -45,17 +45,25 @@ var _ = Describe("GitOpsServiceNoDefaultInstall", func() { }, } - It("Backend resources are created in 'openshift-gitops' namespace", func() { - resourceList := []helper.ResourceList{ - { - Resource: &appsv1.Deployment{}, - ExpectedResources: []string{ - "cluster", - }, - }, - } - err := helper.WaitForResourcesByName(k8sClient, resourceList, existingArgoInstance.Namespace, time.Second*180) - Expect(err).NotTo(HaveOccurred()) + It("openshift-gitops namespace should not be created when DISABLE_DEFAULT_ARGOCD_INSTANCE is true", func() { + // When DISABLE_DEFAULT_ARGOCD_INSTANCE is true, the namespace should not exist + Consistently(func() bool { + ns := &corev1.Namespace{} + err := k8sClient.Get(context.Background(), + types.NamespacedName{Name: existingArgoInstance.Namespace}, + ns) + // Namespace should not exist + return kubeerrors.IsNotFound(err) + }, time.Second*30, interval).Should(BeTrue(), "openshift-gitops namespace should not exist when DISABLE_DEFAULT_ARGOCD_INSTANCE is true") + }) + + It("Backend deployment should not be created when DISABLE_DEFAULT_ARGOCD_INSTANCE is true", func() { + // Backend deployment should not exist (namespace doesn't exist) + deployment := &appsv1.Deployment{} + err := k8sClient.Get(context.Background(), + types.NamespacedName{Name: "cluster", Namespace: existingArgoInstance.Namespace}, + deployment) + Expect(kubeerrors.IsNotFound(err)).To(BeTrue(), "Backend deployment 'cluster' should not exist when DISABLE_DEFAULT_ARGOCD_INSTANCE is true") }) It("Default Argo CD instance should not be found", func() { diff --git a/test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go b/test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go index ed5efb2759c..a644f952d0c 100644 --- a/test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go +++ b/test/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go @@ -29,6 +29,7 @@ import ( statefulsetFixture "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/statefulset" "github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/fixture/utils" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -145,9 +146,21 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { } Eventually(gitopsServerDepl).Should(k8sFixture.NotExistByName()) + By("verifying openshift-gitops namespace no longer exists") + openshiftGitopsNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "openshift-gitops", + }, + } + Eventually(openshiftGitopsNS).Should(k8sFixture.NotExistByName()) + By("remove the DISABLE_DEFAULT_ARGOCD_INSTANCE env var we set above") fixture.RestoreSubcriptionToDefault() + By("verifying openshift-gitops namespace is recreated") + Eventually(openshiftGitopsNS, "3m", "5s").Should(k8sFixture.ExistByName()) + + By("verifying ArgoCD CR is recreated") Eventually(openshiftGitopsArgoCD, "3m", "5s").Should(k8sFixture.ExistByName()) Eventually(openshiftGitopsArgoCD, "5m", "5s").Should(argocdFixture.BeAvailable())