fix(promoter): mount writable temporary storage for the controller by jsell-rh · Pull Request #1302 · redhat-developer/gitops-operator

jsell-rh · 2026-09-17T14:57:52Z

What type of PR is this?

/kind bug

What does this PR do / why we need it:

The operator sets readOnlyRootFilesystem: true on the GitOps Promoter controller but does not mount writable temporary storage. On OpenShift GitOps 1.22 RC2, the controller starts and passes its probes, but a ChangeTransferPolicy fails when it tries to clone its Git repository:

failed to clone repo: failed to create temp directory: mkdir /tmp/…: read-only file system

Add an emptyDir volume mounted at /tmp to the controller Deployment configuration. This provides space for Git clones and temporary index files while retaining the read-only root filesystem. The existing reconciliation code also adds the volume and mount to Deployments created before this fix and preserves them on later reconciliations.

Have you updated the necessary documentation?

No documentation update is required. This restores Git operations without changing the ArgoCD API or configuration requirements.

Which issue(s) this PR fixes:

Observed on an OpenShift GitOps 1.22 RC2 installation. No issue has been filed.

Test acceptance criteria:

Unit Test
E2E Test

How to test changes / Special notes to the reviewer:

cd argocd-operator
go test ./controllers/gitopspromoter -count=1

The package tests pass. The existing TestReconcilePromoterControllerDeployment_PromoterEnabled test retains its structure and adds assertions for the controller volumes and volume mounts. The volume and mount are built by buildControllerVolumes() and buildControllerVolumeMounts(), consistent with the API-server configuration.

For a cluster test, enable Promoter and create a GitRepository and PromotionStrategy. Verify that the controller pod mounts an emptyDir at /tmp and that repository cloning proceeds without the read-only filesystem error. This patch has not yet been tested as a built operator image in a cluster.

Signed-off-by: John Sell <jsell@redhat.com>

openshift-ci · 2026-09-17T14:58:00Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jgwest for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

argocd-operator/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2026-09-17T14:58:06Z

Hi @jsell-rh. Thanks for your PR.

I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

coderabbitai · 2026-09-17T14:58:39Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: bc7c5102-8e4b-4838-9fb2-4e8f5c047c55

📥 Commits

Reviewing files that changed from the base of the PR and between f753476 and 65fbf2f.

📒 Files selected for processing (1)

argocd-operator/controllers/gitopspromoter/deployment_test.go

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

argoproj-labs/argocd-operator (manual)

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

Tests
- Expanded validation of GitOps Promoter controller deployments across creation and upgrade scenarios.
- Confirmed consistent security settings, health checks, command configuration, and temporary storage at /tmp.
Refactor
- Improved deployment configuration organization while preserving existing deployment behavior and runtime settings.

Walkthrough

The Controller deployment now builds its tmp emptyDir volume and /tmp mount through helper functions. Table-driven tests verify the configuration during initial creation and upgrade reconciliation.

Changes

Controller temporary volume

Layer / File(s)	Summary
Volume helpers and reconciliation coverage `argocd-operator/controllers/gitopspromoter/deployment.go`, `argocd-operator/controllers/gitopspromoter/deployment_test.go`	`createControllerConfig` delegates the `tmp` volume and `/tmp` mount to helper functions. Tests verify deployment fields, the read-only root filesystem, and the temporary volume during create and upgrade reconciliation.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 65fbf

The change adds the required writable temporary storage without an identified merge-blocking regression.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main change: mounting writable temporary storage for the GitOps Promoter controller.
Description check	✅ Passed	The description explains the read-only filesystem failure, the /tmp emptyDir mount, reconciliation behavior, and test coverage.

Fix all pre-merge checks with AI

_{Comment @coderabbitai help to get the list of available commands.}

cjcocokrisp

Overall LGTM just a few small comments.

cjcocokrisp · 2026-09-17T15:51:10Z

 	assert.Equal(t, cfg.readinessProbe, retrievedDeployment.Spec.Template.Spec.Containers[0].ReadinessProbe)
 }

