-
Notifications
You must be signed in to change notification settings - Fork 397
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
docker: optimizations #2074
docker: optimizations #2074
Conversation
Thanks for the effort. I will try to test your changes on Mac in the next couple of days and merge it accordingly! |
Can you merge the most recent PRs because they will resolve the failing checks above |
4e4f588
to
ea9827f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works ok in Mac as well. I do get an error in the Jukebox container but I believe this is related to something else, my personal set up. This is not a blocker.
jukebox | 146:daemon.py - jb.daemon - MainThread - ERROR - Plugins with errors during load: player
@@ -1,12 +1,10 @@ | |||
.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we want to ignore this anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The jukebox calls git to show some git values on startup. Without the .git folder this runs into an error. I know it's not a vital function, but I wanted to resolve warnings and errors wherever possible, so when a "real" error occurs you don't get distracted by these "built-in" errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. I understand the reasoning behind it. But this folder can grow a lot over time. So maybe the best approach would be to remove the git command? If it runs into errors, it doesn't work anyways and might be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-added .git along with hidden github dirs and added the git errors to the known to-be-ignored errors in the doc
docker/docker-compose.yml
Outdated
volumes: | ||
- ../shared/audiofolders:/home/pi/RPi-Jukebox-RFID/shared/audiofolders:rw | ||
- ../shared/playlists:/root/.config/mpd/playlists:rw | ||
- ./config/docker.mpd.conf:/root/.config/mpd/mpd.conf:rw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you move them out of the global docker-compose
and duplicate them in the individual ones? Since they are the same in all OS docker compose, they can be unified here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are different in the linux-as-host configuration, they are mapped to /home/pi instead to /root because the linux config uses the pi user in combination with the running users UID to connect to the pulseaudio socket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But they seem to be the same in Windows and Mac. You could use them as default in the docker-compose.yml
and only overwrite them in docker-compose-linux.yml
. This is how it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done; now docker-compose.windows.yml
is completely empty so I removed it
@@ -152,7 +154,9 @@ Test & Develop | |||
|
|||
The Dockerfile is defined to start all Phoniebox related services. | |||
|
|||
Open `http://localhost:3001 <http://localhost:3001>`_ in your browser to see the web application. | |||
Windows: Open `host.docker.internal:3000 <http://host.docker.internal:3000>`_ in your browser to see the web application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be also just http://localhost:3000
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
localhost is changed to 0.0.0.0 by jukebox. I noticed that on windows I only have a route that routes traffic from 0.0.0.0 to my physical network adapter, so the jukebox could not connect. On another windows with docker I noticed that another network route from 0.0.0.0 to the docker network adapter exists, so I'm not sure why it is missing on my installation.
Anyhow I figured that using host.docker.internal on windows might work on more installations (including mine), but if you want I'll undo that change. For mac, my impression was that host.docker.internal also exists and should work, but I'm not sure and can't test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
host.docker.internal
does not work for me on Mac. Maybe we can add a comment mentioning both, so it works for everyone depending on their set up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc changed for Mac to localhost:3000
docs/sphinx/developer/docker.rst
Outdated
@@ -152,7 +154,9 @@ Test & Develop | |||
|
|||
The Dockerfile is defined to start all Phoniebox related services. | |||
|
|||
Open `http://localhost:3001 <http://localhost:3001>`_ in your browser to see the web application. | |||
Windows: Open `host.docker.internal:3000 <http://host.docker.internal:3000>`_ in your browser to see the web application. | |||
Mac: Open `host.docker.internal:3000 <http://host.docker.internal:3000>`_ in your browser to see the web application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least in Mac, it only works with localhost:3000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK good to know, then I'll change it back.
Do you know if that host entry host.docker.internal exists on Mac?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above
3ddfe59
to
a36ee1e
Compare
- linux host: use pulse unix socket - mpd: run as user (pi / root) - mpd: remove port exposure to host, connections to mpd only come from other docker containers - less config adjustments for docker environment
a36ee1e
to
9b339a7
Compare
I was able to get it to work. We need to update the docs in another place. The path has changed from |
I didn't find |
* Start moving docs to markdown * update userguide * add more pages * Rename index.md * Undo last commit * Add autohotspot * Finish userguide without references * Rename bt audio btns * Adding developers and rfid * docker: optimizations (#2074) - linux host: use pulse unix socket - mpd: run as user (pi / root) - mpd: remove port exposure to host, connections to mpd only come from other docker containers - less config adjustments for docker environment Co-authored-by: Christoph Lauer <[email protected]> * Start moving docs to markdown * update userguide * add more pages * Rename index.md * Undo last commit * Add autohotspot * Finish userguide without references * Rename bt audio btns * Adding developers and rfid * Rename docs to documentation * Update document structure * Move rfid to developers * Remove sphinx * Remove even more sphinx * Test pydoc * Revert "Test pydoc" This reverts commit e1c6aeb. Revert "Remove even more sphinx" This reverts commit 6f55597. Revert "Remove sphinx" This reverts commit 23abee3. * Moving sphinx for api and command reference * Remove Sphinx for good * update paths to developers --------- Co-authored-by: notapirate <[email protected]> Co-authored-by: Christoph Lauer <[email protected]>
- Bookworm Support - Documentation in Github (removed Sphinx and Readthedocs) - Webapp Dependency updates - Python Dependency updates - Python venv - Better Dockerfiles - Installation fixes Squashed commit of the following: commit c0d5a20 Author: pabera <[email protected]> Date: Sat Nov 25 23:48:28 2023 +0100 future3 V3.3 - reference webapp build (#2126) commit 12f4f92 Author: Alvin Schiller <[email protected]> Date: Sat Nov 25 23:36:44 2023 +0100 Future3/documentation (#2127) * fixed typo * restored "rpc-commands" * fixed broken links * switched placed for leading documentation * added docu for changing swap size * unified links to source commit 924c7f3 Author: pabera <[email protected]> Date: Sat Nov 25 19:46:42 2023 +0100 Remove content folder from docs (#2124) commit 94aa9dc Author: pabera <[email protected]> Date: Sat Nov 25 00:14:29 2023 +0100 Future3/update docs (#2123) * Update pulseaudio link for docker mac setup * Update Docker docs commit 2a0bb20 Author: pabera <[email protected]> Date: Fri Nov 24 23:24:29 2023 +0100 Update some links in the documentation (#2122) * Update some links in the documentation * Rename docs folder userguide to builders * Update some headlines commit dfb9e93 Author: Alvin Schiller <[email protected]> Date: Sat Nov 18 14:40:10 2023 +0100 Cleanup installation messages (#2109) * cleanup installation messages use clear for each new option added headers use correct urls in finish message fixed some typos and wording * fix usage with multiple ip addresses (ipv4/ipv6) use single call to get all information during installation show and read out only first ip in webui * fix typo in translation * fix order for gateway / interface * harmonize read answer option * fix FIN_MESSAGE * fix line break on OS check commit babb237 Author: s-martin <[email protected]> Date: Thu Nov 16 23:57:28 2023 +0100 fix docker description for windows (#2114) commit acf6ec0 Author: Alvin Schiller <[email protected]> Date: Thu Nov 16 22:49:57 2023 +0100 Future3 fix venv usage (#2111) * fix node setup nodejs includes npm. fails on extra installation folder keyrings might not be created yet * install python packages via pip in venv * move venv to project root * remove outdated comments / messages about venv * remove hardwired path. replace during installation * fix dead variable * #2112 fix 'not tagged as plugin callable' commit d62de5f Merge: b7480b8 26ca404 Author: Simon <[email protected]> Date: Thu Nov 16 22:10:44 2023 +0100 Merge branch 'future3/develop' of https://github.com/MiczFlor/RPi-Jukebox-RFID into future3/develop commit b7480b8 Author: Simon <[email protected]> Date: Thu Nov 16 22:10:07 2023 +0100 fix rename commit 26ca404 Author: s-martin <[email protected]> Date: Thu Nov 16 22:06:23 2023 +0100 * keep docs for template reader at src (#2113) * fix markdown warnings commit e3bc59f Author: s-martin <[email protected]> Date: Thu Nov 16 21:35:53 2023 +0100 More links fixed (#2106) * fix link * fix links * fix indentation * fix link * fix links * fix links * fix link * Update template_reader.md * Make only link to docs * Fix link * Update and rename README.rst to README.md * Update and rename README.rst to README.md * Update and rename README.rst to README.md * Update and rename README.rst to README.md commit b12c941 Author: s-martin <[email protected]> Date: Tue Nov 14 10:15:18 2023 +0100 markdown file commit 13bf5e5 Author: pabera <[email protected]> Date: Mon Nov 13 23:27:35 2023 +0100 Future3/update dependencies (#2103) Update python venv Update Bookworm Update node to v20 (lts) and npm minor dependencies Update docker to all of the above commit 1ca31dd Author: s-martin <[email protected]> Date: Sun Nov 12 20:49:45 2023 +0100 fix #2101 (#2107) * fix #2101 * fix #2101 for de commit 86d608c Author: s-martin <[email protected]> Date: Sun Nov 12 12:31:13 2023 +0100 fix some more doc links (#2105) * fix doc links * add a readme to user guide * add readme to developers section * fix links * fix some links * fix links * fix some links * fix links * fix links * fix links * fix links * fix links * add new line commit b7743da Author: Philipp S. Sommer <[email protected]> Date: Sun Nov 12 00:04:23 2023 +0100 Fix future3 installation for bookworm (#2100) * add --break-system-packages option to pip3 install as required for bookworm, see #2050 (comment) * install libasound2-dev to fix installation of pyalsaaudio via pip3 * install NodeJS and npm via official recommendation see https://github.com/nodesource/distributions * configure break-system-packages option globally see discussion in #2100 (review) * allow nginx (and others) to access /home/pi commit 4667fc1 Author: s-martin <[email protected]> Date: Sat Nov 11 21:47:04 2023 +0100 Fix link commit 7406085 Author: s-martin <[email protected]> Date: Sat Nov 11 21:43:55 2023 +0100 Fix link to feature status (#2102) * Fix link to feature status * Rename to-be-deleted-status.md to status.md commit 1ca5a81 Author: pabera <[email protected]> Date: Sat Nov 11 21:26:19 2023 +0100 future3 - Move docs back to Github & markdown (#2085) * Start moving docs to markdown * update userguide * add more pages * Rename index.md * Undo last commit * Add autohotspot * Finish userguide without references * Rename bt audio btns * Adding developers and rfid * docker: optimizations (#2074) - linux host: use pulse unix socket - mpd: run as user (pi / root) - mpd: remove port exposure to host, connections to mpd only come from other docker containers - less config adjustments for docker environment Co-authored-by: Christoph Lauer <[email protected]> * Start moving docs to markdown * update userguide * add more pages * Rename index.md * Undo last commit * Add autohotspot * Finish userguide without references * Rename bt audio btns * Adding developers and rfid * Rename docs to documentation * Update document structure * Move rfid to developers * Remove sphinx * Remove even more sphinx * Test pydoc * Revert "Test pydoc" This reverts commit e1c6aeb. Revert "Remove even more sphinx" This reverts commit 6f55597. Revert "Remove sphinx" This reverts commit 23abee3. * Moving sphinx for api and command reference * Remove Sphinx for good * update paths to developers --------- Co-authored-by: notapirate <[email protected]> Co-authored-by: Christoph Lauer <[email protected]> commit 0660586 Author: Christian Tietze <[email protected]> Date: Wed Nov 8 17:01:23 2023 +0100 Remove unused _jukebox_core_download_prebuilt_pyzmq (#2097) See report #2094 commit 373fd38 Author: Christian Tietze <[email protected]> Date: Wed Nov 8 17:00:48 2023 +0100 Fix pyzmq installation from source with drafts support (#2096) * Fix pyzmq installation from source with drafts support * Update pyzmq docs link commit 5e35ce8 Author: notapirate <[email protected]> Date: Fri Nov 3 14:33:56 2023 +0100 docker: optimizations (#2074) - linux host: use pulse unix socket - mpd: run as user (pi / root) - mpd: remove port exposure to host, connections to mpd only come from other docker containers - less config adjustments for docker environment Co-authored-by: Christoph Lauer <[email protected]> commit 6f1efb8 Author: Alvin Schiller <[email protected]> Date: Tue Oct 10 21:21:59 2023 +0200 fix unavailable repo for pi-rc522 (#2075) * fix unavailable repo for pi-rc522. Use fix version to ensure compatibility * build trigger * REVERT build trigger * fixed sphinx gpiozero version for pyton < 3.8 * fixed sphinx gpiozero version for pyton >= 3.8 commit f803185 Author: Alvin Schiller <[email protected]> Date: Tue Oct 10 21:15:54 2023 +0200 future3: Abort installation if user is not "pi" (#2077) * check for user 'pi' and home '/home/pi' retrieve correct information even if called with sudo * removed obsolete v2.x workflow files commit b11606a Author: s-martin <[email protected]> Date: Sat Sep 2 12:51:56 2023 +0200 Create dependabot.yml (#2022) commit 99a5536 Author: pabera <[email protected]> Date: Tue Aug 15 00:25:41 2023 +0200 Add info to install script and documentation about 64bit systems #2041 (#2057) * Add info to install script and documentation about 64bit systems #2041 * Fix flake8 error commit 37a7938 Author: Alvin Schiller <[email protected]> Date: Fri May 5 09:02:36 2023 +0200 set indent_size of 2 for js and yaml files (#2026) commit 99bad28 Author: Alvin Schiller <[email protected]> Date: Wed May 3 23:30:01 2023 +0200 future3 - Feature "sync shared" (#2009) * first callback test * default sync_shared settings added * moved test callback methods * fixed logger call * changed logger name * test rpc call sync_folder * settings added * added test rsync call (subprocess) * fixed path * fixed paths * fixed errorlogging * changed subprocess shell=false * added player update * fixed subprocess args * fixed rsync parameter * update database on caller. added return value * added "wait for database update" * added server and directory checks loglevels adjusted * fixed sync for subfolder shortcuts * fixed ignored files * refactored path handling use os.path instead of string concatenation fixed handling for abs path folder name * refactored logic in control class * added check for "on_rfid_scan_enabled" * sync_full added * added check for feature activation * correction of bool value handling evaluate to false if settings not correctly set (e.g. as string) * fix flake8 errors * update log message and fix result code * added ssh support * refactored paths for run_params * speed up ssh mode perform less checks for folder existence * added sync_change_on_rfid_scan * updated default settings format like ConfigHandler would save it * added command binding for Ui * fixed binding of command options and made them lowercase * changed invalid parameter handling * added sync_card_database update card id only on rfid scan sync overwrite on full sync * refactorings added locking on cfg access updated methodnames updated logging methods reordered flake8 corrections * exclude folder.conf if existing from V2.x * changed call on rfid scan to callback * fix flake8 errors * fix indendation for JS * combine settings of credentials for modes * naming convention * refactored function names to be more clear * changed options of sync_change_on_rfid_scan options changed from "true"/"false" to "enable"/"disable" * moved identical prechecks to functions * renamed "sync_full" to "sync_all" * Fix function calls fix for: moved identical prechecks to functions * renamed "sync_full" to "sync_all" correction for logging * added "update_wait" and fixed to much locking * changed call on play_card to callback * changed precheck names to "is sync enabled" * updated function names "is_file" and "is_dir" * reduced nesting complexity * Changed rfid callback state to Enum renamed callback class add state as enum * Changed playcontent callback state to Enum added state as num moved callback and enum to seperate class callback class with generic to be able to use in more play functions * fix import * fixed generic type definition * harmonised precheck for sync_change_on_rfid_scan * refactored methods to util class * renamed syncutil to syncutils. fixed import * fixed flake8 * Moved syncutils up * renamed module sync_shared to rfidcards * renamed sync_shared to sync_rfidcards * fix flake8 * updated documentation * Updated translation for en * Updated language * Updated language * Update docs --------- Co-authored-by: pabera <[email protected]> commit 8eee27b Author: Michael Weinrich <[email protected]> Date: Sun Apr 16 17:13:25 2023 +0200 Fix typo in System Setup chapter (#2012) commit 68138d1 Author: pabera <[email protected]> Date: Wed Apr 12 23:41:31 2023 +0200 update webapp dependencies (#2010) commit 0250733 Author: Alvin Schiller <[email protected]> Date: Wed Apr 12 07:12:57 2023 +0200 Fix Workflow Actions flake8 (future3) (#1989) * flake8 Version pin >= 4.0.0 * flake8 corrections * update action versions * Updated pyton version matrix Pythen versions 3.7 - 3.11 added add quotation for correct intpretation commit a8c56b0 Author: notapirate <[email protected]> Date: Fri Apr 7 23:43:28 2023 +0200 setup_jukebox_core.sh: fix pyzmq installation for current pip versions (#1928) - using --install-options is deprecated, building pyzmq throws an error - use environment variables instead to activate draft support Co-authored-by: Christoph <Christoph Lauer> commit 6fce216 Author: kugelzucker <[email protected]> Date: Fri Apr 7 23:37:35 2023 +0200 a few more typos and wordings (#1952) * typos * GPIOs listed as unsupported is this still true? i used gpio on future3 and it seems fine. if its obsolete then merge. * typos and wording * typos, wording * info about tail -f on installlog * added info about monitoring install log * tail -f INSTALL-log,with proper formating * wording * wording commit 0326c9a Author: s-martin <[email protected]> Date: Tue Jan 3 14:26:31 2023 +0100 Fix count in flake8 config (#1954) commit 5734440 Author: kugelzucker <[email protected]> Date: Mon Jan 2 10:11:41 2023 +0100 typos in docs (#1950) typo and broken link to phoniebox.de, site seems down right now.
The scope of this work is to make the docker environment work out of the box on linux hosts (which it didn't so far for me), reduce errors and warnings to a minimum, and reduce config adjustments for the docker environment if possible.
On Linux hosts, the pulseaudio socket is 'volumed' to the docker containers to be used by dockers pulseaudio. Additionally the UID has to match, which made some adjustments for windows/mac necessary too.
Changes were successfully tested on Linux (Arch) and Windows 10. Mac is untested; the Mac config seems to be closer to the Windows config than to the Linux config, so I adjusted it accordingly.