Skip to content

Added support for the BookStack System CLI #173

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

Merged
merged 6 commits into from
Jun 9, 2023
Merged

Added support for the BookStack System CLI #173

merged 6 commits into from
Jun 9, 2023

Conversation

ssddanbrown
Copy link
Contributor

linuxserver.io


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

Description:

This updates the image with the requirements need to run the now-included "BookStack System CLI".
This is a new CLI that automates certain admin-level operations like backup/restore.

To achieve support, the image needed to be updated with php-zip extension support, and the mariadb-client client.
These changes , specifically the addition of mariadb-client, does lead to an increase in image size from approximately 280MB to 315MB. I do think this is worth it, especially as I can see some utility of having mariadb-client available for debug purposes outside just the context of the CLI, but that's not my call to make hence why I'm highlighting this size increase here.

Upon the above added packages, this also adds the newly added default BookStack backup directory to the folders that get linked to the /config volume, so that this backup folder is accessible via the mounted volume.

With these changes you'd able able to run the CLI like so:

# Create a backup
docker compose exec bookstack /app/www/bookstack-system-cli backup

# Restore an existing backup ZIP
docker compose exec bookstack /app/www/bookstack-system-cli restore /config/www/backups/bookstack-backup-2023-05-23-120213.zip

I did not update the readme yet with details of the CLI, since this CLI is in early alpha stages, so I don't want to actively advise it's usage right now, but it might be something to add in the future (Alongside details of calling the standard artisan command line).

Benefits of this PR and context:

This allows usage of the newly added BookStack System CLI.
Closes #170

While there are alternative or direct means to backup content in this container-based setup (Via direct volume files and/or via DB dumps), this CLI automates these actions in a standard manner, which could be especially useful for users looking to migrate between different BookStack hosting options.

How Has This Been Tested?

I have built the image on Linux (Fedora 38 / AMD64) and on MacOS (ARM), then performed a backup and restore using the CLI on both.

Source / References:

Added required ZIP extension and mariadb-client package to provide
mysqldump which is required by the CLI for backing up.
Also reordered packages in dockerfile to be alphabetical.
@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/bookstack/v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173/index.html
https://ci-tests.linuxserver.io/lspipepr/bookstack/v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173/shellcheck-result.xml

Tag Passed
amd64-v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173
arm32v7-v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173
arm64v8-v23.05.2-pkg-261fc739-dev-e3149f929a62b8caf84f071c6142e0eea0f16b12-pr-173

@nemchik
Copy link
Member

nemchik commented May 30, 2023

This will need to be rebased now that #174 has merged.

Key points:

  • php 8.2
  • php zip extension is now included in the base image (not required in the Dockerfiles here)
  • armhf is deprecated and removed

All that being said, I'm not a fan of adding 200mb+ to the image, and I think it might be better to recommend via the readme to use our universal package install mod https://github.com/linuxserver/docker-mods/tree/universal-package-install if you want this functionality.

@thespad
Copy link
Member

thespad commented May 30, 2023

Sounds like an ideal mod candidate really.

@ssddanbrown
Copy link
Contributor Author

All that being said, I'm not a fan of adding 200mb+ to the image

Just to confirm, the image size change I observed was approximately from 280MB to 315MB, so a ~35MB change. Unless I've misunderstood and the 200mb referenced is via another form of measurement.

@thespad
Copy link
Member

thespad commented May 30, 2023

Total extra size for mariadb-client from scratch is about 85Mb, but will probably be less in practice because some of the dependency packages will already be installed on the image.

@nemchik
Copy link
Member

nemchik commented May 30, 2023

All that being said, I'm not a fan of adding 200mb+ to the image

Just to confirm, the image size change I observed was approximately from 280MB to 315MB, so a ~35MB change. Unless I've misunderstood and the 200mb referenced is via another form of measurement.

I misread your initial post and thought you meant there was roughly a 200mb increase in the image size by including the additional package.

~35mb is definitely a lot less, but still a pretty decent number.

I don't have a strong opinion here, and I don't use bookstack at all, so I'll see if the rest of the team would like to chime in and if there are no other opinions I'll approve once the PR is rebased.

@thespad
Copy link
Member

thespad commented May 30, 2023

Quick build test, existing image is 287MB on disk, adding mariadb-client takes it to 361MB on disk so ~75MB. I don't think that's a problem for core functionality even if it's not going to be used by everyone.

@thespad
Copy link
Member

thespad commented Jun 7, 2023

@ssddanbrown I've rebased against the current master branch, let me know if you're happy and if so we'll get it merged.

@thespad thespad requested a review from a team June 7, 2023 12:45
@thespad thespad self-assigned this Jun 7, 2023
@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/bookstack/v23.05.2-pkg-e7357f94-dev-122d1204292249228dead1fc5fe7c23abef484c4-pr-173/index.html
https://ci-tests.linuxserver.io/lspipepr/bookstack/v23.05.2-pkg-e7357f94-dev-122d1204292249228dead1fc5fe7c23abef484c4-pr-173/shellcheck-result.xml

Tag Passed
amd64-v23.05.2-pkg-e7357f94-dev-122d1204292249228dead1fc5fe7c23abef484c4-pr-173
arm64v8-v23.05.2-pkg-e7357f94-dev-122d1204292249228dead1fc5fe7c23abef484c4-pr-173

@ssddanbrown
Copy link
Contributor Author

Thanks @thespad.
Just gave this a test build and run on mac (arm64).

PHP Timezone Issue

I noticed this error appearing whenever PHP is ran (Particularly via the CLI):

PHP Warning:  PHP Startup: Invalid date.timezone value '', using 'UTC' instead in Unknown on line 0

It looks like PHP date.timezone handling changed in php8.2.
Within the base image, a php-local.ini is generated upon container start-up:

https://github.com/linuxserver/docker-baseimage-alpine-nginx/blob/71cc706de6cd077d4649a3a1a996a95e4601212a/root/etc/s6-overlay/s6-rc.d/init-php/run#L6

If the user is not passing a TZ env option, this generates date.timezone with an empty value.
I don't think the TZ option has ever been a default value for the example compose setup in the readme, and I don't think there's been a fallback default value.
I checked on a few of my past linuxserver compose setups, All of them have an empty date.timezone.
This hasn't been an issue before, but now in PHP8.2 it causes the above warning.
I think many setups upgrading to the latest image versions would have this warning appear (Also shows up in container start-up logs as PHP commands are ran).

BookStack System CLI

The CLI exits upon unexpected PHP output, so the above scenario causes the CLI to stop upon certain actions.
Otherwise, running with a valid date.timezone (Via setting that option in php-local.ini or originally running with a TZ env option), allows backup and restore via the CLI to work as expected.

Summary

I see the timezone issue to be tangential to this PR.
I'm happy to go ahead with this being merged, but users with an invalid/empty timezone may have trouble using this until that's addressed for their setup.
Probably could do with a separate follow-up PR to update the example docker-compose with a default valid TZ option, and/or a script to fix existing empty date.timezone (Could maybe wait to assess impact before going down that road).

@nemchik
Copy link
Member

nemchik commented Jun 8, 2023

The timezone issue will be fixed by linuxserver/docker-baseimage-alpine-nginx#143

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.

[BUG] bookstack-system-cli backup can't be run
4 participants