Durss / Twitchat

Full featured Twitch chat alternative to fill gaps from the official one.
https://twitchat.fr
GNU General Public License v3.0
269 stars 30 forks source link

Add dockerized production version #58

Closed Luzifer closed 2 weeks ago

Luzifer commented 9 months ago

This PR adds a Dockerfile to generate Twitchat's production version into a Docker image following the build-instructions from the README file.

Alongside it modifies the language compiler to work on Windows as well as POSIX systems (Linux, MacOS) by using system-specific path separator constant.

Durss commented 9 months ago

Hello! Thank you very much for taking time to help on this! Docker is something I haven't taken time to understand yet, but I know kinda everyone uses it now so it's definitely somehting that would help people running their own Twitchat instance.

I have a question regarding your modification of the src_labels/index.js. You replace the split("\\") with split(path.sep), I didn't know about that prop, thank you for teaching me this šŸ™ But wouldn't it make sense to also use this on the replace() just before? To be more clear, replace this...:

f.replace(/\\+/g, "\\").replace(rootDir, "").replace(/\.json$/gi, "").split("\\");

...with trhe following:

f.replace(new RegExp(path.sep+"+","g"), path.sep).replace(rootDir, "").replace(/\.json$/gi, "").split(path.sep);
Durss commented 9 months ago

Isn't it necessary to use that path.sep also on the server scripts? Like on the src_back/utils/Config.ts that has many hardcoded "/", or the src_back/controllers/FileServerController.ts and maybe other controllers.

Luzifer commented 9 months ago

But wouldn't it make sense to also use this on the replace() just before?

In general, yesā€¦ ā€¦though I don't know any way to produce a path like some//path in Linux / MacOS, so that shouldn't happenā€¦ šŸ¤” (But yes, if that can happen on Windows, it would be cleaner to switch to the path.sep too.)

Isn't it necessary to use that path.sep also on the server scripts?

Hmm in the server scripts there is already a / so it works perfectly fine on POSIX systemsā€¦ In that case someone hosting the application on a Windows server would need to tell us whether that works or is brokenā€¦ šŸ¤” (Though, does anyone want to host node-applications on Windows servers? Is that even possible?)

Luzifer commented 9 months ago

Okay I think I got the regex replace right nowā€¦ What a messā€¦

Without the extra replace just adding in the separator on Windows systems it would search for a literal +, not a multiple-\:

> sep = '\\'
'\\'
> new RegExp(`${sep}+`, 'g')
/\+/g
> new RegExp(sep+'+', 'g')
/\+/g
> new RegExp(`${sep}+`.replace('\\', '\\\\'), 'g')
/\\+/g

That being said: Please test the change on a Windows machine before mergingā€¦ ā€¦that has potential to do weird thingsā€¦ šŸ˜…

Durss commented 9 months ago

Hello ! Sorry for the late answer, I wanted take time to first try to make a docker isntance with your PR :)

Being a total Docker noob I propably missed something. I get this when running docker build . :

The npm ci command can only install with an existing package-lock.json " when trying to build.

EDIT: the issue came from me doing stupid things! The error is actually something else. Building process runs out of memory on my laptop. I just need to figure out how to expand memory allocation.

BTW I asked to few people knowing their things with docker if they had feedbacks on your PR and they told me you did a great job so thank you for that!

Durss commented 9 months ago

I finally succeeded to get a running docker instance šŸ„³ I'm not sure I did it the right way, but it works!

Do you think it would be worth adding the process to run the docker image on the README for newbies like I? Here is what I did:

 docker build -t twitchat .
 docker run -p 3018:3018 twitchat

After this, starting failed because credentials were missing. I had to push them to the volume with this:

 docker cp .\credentials\ {CONTAINER_ID}:/opt/twitchat/

Then starting the container properly starts the app. Is there a better way to push the credentials? Would it make sense to copy them during the build process?

Luzifer commented 9 months ago

Would it make sens to copy them during the build process?

Please don't! Docker images are not supposed to contain credentials / secrets! If you push that image somewhere you will leak those secrets.

That's why there are mounts:

docker run --rm -ti \
    -p 3018:3018 \
    -v ./credentials:/opt/twitchat/credentials:ro \
    -v ./data/beta:/opt/twitchat/beta \
    -v ./data/donors:/opt/twitchat/donors \
    -v ./data/userData:/opt/twitchat/userData \
    twitchat

The first mount -v ./credentials:/opt/twitchat/credentials:ro will take the local credentials folder and mount it to the /opt/twitchat/credentials folder read-only (ro). So if you place a credentials.json into the local folder, it's available to the application read-only.

The three other mounts "export" the stored data from the three folders beta, donors and userData to the local file-system (and therefore persist the data inside even over restarts).

(The option --rm will remove the container after its exit, -ti will bind the local terminal to the containers output. If you want it to run in background just replace the first line with docker run -d \.)

Durss commented 9 months ago

I had a feeling i was doing things wrong haha I saw it was possible to make an image load files from host filesystem but i didn't get that I had to define that mapping on the run command. Thank you very much for making it clearer :)

Is the "./data/.." you added before the 3 last volumes a specificity of your local env?

I tried running this from the project's root;

docker run --rm -ti -p 3018:3018 -v ./credentials:/opt/twitchat/credentials:ro -v ./beta:/opt/twitchat/beta -v ./donors:/opt/twitchat/donors -v ./userData:/opt/twitchat/userData twitchat

But i still get this errors on server startup:

 no such file or directory, open '/opt/twitchat/credentials/credentials.json'

Have I missed something? Maybe a windows' specificity on volume handling?

Luzifer commented 9 months ago

Eh on Windows maybe try to pass the part in front of the colon with backslashes instead of slashes:

docker run --rm -ti \
    -p 3018:3018 \
    -v .\credentials:/opt/twitchat/credentials:ro \
    -v .\data\beta:/opt/twitchat/beta \
    -v .\data\donors:/opt/twitchat/donors \
    -v .\data\userData:/opt/twitchat/userData \
    twitchat

(That looks really weird to me but might do the trick on Windows, not sure about thisā€¦)

Durss commented 9 months ago

Backslashes did the trick thank you! Almost good then ! Can you just let me know about the "./data/" prefix for the last 3 volumes? I have to remove it on my side for it work, which makes sense as those folders are not within a "data" folder but on the root directory (althought i often tell myself they would have definitely been much better in a "data" folder...)

Luzifer commented 9 months ago

Well, where you place those folders is entirely up to you. Back when I was using plain Docker I had persistent data stored at /data/<application>/..., some prefer /home/<user>/..., so basically choose the best place for you to store the dataā€¦ It's just me to have the habit to store data in a data folderā€¦ :D

The only thing you can't change is the /opt/twitchat/... folder as the application resides in /opt/twitchat and expects the folders to be at those placesā€¦

Durss commented 9 months ago

Oh my bad I didn't see you added this line to the readme :

The command below will persist the data stored by Twitchat into the local data folder

I feel like that "data" folder may add confusion. As the README tells those folders must be on the root folder, if people follow this, then run the docker app blindly, it will fail. I tend to think that if experienced users like you prefer to set those folders within a "data" folder, they'll know how to do it whereas newbies like I might get lost not understanding they actually have to put those folders within a "data" folder, not necessarily understanding the local filesystem structure does not have to reflect the one expected by the server. Does it makes sense.? Not sure I'm super clear haha

Ideally I should move all those folders within a "data" folder because, I totally agree, it's actually much cleaner. I'm just a little reluctant as it will break things on users self hosting twitchat. Unless I accept both folder structures šŸ¤” . That might be a solution actually. I update the README to put everything within a "data" folder but I add retro-compatibility stuff to still accept the old data structure.

Luzifer commented 9 months ago

Hm if you adjust the app to use one data folder it makes sense to mount that one instead of mounting three different folders. Would make it more easy run the application.

As the README tells those folders must be on the root folder, if people follow this, then run the docker app blindly, it will fail.

Ideally the dockerized version shouldn't even be running within the repo folders. Not even on the same machineā€¦ :D The repo contents don't need to exist on the server running the Docker image, it just needs a place to store its dataā€¦

Durss commented 8 months ago

Hey there ! Just to say I'm forgetting you !

I changed the data structure to put everything inside a "data" folder on the dev branch. I just need to adapt your pull request accordingly and test everythign works properly on my side.

I also added SSE pipe (Server Sent Events) so the server can push events to the client while having something more scalable than a websocket. I don't think it will be necessary to add a docker configuration for this as it's going over HTTP but let me know if I'm wrong.