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

Ssh remote console #388

Merged
merged 5 commits into from
Jan 6, 2024
Merged

Ssh remote console #388

merged 5 commits into from
Jan 6, 2024

Conversation

Kaiede
Copy link
Contributor

@Kaiede Kaiede commented Jan 6, 2024

Purpose

This enables the work that was started in the mc-server-runner pull request to add a remote SSH console.
itzg/mc-server-runner#56

  • Adds mc-server-runner and openssl to the container
  • Adds ENABLE_SSH setting
  • Fixes box64 pkg that got renamed
  • Ensures that if RCON_PASSWORD is not set, that it uses a scrambled password to match the docker-minecraft-server behavior.

Validation Performed

Built the docker container locally on an ARM64 system (M1 Mac mini with an Ubuntu VM running docker). Started a server with ENABLE_SSH set and the 2222 port exposed. Was able to login from a standard ssh client and issue commands to it. Docker logs were checked and confirmed that the output was sent to both locations and I could audit SSH connections.

Also checked to make sure that the default 'minecraft' password did not work if RCON_PASSWORD is not set.

Checked to ensure that the server still works when ENABLE_SSH is not defined, and logs still work.

Copy link
Owner

@itzg itzg left a comment

Choose a reason for hiding this comment

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

Looking really good! I'm not sure why I feared it would be a bigger deal than that.

Just to double check, the server stops gracefully still when the container is told to stop? (I should probably sanity test the changes myself but will do that a bit later.)

bedrock-entry.sh Show resolved Hide resolved
@Kaiede
Copy link
Contributor Author

Kaiede commented Jan 6, 2024

Just to double check, the server stops gracefully still when the container is told to stop? (I should probably sanity test the changes myself but will do that a bit later.)

Let me look at that more closely. I’ve just been noticing that the bedrock container closes relatively quickly like I expect when using docker compose down, but didn’t check the logs or anything like that. I’ll follow up with the findings.

@Kaiede
Copy link
Contributor Author

Kaiede commented Jan 6, 2024

Looking really good! I'm not sure why I feared it would be a bigger deal than that.

Just to double check, the server stops gracefully still when the container is told to stop? (I should probably sanity test the changes myself but will do that a bit later.)

Before:

[2024-01-06 16:50:13:739 INFO] IPv4 supported, port: 19132: Used for gameplay and LAN discovery
[2024-01-06 16:50:13:739 INFO] IPv6 not supported
[2024-01-06 16:50:13:746 INFO] Server started.
[2024-01-06 16:50:13:747 INFO] ================ TELEMETRY MESSAGE ===================
[2024-01-06 16:50:13:747 INFO] Server Telemetry is currently not enabled. 
[2024-01-06 16:50:13:747 INFO] Enabling this telemetry helps us improve the game.
[2024-01-06 16:50:13:747 INFO] 
[2024-01-06 16:50:13:747 INFO] To enable this feature, add the line 'emit-server-telemetry=true'
[2024-01-06 16:50:13:747 INFO] to the server.properties file in the handheld/src-server directory
[2024-01-06 16:50:13:747 INFO] ======================================================
DEBU[0019] Forwarding signal                             signal=terminated
DEBU[0019] Sending message on stdin due to SIGTERM       message=stop
[2024-01-06 16:50:25:027 INFO] Server stop requested.
[2024-01-06 16:50:25:034 INFO] Stopping server...
Quit correctly

After:

[2024-01-06 16:45:34:895 INFO] IPv6 not supported
[2024-01-06 16:45:34:956 INFO] Server started.
[2024-01-06 16:45:34:957 INFO] ================ TELEMETRY MESSAGE ===================
[2024-01-06 16:45:34:957 INFO] Server Telemetry is currently not enabled. 
[2024-01-06 16:45:34:957 INFO] Enabling this telemetry helps us improve the game.
[2024-01-06 16:45:34:957 INFO] 
[2024-01-06 16:45:34:957 INFO] To enable this feature, add the line 'emit-server-telemetry=true'
[2024-01-06 16:45:34:957 INFO] to the server.properties file in the handheld/src-server directory
[2024-01-06 16:45:34:957 INFO] ======================================================
DEBU[0216] Forwarding signal                             signal=terminated
DEBU[0216] Sending message on stdin due to SIGTERM       message=stop
[2024-01-06 16:49:06:853 INFO] Server stop requested.
[2024-01-06 16:49:06:884 INFO] Stopping server...
Quit correctly
2024-01-06T16:49:07.819Z	INFO	mc-server-runner	Done

@itzg
Copy link
Owner

itzg commented Jan 6, 2024

Thanks for confirming the stop behavior. I forgot that I had entrypoint demoter intercept the term signal and write "stop" to stdin. I'm now thinking that's why I kept thinking demoter and server runner were orthogonal tools not to be used in the same container. Turns out it's a just a little overkill that both do signal conversion and even better demoter intercepts first and avoids the rcon check in server runner.

@Kaiede
Copy link
Contributor Author

Kaiede commented Jan 6, 2024

Alright, I've closed everything out. Let me know if there's anything else, otherwise I'll be focused on my final changes to wrap this up.

Copy link
Owner

@itzg itzg left a comment

Choose a reason for hiding this comment

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

Latest looks great. Thanks!

@itzg itzg merged commit d04be86 into itzg:master Jan 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants