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

Daemon/Init/Start/Stop to keep brewpi script alive #16

Open
elcojacobs opened this issue Oct 12, 2013 · 18 comments
Open

Daemon/Init/Start/Stop to keep brewpi script alive #16

elcojacobs opened this issue Oct 12, 2013 · 18 comments
Assignees

Comments

@elcojacobs
Copy link
Member

This is a summary of what has been discussed in the IRC channel and a list of requirements for starting/restarting brewpi.

The old way of doing things (master)

  • The python script brewpi.py is started by the brewpi user's crontab, using this line:
* * * * * python -u /home/brewpi/brewpi.py --dontrunfile 1>/home/brewpi/logs/stdout.txt 2>>/home/brewpi/logs/stderr.txt &

This starts the script unbuffered (-u) and redirects the output to log files.

  • The web interface can stop the script by sending a message over the socket. To prevent CRON from restarting it, it writes a flag file (dontrunfile).
  • The dontrunfile is checked by the script at startup and it exits when it is found.
  • CRON starts the script every minute.
  • At startup, the script uses the python module psutil to check for other running instances. When a conflicting instance is found, it quits. To find conflicts BrewPiProcess.py compares the config's of both scripts to identify conflicts

Motivation

The web server runs as the www-data user and it should not be able to access/execute anything outside of it's own dir and/or usergroup. Stopping the script is no problem, because brewpi is listening on the socket. To start it, the www-data user can just remove the file and wait for the cron job to restart the script as the brewpi user. This keeps the permissions for the www-data user limited.
The brewpi user is a member of the www-data group, but not the other way around.

Limitations

  • Starting the script takes up to a minute. The script is also not restarted immediately after a crash.
  • Modifying the crontab is hard to version control. The current code in develop moved towards using cron.d to solve this issue.
  • psutil does not work on mac. It needs root privileges, but sudo is not transfered to python submodules. So even starting the process with sudo does not work.
  • Each time the cron job executes, it overwrites stdout.txt. This is a bug. I added a command line option to log to files and let python handle redirecting stdout to the file after the startup checks, but I was told this was dirty. Mdma offered a different solution here m-mcgowan@ee503a1 by separating the dontrunfile check from the rest of the code.

The main requirements for the daemon are:

  • When the script crashes, it is restarted automatically (watchdog). It is responsible for logging data and following the temperature profile. A crash could have several causes (hardware fault, communication error, bug in code). Ideally, we except all of them, but we are all programmers that drink a lot ;)
  • BrewPi is started at boot
  • The brewpi service can be started/stopped.
    • This is the main point of discussion. Who should be able to start/restart brewpi?
    • Starting/stopping the script in the web interface might not be a requirement anymore, with the introduction of the feature to stop/pause logging (in develop). When data logging is stopped, brewpi only logs to stderr and stdout files.
    • We need to stop/restart BrewPi for updates
    • Developers need to be able to prevent brewpi from restarting automatically. (eg. sudo service brewpi stop)
  • Changes to the watchdog/init process should be easy to version control
  • Multiple conflicting instances of brewpi should be prevented.
  • Perhaps take into account future implementation of brewpi: In the future, we are likely to ditch apache and to integrate the webserver into python. BrewPi will consists of two (or 3) python scripts, of which the webserver is always running. A likely implementation is that each Arduino is wrapped by a driver script. Another process communicates with these drivers. The data logger could be a separate process as well.

Our options so far:

So now we have a place to discuss this properly. I don't know much about this topic, so your opinions are very welcome.

@ghost ghost assigned elcojacobs and bschwinn Oct 12, 2013
@vanosg
Copy link
Collaborator

vanosg commented Oct 12, 2013

The brewpi service can be started/stopped.
** Do we really need it to be stopped? Or just restarted/reloaded? What reason does a user have to stop brewpi? See bullet #4 below

Who should be able to start/restart brewpi?
** Why do we care 'who' should be able to perform these functions? Why would one type of user not be able to perform this function?

We need to stop/restart BrewPi for updates
** Again, do we really need to stop it? (Not that I can see) A restart would suffice for this.

Developers need to be able to prevent brewpi from restarting automatically. (eg. sudo service brewpi stop)
** We generally don't code in production lines to meet development needs- we instead use test frames and other external development scripts to meet development needs. I strongly recommend separating development needs from user needs, and focusing on the user piece. We can meet dev needs in different ways that should not exist in the production code.

Multiple conflicting instances of brewpi should be prevented.
** This code is currently contained within BrewPi, and I think it should stay there. (I can elaborate more if wanted)

Perhaps take into account future implementation of brewpi: In the future, we are likely to ditch apache and to integrate the webserver into python. BrewPi will consists of two (or 3) python scripts, of which the webserver is always running. A likely implementation is that each Arduino is wrapped by a driver script. Another process communicates with these drivers. The data logger could be a separate process as well.
** This is a seperate issue from this topic, and probably shouldn't be discussed in this thread. Or does it relate that I'm not seeing? If you move to a python-based web server, it should be seperate from brewpi code totally, just like apache is now (I disagree with moving to a python server anyway, but like I said, thats a topic for another thread :) )

@bschwinn
Copy link
Collaborator

asked a couple sys admins what they thought, they recommended daemontools:

http://cr.yp.to/daemontools.html

might be worth trying to compile for the pi, author declares it public domain:
http://cr.yp.to/distributors.html

-b

@vanosg
Copy link
Collaborator

vanosg commented Oct 13, 2013

Daemontools is definitely a solid program, but it requires another install process for the user to perform. Great for a sysadmin, but tough to distribute

What comments do you have on the requirements?

@elcojacobs
Copy link
Member Author

Why do we care 'who' should be able to perform these functions? Why would one type of user not be able to perform this function?

I was thinking about root/brewpi/www-data. Starting/stopping daemons might need root access. If we decide to have a script always running, it does not really matter. It can be stopped in ssh with sudo.

I think splitting this in a script that manages what runs and the actual program might be the most flexible way forward. This script could read lock files and start the application script (perhaps with start-stop-daemon).

Running a separate script as a watchdog gives us more freedom than using inittab on the application script directly. We could even have a windows and mac alternative for this script.

daemontools is on apt-get, so why would it be a problem?

@vanosg
Copy link
Collaborator

vanosg commented Oct 14, 2013

We keep shooting to discussing solutions, before we fully understand the problem. Got your rationale on user access levels, but we can we flesh out if we really ever need to stop the script?

@rparenton
Copy link

Here are a couple of reasons that the script should be able to be stopped:

  • System maintenance, such as replacing an Arduino. Although you could restart the script after doing so, it's cleaner to stop the script when doing something disruptive.
  • The RPi in use isn't dedicated to BrewPi use. This may not be that common, but it's a reasonable scenario.

It's also bad form to not provide an easy way to stop a daemon.

@elcojacobs
Copy link
Member Author

I agree with Robbert here. If we choose not to stop the script through the web interface (which is simple, but should be omitted when we don't have a way to start it through the web interface probably, which is not simple).

I think the requirements can be simplified:

  • Sudo users can stop (and start) the daemon. (for maintenance).
  • When the daemon is enabled, the script is considered always running.
  • Application script can restart the script (but not stop the daemon). This can be done in python, similar to how the programming script is started. This can be triggered from the web interface.

So let's get this out of the way first: do we want to get rid of the ability to start/stop the script from the web interface?

@rparenton
Copy link

I think the requirements Elco listed are reasonable and that there isn't a need to be able to start/stop the script from the web interface. Anybody doing the type of maintenance (or anything else) where they would want to stop the script would most likely have sudo access.

@elcojacobs
Copy link
Member Author

I postpone this to next release, so we can properly develop this in a feature branch. There has been enough feature creep. This release will be on cron.d with m-mcgowan@ee503a1 m-mcgowan@7c9feee (no idea why the reverse commit is there)

I don't want to stop the discussion here, but let's not hold up the release any longer.

@elcojacobs
Copy link
Member Author

Temporary fix with cron.d done:
c7a62d3

@vanosg
Copy link
Collaborator

vanosg commented Oct 18, 2013

Just remember, if you release with cron.d, we'll have to add in a check to remove it, should we go a different way in the future ;)

@elcojacobs
Copy link
Member Author

I know, but it doesn't seem like we will resolve this issue soon. There has
been too much feature creep already and the release has been delayed for
too long.
On Oct 18, 2013 2:20 AM, "vanosg" [email protected] wrote:

Just remember, if you release with cron.d, we'll have to add in a check to
remove it, should we go a different way in the future ;)


