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

Docker support #2

Open
ShaddyDC opened this issue Jul 9, 2019 · 5 comments
Open

Docker support #2

ShaddyDC opened this issue Jul 9, 2019 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ShaddyDC
Copy link

ShaddyDC commented Jul 9, 2019

The bot looked kind of nice, but I'm a lazy bum who hates installation steps and a sucker for docker.

Therefore adding support for Docker by providing a Dockerfile would be nice since you could then easily spin up an instance wherever docker is installed. Maybe even throw it on dockerhub so you could pull and run it with one command.

I don't have access to my computer currently, but I can probably write one in a week or so if that's something you could see adding.

@LeaPhant
Copy link
Owner

LeaPhant commented Jul 9, 2019

Definitely something I wanted to pursue with this bot! Adding it to my to-do. :)

@LeaPhant LeaPhant added the enhancement New feature or request label Jul 9, 2019
@LeaPhant
Copy link
Owner

I published a Dockerfile in the docker-test branch and put it on Docker Hub.

To run it first create a volume:

docker volume create --name flowabot

Then set up the docker container which will also lead you through the configuration wizard:

docker run --name flowabot --restart unless-stopped -it -v flowabot:/usr/src/app leaphant/flowabot

Now the bot can be stopped by pressing Ctrl+C and then started in the background with the following command:

docker start flowabot

If you (@ShaddyDC) or someone else knows more about Docker and can help me, as I've only just learned about it during the last two days, please comment on issues before I'll merge this. Thank you!

@LeaPhant LeaPhant added the help wanted Extra attention is needed label Jul 12, 2019
@ShaddyDC
Copy link
Author

So, I haven't got around to playing around with it, so I don't know which parts are exactly needed where, and what I'm saying is only what I got from looking it over without any actual testing or investigating. I also have to say that while I've worked with docker some, I'm probably not aware of a bunch of best practices.

I'm not sure if .net is required for actually running it or only for building. In the latter case I would suggest using a multi-stage build with the corresponding image. In that case you also wouldn't need the ENV variables anymore.

I think I've read somewhere about clearing the apt cache after doing your installations or removing the tools after they're not needed anymore to keep the image slim, but it isn't hugely important, at least to me. However, if it works, you could try using the node:10-alpine image to make a significant difference in size.

You can save the WORKDIR /opt line by directly using WORKDIR /opt/osu-tools and then cloning into the current directory. Nothing much, but it makes the Dockerfile look more neat, imo.

Similarly, is there a specific reason you're using COPY twice? The second call would include all the files of the first one, so I'm not sure it's necessary. I don't know if it makes a difference in caching though.

You can also reorder some stuff for better caching of build stages, but I don't know which of those commands can be cached or any details from the top of my head, so I won't comment until I've played with it around some. I can properly look into it later this weekend or early next week.

@LeaPhant
Copy link
Owner

Alright, thanks for the insight. I'll play around with the Alpine image.

The .NET Core SDK could indeed be removed and replaced by the runtime after building osu-tools, this would make multi-stage builds even more complicated though so I'd rather keep it the current way even though it's a little messy.

The reason I run COPY twice like that is simply because I've seen it written that way in every Docker for Node.js example. 🤷

@ShaddyDC
Copy link
Author

ShaddyDC commented Jul 17, 2019

Some small things. I've never worked with node.js, so I haven't really dug deep into it and can only give some superficial feedback.

Firstly it would be nice if the config file was detached form the bot folder. Then you could just take an empty local directory and use that as the volume, docker run -it --rm -v $PWD/flowabot:/config leaphant/flowabot or whatever. This currently produces an error as the local folder overwrites whatever is inside the container folder, and in this case it would then lack the files to run the bot.
On that note, you also wouldn't want the entire bot to live in the volume. Docker is all about applications being ephemeral. Then running git pull on the volume to update also completely bypasses the docker version system. You only want to update by pulling the later version of the image manually. This would be especially true for tags, but I guess that isn't an issue in this case. lol
Having only the config persist would also allow to just swap out different versions of the bot, downgrade or temporarily run a dev test branch or whatever, and just plug the config volume in there.
There's also things like watchtower to do the updating automatically. These wouldn't work with this mechanism.

One more thing which is mostly for convenience, just throw the bot into /app instead of /usr/src/app. It's just shorter, and it's not like you've got other stuff in the container so you'd have to keep things orderly and stuff.

Some minor things: Maybe run apt-get clean and rm -rf /var/lib/apt/lists/* after the install to keep the image smaller, though I haven't checked how much that actually saves. Lastly, it's odd to have COPY origin . and COPY origin ./ each once. Doesn't really matter though.

Edit: I just remembered, you could use oppai-ng instead of the osu tools to avoid the need for .net. I don't know how accurate it actually is, but it probably is accurate enough. It looks like it also supports star difficulty calculation. Though you've commented on an issue there, so you've probably got a reason for not using it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants