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

"VOLUME /books" was missing from Dockerfile #41

Closed
wants to merge 1 commit into from

Conversation

salfter
Copy link

@salfter salfter commented Dec 5, 2024

linuxserver.io


  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

The Dockerfile is missing a VOLUME /books declaration, so you can't mount a library against it.

Benefits of this PR and context:

As it's currently shipping, the container doesn't work because it can't access libraries.

How Has This Been Tested?

In my docker-compose.yml, I replaced image: lscr.io/linuxserver/docker-readarr with build: docker-readarr, built a new container, and brought it up. My books are now visible within the container.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pull request! Be sure to follow the pull request template!

@j0nnymoe
Copy link
Member

j0nnymoe commented Dec 5, 2024

This is not needed. We only add the /config volume as part of our containers docker files as it's the main one that's required. Outside of that, users are welcome to add whatever folder's they need via bind mounts.

For Example: https://github.com/linuxserver/docker-sonarr/blob/5f634a338cbf0c28fd4373894ac04dfed285f292/Dockerfile#L50 We don't have /tv in sonarr.

@j0nnymoe j0nnymoe closed this Dec 5, 2024
@LinuxServer-CI
Copy link

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/readarr/develop-0.4.4.2686-pkg-0dc06621-dev-236ddc587fa33ca63036ea8eba14ad4a52c274b6-pr-41/index.html
https://ci-tests.linuxserver.io/lspipepr/readarr/develop-0.4.4.2686-pkg-0dc06621-dev-236ddc587fa33ca63036ea8eba14ad4a52c274b6-pr-41/shellcheck-result.xml

Tag Passed
amd64-develop-0.4.4.2686-pkg-0dc06621-dev-236ddc587fa33ca63036ea8eba14ad4a52c274b6-pr-41
arm64v8-develop-0.4.4.2686-pkg-0dc06621-dev-236ddc587fa33ca63036ea8eba14ad4a52c274b6-pr-41

@salfter
Copy link
Author

salfter commented Dec 5, 2024

This is not needed. We only add the /config volume as part of our containers docker files as it's the main one that's required. Outside of that, users are welcome to add whatever folder's they need via bind mounts.

For Example: https://github.com/linuxserver/docker-sonarr/blob/5f634a338cbf0c28fd4373894ac04dfed285f292/Dockerfile#L50 We don't have /tv in sonarr.

Noted, and Sonarr does work properly that way (I have /mnt/storage/tv-shows mounted to /tv). However, a similar declaration wasn't working with Readarr for /books (though it was working for /downloads), at least not when using Docker Compose. echo ls /books | docker run -i --rm --entrypoint ash -v /mnt/storage/books/Calibre\ Library/:/books lscr.io/linuxserver/readarr:develop produced the expected results, however.

Just tried again with the published container and Docker Compose, and I'll be damned if it isn't working properly now. I don't know why it wasn't working earlier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants