From d6e856f057f7c87298865387a30ef1b6635f809b Mon Sep 17 00:00:00 2001 From: John Sell Date: Thu, 17 Sep 2026 10:57:43 -0400 Subject: [PATCH 1/3] fix(promoter): mount writable temporary storage for the controller Signed-off-by: John Sell --- .../controllers/gitopspromoter/deployment.go | 15 +++++++ .../gitopspromoter/deployment_test.go | 39 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/argocd-operator/controllers/gitopspromoter/deployment.go b/argocd-operator/controllers/gitopspromoter/deployment.go index 8893fd0ed27..2d92742ce48 100644 --- a/argocd-operator/controllers/gitopspromoter/deployment.go +++ b/argocd-operator/controllers/gitopspromoter/deployment.go @@ -68,6 +68,21 @@ func createControllerConfig() deploymentConfig { securityContext: buildControllerSecurityContext(), livenessProbe: buildControllerLivenessProbe(), readinessProbe: buildControllerReadinessProbe(), + // Git clones and temporary index files need writable storage. + volumes: []corev1.Volume{ + { + Name: "tmp", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + }, + volumeMounts: []corev1.VolumeMount{ + { + Name: "tmp", + MountPath: "/tmp", + }, + }, } } diff --git a/argocd-operator/controllers/gitopspromoter/deployment_test.go b/argocd-operator/controllers/gitopspromoter/deployment_test.go index 17fcedf75e2..ca273666c3b 100644 --- a/argocd-operator/controllers/gitopspromoter/deployment_test.go +++ b/argocd-operator/controllers/gitopspromoter/deployment_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -479,6 +480,44 @@ func TestReconcilePromoterControllerDeployment_PromoterEnabled(t *testing.T) { assert.Equal(t, cfg.readinessProbe, retrievedDeployment.Spec.Template.Spec.Containers[0].ReadinessProbe) } +func TestReconcilePromoterControllerDeployment_WritableTmp(t *testing.T) { + for _, existing := range []bool{false, true} { + name := "create" + if existing { + name = "upgrade" + } + t.Run(name, func(t *testing.T) { + cr := makeTestArgoCD(withPromoterEnabled(true)) + sa := makeExistingServiceAccount(cr) + objects := []client.Object{cr} + if existing { + deployment := makeExistingDeployment(sa, cr) + deployment.Spec.Template.Spec.Volumes = nil + deployment.Spec.Template.Spec.Containers[0].VolumeMounts = nil + objects = append(objects, deployment) + } + scheme := makeTestReconcilerScheme() + c := makeTestReconcilerClient(scheme, objects) + + // Check creation or upgrade, then check that another reconciliation preserves the mount. + for range 2 { + deployment, err := ReconcilePromoterControllerDeployment(c, testCompName, sa, cr, scheme) + require.NoError(t, err) + require.NotNil(t, deployment) + retrieved := &appsv1.Deployment{} + require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(deployment), retrieved)) + container := retrieved.Spec.Template.Spec.Containers[0] + assert.Equal(t, ptr.To(true), container.SecurityContext.ReadOnlyRootFilesystem) + assert.Equal(t, []corev1.Volume{{ + Name: "tmp", + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + }}, retrieved.Spec.Template.Spec.Volumes) + assert.Equal(t, []corev1.VolumeMount{{Name: "tmp", MountPath: "/tmp"}}, container.VolumeMounts) + } + }) + } +} + func TestReconcilePromoterAPIServerDeployment_PromoterDisabled(t *testing.T) { // Test Case: Promoter is disabled and API Server Deployment does not exist // Expected: API Server Deployment should not be created From f753476ee5b56d2604e4b13a2a8b9f7f3d358751 Mon Sep 17 00:00:00 2001 From: John Sell Date: Thu, 17 Sep 2026 12:02:27 -0400 Subject: [PATCH 2/3] refactor(promoter): address temporary storage review comments --- .../controllers/gitopspromoter/deployment.go | 39 +++++++----- .../gitopspromoter/deployment_test.go | 61 ++++++------------- 2 files changed, 43 insertions(+), 57 deletions(-) diff --git a/argocd-operator/controllers/gitopspromoter/deployment.go b/argocd-operator/controllers/gitopspromoter/deployment.go index 2d92742ce48..20e56706910 100644 --- a/argocd-operator/controllers/gitopspromoter/deployment.go +++ b/argocd-operator/controllers/gitopspromoter/deployment.go @@ -68,21 +68,8 @@ func createControllerConfig() deploymentConfig { securityContext: buildControllerSecurityContext(), livenessProbe: buildControllerLivenessProbe(), readinessProbe: buildControllerReadinessProbe(), - // Git clones and temporary index files need writable storage. - volumes: []corev1.Volume{ - { - Name: "tmp", - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, - }, - }, - volumeMounts: []corev1.VolumeMount{ - { - Name: "tmp", - MountPath: "/tmp", - }, - }, + volumes: buildControllerVolumes(), + volumeMounts: buildControllerVolumeMounts(), } } @@ -460,6 +447,28 @@ func buildAPIServerArgs() []string { } } +// buildControllerVolumes provides writable storage for Git clones and temporary index files. +func buildControllerVolumes() []corev1.Volume { + return []corev1.Volume{ + { + Name: "tmp", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + } +} + +// buildControllerVolumeMounts mounts the controller's writable temporary storage. +func buildControllerVolumeMounts() []corev1.VolumeMount { + return []corev1.VolumeMount{ + { + Name: "tmp", + MountPath: "/tmp", + }, + } +} + // buildAPIServerVolumes builds the Volumes for the API Server func buildAPIServerVolumes(cr *argoproj.ArgoCD) []corev1.Volume { return []corev1.Volume{ diff --git a/argocd-operator/controllers/gitopspromoter/deployment_test.go b/argocd-operator/controllers/gitopspromoter/deployment_test.go index ca273666c3b..41e7da1bcb4 100644 --- a/argocd-operator/controllers/gitopspromoter/deployment_test.go +++ b/argocd-operator/controllers/gitopspromoter/deployment_test.go @@ -447,50 +447,18 @@ func TestReconcilePromoterControllerDeployment_PromoterDisabled(t *testing.T) { } func TestReconcilePromoterControllerDeployment_PromoterEnabled(t *testing.T) { - // Test Case: Promoter is enabled and Controller Deployment does not exist - // Expected: Controller Deployment should be created - - cr := makeTestArgoCD(withPromoterEnabled(true)) - - sa := makeExistingServiceAccount(cr) - - resObjs := []client.Object{cr} - sch := makeTestReconcilerScheme() - client := makeTestReconcilerClient(sch, resObjs) - - deployment, err := ReconcilePromoterControllerDeployment(client, testCompName, sa, cr, sch) - assert.NoError(t, err) - assert.NotNil(t, deployment) - - retrievedDeployment := &appsv1.Deployment{} - err = client.Get(context.Background(), types.NamespacedName{ - Name: deployment.Name, - Namespace: cr.Namespace, - }, retrievedDeployment) - assert.NoError(t, err) - - assert.Equal(t, deployment.Name, retrievedDeployment.Name) - assert.Equal(t, cr.Namespace, retrievedDeployment.Namespace) - assert.Equal(t, buildLabelsForPromoterResources(testCompName, cr), retrievedDeployment.Labels) - - cfg := createControllerConfig() - assert.Equal(t, cfg.command, retrievedDeployment.Spec.Template.Spec.Containers[0].Command) - assert.Equal(t, cfg.securityContext, retrievedDeployment.Spec.Template.Spec.Containers[0].SecurityContext) - assert.Equal(t, cfg.livenessProbe, retrievedDeployment.Spec.Template.Spec.Containers[0].LivenessProbe) - assert.Equal(t, cfg.readinessProbe, retrievedDeployment.Spec.Template.Spec.Containers[0].ReadinessProbe) -} - -func TestReconcilePromoterControllerDeployment_WritableTmp(t *testing.T) { - for _, existing := range []bool{false, true} { - name := "create" - if existing { - name = "upgrade" - } - t.Run(name, func(t *testing.T) { + for _, tt := range []struct { + name string + existing bool + }{ + {name: "create"}, + {name: "upgrade", existing: true}, + } { + t.Run(tt.name, func(t *testing.T) { cr := makeTestArgoCD(withPromoterEnabled(true)) sa := makeExistingServiceAccount(cr) objects := []client.Object{cr} - if existing { + if tt.existing { deployment := makeExistingDeployment(sa, cr) deployment.Spec.Template.Spec.Volumes = nil deployment.Spec.Template.Spec.Containers[0].VolumeMounts = nil @@ -499,14 +467,23 @@ func TestReconcilePromoterControllerDeployment_WritableTmp(t *testing.T) { scheme := makeTestReconcilerScheme() c := makeTestReconcilerClient(scheme, objects) - // Check creation or upgrade, then check that another reconciliation preserves the mount. + // Check creation or upgrade, then check that reconciliation preserves the configuration. for range 2 { deployment, err := ReconcilePromoterControllerDeployment(c, testCompName, sa, cr, scheme) require.NoError(t, err) require.NotNil(t, deployment) retrieved := &appsv1.Deployment{} require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(deployment), retrieved)) + assert.Equal(t, deployment.Name, retrieved.Name) + assert.Equal(t, cr.Namespace, retrieved.Namespace) + assert.Equal(t, buildLabelsForPromoterResources(testCompName, cr), retrieved.Labels) + + cfg := createControllerConfig() container := retrieved.Spec.Template.Spec.Containers[0] + assert.Equal(t, cfg.command, container.Command) + assert.Equal(t, cfg.securityContext, container.SecurityContext) + assert.Equal(t, cfg.livenessProbe, container.LivenessProbe) + assert.Equal(t, cfg.readinessProbe, container.ReadinessProbe) assert.Equal(t, ptr.To(true), container.SecurityContext.ReadOnlyRootFilesystem) assert.Equal(t, []corev1.Volume{{ Name: "tmp", From 65fbf2ff42d13e3b429653c3266fb1aa8576b3da Mon Sep 17 00:00:00 2001 From: John Sell Date: Thu, 17 Sep 2026 12:12:44 -0400 Subject: [PATCH 3/3] test(promoter): keep the existing deployment test structure --- .../gitopspromoter/deployment_test.go | 80 ++++++++----------- 1 file changed, 33 insertions(+), 47 deletions(-) diff --git a/argocd-operator/controllers/gitopspromoter/deployment_test.go b/argocd-operator/controllers/gitopspromoter/deployment_test.go index 41e7da1bcb4..a7f5c8283cd 100644 --- a/argocd-operator/controllers/gitopspromoter/deployment_test.go +++ b/argocd-operator/controllers/gitopspromoter/deployment_test.go @@ -20,7 +20,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -447,52 +446,39 @@ func TestReconcilePromoterControllerDeployment_PromoterDisabled(t *testing.T) { } func TestReconcilePromoterControllerDeployment_PromoterEnabled(t *testing.T) { - for _, tt := range []struct { - name string - existing bool - }{ - {name: "create"}, - {name: "upgrade", existing: true}, - } { - t.Run(tt.name, func(t *testing.T) { - cr := makeTestArgoCD(withPromoterEnabled(true)) - sa := makeExistingServiceAccount(cr) - objects := []client.Object{cr} - if tt.existing { - deployment := makeExistingDeployment(sa, cr) - deployment.Spec.Template.Spec.Volumes = nil - deployment.Spec.Template.Spec.Containers[0].VolumeMounts = nil - objects = append(objects, deployment) - } - scheme := makeTestReconcilerScheme() - c := makeTestReconcilerClient(scheme, objects) - - // Check creation or upgrade, then check that reconciliation preserves the configuration. - for range 2 { - deployment, err := ReconcilePromoterControllerDeployment(c, testCompName, sa, cr, scheme) - require.NoError(t, err) - require.NotNil(t, deployment) - retrieved := &appsv1.Deployment{} - require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(deployment), retrieved)) - assert.Equal(t, deployment.Name, retrieved.Name) - assert.Equal(t, cr.Namespace, retrieved.Namespace) - assert.Equal(t, buildLabelsForPromoterResources(testCompName, cr), retrieved.Labels) - - cfg := createControllerConfig() - container := retrieved.Spec.Template.Spec.Containers[0] - assert.Equal(t, cfg.command, container.Command) - assert.Equal(t, cfg.securityContext, container.SecurityContext) - assert.Equal(t, cfg.livenessProbe, container.LivenessProbe) - assert.Equal(t, cfg.readinessProbe, container.ReadinessProbe) - assert.Equal(t, ptr.To(true), container.SecurityContext.ReadOnlyRootFilesystem) - assert.Equal(t, []corev1.Volume{{ - Name: "tmp", - VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, - }}, retrieved.Spec.Template.Spec.Volumes) - assert.Equal(t, []corev1.VolumeMount{{Name: "tmp", MountPath: "/tmp"}}, container.VolumeMounts) - } - }) - } + // Test Case: Promoter is enabled and Controller Deployment does not exist + // Expected: Controller Deployment should be created + + cr := makeTestArgoCD(withPromoterEnabled(true)) + + sa := makeExistingServiceAccount(cr) + + resObjs := []client.Object{cr} + sch := makeTestReconcilerScheme() + client := makeTestReconcilerClient(sch, resObjs) + + deployment, err := ReconcilePromoterControllerDeployment(client, testCompName, sa, cr, sch) + assert.NoError(t, err) + assert.NotNil(t, deployment) + + retrievedDeployment := &appsv1.Deployment{} + err = client.Get(context.Background(), types.NamespacedName{ + Name: deployment.Name, + Namespace: cr.Namespace, + }, retrievedDeployment) + assert.NoError(t, err) + + assert.Equal(t, deployment.Name, retrievedDeployment.Name) + assert.Equal(t, cr.Namespace, retrievedDeployment.Namespace) + assert.Equal(t, buildLabelsForPromoterResources(testCompName, cr), retrievedDeployment.Labels) + + cfg := createControllerConfig() + assert.Equal(t, cfg.command, retrievedDeployment.Spec.Template.Spec.Containers[0].Command) + assert.Equal(t, cfg.securityContext, retrievedDeployment.Spec.Template.Spec.Containers[0].SecurityContext) + assert.Equal(t, cfg.livenessProbe, retrievedDeployment.Spec.Template.Spec.Containers[0].LivenessProbe) + assert.Equal(t, cfg.readinessProbe, retrievedDeployment.Spec.Template.Spec.Containers[0].ReadinessProbe) + assert.Equal(t, cfg.volumeMounts, retrievedDeployment.Spec.Template.Spec.Containers[0].VolumeMounts) + assert.Equal(t, cfg.volumes, retrievedDeployment.Spec.Template.Spec.Volumes) } func TestReconcilePromoterAPIServerDeployment_PromoterDisabled(t *testing.T) {