Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Allow override of default /tmp volume mount in repo server #1459

Conversation

jgwest
Copy link
Member

@jgwest jgwest commented Jul 16, 2024

What type of PR is this?
/kind bug

What does this PR do / why we need it:

The ArgoCD HA guidelines [1] says:

argocd-repo-server clones the repository into /tmp (or the path specified in the TMPDIR env variable). The Pod might run out of disk space if it has too many repositories or if the repositories have a lot of files. To avoid this problem mount a persistent volume.

However, with the current behaviour of argocd-operator, it is not possible to override the tmp path: when you attempt to do so, you will see this error in the operator logs:

2024-07-16T07:32:47Z ERROR Reconciler error {"controller": "argocd", "controllerGroup": "argoproj.io", "controllerKind": "ArgoCD", "ArgoCD": {"name":"argocd","namespace":"jgw"}, "namespace": "jgw", "name": "argocd", "reconcileID": "6979a388-fdae-453d-b999-b026aa1e260b", "error": "Deployment.apps \"argocd-repo-server\" is invalid: spec.template.spec.containers[0].volumeMounts[8].mountPath: Invalid value: \"/tmp\": must be unique"}
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
/remote-source/deps/gomod/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:329
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
/remote-source/deps/gomod/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
/remote-source/deps/gomod/pkg/mod/sigs.k8s.io/controller-runtime@v0.17.2/pkg/internal/controller/controller.go:227
W0716 07:33:31.475998 1 warnings.go:70] apps.openshift.io/v1 DeploymentConfig is deprecated in v4.14+, unavailable in v4.10000+

using an ArgoCD CR like this:

kind: ArgoCD
# (...)
spec:
  # (...)
  repo:
    resources:
      limits:
        cpu: '1'
        memory: 1Gi
      requests:
        cpu: 250m
        memory: 256Mi
    volumeMounts:
      - mountPath: /tmp
        name: my-volume
    volumes:
      - emptyDir: {} # in practice, a user would likely use a persistent volume for increased storage capacity
        name: my-volume

This PR skips creation of the default /tmp mount path in the case where the user is already providing one via .spec.repo.volumeMounts in the ArgoCD CR.

Have you updated the necessary documentation?

  • Documentation update is required by this PR.
  • Documentation has been updated.

Which issue(s) this PR fixes:
N/A

How to test changes / Special notes to the reviewer:

@jgwest jgwest changed the title Allow override of default /tmp volume mount in repo server fix: Allow override of default /tmp volume mount in repo server Jul 16, 2024
@jgwest
Copy link
Member Author

jgwest commented Jul 16, 2024

Red Hat tracking JIRA: https://issues.redhat.com/browse/GITOPS-4602

@jgwest
Copy link
Member Author

jgwest commented Jul 16, 2024

Failed on:
--- FAIL: kuttl/harness/1-036_validate_role_rolebinding_for_source_namespace (63.91s)

Copy link
Collaborator

@svghadi svghadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @jgwest. I have same exact fix for one the issue I am working on 😆. If you can rebase we can get this in.

internal/controller/argocd/deployment.go Outdated Show resolved Hide resolved
@jgwest jgwest force-pushed the allow-override-of-default-tmp-repo-server-volume-july-2024 branch from b28d9dc to f5a7b9f Compare July 17, 2024 07:41
Signed-off-by: Jonathan West <jonwest@redhat.com>
@jgwest jgwest force-pushed the allow-override-of-default-tmp-repo-server-volume-july-2024 branch from f5a7b9f to c06f8fe Compare July 17, 2024 13:11
Copy link
Collaborator

@svghadi svghadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Will merge it.

@svghadi svghadi merged commit a9218b8 into argoproj-labs:master Jul 17, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants