Use Node Alpine Version as Docker image base & use Yarn instead of NPM

[stevefan1999-personal]

Well the prebuilt binaries for benchpress-rust is built against GLIBC, which is a big no no for Alpine.

I tried to use Alpine and thus that prebuilt binary cannot import the symbols for GLIBC, neither libc6-compat do. I have succeeded to install rust from Alpine Edge and let Neon do its job, but it will increase the build time but roughly 1.5x.

It is fixed a year later

The image size will drop more than 100MB when compared normally.

[CLAassistant]

All committers have signed the CLA.

[julianlam]

Hi @stevefan1999-personal -- is there a specific reason you've updated the Dockerfile to use these configurations?

[stevefan1999-personal]

@julianlam I want to image size to be as small as possible to better fit in a k8s cluster. Almost all of us use Alpine as base image.

[julianlam]

Right, and the choice between Debian Buster and Alpine...?

You should not bundle imagemagick in the file, as it is not required by NodeBB unless you are using the imagemagick plugin, and if so you'll want to use a forked Dockerfile.

[stevefan1999-personal]

@julianlam And I think that hacking the Dockerfile to use Alpine will be very dangerous and it will affect downstream images as well, so I rather take a step back and look for safer alternatives. And immediately I see that a slim version of node is available so I changed to that, plus I deleted all that apt cache so that it should have a comparable size to what you would get with Alpine.

Also in the official guide to install NodeBB in Debian, ImageMagick is required. And I see that makes sense for others to derive a new image that install ImageMagick on their own. Now I removed it so the size should be the bare minimum now.

I have pushed my modifications to Docker Hub. You can compare the size difference here:
https://hub.docker.com/r/stevefan1999/nodebb_node/tags
https://hub.docker.com/r/nodebb/docker/tags

[julianlam]

Thanks for the explanation. The change to use yarn is certainly ok, although I think some flag needs to be set in NodeBB setup, I will review again later today.

[julianlam]

@nilsramsperger this is an unsolicited plea for a second pair of eyes on this PR 😁

[stevefan1999-personal]

I think it LGTM. The image size now is trimmed down by 2,27x, a very considerable shrink in sizes.

[nilsramsperger]

Switching to a smaller base image to shrink image size is a good idea. Buster-slim is a step in the right direction.
If I understand the conversation right. The use of alpine is problematic with the initial image magic integration. In general, alpine is usable. I use it in my NodeBB image with no problems.
I'm not familiar with the advantages of yarn compared to npm. A quick look a yarn's documentation just points out a better speed. So for precompiled Docker images from Dockerhub it bears no real benefit for the downloading user.

[julianlam]

@stevefan1999-personal Let's keep it simple and revert back to npm. I am okay if you want to switch to Alpine Linux since @nilsramsperger is using it. If you need imagemagick support, then ok to stick with Debian Buster as well.

For reference here is his Dockerfile

[pitaj]

If somebody wants to open an issue on benchpress I can switch it to use musl-libc instead which should make it more portable across Linux distros.

[stevefan1999-personal]

@nilsramsperger fine, i'll revert to npmI found out that one key point of having a dramatic size decrease is due yarn and how it deletes cache. I need to have some reconsideration now...🤔 from npm with 656.39 to yarn with 540.54 in image MB size...
@julianlam well in alpine you can also just apk --no-cache add imagemagick
@pitaj rather I think you should provide extra builds with musl and not replace them. right now i can manually compile benchpress-rust by using the edge repo and then download the prerequisites for neon

I will squash the commits for the one final round

[julianlam]

Thanks @stevefan1999-personal 😄

[nilsramsperger]

@stevefan1999-personal good to know that yarn is somewhat more efficient on cache deletion than npm.
As in most cases, there are multiple ways to reach a goal. While optimizing my NodeBB image for size, I came to Docker multistage builds. That way you only take the files you really need for the image and dump everything else. In the end, it's an efficient way of cache deletion as well.

[NazimHAli]

Any news on this? Need help with any outstanding issues? After switching to yarn, I just can't use npm anymore unless I have to :p

[stevefan1999-personal]

@NazimHAli yes, there is no way to keep node_modules for some reason, especially when it involved plugins, and I realized Docker might not be the right tech for this.

[stevefan1999-personal]

I planned to use it in Kubernetes, but then I realized I cannot do layered volumes on demand, so I wanted to have a separate plugin volume that layered on top of the base image, so I don't have to worry about losing plugin after restarting.

Someone made it with Synching and decided to recursively link everything into it, and I think this is not elegant.

It is also possible if I use FUSE based OverlayFS implementation (because it requires root), but the performance won't be pretty.

[Brutus5000]

@stevefan1999-personal I made it working on kubernetes, but it is ugly so far.
(It could be improved a little bit by replacing sed with envsubst.)

My PR #10036 would solve the module problem.
Also PR #9850 would solve the splitting of secret and config problem.

But until then I hacked it with an initContainer in the deployment and a config.json template and a buildscript mounted from the configMap.
Also I added a one-time installer script

[stevefan1999-personal]