Reply to this email directly or view it on GitHubhttps://github.com//issues/16#issuecomment-26563967
.

@bschwinn
Copy link
Collaborator

looking into supervisord...

@elcojacobs
Copy link
Member Author

cron job in cron.d works fine for now. In a next version with a new service layer we might reconsider. Issue closed for now.

@m-mcgowan
Copy link
Contributor

Elco, shouldn't this be reopened and put in the backlog? Maybe some new tags for version numbers, and a tag for general backlog.

@elcojacobs elcojacobs reopened this Feb 9, 2014
@elcojacobs
Copy link
Member Author

Re-opened for discussing running BrewPi as a service.

@vanosg
Copy link
Collaborator

vanosg commented Feb 12, 2014

daemontools is indeed packaged under aptitude now (boy am I getting old), so that clears the first big hurdle I saw. Additionally, daemontools:

  • gives each service its own directory/file structure, as opposed to adding lines to a single file, making it easy for us to 'blow away' a previous version, versus searching for relevant lines
  • has automated restarting
  • allows granular start, stop, and restart options

like bswchinn, several admins I spoke with say this is the 'new hotness' and cannot think of anything that another service monitor offers that daemontools does not; but dameontools has it all.

Plus, init.d is going away in debian, to be replaced by systemd. I don't know a lot about this, but it seems the community is widely against this. So, that leads to a potential lack of support in the future for either init.d or systemd.

@vanosg
Copy link
Collaborator

vanosg commented Feb 21, 2014

Per discussion in channel, here's the proposed implementation plan for Daemontools:

Install/Upgrade scripts

(changes will be made appropriately in each of these scripts to install DT/daemon)

  • Add daemontools, daemontools-run to apt-get install list
  • add brewpi/ to /services/
  • create run files (from context line of brewpi.cron) to start at startup
  • reboot
  • delete cron.d entry (upgrade)

brewpi.py

brewpi.py will be able to issue the stop and (re)start commands, in place of writing the dontrunfile.

  • Update commandline arg names (donotrun to stop, etc)
  • update commandline help
  • modify messagetype = stopScript to stop brewpi via daemontools
  • clean up dontrunfile checks, such as checkDontRunFile
  • delete checks for other instances of brewpi (Don't think we need this anymore? I suppose its still possible the user could manually try, so perhaps we still need it? or do we could chmod it so they cant "by accident"...)
  • delete checkstartuponly

?? remove checkrunning.sh ??

brewpiProcess.py

  • Update stopAll function

brewpi-www

  • Mod stop/start script stuff... not sure here

Modules

  • We are currently using cron.d to run modules. Daemontools could work for modules that need to run all the time, but not for recurring task-based modules, such as wifichecker. If we want to go this route, we would need to require that a module able to run standalone, 'always on', using 'sleep', 'at', or something similar to execute the interval-based tasks.

We could still use cron.d/module.cron for scheduled tasks like this, but we again risk spreading code out all across the OS. Not necessarily a bad thing, but it becomes more cumbersome for us, as software, to track.

In summary

This is my first stab at changes that could be made. I don't know the guts of brewpi as well as others, what other ramifications would there be that need to be thought of?

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

No branches or pull requests

5 participants