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

Use a configuration file for Canto's init script #2478

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jseager7
Copy link
Collaborator

(Fixes #2476)

This PR adds a configuration file for the canto-docker-initd script. The configuration file is at etc/canto.defaults.

On the server, the configuration file is copied to /etc/default/canto. The init script will source the file and use any (uncommented) variables in the file to override the default values for variables in the script. Currently the file can override Canto's port number, base directory, number of workers for Server::Starter, and the path to the PID file for Server::Starter (although technically it can override any named variable).

I've removed the intermediary example script that only existed to run the actual service script (formerly at etc/example-canto-docker-init.d) since there's no need to pass arguments into the script now. Note that the default values in canto-docker-initd are not the same as in the example script, specifically the port number. I hope this doesn't affect any other users of Canto, like FlyBase.

I've updated the documentation at etc/canto-init.d.md with instructions for the new configuration file, and instructions for installing the init script using systemd.

@kimrutherford You might want to double-check the documentation for any mistakes or omissions. I've tested the script with the configuration file on a virtual machine and it all seems to work fine.

One other thing we might want to resolve at this point is the differences between the PomBase init script and this init script. For example, the PomBase script has an additional environment variable, DBIC_TRACE=0, passed into the command that runs Server::Starter, and it also uses tee for the log file instead of appending:

2>&1 | tee -a $CANTO/logs/$app.log &

Should any of these changes be applied to this init script? I'm guessing not, since this script was authored later.

@jseager7 jseager7 self-assigned this Jun 30, 2021
@kimrutherford
Copy link
Member

Thanks James. That all looks great. And thanks for the thorough documentation.

@vmt25 Vitor do you use the canto-docker-initd script from the Canto repository or have you got your own copy? It's this file that I'm thinking about: https://github.com/pombase/canto/blob/master/etc/canto-docker-initd

James has done work to tidy it up and make it more reusable. Will changing cause any problems for you?

@vmt25
Copy link

vmt25 commented Jul 9, 2021

Thanks James. That all looks great. And thanks for the thorough documentation.

@vmt25 Vitor do you use the canto-docker-initd script from the Canto repository or have you got your own copy? It's this file that I'm thinking about: https://github.com/pombase/canto/blob/master/etc/canto-docker-initd

James has done work to tidy it up and make it more reusable. Will changing cause any problems for you?

Hi Kim,

We use an edited version of canto-docker-initd script, which just has a different value (location) for the CANTO_SPACE variable.
I don't really know how this would affect our stuff until I try to better understand what the changes imply... and maybe test on a local test instance. Apologies @jseager7, @kimrutherford but I am not an IT person/coder and this is a bit confusing.

Could you please confirm whether my understanding of the changes is correct:
'canto-docker-initd' would incorporate the default port as 5000, and no defined log for 'stop' (and no instructions for restart?).
But isn't 'canto-docker-initd' executing 'canto_for_etc-initd'? I assume canto_for_etc-initd would cease to exist because plenty of it has been transfered canto-docker-initd, right?
And then 'etc/canto.defaults' would have all configurable information used by 'canto-docker-initd'

The bigger issue is that the setup of how canto is run by the system was done by another person and I don't really know how your systemd setup would play out.

Apologies if some/all of this does not make much sense but, like I said, I am not a IT person/coder

@kimrutherford
Copy link
Member

Hi Vitor. If you are happy to have your own copy of the scripts then you could just ignore these changes.

I assume canto_for_etc-initd would cease to exist because plenty of it has been transfered canto-docker-initd, right?
And then 'etc/canto.defaults' would have all configurable information used by 'canto-docker-initd'

Yep. The main thing James has done is to combine the two scripts into one. That makes sense because one of them does very little except for executing the other script.

So James has removed etc/example-canto-docker-init.d and moved the contents to etc/canto-docker-initd.

Probably the new versions from James would work on your server after editing the new settings file but if your current script works for you there is probably no reason to change. I'm very happy to have a Skype chat about this sometime.

@jseager7
Copy link
Collaborator Author

jseager7 commented Jul 12, 2021

@vmt25 Just to add a bit more in response:

We use an edited version of canto-docker-initd script, which just has a different value (location) for the CANTO_SPACE variable.

Under this new system, you would need to set this location value in /etc/default/canto (which would be the path to the canto.defaults file on your server). In that file, you would specify the path to the canto-space directory on the line with the CANTO_SPACE variable. You could probably just copy the custom configuration from your current script file to the canto.defaults file. You're correct that you wouldn't need to edit the script file (canto-docker-initd) to customise its configuration under this new system.

'canto-docker-initd' would incorporate the default port as 5000, and no defined log for 'stop' (and no instructions for restart?).

The new service script defaults to port number 5000, but the port number can be configured in the canto.defaults file by setting a value for the PORT variable. If you already use port number 5000, then you can use the new script as-is.

I'm not sure I understand the part about "no defined log for 'stop' (and no instructions for restart?)". The new script logs output to a file called canto.log in whatever directory is specified for CANTO_SPACE (which is the same behaviour as the old script). If you're wondering about the stop and restart commands, those can still be issued to the new script the same as before.

The bigger issue is that the setup of how canto is run by the system was done by another person and I don't really know how your systemd setup would play out.

The new script doesn't require systemd; the script is just a plain init script (technically a SysV-style init script). If your team followed the previous instructions for installing the service using update-rc.d, the new script should work just the same. The documentation still has instructions for how to set up the script with update-rc.d. It just so happens that systemd has backwards compatibility for SysV-style init scripts, so the script will work with either system.

@jseager7
Copy link
Collaborator Author

@kimrutherford I think there might be a typo in the default path for CANTO_SPACE in the init script. The scripts and documentation has /var/canto-space, but our server has /var/canto_space. Which of these is the 'correct' name? Does PomBase use the version with a hyphen?

@kimrutherford
Copy link
Member

Which of these is the 'correct' name? Does PomBase use the version with a hyphen?

PomBase doesn't use Docker so we don't have a canto_space or canto-space. I'm happy with either. I have email from Vitor mentioning canto_space so maybe that the one to use?

@jseager7
Copy link
Collaborator Author

I have email from Vitor mentioning canto_space so maybe that the one to use?

That sounds like a sensible default. I guess underscores are more conventional than hyphens for directory names. I'll amend the pull request.

@jseager7
Copy link
Collaborator Author

@kimrutherford One more question: I've just checked the documentation for Starman, and it mentions that Starman uses the QUIT signal "to gracefully shutdown the worker processes". However, canto-docker-initd sends the TERM signal through docker when the service script receives the stop command:

  stop)
    pid=`/bin/cat $CANTO_SPACE/$PID_PATH`
    echo stopping $pid
    (cd $CANTO_SPACE; docker exec canto kill -TERM $pid)
    ;;

Does this cause different behaviour, or would QUIT and TERM be functionally equivalent in this case? I'm guessing that a graceful shutdown doesn't necessarily mean that Canto instances would wait to complete ongoing HTTP requests before shutting down given either signal, but please let me know if I'm wrong.

kimrutherford added a commit to kimrutherford/canto that referenced this pull request Jul 27, 2021
@kimrutherford
Copy link
Member

Does this cause different behaviour, or would QUIT and TERM be functionally equivalent in this case?

Good catch! The documentation says they are different. I've just read the docs (twice) and I think the change we need is to add --signal-on-hup=QUIT --signal-on-term=QUIT to the start_server command. We can also remove --preload because I don't think it does anything: canto_start does the preloading itself.

I've made a PR with my suggested change: jseager7#7

@jseager7
Copy link
Collaborator Author

@kimrutherford I think this PR is almost ready now. I've just noticed one more thing from testing on our demo server: the canto.log file (which the script is supposed to redirect output to) doesn't exist in my canto_space directory on the host filesystem.

I assume that there should be something logged there, since the command that runs Canto should at least append the date to the file:

(date; cd $CANTO_SPACE; canto/script/canto_docker --non-interactive --use-container-name start_server --pid-file=/$PID_PATH --port $PORT --signal-on-hup=QUIT --signal-on-term=QUIT -- script/canto_start --workers $WORKERS --keepalive-timeout 5 -s Starman) >> canto.log 2>&1 &

Do I need to touch the file before redirecting to it? Or is the file somewhere other than where I'm expecting?

@kimrutherford
Copy link
Member

The log file might be ending up somewhere unhelpful. You can probably find it with:

  sudo find / -name 'canto.log'

which will search the whole filesystem.

Do I need to touch the file before redirecting to it?

It should create the file or fail if it doesn't have the correct permissions.

Or is the file somewhere other than where I'm expecting?

I think so. It's probably in the home directory of the user running the script. Which is probably root. :-/

To control this better you could move the >> canto.log 2>&1 inside the brackets like:

(date; cd $CANTO_SPACE; canto/script/canto_docker --non-interactive --use-container-name start_server --pid-file=/$PID_PATH --port $PORT --signal-on-hup=QUIT --signal-on-term=QUIT -- script/canto_start --workers $WORKERS --keepalive-timeout 5 -s Starman >> canto.log 2>&1) &

which will put the log in $CANTO_SPACE (untested)

Or give a full path to the log file:

(date; cd $CANTO_SPACE; canto/script/canto_docker --non-interactive --use-container-name start_server --pid-file=/$PID_PATH --port $PORT --signal-on-hup=QUIT --signal-on-term=QUIT -- script/canto_start --workers $WORKERS --keepalive-timeout 5 -s Starman) >> $CANTO_SPACE/canto.log 2>&1 &

(also untested)

I don't have a preference as to which is best.

@jseager7
Copy link
Collaborator Author

find / -name 'canto.log' revealed that the log file is at the root of the file system. Strange.

I'd opt for providing an absolute path to the log file, like the second option you suggested. I'll test this locally and then commit here once it's working.

@jseager7
Copy link
Collaborator Author

Looks like the log file is working with an absolute path. Now I can also see the handling of the new signal flags:

received TERM, sending QUIT to all workers:6
2021/07/28-12:12:00 Received QUIT. Running a graceful shutdown
Sending children hup signal
2021/07/28-12:12:11 Worker processes cleaned up
2021/07/28-12:12:11 Server closing!
worker 6 died, status:0
exiting

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.

Use /etc/default/ to source configuration for Canto init script
3 participants