By merging the user layer with the base layer using fuse-overlayfs, you can successfully save the user-initiated changes such as plugin and theme install. The only problem: it requires FUSE and SYS_ADMIN, which is considered unsafe to some people with a concern of breaching container security. But so far it works for me.

[julianlam]

@nilsramsperger Can you kindly do a test run and let us know if there are any unexpected behaviours with this new image?

[stevefan1999-personal]

@nilsramsperger Can you kindly do a test run and let us know if there are any unexpected behaviours with this new image?

Oh also expect longer build times for workflow since qemu emulation is prohibitively slow. At the very best we could just include arm64 and amd64 to lose 32-bit Raspberry Pi support (armv7). That might pity some people but any hobbyst should be able to figure out how to build an image matching their platform. I needed arm64 since I run it on Ampere.

[oplik0]

I'd say at the very least ppc64le (I wonder how many PowerPC devices run NodeBB...) and arm v6 should go. I'd even be in favor of dropping v7.
The last fairly popular device pre-arm v7 was the original RPi Zero, but even the Zero 2W is ARMv8 now. Mainline RPis are on v8 since RPi3 and there is now official 64 bit Raspberry Pi OS.

Also, not sure if MongoDB docker image works on 32 bit ARM anyway.

EDIT: Nope, it doesn't - amd64 and arm64 only. Redis and Postgres do, but I think it might be fine to just follow the default DB choice for NodeBB. The question is how much time it adds though, if it's not a lot I don't think keeping v7 would be bad.

[stevefan1999-personal]

I'd say at the very least ppc64le (I wonder how many PowerPC devices run NodeBB...) and arm v6 should go. I'd even be in favor of dropping v7. The last fairly popular device pre-arm v7 was the original RPi Zero, but even the Zero 2W is ARMv8 now. Mainline RPis are on v8 since RPi3 and there is now official 64 bit Raspberry Pi OS.

Also, not sure if MongoDB docker image works on 32 bit ARM anyway.

EDIT: Nope, it doesn't - amd64 and arm64 only. Redis and Postgres do, but I think it might be fine to just follow the default DB choice for NodeBB. The question is how much time it adds though, if it's not a lot I don't think keeping v7 would be bad.

Okay. That should do it

[nilsramsperger]

@julianlam I just did a small test run on the code, using the redis profile. The forum can be set up and be started. No problems this far.

As mentioned in previous posts, the startup takes quite some time. This leads to some bad user experience, since the software feels like it hung up several times:

• The web installer can't be accessed right after starting the containers.
• The installation itself, after providing user and db parameters, takes very long. I had to F5, to get to the "Start NodeBB" screen.

The performance definitely needs some improvement (IMHO).

Greets
Nils

[oplik0]

I just noticed that this is running install instead of setup like the current dockerfile. This would be a breaking change AFAIK, since unlike setup this requires user interaction to actually start NodeBB.

However, current Dockerfile requires a SETUP env variable to run setup, as such this could probably be left as-is and instead before the config file is checked the dockerfile could check for that variable and run ./nodebb setup if it is set.

[stevefan1999-personal]

IIRC setup runs an interactive CLI that requires user input but on a containerized environment this is close to impossible (and there is a lot of gimmicks using docker attach). Since I use Kubernetes to host NodeBB, that's even more impossible, and that's why I need to resort to using web installer instead. But I do agree this is a breaking change. Maybe we could add a switch for this.

[oplik0]

Setup can run with just env variables, which is why the current dockerfile has it (and probably why it requires a separate variable to run it).

As I said, I think a preferred solution might be leaving install as is and instead checking for the SETUP env variable before the two existing paths for running NodeBB. It'd maintain compatibility with current Dockerfile while adding the installation functionality if config doesn't exist and the user didn't prepare for unattended setup.

[stevefan1999-personal]

Oh I have implemented the "init" verb now, but should I keep it as setup or install, though?

[oplik0]

As I said: I think install by default is a good choice, but for the sake of backwards compatibility the script should run setup if an environmental variable called SETUP is set (see this part of the current dockerfile: test -n "${SETUP}" && ./nodebb setup).
So essentially, I think your solution is better for most cases (and being able to specify command is even better), but I think the previous one should still work not to break someone's workflow without a decent reason.

Can't suggest code changes in this thread since it's outdated, but something like

if [ -n $SETUP ]; then
  ./nodebb setup --config $CONFIG
elif [ -f $CONFIG ]; then
(rest of current code)

Would ensure current setups work.

[oplik0]

Hmm... The image from this PR doesn't seem to work at all for me - I'm mostly getting permission errors:

nodebb-7-nodebb-1  | chmod: /opt/config: Operation not permitted
nodebb-7-nodebb-1  | chmod: /opt/config: Operation not permitted
nodebb-7-nodebb-1  | cp: can't create '/opt/config/package.json': Permission denied
nodebb-7-nodebb-1  | touch: /opt/config/package-lock.json: Permission denied
nodebb-7-nodebb-1  | ln: package.json: File exists
nodebb-7-nodebb-1  | ln: package-lock.json: File exists
[...]
nodebb-7-nodebb-1  | npm ERR! code EACCES
nodebb-7-nodebb-1  | npm ERR! syscall open
nodebb-7-nodebb-1  | npm ERR! path /usr/src/app/package-lock.json
nodebb-7-nodebb-1  | npm ERR! errno -13
nodebb-7-nodebb-1  | npm ERR! Error: EACCES: permission denied, open '/usr/src/app/package-lock.json'
[...]
nodebb-7-nodebb-1  | Error: EACCES: permission denied, copyfile '/usr/src/app/install/package.json' -> '/usr/src/app/package.json'

The ln error is caused by ln -s attempting to run on every container boot which will fail if these files are already there. It can be remediated simply by adding the -f flag (so that part of entrypoint would look like this:

ln -fs $CONFIG_DIR/package.json package.json
ln -fs $CONFIG_DIR/package-lock.json package-lock.json

For a dirty fix of the permission errors I even ran as root and found another issue, and I 100% blame NPM for it. What is it?
NPM v7 won't work with NodeBB without ssh keys.
Why? It appears that something in NodeBB dependency tree depends on git+https://github.com/Mango/emitter.git (more specifically, this something: https://github.com/Mango/slideout).

So, logically, what protocol should this URL starting with git+https:// use? According to NPM, it's obviously SSH. Which means that using this kind of specifiers for dependencies now requires having SSH keys set up to download them, even if they are public.

The "workaround" is to use git+https://git@github.com/Mango/emitter.git which would require NodeBB to fork the slideout package.

Oh, and the issue in NPM repo has been open since last february: npm/cli#2610

[oplik0]

It still seems the permissions for the default config directory are broken, I'm still getting

nodebb-nodebb-1  | chmod: /opt/config: Operation not permitted
nodebb-nodebb-1  | panic: no write permission for /opt/config

I'm just running docker compose --profile mongo up without any additional arguments or changes to docker-compose.yml or the dockerfile. Same thing happens both on Linux and Windows hosts.

[stevefan1999-personal]

It still seems the permissions for the default config directory are broken, I'm still getting

nodebb-nodebb-1  | chmod: /opt/config: Operation not permitted
nodebb-nodebb-1  | panic: no write permission for /opt/config

I'm just running docker compose --profile mongo up without any additional arguments or changes to docker-compose.yml or the dockerfile. Same thing happens both on Linux and Windows hosts.

Interesting. It seems like you are running the container as another user that the UID-GID mapping isn't cleanly remapped. I need to think about that as well, since it could mean one of your upper directory does not give permission to that location (far as I remember it is chained, so if you can't the top/"leftmost" directory, the rest won't as well)

[julianlam]

I will defer to @oplik0's opinion on merging this wrt technical details.

However, I noticed that you updated the readme to include information about the Docker config. That's great! I do want to move that out to a dedicated page on the documentation portal instead.

So I will do that and then merely link out to it from the readme, just like the installation/upgrade sections.

[julianlam]

This is now done, NodeBB/docs#78 will need to be merged at the same time for best effect 🪄

[oplik0]

This time I actually found how to fix the issues I ran into, so with just a few changes I'm all for this. It's far better than current solution, both by optimizing some parts of the image by using alpine, and by greatly improving usability with the volumes.

[oplik0]

This was already changed, see line 60 - I guess I didn't notice this was also a part of this PR

[oplik0]

This actually returns true if $CONFIG_DIR/package.json doesn't exist. You need to check the directory itself here (just if [[ ! -w $CONFIG_DIR ]])

Fixing this resolves my issue from #8704 (comment) - though chmod still throws an error

[oplik0]

I'd redirect the errors from this to /dev/null - volumes on at least some platforms mount owned by root, making this fail despite the folder still being writable.

Though I'd also add a comment explaining this to help debugging. E.g.

# If the folder is mounted as a volume this can fail, the check below is to ensure there is still write access
chmod -fR 760 $CONFIG_DIR 2> /dev/null

[oplik0]

npm moved to --omit instead of --only for some reason, so this throws a deprecation warning. So this should now be

npm install --omit=dev

(note that this is from npm >=8, which became the default in Node 16, so it's currently in all supported node versions)

[oplik0]

Correction: also add --no-optional because sass-embedded currently doesn't support alpine Linux (it might soon-ish - it appears dart finally added support for musl builds a few months ago)
But npm doesn't catch this issue on the optional dependency because it only seems to look for generic os/architecture.

And sass removed musl-specific error handling in a refactor for some reason

[oplik0]

This has been held up so long that 2 alpine versions came out, so it should be 3.18 now :)

[oplik0]

@stevefan1999-personal I just wanted to ask if you're still interested in pursuing this PR (I think all that's left are the fixes from stevefan1999-personal#71 and possibly rebasing to develop, but I'm not sure how important that is), or if that's not the case if you'd be fine with me making a new PR based on your work here?

[stevefan1999-personal]

@oplik0 You can take over some of it. You can choose what was essential first. I will split this PR into different parts.