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

feat(argo-cd): Add shareProcessNamespace option to repoServer #291

Closed
wants to merge 2 commits into from

Conversation

k
Copy link

@k k commented Apr 2, 2020

Checklist:

  • I have update the chart version in Chart.yaml following Semantic Versioning.
  • Any new values are backwards compatible and/or have sensible default.
  • I have followed the testing instructions in the contributing guide.
  • I have signed the CLA and the build is green.
  • I will test my changes again once merged to master and published.

Changes are automatically published when merged to master. They are not published on branches.

@k k requested review from seanson and spencergilbert as code owners April 2, 2020 18:28
@k k force-pushed the share-process-namespace branch from 4ac3248 to 507e46d Compare April 2, 2020 18:29
@spencergilbert
Copy link
Contributor

@k can you tell me a little more about the use case here? I'm not super familiar with that spec, but it seems like it would only be useful if you're running multiple containers in the repo-server pod. Are you using something like an istio-sidecar and running into problems?

Copy link
Contributor

@seanson seanson left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @k!
Just a minor suggestion change and could you please add the value to the README.md with an explanation of the use case for enabling shareProcessNamespace

Comment on lines +42 to +44
{{- if .Values.repoServer.shareProcessNamespace }}
shareProcessNamespace: true
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change the invocation of the template to with so it's easier to template and future proof.

Suggested change
{{- if .Values.repoServer.shareProcessNamespace }}
shareProcessNamespace: true
{{- end }}
{{- with .Values.repoServer.shareProcessNamespace }}
shareProcessNamespace: {{ . }}
{{- end }}

@seanson seanson added argo-cd enhancement New feature or request labels Apr 21, 2020
@alexec alexec changed the title feat(argocd): Add shareProcessNamespace option to repoServer feat(argo-cd): Add shareProcessNamespace option to repoServer Jan 21, 2021
@alexec
Copy link
Contributor

alexec commented Jan 21, 2021

Stale.

@alexec alexec closed this Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
argo-cd enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants