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

Add Docker suport #80

Closed
wants to merge 2 commits into from
Closed

Conversation

SvenDowideit
Copy link

for #4
I'm likely to use it with Docker Swarm for now, so will be able to follow up with a docker-compose file that leverages Traefik at some point.

@SvenDowideit SvenDowideit force-pushed the dockerize branch 2 times, most recently from cbaa45f to 1c0f63d Compare July 31, 2018 00:56
@SvenDowideit
Copy link
Author

the setup_and_start part now does the same thing as https://github.com/belsander/docker-cronicle

@belsander has done the right thing, and has documented it well - are you 2 interested in combining your work??

@SvenDowideit SvenDowideit force-pushed the dockerize branch 2 times, most recently from 45074fe to 28e751e Compare July 31, 2018 03:12
@belsander
Copy link

@SvenDowideit sure thing, I'll have a look at it tonight!

@belsander
Copy link

belsander commented Aug 1, 2018

Damn, sorry man. Really having a hard time, getting some time free. I quickly checked it out, if you want, I'll merge my fork in here.

EDIT: let me know what you want to do, seems like you copied all my stuff already in here.

@soulteary
Copy link

soulteary commented Aug 3, 2018

@belsander hi, @SvenDowideit just write a dockerfile describe which environment let the Cronicle running, eg: node:latest.

And he wanna keep the cronicle process running as fg process use debug.sh, otherwise this docker container will exit(without hacks).

It's good work, but i think use node image base on ubuntu is too large, not good for spread, maybe use alpine?

Dowideit, Sven (O&A, St. Lucia) added 2 commits August 10, 2018 14:26
Signed-off-by: Dowideit, Sven (O&A, St. Lucia) <[email protected]>
Signed-off-by: Dowideit, Sven (O&A, St. Lucia) <[email protected]>
@SvenDowideit
Copy link
Author

@soulteary good point - using node:alpine now, should save at least 250MB

@SvenDowideit
Copy link
Author

@jhuckaby wdyt?

@soulteary
Copy link

@SvenDowideit Good job.

But I suggest use alpine:3.x instead of latest, to keep the docker image fixed and more stable, alpine:latest is okay as well.

Maybe I'll make a pr after you, let it work together with traefik: make it more simple, scale, stable..

@belsander
Copy link

belsander commented Aug 10, 2018

Just one remark, is there no need for extra alpine dependencies? I have already forgotten why, but I've added the following packages to make it work in my case: belsander/docker-cronicle/Dockerfile

RUN        apk add --no-cache git curl wget perl bash perl-pathtools tar procps jq

using node:9.11-alpine though...

FROM node:9.11-alpine

EDIT: if you'll be using Traefik with Let's Encrypt certificates, have a look at the following:
belsander/docker-cronicle/blob/master/Dockerfile.letsencrypt
in combination with:
belsander/docker-cronicle/blob/master/entrypoint.sh

NODE_EXTRA_CA_CERTS=/etc/ssl/certs/ca-certificates.crt $BIN_DIR/debug.sh

@jhuckaby
Copy link
Owner

Hey everyone,

I see that I've been pinged on this issue. First of all, thank you for all your hard work! I am really pleased to see so many people interested in Cronicle. Now, unfortunately I know absolutely nothing about Docker, having never used it once, so I really cannot merge this PR into the main repo.

Please understand my reasoning here -- it's not that your work is sub-par (it's not), or I think Docker sucks (I don't), or anything of the sort. The problem is, I simply wouldn't be able to support it. I haven't a clue what these files are or what they do, or how to change them when I upgrade Cronicle in the future. And if people see a Dockerfile in the repo, they will expect that Cronicle fully support Docker, which it does not.

I have to understand everything that goes in my apps, so if something comes up or I decide to make a change, I know how to fix problems, or make huge architectural shifts. With these Docker additions, I would be unable to update or change them. This is my own fault. Docker is really taking over the world, and is used practically everywhere, so I am solely to blame for not knowing how to use it. Maybe someday I'll learn, but it's probably a long way off.

The last point I have to make here is that Cronicle is still in a pre-release state (still sub-v1.0), and its state is in flux. I may change all kinds of things before v1.0 which may break this Docker support, and for that reason I cannot accept any PRs (I've already had to reject several).

I think @belsander has the right idea here, in creating his own repo that augments Cronicle for running in a Docker environment. Could this PR be redesigned to live in it's own repo? Heck, it even could be made into an npm module, with a post-install script that moves the files into /opt/cronicle/ or whatever.

I tried to make Cronicle super easy to install, so it could be integrated into things like container systems without much trouble. I see that you guys are still running into roadblocks, though, and having to work around them with hacks. So maybe I can help with some of those to make the Docker integration cleaner.

First of all, I see that you are using environment variables, and then using jq to insert those into the Cronicle JSON configuration file. Perhaps I can help here. What if Cronicle simply understood a special environment variable syntax, and allowed those to be configuration overrides on startup? Example:

Environment Variable Sample Value
CRONICLE_web_socket_use_hostnames true
CRONICLE_server_comm_use_hostnames true
CRONICLE_WebServer__http_port 3012
CRONICLE_WebServer__https_port 3013
CRONICLE_base_app_url http://myserver.com:3012

Note the use of double-underscore (__) to denote an object traversal (I don't think you can include dots in environment variable names). Would this help make the Docker setup process easier?

I also plan on allowing any configuration option to be overridden on the command-line using dot notation, so you can (soon) do things like this:

/opt/cronicle/bin/control.sh start --web_socket_use_hostnames true --server_comm_use_hostnames true --WebServer.http_port 3012 --WebServer.https_port 3013 --base_app_url "http://myserver.com:3012"

Would any of these changes help you make the Docker integration cleaner?

Secondly, I see that you have to use debug.sh to start Cronicle in the foreground (no daemon fork), otherwise Docker will exit the container (sigh). This is bad for a number of reasons, not the least of which is, it is launched in debug mode, emits the entire debug log to STDOUT, runs with Node deprecation warnings and garbage collection debugging, and cannot do things like restart itself. Debug mode is meant for debugging, not running in a container.

Someone recently contacted me and asked if Cronicle could have a --foreground command-line option. This would simply prevent the background daemon fork, but not enable debug mode. Would this help? I can easily add something like this in.

Anyway, I am extremely sorry that I cannot merge this PR, but I am more than happy to add features to Cronicle that make sense for all users, and that includes things that may help you, like environment variable support, CLI config overrides, and foreground launching. Let me know if any of these would be of interest to you!

And seriously, thank you all for your interest in Cronicle, and your efforts to make it better.

@belsander
Copy link

Hi @jhuckaby! And thanks for your hard work! I can keep supporting Cronicle as a docker container, that works fine for me, but I'm far off for being a Javascript expert. So I can't support it as an npm module.

First things first. Let's get that debug out of the way! If you could enable the flag '--foreground', I would be very pleased, so yes, that would be a huge improvement.
Number 2, environment variable support. Another huge improvement. The jq way was more of a workaround anyways. If the configuration of Cronicle is exposed as environmental variables, the configuration could be fully controlled by the environment. This makes is very easy in a Docker container to configure. For the naming, I would indeed suggest using double underscores like you proposed.

@SvenDowideit
Copy link
Author

@jhuckaby heya! :)

yup, 90% of what we'd love for Docker support is environment variable based settings - so we don't need to create / modify / manage application specific config files, and a start script that can do setup (if the instance wasn't previously setup) and run in foreground.

After that, the Dockerfile would be the only needed part.

Personally, I like the Dockerfile to be in the main repository, so that any time you do a release, it triggers (via some hub.docker.com automation) a rebuild of the Docker image - but I can understand that doesn't make sense for you :)

@jhuckaby
Copy link
Owner

@belsander @SvenDowideit Just released Version 0.8.23 which introduces environment variables and foreground mode. Hope it helps!

@belsander
Copy link

Awesome, thanks @jhuckaby ! I'll make the changes next week

@belsander
Copy link

the docker image has been updated with the environmental variables and no longer using debug to start cronicle. nice improvement! One remark though, @jhuckaby. I haven't looked at it yet, but it seems when stopping the Docker container a SIGKILL is used, because control.sh start does not shutdown nicely.

@jhuckaby
Copy link
Owner

@belsander The control.sh is a very simple shell script, which does nothing except launch the Node.js main script (main.js) for Cronicle. The shell script doesn't respond to any signals, which is probably why Docker resorts to a SIGKILL. What you probably want is to skip control.sh entirely, and just have Docker launch the main Node.js script yourself. So instead of calling control.sh start, do this instead:

node /opt/cronicle/lib/main.js

@belsander
Copy link

belsander commented Aug 24, 2018

@jhuckaby Makes sense, I'll make the changes

EDIT: any need for the garbage collection flags you are using in control.sh?

@kdolan
Copy link

kdolan commented Aug 30, 2018

@jhuckaby Really appreciate your efforts to make Docker integration a piece of cake for Cronicle. I've had great success using the Dockerfile made by @belsander to deploy Cronicle in our Docker swarm cluster.

The only thing I am missing is email support. We use Google's SMTP for our mail so naturally we need to somehow provide Cronicle with a password to authenticate. The only problem is that would either result in me putting our password as an environment variable in our docker-compose file or in plain text in a config file that we inject into the container. Both of these would result in the password in source control which is an obvious no-no.

If Cronicle could support reading the password from a text file that would allow me to easily inject it using secrets support in Docker swarm which securely mounts secrets into the container at run time in /run/secrets. For example, then I could set a new Cronicle config option mail_options_password_path=/run/secrets/smtp_password.txt

This would also be an issue if we choose to use a storage engine that would require authentication so it might be appropriate for a more generic solution. Perhaps a better approach would be to have an environment variable/config options secrets_path that would be a JSON file with any sensitive configuration options. Before Cronicle consumed the configuration it would merge its regular configuration with the secrets configuration.

For example:
config.json

{
  "auth": { "username": "admin"}
}  

secrets.json

{
  "auth": { "password": "SecreT"}
}  

combined.json

{
  "auth": { "username": "admin", "password": "SecreT"}
}  

@jhuckaby
Copy link
Owner

jhuckaby commented Sep 2, 2018

@belsander Oops, I'm so sorry -- I didn't see the edit to your comment. GitHub didn't notify me when it was updated. Yes, you can safely remove all those old garbage collection flags. They were leftover from the old Node v0.12 days, and are now removed entirely as of Cronicle 0.8.26.

@kdolan That's a rather complicated and specific feature, for something that I believe Linux already provides. All Linux flavors support adding a script into the /etc/profile.d/ directory, and it'll be executed on boot. In here you can specify your own secrets that go into environment variables, using the EXPORT shell keyword. For example, create this file: /etc/profile.d/cronicle-secrets.sh in your Docker container with these contents:

#!/bin/sh
EXPORT CRONICLE_mail_options__auth__pass=YOURPASSWORDHERE

Would that solve your problem? I think you might have to set the file permissions to 755 (executable), but I am not sure.

@kdolan
Copy link

kdolan commented Sep 2, 2018

That would work perfectly! Did not think about approaching it from that angle. Thanks!

I also found a need to write my own simple plugin to make HTTP requests with our internal API keys since we could not just add them to header of the default HTTP plugin for security reasons. At that point I just added email support for failed jobs into the plugin itself.

Added these two lines to @belsander's Dockerfile to support adding custom plugins to the image.

COPY       plugins /plugins
RUN        cd /plugins && npm install && rm -f .npmrc && chmod +x *.js

@belsander
Copy link

Thanks, @jhuckaby. I removed the garbage collection parameters.

@kdolan , I added a plugins directory /opt/cronicle/plugins, if there is anything else you would like to see added, let me know, or just create a PR.

I'll also have a look at versioning the docker container, for the moment it only supports tag latest (and letsencrypt).

@SvenDowideit
Copy link
Author

@belsander and @jhuckaby awesome work - I'm finally circling around to this again in our infra-stack (using docker swarm) and it looks alot like the intelliops/cronicle image will do what we need.

but there is the problem of updating the images, tagging for releases etc - perhaps we could get a github webhook setup on this repo, which triggers a build?

This is one circumstance where having the Dockerfile in your repository would be simpler - as Docker auto-builds trigger from Github repo updates - and can build different images based on git tags and branch names.

anyhow - looking good :)

@belsander
Copy link

@SvenDowideit sure, a webhook would do the job. I just fixed the version in de docker repo for the moment, so a manual update can be done. If we can set up a webhook, then I'll set the master branch back to build the latest version of Cronicle.

@jhuckaby
Copy link
Owner

jhuckaby commented Oct 1, 2018

@SvenDowideit @belsander I have no problem adding a webhook trigger for you. Just get me the URL you want to ping and I'll add it in.

@belsander
Copy link

@jhuckaby is there a way to contact you directly? Then I can send you the build URL.

@jhuckaby
Copy link
Owner

jhuckaby commented Oct 6, 2018

@belsander Sure, my e-mail address is my GitHub username at gmail dot com.

@lukasmrtvy
Copy link

@ckt114
Copy link

ckt114 commented Feb 26, 2019

For anyone that's interested, I've got cronicle to work in k8s using container below so I think it'll be the same for running in docker with little tweak of course. If you're interested then give me your email and I'll send you the manifests I use for k8s. My setup is one primary master and two back masters and any amount of slaves.

https://hub.docker.com/r/intelliops/cronicle

@belsander
Copy link

@chngtrn you are always free to create a PR with the relevant part of your k8s config as an example. Much appreciated :)

@ckt114
Copy link

ckt114 commented Mar 1, 2019

@belsander I've added k8s manifests via pull request below.

#158

@lukasmrtvy
Copy link

Here is my docker image, based on linuxserver.io requirements:
https://github.com/lukasmrtvy/lsiobase-cronicle

@soulteary
Copy link

If someone wants to start Cronicle quickly and painlessly in the docker, you can try:

https://github.com/soulteary/docker-cronicle

I made a simple startup script that can make data reconstruction and migration in docker environment more easily.

And, no one needs to wait another 60 seconds before the initial initialization. ( Only Single Master Node Mode)

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

Successfully merging this pull request may close these issues.

7 participants