Add mc_health_extra_args to autopause-fcns.sh functions #2832
Conversation
|
|
||
| mc_server_listening() { | ||
| mc-monitor status --host localhost --port "$SERVER_PORT" --timeout 10s >& /dev/null | ||
| mc-monitor status "${MC_HEALTH_EXTRA_ARGS[@]}" --host localhost --port "$SERVER_PORT" --timeout 10s >& /dev/null |
There was a problem hiding this comment.
The idea of this change is great, but I don't the think the syntax or usage is correct. Have you tested this?
MC_HELATH_EXTRA_ARGS is being used as an array type since normally it is declared in /data/.mc-health.env here
docker-minecraft-server/scripts/start-finalExec
Lines 226 to 229 in 7419a66
however, the file .mc-health.env file doesn't seem to be getting source'd in the autopause scripts. So that needs to be added, such as this
Setting the env var MC_HEALTH_EXTRA_ARGS is not correct since env variables only have string values, not array values. So the array expansion ${MC_HEALTH_EXTRA_ARGS[@]} will not be correct.
I would suggest introducing a new env var named something like USES_PROXY_PROTOCOL that is "true" or "false". In the start-finalExec location above add a conditional on isTrue "$USES_PROXY_PROTOCOL" and add --use-proxy to MC_HEALTH_EXTRA_ARGS there.
There was a problem hiding this comment.
Yes, before making this pull request I've tested it with the --use-proxy option but that was about it.
There was a problem hiding this comment.
If you wish I can make the changes you mentioned, but it will probably be in about 1.5 weeks time from now as I'm working on other projects at the moment.
There was a problem hiding this comment.
Yes, making those changes would be good. mc-health can also make use of that option then. I don't mind waiting for you to make that change.
BTW, MC_HEALTH_EXTRA_ARGS was purposely undocumented since it was only meant to communicate internally the start-time discovery for legacy ping protocol. So, it'll be great to document the new variable since it actually should be visible to users.
github-actions
bot
added
the
status/stale
2 participants
This adds ability that was non-working in issue #2828, allowing for use of undocumented environment variable MC_HEALTH_EXTRA_ARGS setting.
This functionality was originally in bin/mc-health but not in autopause-fcns.sh which is used to autostop and autopause, and which was non-working when for example proxy protocol acceptance was enabled in Paper.