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

Bump base to ubuntu 22.04 and use jq for healthcheck #282

Merged
merged 2 commits into from
Sep 12, 2024

Conversation

sinuscosinustan
Copy link
Contributor

This patch series focuses on two main improvements:

  1. Bump the base image to Ubuntu 22.04, because Ubuntu 18.04 has reached its end-of-life in May 2023 and shouldn't be used in new images anymore.

  2. Improve the healthcheck by using jq instead of grep.

Ubuntu 18.04 (Bionic) is unmaintained and marked as EOL since May 2023.
The issue with the `ubuntu` (PID 1000) user was introduced in Ubuntu
24.04 (Noble).

Signed-off-by: Tan Siewert <[email protected]>
The use of `grep` requires the HTTP server to return the JSON body
in one line. This could change with an update (either intended or not)
and would break the healthcheck.

Instead, we should parse the JSON and exit with code 1 if the health
is not healthy.

Signed-off-by: Tan Siewert <[email protected]>
@wolveix
Copy link
Owner

wolveix commented Sep 11, 2024

Awesome stuff, appreciate the PR! Based on your comments, I assume you'd already looked over the trailing comments in #266

Any thoughts @putnam?

I'll properly review this once I'm awake. Thanks so much!

@sinuscosinustan
Copy link
Contributor Author

Based on your comments, I assume you'd already looked over the trailing comments in #266

Actually, no 🙈 I ran into the "uid 1000" issue myself for another service using the steamcmd image.

@wolveix
Copy link
Owner

wolveix commented Sep 11, 2024

@tomsiewert have you tested this for the permissions issues mentioned in the other threads?

@LevantinLynx
Copy link

Just a general question regarding security and dependencies.
Why are we adding dependencies/packages like jq to the project? There is no real benefit over grep. I could understand if it's in preparation for future API support but otherwise it just adds "junk" to the container.

I am all for updating the base image to a higher ubuntu version, maybe even 24?

@wolveix
Copy link
Owner

wolveix commented Sep 11, 2024

@LevantinLynx please read the related issues regarding Ubuntu 24.

Regarding jq, I had the exact same initial thought. However, it's very likely that we'll be doing more with the API soon, so I'm fine with it being included

@sinuscosinustan
Copy link
Contributor Author

@tomsiewert have you tested this for the permissions issues mentioned in the other threads?

@wolveix I haven't triggered this issue so far and wasn't really able to re-produce it. If you have a reproducer I'd be happy if you can share it!

Why are we adding dependencies/packages like jq to the project?

As described in my commit message, the grep approach requires the string to be exact like that to match for the health check. If the game server suddenly starts to return multiline JSON or a slight different formatted JSON body, the health check would (always) fail.

@schildbach
Copy link
Contributor

Don't want to derail this PR, but is there a reason for using Ubuntu, as opposed to Debian, at all for a Docker image?

@wolveix
Copy link
Owner

wolveix commented Sep 11, 2024

@schildbach usually Debian is my first choice! I seem to recall there being a reason for us using Ubuntu instead of Debian, but that decision would have been years ago and I'd have to do some digging :)

@wolveix
Copy link
Owner

wolveix commented Sep 11, 2024

@tomsiewert @natewalck could you test wolveix/satisfactory-server:dev-282 please? Mainly interested to hear if it works, and whether data persists. It seems to be working for me

@natewalck
Copy link

@tomsiewert @natewalck could you test wolveix/satisfactory-server:dev-282 please? Mainly interested to hear if it works, and whether data persists. It seems to be working for me

I'm not seeing this in a github branch, but I can try it on docker in a bit.

Currently I'm on the latest docker image and its been working great.

@wolveix
Copy link
Owner

wolveix commented Sep 11, 2024

@natewalck it's from DockerHub, the tag is there :) I'd appreciate it if you could give it a go, as you had issues with the previous image, so I'm curious for your usage specifically.

Make sure to backup your saves etc. first!

@natewalck
Copy link

@natewalck it's from DockerHub, the tag is there :) I'd appreciate it if you could give it a go, as you had issues with the previous image, so I'm curious for your usage specifically.

Make sure to backup your saves etc. first!

If the current image is working well, what is the use case for this one? Going back to modern Ubuntu? I have friends playing on the server and I am loathe to change it atm.

I can try spinning up a fresh server on another host and test it there.

@wolveix
Copy link
Owner

wolveix commented Sep 11, 2024

@natewalck because Ubuntu 18 is EOL. I believe the issue you experienced was with Ubuntu 24, and I'm interested in trying it on your same server given that it's an isolated issue. I understand if you'd rather not, but it'd be incredible if you could spend just 10 minutes trying it

@Zenkala
Copy link

Zenkala commented Sep 12, 2024

I'm not natewalck, but I did do a quick test. Moving from satisfactory-server:latest to satisfactory-server:dev-282 went flawless for me. Game loaded with the existing save data and as a player, I was none the wiser.

satisfactory-server  | LogInit: ExecutableName: FactoryServer-Linux-Shipping
satisfactory-server  | LogInit: Build: ++FactoryGame+rel-main-1.0.0-CL-365306
satisfactory-server  | LogInit: Engine Version: 5.3.2-365306+++FactoryGame+rel-main-1.0.0
satisfactory-server  | LogInit: Compatible Engine Version: 5.3.2-365306+++FactoryGame+rel-main-1.0.0
satisfactory-server  | LogInit: Net CL: 365306
satisfactory-server  | LogInit: OS: Ubuntu 22.04.4 LTS (6.8.0-41-generic), CPU: AMD Ryzen 5 5500, GPU: NVIDIA PCI-id: 10de-1380 (10de-1380)
satisfactory-server  | LogInit: Compiled (64-bit): Sep  9 2024 11:32:40
satisfactory-server  | LogInit: Architecture: x64
satisfactory-server  | LogInit: Compiled with Clang: 15.0.1 (https://github.com/llvm/llvm-project b73d2c8c720a8c8e6e73b11be4e27afa6cb75bdf)

@wolveix
Copy link
Owner

wolveix commented Sep 12, 2024

Thank you for confirming, @Zenkala! I'm particularly eager for anyone who reported issues before to test it. The issues with Ubuntu 24 were due to the users, but we had issues even when bumping to Ubuntu 20 in the past (if memory serves right). That was due to host Docker versions, but I wanted to try and verify that this isn't the case with Ubuntu 22 now.

@Zenkala
Copy link

Zenkala commented Sep 12, 2024

Ah I see. Well it works with Docker version 27.1.0, build 6312585. Hope we get more people to test it.

@wolveix wolveix merged commit 99a972a into wolveix:main Sep 12, 2024
2 checks passed
@wolveix
Copy link
Owner

wolveix commented Sep 12, 2024

Thanks everyone :D No complaints so far 👀

@sinuscosinustan sinuscosinustan deleted the few-changes branch September 12, 2024 13:40
@natewalck
Copy link

@tomsiewert @natewalck could you test wolveix/satisfactory-server:dev-282 please? Mainly interested to hear if it works, and whether data persists. It seems to be working for me

I tried to test this, it said it does't exist. I assume this is just in main now, so I'll update my base.

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.

6 participants