+func TestReconcilePromoterControllerDeployment_WritableTmp(t *testing.T) {


The check to see if the volume and volume mounts are there should be added to the existing tests that check if the deployment exists instead of making a new test IMO.

cjcocokrisp · 2026-09-17T15:51:14Z

 		securityContext: buildControllerSecurityContext(),
 		livenessProbe:   buildControllerLivenessProbe(),
 		readinessProbe:  buildControllerReadinessProbe(),
+		// Git clones and temporary index files need writable storage.


Can this be moved to a function for readability.

See buildAPIServerVolumeMounts() and buildAPIServerVolumes().

cjcocokrisp · 2026-09-17T16:11:45Z

-	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)
+	for _, tt := range []struct {


I was thinking something more of just adding something like this. No need to change the whole test structure.

assert.Equal(t, cfg.volumeMounts, retrievedDeployment.Spec.Template.Spec.Containers[0].VolumeMounts) assert.Equal(t, cfg.volumes, retrievedDeployment.Spec.Template.Spec.Volumes)

cjcocokrisp

One more small comment.

cjcocokrisp

LGTM!

cjcocokrisp · 2026-09-21T13:20:25Z

@jsell-rh Can you please fix the DCO sign off issues?

olivergondza · 2026-09-21T14:00:58Z

/ok-to-test
/lgtm

cjcocokrisp · 2026-09-21T17:28:27Z

/retest

jgwest

LGTM

jgwest · 2026-09-22T00:44:27Z

  [FAIL] GitOps Operator Sequential E2E Tests 1-132_validate_sensitive_annotation_masking_test [It] verifies that resource.sensitive.mask.annotations is set in argocd-cm and that the openshift.io/token-secret.value annotation is hidden from diff computation so the app stays Synced and the token is never visible in CLI output
  /go/src/github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/sequential/1-132_validate_sensitive_annotation_masking_test.go:163



Summarizing 1 Failure:
  [FAIL] GitOps Operator Sequential E2E Tests 1-124_validate_console_plugin_in_operator_namespace [It] cleans up plugin resources from the old namespace when they exist (simulating upgrade from older version)
  /go/src/github.com/redhat-developer/gitops-operator/test/openshift/e2e/ginkgo/sequential/1-124_validate_console_plugin_in_operator_namespace_test.go:160

/retest

jgwest · 2026-09-22T01:58:17Z

@jsell-rh - DCO check is failing:

This repo requires commits to be commited via git commit -s, producing Signed-off-by: John Sell <jsell@redhat.com> in the commit log.
It looks like you did this with the first commit, but not the subsequent commits:

commit 65fbf2ff42d13e3b429653c3266fb1aa8576b3da (HEAD -> fix/promoter-writable-tmp, origin/fix/promoter-writable-tmp)
Author: John Sell <jsell@redhat.com>
Date:   Thu Sep 17 12:12:44 2026 -0400

    test(promoter): keep the existing deployment test structure

commit f753476ee5b56d2604e4b13a2a8b9f7f3d358751
Author: John Sell <jsell@redhat.com>
Date:   Thu Sep 17 12:02:27 2026 -0400

    refactor(promoter): address temporary storage review comments

commit d6e856f057f7c87298865387a30ef1b6635f809b
Author: John Sell <jsell@redhat.com>
Date:   Thu Sep 17 10:57:43 2026 -0400

    fix(promoter): mount writable temporary storage for the controller
    
    Signed-off-by: John Sell <jsell@redhat.com>

openshift-ci · 2026-09-22T02:23:11Z

@jsell-rh: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/v4.14-kuttl-sequential	`65fbf2f`	link	false	`/test v4.14-kuttl-sequential`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

fix(promoter): mount writable temporary storage for the controller

d6e856f

Signed-off-by: John Sell <jsell@redhat.com>

openshift-ci Bot added the kind/bug Something isn't working label Sep 17, 2026

openshift-ci Bot requested review from Rizwana777 and jparsai September 17, 2026 14:57

openshift-ci Bot added the needs-ok-to-test label Sep 17, 2026

cjcocokrisp suggested changes Sep 17, 2026

View reviewed changes

refactor(promoter): address temporary storage review comments

f753476

cjcocokrisp reviewed Sep 17, 2026

View reviewed changes

test(promoter): keep the existing deployment test structure

65fbf2f

cjcocokrisp approved these changes Sep 17, 2026

View reviewed changes

openshift-ci Bot added the ok-to-test label Sep 21, 2026

openshift-ci Bot assigned olivergondza Sep 21, 2026

openshift-ci Bot added lgtm and removed needs-ok-to-test labels Sep 21, 2026

jgwest approved these changes Sep 21, 2026

View reviewed changes

cjcocokrisp mentioned this pull request Sep 22, 2026

fix: tmp not writable for promoter controller deployment argoproj-labs/argocd-operator#2395

Merged

2 tasks

Conversation

jsell-rh commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci Bot commented Sep 17, 2026

Uh oh!

openshift-ci Bot commented Sep 17, 2026

Uh oh!

coderabbitai Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

cjcocokrisp left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjcocokrisp Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjcocokrisp Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjcocokrisp Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

cjcocokrisp left a comment

Choose a reason for hiding this comment

Uh oh!

cjcocokrisp left a comment

Choose a reason for hiding this comment

Uh oh!

cjcocokrisp commented Sep 21, 2026

Uh oh!

olivergondza commented Sep 21, 2026

Uh oh!

cjcocokrisp commented Sep 21, 2026

Uh oh!

jgwest left a comment

Choose a reason for hiding this comment

Uh oh!

jgwest commented Sep 22, 2026

Uh oh!

jgwest commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jsell-rh commented Sep 17, 2026 •

edited

Loading

coderabbitai Bot commented Sep 17, 2026 •

edited

Loading

cjcocokrisp left a comment •

edited

Loading

cjcocokrisp Sep 17, 2026 •

edited

Loading

cjcocokrisp Sep 17, 2026 •

edited

Loading

jgwest commented Sep 22, 2026 •

edited

Loading

openshift-ci Bot commented Sep 22, 2026 •

edited

Loading