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

When providing the UID or GID as a number the chart fails to deploy, … #94

Merged
merged 1 commit into from Nov 14, 2023

Conversation

JoelBChapmanII
Copy link
Contributor

@JoelBChapmanII JoelBChapmanII commented Nov 14, 2023

I did not see a contributing.md before opening this PR, if there are steps I've missed please let me know.

Problem: When specifying the gid (PLEX_GID) or uid (PLEX_UID) as numeric values under extraEnv helm fails to deploy because the options are provided as integers instead of strings, regardless of how the values are provided within the values.yaml. For example all of these will fail to deploy

extraEnv:
  PLEX_UID: '1000'
  PLEX_GID: "1000"
  OTHER_ENVVAR: 1000

Error Message:

json: cannot unmarshal number into Go struct field EnvVar.spec.template.spec.containers.env.value of type string

Potential Solution: I've added quotations around the key/value pair that are land the env vars, if there is a more preferable solution I can update the pull request to accommodate. I've tested this solution works for me on my cluster.

@JoelBChapmanII JoelBChapmanII requested a review from a team as a code owner November 14, 2023 19:55
Copy link
Member

@cilindrox cilindrox 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 contribution @JoelBChapmanII - LGTM, although my preference would be to use the quote function instead of the hard quotes.

Edit: also, if you could bump the chart version to a patch (0.1.7 afaict), we can release this straight away.

charts/plex-media-server/templates/statefulset.yaml Outdated Show resolved Hide resolved
…adding quotations to where extraEnv lands the extra environment variables fixes the issue

Updating method to quote the env values per PR conversation, updating chart version with a patch bump
Copy link
Member

@cilindrox cilindrox 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 @JoelBChapmanII !

@cilindrox cilindrox merged commit cbf988e into plexinc:master Nov 14, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants