Repository navigation
fix: set websocket URL env vars regardless of ingress config - #287
Merged
Merged
Conversation
- Remove conditional wrapper that only set env vars when ingress enabled - Fix inverted defaults (ws vs wss) - Add CI test values for LoadBalancer and ingress scenarios - Add CI tests to prevent regression
The readiness probe tries to connect to the RCON server which doesn't exist in CI, causing the pod to appear unhealthy. This is expected behavior - the important thing is that the chart deploys and has the correct env vars.
Contributor
Author
|
@itzg let me know if any additional fixes needed in order to merge this. Happy to help with other helm template issues you need to be addressed. |
Owner
|
I'm so sorry, I forgot to merge this since I kicked off a verify build and moved on. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a bug in my last PR that I missed. Details below.
Summary
Fixes a bug where the
RWA_WEBSOCKET_URLandRWA_WEBSOCKET_URL_SSLenvironment variables were only set when ingress was enabled. This caused the web UI to fail connecting to the Minecraft server when using a LoadBalancer service.Changes
deployment.yaml): Remove conditional wrapper and fix inverted defaultsDetails
The bug was in
charts/rcon-web-admin/templates/deployment.yamlwhere the websocket env vars were wrapped in{{- if .Values.ingress.enabled }}. This meant they were only set when ingress was enabled, breaking LoadBalancer deployments.Additionally fixed inverted defaults:
websocketUrlSslwas defaulting tows://(should bewss://)websocketUrlwas defaulting towss://(should bews://)Testing