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

Feature extra env init container #2602

Conversation

kenzaboussaoud8
Copy link

This PR adds support for the extraEnvVars section to the migrations section of the api.

Copy link

vercel bot commented Sep 4, 2024

Someone is attempting to deploy a commit to the neosync Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@nickzelei nickzelei left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the PR! Left a comment.

You'll also want to run make helm/docs to generate the helm documentation and get that added to the commit.

backend/charts/api/templates/deployment.yaml Show resolved Hide resolved
@nickzelei
Copy link
Member

Separately, curious as to what your usecase is for this feature addition?

@nickzelei nickzelei added the enhancement New feature or request label Sep 4, 2024
@kenzaboussaoud8
Copy link
Author

Separately, curious as to what your usecase is for this feature addition?

Hello Nick, this is to simplify our deployment process by reducing the need for manual patches. Specifically, we need to avoid applying patches for db credentials.

@nickzelei
Copy link
Member

Separately, curious as to what your usecase is for this feature addition?

Hello Nick, this is to simplify our deployment process by reducing the need for manual patches. Specifically, we need to avoid applying patches for db credentials.

Hmm, still not super clear. The db migrations container is fully configurable via the helm chart through the values. What are you looking to specify in the extra that isn't already available to configure through the helm chart as-is today?

fieldRef:
fieldPath: status.hostIP

{{- with .Values.migrations.extraEnvVars }}
Copy link
Member

Choose a reason for hiding this comment

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

I tried running this but the helm output failed due to indentation I believe. I think it might need to be indented one more section so that it appears underneath the env.

@nickzelei
Copy link
Member

Hey @kenzaboussaoud8 - I went ahead and took this over the line. Since outside collabs branches are slightly different, i wasn't able to push to this pull request directly.
You can see my final PR here: #2717

Thank you for the contribution!

@nickzelei nickzelei closed this Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants