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

Add Helm Chart Variable for Configuring SIDECAR_IMAGE Independently #3979

Open
kangjk1017 opened this issue Sep 6, 2024 · 2 comments · May be fixed by #4051
Open

Add Helm Chart Variable for Configuring SIDECAR_IMAGE Independently #3979

kangjk1017 opened this issue Sep 6, 2024 · 2 comments · May be fixed by #4051
Assignees
Labels
kind/feature New features for Agones

Comments

@kangjk1017
Copy link

Is your feature request related to a problem? Please describe.
Currently, the SIDECAR_IMAGE environment variable can be set in the Agones controller, but it cannot be easily modified when installing Agones via Helm. This limitation forces the image registry to follow the common Agones settings, and it cannot be customized through deployment commands, arguments, or flags. For instance, if there is a need to use a different registry for the sidecar image, such as in cases where the sidecar image must be pulled from a private registry due to issues like those described in issue #3978, the current setup does not provide an easy way to achieve this.

Describe the solution you'd like
I would like to see the introduction of a Helm chart variable that allows the SIDECAR_IMAGE to be configured independently. This would enable users to specify a different image registry for the sidecar without affecting the main Agones configuration. It would provide more flexibility in managing image sources, especially in scenarios where different registries are required for different components.

Describe alternatives you've considered
The current alternative is to manually modify the deployment after the Helm installation or to maintain a custom fork of the Helm chart that includes the desired changes. However, these approaches are less efficient and more error-prone compared to having an official Helm variable for this purpose.

Additional context
This feature would be particularly useful in situations where the sidecar image needs to be pulled from a private registry, as in the case described in issue #3978. It would allow for easier maintenance and customization of Agones installations in environments with specific registry requirements.

@markmandel
Copy link
Member

markmandel commented Oct 24, 2024

@kangjk1017 any reason you can't use the agones.image.registry, agones.image.sdk.name and agones.image.sdk.tag variables in the helm configuration?

Just looking at this a bit more - it sounds like you have a fix for the Windows issue you also filed #3978 😄 since you have it running in a private repository. Would you be so kind as to share it back, if that is the case?

@kangjk1017
Copy link
Author

Thank you for taking an interest in resolving this issue!

In my case, the game server nodes can be either Linux or Windows. That means I create a single Kubernetes cluster for game servers with worker nodes configured for both Windows and Linux, and then deploy Agones using a single Helm chart.

Simply changing the agones.image.sdk.tag to something like {$VERSION}-windows_amd64-ltsc2022 alone might not fully resolve the issue I'm experiencing. That's because the Agones sidecar needs to run on both Windows and Linux nodes under the same SDK version tag, such as 1.44, without any platform-specific postfix (like windows_amd64-ltsc2022).

This is why I proposed the option to modify the registry for the SDK image.

In reality, if the os.version in the Docker manifest were correctly set as it was in the past, there wouldn't be any issue at all.

@0xaravindh 0xaravindh linked a pull request Nov 26, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New features for Agones
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants