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

Various Fixes for Podman/ Docker #113

Merged
merged 26 commits into from May 12, 2024
Merged

Various Fixes for Podman/ Docker #113

merged 26 commits into from May 12, 2024

Conversation

sHedC
Copy link
Collaborator

@sHedC sHedC commented May 7, 2024

From the latest have updated to fix the following, mainly for podman but tested with both docker and podman on xwayland, x11, wayland in both KDE (Manjaro) and Gnome (Fedora).

  • podman/ docker now detected in entrypoint and script split to start correctly.
  • properly supports xwayland using xAuthority
  • fixed permissions when setting up new zwift user volume for podman
  • added ability to specify user and group id in podman
  • can locally build and run in podman using bin/test-build.sh (also works for docker)
  • fixed entrypoint for docker initial build when using same userid as host
  • fixed passing of arguments from test-build.sh and zwift scripts

Wayland i tried to fix, it starts wine notepad however zwift complains about screen size and doesn't work correctly.

run_zwift.sh Show resolved Hide resolved
zwift.sh Outdated Show resolved Hide resolved
zwift.sh Show resolved Hide resolved
Copy link
Owner

@netbrain netbrain left a comment

Choose a reason for hiding this comment

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

Your bash skills are a notch above mine 👍

lgtm,. however one question.

@netbrain
Copy link
Owner

netbrain commented May 9, 2024

@hobeone do you have any inputs on this?

entrypoint.sh Show resolved Hide resolved
entrypoint.sh Outdated Show resolved Hide resolved
@hobeone
Copy link
Contributor

hobeone commented May 9, 2024

Only other input is Thank You for doing all of this!

@sHedC
Copy link
Collaborator Author

sHedC commented May 9, 2024

Your bash skills are a notch above mine 👍

lgtm,. however one question.

I have been coding for over 35 years, I actually was trying not to change too much :)
I am more than happy to help with Podman integration and Wayland as this is what I use for my other developments.

@sHedC
Copy link
Collaborator Author

sHedC commented May 9, 2024

Fixed the comments above, I have updated bin/test_build.sh to trap for errors and clean up after (annoying if you have to manually delete the container on failure).

I didn't update the bin/build.sh but the test one should work for both Podman and Docker, if you are happy I will move it into build.sh and update the readme.

I am just running docker tests now.

@sHedC
Copy link
Collaborator Author

sHedC commented May 9, 2024

There is an issue with the docker and using a different ZWIFT_UID/ GID, checking it.

@sHedC
Copy link
Collaborator Author

sHedC commented May 9, 2024

Ok docker is working but when I try using a different ZWIFT_UID/GID I get an error later when starting, this might just be because I am not using it right as not sure why it would be used.

The entrypoint is setting all the permissions correctly and setting up the right mapping. Can you seee if that part works?

@hobeone
Copy link
Contributor

hobeone commented May 9, 2024

What error are you seeing?

@sHedC
Copy link
Collaborator Author

sHedC commented May 9, 2024

What error are you seeing?

image

@netbrain
Copy link
Owner

netbrain commented May 9, 2024

Looks like an error from the runfromprocess-rs binary. @quietvoid

@sHedC
Copy link
Collaborator Author

sHedC commented May 9, 2024

This is only when I put ZWIFT_UID and ZWIFT_GID different to my local id of 1000. Without setting those it works fine.

@sHedC
Copy link
Collaborator Author

sHedC commented May 9, 2024

ok got it, because I mount /run/user/??? directly it is not setting the ownership correctly. Problem is /run/user//?? is neede for wayland, need to re-adjust.

@sHedC
Copy link
Collaborator Author

sHedC commented May 9, 2024

Have changed it back a bit, so basically to use Wayland or XWayland we need to map /run/user/xxxx, however doing this means you can not change ownership of /run/user/xxxx to the UID/ GID provided in ZWIFT_UID and GID.

So in short at the moment if you use ZWIFT_UID / GID you have to use X11, but wayland works for default.

Will look into this more but fixed to Podman/ Docker works with defefault in Wayland and X11 but adding ZWIFT_UID/GID requires using X11

@sHedC sHedC marked this pull request as draft May 10, 2024 15:59
@sHedC
Copy link
Collaborator Author

sHedC commented May 10, 2024

Found a couple of minor issues with podman savng documents and sound since fixing the ZWIFT_UID issue.

@netbrain
Copy link
Owner

Nice, is it all done now or are there any remaining issues?

@sHedC
Copy link
Collaborator Author

sHedC commented May 10, 2024

Nice, is it all done now or are there any remaining issues?

Just testing and one more I notice the update script should e reversed, "UPDATE" should be checked after install is checked.

@sHedC sHedC marked this pull request as ready for review May 10, 2024 19:57
@sHedC
Copy link
Collaborator Author

sHedC commented May 10, 2024

Fixed Tested with ZWIFT_UID and docker (podman fails for some weird reson but nearly works)
Fix Audio on Docker
Build now supports docker and podman
Fixed Podman permissions

Tested everything I can

Copy link
Owner

@netbrain netbrain left a comment

Choose a reason for hiding this comment

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

A few questions & clarifications.

bin/build-image.sh Show resolved Hide resolved
zwift.sh Outdated Show resolved Hide resolved
@sHedC
Copy link
Collaborator Author

sHedC commented May 11, 2024

Weirdly Podman now supports using ZWIFT_UID under X11 not sure how that happened.

@sHedC
Copy link
Collaborator Author

sHedC commented May 12, 2024

I this all good now anything else you notice?

@netbrain
Copy link
Owner

What is the reasoning behind the different names? i.e why when podman you have localhost/zwift instead of netbrain/zwift? relic from testing?

@sHedC
Copy link
Collaborator Author

sHedC commented May 12, 2024

What is the reasoning behind the different names? i.e why when podman you have localhost/zwift instead of netbrain/zwift? relic from testing?

Think you missed my answer or it's not commenting properly from my phone.

Podman always prefixes with localhost automatically unless you build against a real domain. So wasn't really a choice to use localhost for podman local build.

@netbrain netbrain merged commit f679ae7 into netbrain:master May 12, 2024
@netbrain
Copy link
Owner

Thanks for your contribution, The community appreciates it!

@netbrain
Copy link
Owner

Just got around to test this myself now, seems to work well. 👏 Have released this as a new image built from scratch. So for the next upcoming releases we'll see if the updating routine works aswell.

@sHedC
Copy link
Collaborator Author

sHedC commented May 12, 2024

Just got around to test this myself now, seems to work well. 👏 Have released this as a new image built from scratch. So for the next upcoming releases we'll see if the updating routine works aswell.

Perfect glad I can help i am more than willing to help in the future I use this a few times a week and it was one of the last things I used windows for, really appreciate the effort you have all putin to make this work so happy I can assist.

@netbrain
Copy link
Owner

Happy to hear! 🤗

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.

None yet

4 participants