diff --git a/common/common.go b/common/common.go index 65e7347e82c..ada9bc7f3b3 100644 --- a/common/common.go +++ b/common/common.go @@ -48,6 +48,10 @@ const ( InfraNodeSelectorAnnotation = "openshift.io/node-selector" // InfraNodeSelectorAnnotationValue is the value for the infra node selector annotation InfraNodeSelectorAnnotationValue = "node-role.kubernetes.io/infra=" + // NodePlacementManagedByGitopsServiceAnnotation marks NodePlacement on the default ArgoCD CR + // that was applied by the GitopsService controller. Used to distinguish GitopsService-managed + // placement from admin edits on the ArgoCD CR directly. + NodePlacementManagedByGitopsServiceAnnotation = "gitops.openshift.io/node-placement-managed-by-gitopsservice" ) // InfraNodeSelector returns openshift label for infrastructure nodes diff --git a/controllers/gitopsservice_controller.go b/controllers/gitopsservice_controller.go index ade2d0792b2..d3b7a007392 100644 --- a/controllers/gitopsservice_controller.go +++ b/controllers/gitopsservice_controller.go @@ -495,6 +495,14 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli } } + gitopsServiceConfiguresNodePlacement := len(instance.Spec.NodeSelector) > 0 || len(instance.Spec.Tolerations) > 0 + if gitopsServiceConfiguresNodePlacement { + if defaultArgoCDInstance.Annotations == nil { + defaultArgoCDInstance.Annotations = map[string]string{} + } + defaultArgoCDInstance.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation] = "true" + } + // Get or create ArgoCD instance in default namespace existingArgoCD := &argoapp.ArgoCD{} err = r.Client.Get(context.TODO(), types.NamespacedName{Name: defaultArgoCDInstance.Name, Namespace: defaultArgoCDInstance.Namespace}, existingArgoCD) @@ -560,15 +568,32 @@ func (r *ReconcileGitopsService) reconcileDefaultArgoCDInstance(instance *pipeli changed = true } - // if user is patching nodePlacement through GitopsService CR, then existingArgoCD NodePlacement is updated. - if defaultArgoCDInstance.Spec.NodePlacement != nil { - if !reflect.DeepEqual(existingArgoCD.Spec.NodePlacement, defaultArgoCDInstance.Spec.NodePlacement) { + // Sync NodePlacement when GitopsService configures placement fields. Do not wipe NodePlacement + // that admins set directly on the ArgoCD CR when GitopsService placement fields are empty. + if gitopsServiceConfiguresNodePlacement { + if defaultArgoCDInstance.Spec.NodePlacement != nil { + if !reflect.DeepEqual(existingArgoCD.Spec.NodePlacement, defaultArgoCDInstance.Spec.NodePlacement) { + existingArgoCD.Spec.NodePlacement = defaultArgoCDInstance.Spec.NodePlacement + changed = true + } + } else if existingArgoCD.Spec.NodePlacement != nil { existingArgoCD.Spec.NodePlacement = defaultArgoCDInstance.Spec.NodePlacement changed = true } - // Handle the case where NodePlacement should be removed - } else if existingArgoCD.Spec.NodePlacement != nil { - existingArgoCD.Spec.NodePlacement = defaultArgoCDInstance.Spec.NodePlacement + if existingArgoCD.Annotations == nil { + existingArgoCD.Annotations = map[string]string{} + } + if existingArgoCD.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation] != "true" { + existingArgoCD.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation] = "true" + changed = true + } + } else if existingArgoCD.Annotations != nil && + existingArgoCD.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation] == "true" { + if existingArgoCD.Spec.NodePlacement != nil { + existingArgoCD.Spec.NodePlacement = nil + changed = true + } + delete(existingArgoCD.Annotations, common.NodePlacementManagedByGitopsServiceAnnotation) changed = true } diff --git a/controllers/gitopsservice_controller_test.go b/controllers/gitopsservice_controller_test.go index f499d9a5c75..3402d697bcf 100644 --- a/controllers/gitopsservice_controller_test.go +++ b/controllers/gitopsservice_controller_test.go @@ -201,6 +201,177 @@ func TestReconcileDefaultForArgoCDNodeplacement(t *testing.T) { assertNoError(t, err) assert.Check(t, existingArgoCD.Spec.NodePlacement != nil) assert.DeepEqual(t, existingArgoCD.Spec.NodePlacement.NodeSelector, gitopsService.Spec.NodeSelector) + assert.Equal(t, existingArgoCD.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation], "true") +} + +func TestReconcileDefaultArgoCDNodePlacementPreservesDirectEdit(t *testing.T) { + logf.SetLogger(argocd.ZapLogger(true)) + s := scheme.Scheme + addKnownTypesToScheme(s) + + gitopsService := &pipelinesv1alpha1.GitopsService{ + ObjectMeta: v1.ObjectMeta{ + Name: serviceName, + }, + } + + directNodePlacement := &argoapp.ArgoCDNodePlacementSpec{ + NodeSelector: map[string]string{ + "machine.openshift.io/cluster-api-machineset": "cl01-infra-0", + }, + Tolerations: []corev1.Toleration{{ + Effect: corev1.TaintEffectNoSchedule, + Key: "infra", + Value: "reserved", + }}, + } + + fakeClient := fake.NewFakeClient(gitopsService) + reconciler := newReconcileGitOpsService(fakeClient, s) + + existingArgoCD := &argoapp.ArgoCD{ + ObjectMeta: v1.ObjectMeta{ + Name: serviceNamespace, + Namespace: serviceNamespace, + }, + Spec: argoapp.ArgoCDSpec{ + NodePlacement: directNodePlacement, + Server: argoapp.ArgoCDServerSpec{ + Route: argoapp.ArgoCDRouteSpec{ + Enabled: true, + }, + }, + ApplicationSet: &argoapp.ArgoCDApplicationSet{}, + SSO: &argoapp.ArgoCDSSOSpec{ + Provider: "dex", + Dex: &argoapp.ArgoCDDexSpec{ + Config: "test-config", + }, + }, + }, + } + + err := fakeClient.Create(context.TODO(), existingArgoCD) + assertNoError(t, err) + + _, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test")) + assertNoError(t, err) + + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: common.ArgoCDInstanceName, Namespace: serviceNamespace}, + existingArgoCD) + assertNoError(t, err) + assert.DeepEqual(t, existingArgoCD.Spec.NodePlacement, directNodePlacement) + _, hasManagedAnnotation := existingArgoCD.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation] + assert.Assert(t, !hasManagedAnnotation) +} + +func TestReconcileDefaultArgoCDNodePlacementClearsWhenGitopsServiceCleared(t *testing.T) { + logf.SetLogger(argocd.ZapLogger(true)) + s := scheme.Scheme + addKnownTypesToScheme(s) + + gitopsService := &pipelinesv1alpha1.GitopsService{ + ObjectMeta: v1.ObjectMeta{ + Name: serviceName, + }, + Spec: pipelinesv1alpha1.GitopsServiceSpec{ + NodeSelector: map[string]string{ + "key1": "value1", + }, + }, + } + + fakeClient := fake.NewFakeClient(gitopsService) + reconciler := newReconcileGitOpsService(fakeClient, s) + + existingArgoCD := &argoapp.ArgoCD{ + ObjectMeta: v1.ObjectMeta{ + Name: serviceNamespace, + Namespace: serviceNamespace, + }, + Spec: argoapp.ArgoCDSpec{ + Server: argoapp.ArgoCDServerSpec{ + Route: argoapp.ArgoCDRouteSpec{ + Enabled: true, + }, + }, + ApplicationSet: &argoapp.ArgoCDApplicationSet{}, + SSO: &argoapp.ArgoCDSSOSpec{ + Provider: "dex", + Dex: &argoapp.ArgoCDDexSpec{ + Config: "test-config", + }, + }, + }, + } + + err := fakeClient.Create(context.TODO(), existingArgoCD) + assertNoError(t, err) + + _, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test")) + assertNoError(t, err) + + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceName}, gitopsService) + assertNoError(t, err) + gitopsService.Spec.NodeSelector = nil + err = fakeClient.Update(context.TODO(), gitopsService) + assertNoError(t, err) + + _, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test")) + assertNoError(t, err) + + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: common.ArgoCDInstanceName, Namespace: serviceNamespace}, + existingArgoCD) + assertNoError(t, err) + assert.Assert(t, existingArgoCD.Spec.NodePlacement == nil) + _, hasManagedAnnotation := existingArgoCD.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation] + assert.Assert(t, !hasManagedAnnotation) +} + +func TestReconcileDefaultArgoCDNodePlacementClearsWhenCreatedByReconcile(t *testing.T) { + logf.SetLogger(argocd.ZapLogger(true)) + s := scheme.Scheme + addKnownTypesToScheme(s) + + gitopsService := &pipelinesv1alpha1.GitopsService{ + ObjectMeta: v1.ObjectMeta{ + Name: serviceName, + }, + Spec: pipelinesv1alpha1.GitopsServiceSpec{ + NodeSelector: map[string]string{ + "key1": "value1", + }, + }, + } + + fakeClient := fake.NewFakeClient(gitopsService) + reconciler := newReconcileGitOpsService(fakeClient, s) + + _, err := reconciler.Reconcile(context.TODO(), newRequest("test", "test")) + assertNoError(t, err) + + existingArgoCD := &argoapp.ArgoCD{} + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: common.ArgoCDInstanceName, Namespace: serviceNamespace}, + existingArgoCD) + assertNoError(t, err) + assert.Check(t, existingArgoCD.Spec.NodePlacement != nil) + assert.Equal(t, existingArgoCD.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation], "true") + + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: serviceName}, gitopsService) + assertNoError(t, err) + gitopsService.Spec.NodeSelector = nil + err = fakeClient.Update(context.TODO(), gitopsService) + assertNoError(t, err) + + _, err = reconciler.Reconcile(context.TODO(), newRequest("test", "test")) + assertNoError(t, err) + + err = fakeClient.Get(context.TODO(), types.NamespacedName{Name: common.ArgoCDInstanceName, Namespace: serviceNamespace}, + existingArgoCD) + assertNoError(t, err) + assert.Assert(t, existingArgoCD.Spec.NodePlacement == nil) + _, hasManagedAnnotation := existingArgoCD.Annotations[common.NodePlacementManagedByGitopsServiceAnnotation] + assert.Assert(t, !hasManagedAnnotation) } // If the DISABLE_DEFAULT_ARGOCD_INSTANCE is set, ensure that the default ArgoCD instance is not created.