diff --git a/controllers/argocd/deployment.go b/controllers/argocd/deployment.go index 71f031f3f..52ba1f800 100644 --- a/controllers/argocd/deployment.go +++ b/controllers/argocd/deployment.go @@ -834,43 +834,56 @@ func (r *ReconcileArgoCD) reconcileRedisHAProxyDeployment(cr *argoproj.ArgoCD) e changed = true } updateNodePlacement(existing, deploy, &changed, &explanation) - - if !reflect.DeepEqual(deploy.Spec.Template.Spec.Containers[0].Resources, existing.Spec.Template.Spec.Containers[0].Resources) { - existing.Spec.Template.Spec.Containers[0].Resources = deploy.Spec.Template.Spec.Containers[0].Resources + if !reflect.DeepEqual(deploy.Spec.Template.Spec.Volumes, existing.Spec.Template.Spec.Volumes) { + existing.Spec.Template.Spec.Volumes = deploy.Spec.Template.Spec.Volumes if changed { explanation += ", " } - explanation += "container resources" + explanation += "volumes" changed = true } - - if !reflect.DeepEqual(deploy.Spec.Template.Spec.Containers[0].SecurityContext, existing.Spec.Template.Spec.Containers[0].SecurityContext) { - existing.Spec.Template.Spec.Containers[0].SecurityContext = deploy.Spec.Template.Spec.Containers[0].SecurityContext + if !reflect.DeepEqual(deploy.Spec.Template.Spec.Containers[0].VolumeMounts, + existing.Spec.Template.Spec.Containers[0].VolumeMounts) { + existing.Spec.Template.Spec.Containers[0].VolumeMounts = deploy.Spec.Template.Spec.Containers[0].VolumeMounts if changed { explanation += ", " } - explanation += "container security context" + explanation += "container volume mounts" changed = true } - - if !reflect.DeepEqual(deploy.Spec.Template.Spec.InitContainers[0].Resources, existing.Spec.Template.Spec.InitContainers[0].Resources) { - existing.Spec.Template.Spec.InitContainers[0].Resources = deploy.Spec.Template.Spec.InitContainers[0].Resources + if !reflect.DeepEqual(deploy.Spec.Template.Spec.InitContainers, existing.Spec.Template.Spec.InitContainers) { + existing.Spec.Template.Spec.InitContainers = deploy.Spec.Template.Spec.InitContainers if changed { explanation += ", " } - explanation += "init container resources" + explanation += "init containers" changed = true } - - if !reflect.DeepEqual(deploy.Spec.Template.Spec.InitContainers[0].SecurityContext, existing.Spec.Template.Spec.InitContainers[0].SecurityContext) { - existing.Spec.Template.Spec.InitContainers[0].SecurityContext = deploy.Spec.Template.Spec.InitContainers[0].SecurityContext + if !reflect.DeepEqual(deploy.Spec.Template.Spec.Containers[0].Env, + existing.Spec.Template.Spec.Containers[0].Env) { + existing.Spec.Template.Spec.Containers[0].Env = deploy.Spec.Template.Spec.Containers[0].Env if changed { explanation += ", " } - explanation += "init container security context" + explanation += "container env" + changed = true + } + if !reflect.DeepEqual(deploy.Spec.Template.Spec.Containers[0].Resources, existing.Spec.Template.Spec.Containers[0].Resources) { + existing.Spec.Template.Spec.Containers[0].Resources = deploy.Spec.Template.Spec.Containers[0].Resources + if changed { + explanation += ", " + } + explanation += "container resources" + changed = true + } + if !reflect.DeepEqual(deploy.Spec.Template.Spec.Containers[0].SecurityContext, existing.Spec.Template.Spec.Containers[0].SecurityContext) { + existing.Spec.Template.Spec.Containers[0].SecurityContext = deploy.Spec.Template.Spec.Containers[0].SecurityContext + if changed { + explanation += ", " + } + explanation += "container security context" changed = true } - if changed { argoutil.LogResourceUpdate(log, existing, "updating", explanation) return r.Client.Update(context.TODO(), existing) diff --git a/controllers/argocd/deployment_test.go b/controllers/argocd/deployment_test.go index aa117cd99..f2a83a3e5 100644 --- a/controllers/argocd/deployment_test.go +++ b/controllers/argocd/deployment_test.go @@ -887,6 +887,118 @@ func TestReconcileArgoCD_reconcileDeployments_HA_proxy_with_resources(t *testing assert.Equal(t, deployment.Spec.Template.Spec.Containers[0].Resources, newResources) assert.Equal(t, deployment.Spec.Template.Spec.InitContainers[0].Resources, newResources) } +func TestReconcileArgoCD_reconcileRedisHAProxyDeployment_ModifyContainerSpec(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + + a := makeTestArgoCD(func(a *argoproj.ArgoCD) { + a.Spec.HA.Enabled = true + }) + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch) + + assert.NoError(t, r.reconcileRedisHAProxyDeployment(a)) + + deployment := &appsv1.Deployment{} + assert.NoError(t, r.Client.Get( + context.TODO(), + types.NamespacedName{ + Name: "argocd-redis-ha-haproxy", + Namespace: a.Namespace, + }, + deployment)) + + // Modify the deployment container environment variables + deployment.Spec.Template.Spec.Containers[0].Env = append(deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "TEST_ENV", + Value: "test", + }) + + assert.NoError(t, r.Client.Update(context.TODO(), deployment)) + + // Reconcile again + assert.NoError(t, r.reconcileRedisHAProxyDeployment(a)) + + // Check if the environment variable changes were reverted + assert.NoError(t, r.Client.Get( + context.TODO(), + types.NamespacedName{ + Name: "argocd-redis-ha-haproxy", + Namespace: a.Namespace, + }, + deployment)) + + assert.NotContains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "TEST_ENV", + Value: "test", + }) + + // Modify the deployment initcontainer environment variables + deployment.Spec.Template.Spec.InitContainers[0].Env = append(deployment.Spec.Template.Spec.InitContainers[0].Env, corev1.EnvVar{ + Name: "TEST_ENV", + Value: "test", + }) + + assert.NoError(t, r.Client.Update(context.TODO(), deployment)) + + // Reconcile again + assert.NoError(t, r.reconcileRedisHAProxyDeployment(a)) + + // Check if the environment variable changes were reverted + assert.NoError(t, r.Client.Get( + context.TODO(), + types.NamespacedName{ + Name: "argocd-redis-ha-haproxy", + Namespace: a.Namespace, + }, + deployment)) + + assert.NotContains(t, deployment.Spec.Template.Spec.InitContainers[0].Env, corev1.EnvVar{ + Name: "TEST_ENV", + Value: "test", + }) + + // Modify the deployment volumes + deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, corev1.Volume{ + Name: "test-volume", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }) + deployment.Spec.Template.Spec.Containers[0].VolumeMounts = append(deployment.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "test-volume", + MountPath: "/test", + }) + + assert.NoError(t, r.Client.Update(context.TODO(), deployment)) + + // Reconcile again + assert.NoError(t, r.reconcileRedisHAProxyDeployment(a)) + + // Check if the volume changes were reverted + assert.NoError(t, r.Client.Get( + context.TODO(), + types.NamespacedName{ + Name: "argocd-redis-ha-haproxy", + Namespace: a.Namespace, + }, + deployment)) + + assert.NotContains(t, deployment.Spec.Template.Spec.Volumes, corev1.Volume{ + Name: "test-volume", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }) + assert.NotContains(t, deployment.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "test-volume", + MountPath: "/test", + }) +} func TestReconcileArgoCD_reconcileRepoDeployment_updatesVolumeMounts(t *testing.T) { logf.SetLogger(ZapLogger(true)) diff --git a/controllers/argocd/statefulset.go b/controllers/argocd/statefulset.go index 205b57ba1..b70218c53 100644 --- a/controllers/argocd/statefulset.go +++ b/controllers/argocd/statefulset.go @@ -452,6 +452,14 @@ func (r *ReconcileArgoCD) reconcileRedisStatefulSet(cr *argoproj.ArgoCD) error { explanation += fmt.Sprintf("container '%s' image", container.Name) changed = true } + if !reflect.DeepEqual(ss.Spec.Template.Spec.Containers[i].VolumeMounts, existing.Spec.Template.Spec.Containers[i].VolumeMounts) { + existing.Spec.Template.Spec.Containers[i].VolumeMounts = ss.Spec.Template.Spec.Containers[i].VolumeMounts + if changed { + explanation += ", " + } + explanation += fmt.Sprintf("container '%s' VolumeMounts", container.Name) + changed = true + } if !reflect.DeepEqual(ss.Spec.Template.Spec.Containers[i].Resources, existing.Spec.Template.Spec.Containers[i].Resources) { existing.Spec.Template.Spec.Containers[i].Resources = ss.Spec.Template.Spec.Containers[i].Resources @@ -473,33 +481,29 @@ func (r *ReconcileArgoCD) reconcileRedisStatefulSet(cr *argoproj.ArgoCD) error { if !reflect.DeepEqual(ss.Spec.Template.Spec.Containers[i].Env, existing.Spec.Template.Spec.Containers[i].Env) { existing.Spec.Template.Spec.Containers[i].Env = ss.Spec.Template.Spec.Containers[i].Env + if changed { + explanation += ", " + } + explanation += fmt.Sprintf("container '%s' env", container.Name) changed = true } } - - if !reflect.DeepEqual(ss.Spec.Template.Spec.InitContainers[0].Resources, existing.Spec.Template.Spec.InitContainers[0].Resources) { - existing.Spec.Template.Spec.InitContainers[0].Resources = ss.Spec.Template.Spec.InitContainers[0].Resources + if !reflect.DeepEqual(ss.Spec.Template.Spec.Volumes, existing.Spec.Template.Spec.Volumes) { + existing.Spec.Template.Spec.Volumes = ss.Spec.Template.Spec.Volumes if changed { explanation += ", " } - explanation += fmt.Sprintf("init container '%s' resources", existing.Spec.Template.Spec.InitContainers[0].Name) + explanation += "volumes" changed = true } - - if !reflect.DeepEqual(ss.Spec.Template.Spec.InitContainers[0].SecurityContext, existing.Spec.Template.Spec.InitContainers[0].SecurityContext) { - existing.Spec.Template.Spec.InitContainers[0].SecurityContext = ss.Spec.Template.Spec.InitContainers[0].SecurityContext + if !reflect.DeepEqual(ss.Spec.Template.Spec.InitContainers, existing.Spec.Template.Spec.InitContainers) { + existing.Spec.Template.Spec.InitContainers = ss.Spec.Template.Spec.InitContainers if changed { explanation += ", " } - explanation += fmt.Sprintf("init container '%s' security context", existing.Spec.Template.Spec.InitContainers[0].Name) - changed = true - } - - if !reflect.DeepEqual(ss.Spec.Template.Spec.InitContainers[0].Env, existing.Spec.Template.Spec.InitContainers[0].Env) { - existing.Spec.Template.Spec.InitContainers[0].Env = ss.Spec.Template.Spec.InitContainers[0].Env + explanation += "init containers" changed = true } - if changed { argoutil.LogResourceUpdate(log, existing, "updating", explanation) return r.Client.Update(context.TODO(), existing) diff --git a/controllers/argocd/statefulset_test.go b/controllers/argocd/statefulset_test.go index 58b2b4426..e80502f07 100644 --- a/controllers/argocd/statefulset_test.go +++ b/controllers/argocd/statefulset_test.go @@ -836,3 +836,96 @@ func TestReconcileArgoCD_sidecarcontainer(t *testing.T) { assert.Equal(t, 1, len(ss.Spec.Template.Spec.Containers)) } +func TestReconcileArgoCD_reconcileRedisStatefulSet_ModifyContainerSpec(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + + a := makeTestArgoCD() + a.Spec.HA.Enabled = true + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch) + + // Initial reconciliation to create the StatefulSet + assert.NoError(t, r.reconcileRedisStatefulSet(a)) + + s := newStatefulSetWithSuffix("redis-ha-server", "redis", a) + assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: s.Name, Namespace: a.Namespace}, s)) + + // Modify the container environment variable + s.Spec.Template.Spec.Containers[0].Env = append(s.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{ + Name: "NEW_ENV_VAR", + Value: "new-value", + }) + assert.NoError(t, r.Client.Update(context.TODO(), s)) + + // Reconcile again and check if the environment variable is reverted + assert.NoError(t, r.reconcileRedisStatefulSet(a)) + assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: s.Name, Namespace: a.Namespace}, s)) + + envVarFound := false + for _, env := range s.Spec.Template.Spec.Containers[0].Env { + if env.Name == "NEW_ENV_VAR" { + envVarFound = true + break + } + } + assert.False(t, envVarFound, "NEW_ENV_VAR should not be present") + + // Modify the initcontainer environment variable + s.Spec.Template.Spec.Containers[0].Env = append(s.Spec.Template.Spec.InitContainers[0].Env, corev1.EnvVar{ + Name: "NEW_ENV_VAR", + Value: "new-value", + }) + assert.NoError(t, r.Client.Update(context.TODO(), s)) + + // Reconcile again and check if the environment variable is reverted + assert.NoError(t, r.reconcileRedisStatefulSet(a)) + assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: s.Name, Namespace: a.Namespace}, s)) + + envVarFound = false + for _, env := range s.Spec.Template.Spec.InitContainers[0].Env { + if env.Name == "NEW_ENV_VAR" { + envVarFound = true + break + } + } + assert.False(t, envVarFound, "NEW_ENV_VAR should not be present") + + // Modify the container volume and volume mount + s.Spec.Template.Spec.Containers[0].VolumeMounts = append(s.Spec.Template.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{ + Name: "new-volume", + MountPath: "/new/path", + }) + s.Spec.Template.Spec.Volumes = append(s.Spec.Template.Spec.Volumes, corev1.Volume{ + Name: "new-volume", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }) + assert.NoError(t, r.Client.Update(context.TODO(), s)) + // Reconcile again and check if the volume mount is reverted + assert.NoError(t, r.reconcileRedisStatefulSet(a)) + assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: s.Name, Namespace: a.Namespace}, s)) + + volumeMountFound := false + for _, vm := range s.Spec.Template.Spec.Containers[0].VolumeMounts { + if vm.Name == "new-volume" { + volumeMountFound = true + break + } + } + assert.False(t, volumeMountFound, "new-volume should not be present in volume mounts") + + volumeFound := false + for _, v := range s.Spec.Template.Spec.Volumes { + if v.Name == "new-volume" { + volumeFound = true + break + } + } + assert.False(t, volumeFound, "new-volume should not be present in volumes") +}