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

Systemd service file for Turbinia server and worker #169

Merged
merged 7 commits into from Apr 12, 2018

Conversation

beamcodeup
Copy link
Contributor

Fix/Update init scripts #67

A new systemd service file for Turbinia. This will officially replace the scripts in turbinia/tools/gcp_init/.

Note: remember to delete the gcp init scripts in the future.

@beamcodeup beamcodeup requested a review from aarontp March 7, 2018 01:57
Copy link
Member

@aarontp aarontp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. A few comments in line.

Group=turbinia
Restart=always
RestartSec=10
ExecStartPre=/bin/sh -c 'sudo systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=/home/turbinia/turbinia-service-account-creds.json'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the right way to set environment variables in this case? I don't know systemd much, but a quick search shows that there's also an 'Environment' key that can be set.

Copy link
Contributor Author

@beamcodeup beamcodeup Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with the Environment key is that it needs to be hardcoded, which is why i'm using execstartpre.

Restart=always
RestartSec=10
ExecStartPre=/bin/sh -c 'sudo systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=/home/turbinia/turbinia-service-account-creds.json'
ExecStartPre=/bin/sh -c 'sudo systemctl set-environment TURBINIA_ROLE=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py ROLE)'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about the environment, but it looks like in this case this might be the way to go because it does need an exec?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What user will this run as? Does this mean that turbinia will need sudo privs for this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring to the User and Group attribute above, the processes will be ran as turbinia. However, in sudo, systemctl will run as root (which is required). Yes, turbinia will need sudo privs, which we have already set up in the /etc/sudoers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per Table 1 at https://www.freedesktop.org/software/systemd/man/systemd.exec.html, the prefix "+" set in any Exec*= denotes "the process is executed with full privileges". Therefore, sudo is not needed here. This means systemd will run Exec* as turbinia/turbinia unless they are denoted with special executiable prefixes. So, I applied the + prefix for ExecStartPre and ExecStartPost lines.

Group=turbinia
Restart=always
RestartSec=10
ExecStartPre=/bin/sh -c 'sudo systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=/home/turbinia/turbinia-service-account-creds.json'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should grab this filename from the config rather than hard-coding it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

RestartSec=10
ExecStartPre=/bin/sh -c 'sudo systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=/home/turbinia/turbinia-service-account-creds.json'
ExecStartPre=/bin/sh -c 'sudo systemctl set-environment TURBINIA_ROLE=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py ROLE)'
ExecStart=/bin/sh -c '/home/turbinia/turbinia-dev2/bin/python /home/turbinia/turbinia/turbiniactl -L /tmp/turbinia-$TURBINIA_ROLE.log -o /tmp $TURBINIA_ROLE 2>> /tmp/turbinia-$TURBINIA_ROLE.stdout.log'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the '/bin/sh -c' and the python wrappers here? Why can't we execute turbiniactl directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, without using /bin/sh, systemd complains that it cannot read this line because of the stderr symbol. To workaround, /bin/sh -c '' is used.

RestartSec=10
ExecStartPre=/bin/sh -c 'sudo systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=/home/turbinia/turbinia-service-account-creds.json'
ExecStartPre=/bin/sh -c 'sudo systemctl set-environment TURBINIA_ROLE=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py ROLE)'
ExecStart=/bin/sh -c '/home/turbinia/turbinia-dev2/bin/python /home/turbinia/turbinia/turbiniactl -L /tmp/turbinia-$TURBINIA_ROLE.log -o /tmp $TURBINIA_ROLE 2>> /tmp/turbinia-$TURBINIA_ROLE.stdout.log'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to set the -o flag with a variable from the config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I overlooked this one. the value for -o will be referenced to TMP_DIR in the config.

Done.

@beamcodeup
Copy link
Contributor Author

@aarontp PTAL!

@beamcodeup
Copy link
Contributor Author

beamcodeup commented Mar 20, 2018

After testing this extensively, I believe it would make more sense to have two seperate systemd services for, respectively, server and worker(s). Systemd supports Instantiated Services, which is great for workers per this doc. This overall approach would allow us to run both server and worker(s) daemons on the same machine. Currently working on this now.

@beamcodeup
Copy link
Contributor Author

With systemd 236 or newer we can write directly to a file using StandardError=file:/some/path systemd/systemd#7198 instead of using stdout/stderr symbols in ExecStart. Unforutnately, the stable Debian 9.3 is shipped with systemd 232, per https://packages.debian.org/stretch/systemd.

systemd 232 supports:
inherit, null, tty, journal, syslog, kmsg, journal+console, syslog+console, kmsg+console or socket

For now, we can use the syntax /bin/sh -c ".... >> ...." for ExecStart. We could explore the use of syslog in the future for this project.

@aarontp
Copy link
Member

aarontp commented Apr 10, 2018

LMK if you're waiting for anything from me on this. AFAIK we still want to split this into two init scripts for server and worker with most everything in a template. I think we talked about changing the ROLE config var into something like SERVER and PSQWORKER booleans so that we could run both roles on the same host. Thanks!

@beamcodeup
Copy link
Contributor Author

Systemd supports service instantiations from a single template file. So, this service file will serve as a template from which two units will be instantiated as "server" and "psqworker". This will eliminate the need of SERVER/PSQWORKER booleans.

@beamcodeup
Copy link
Contributor Author

@aarontp PTAL

Restart=on-abnormal
RestartSec=10
RestartPreventExitStatus=100 101 102
ExecStartPre=+/bin/sh -c 'GOOGLE_APPLICATION_CREDENTIALS=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py GCE_SERVICE_ACCOUNT_KEYS_FILE) && /bin/systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=$GOOGLE_APPLICATION_CREDENTIALS'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to remove the '/home/turbinia/turbinia_config_parser.py' hard-coded path here and assume that the script is somewhere in the $PATH, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. It was a bad copy and paste. Fixed.

Restart=on-abnormal
RestartSec=10
RestartPreventExitStatus=100 101 102
ExecStartPre=+/bin/sh -c 'GOOGLE_APPLICATION_CREDENTIALS=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py GCE_SERVICE_ACCOUNT_KEYS_FILE) && /bin/systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=$GOOGLE_APPLICATION_CREDENTIALS'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this effectively sets the env var twice here. I guess the second one is for the global systemd settings, why do we need both? Is that used for other calls in this script? I guess what I'm asking is why can't we just do:
ExecStartPre=+/bin/sh -c '/bin/systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=$(/usr/bin/python turbinia_config_parser.py GCE_SERVICE_ACCOUNT_KEYS_FILE)'

Same comment for the other env vars.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key difference between those lines is which will return the exit code to systemd. In your proposed line, systemd will receive the exit code from systemctl (which is 2) if the parser failed with any exit code (standard codes, including our customized ones like 100-102). Based on our chat offline, those custom exit codes and RestartPreventExitStatus are not needed and that Restart=always. The only withdraw is that if the parser failed, the systemd will restart in loop. This can be addressed seperately with some kind of monitoring/alerting So, rolling this back.

RestartPreventExitStatus=100 101 102
ExecStartPre=+/bin/sh -c 'GOOGLE_APPLICATION_CREDENTIALS=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py GCE_SERVICE_ACCOUNT_KEYS_FILE) && /bin/systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=$GOOGLE_APPLICATION_CREDENTIALS'
ExecStartPre=+/bin/sh -c 'TURBINIA_TMPDIR=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py TMP_DIR) && /bin/systemctl set-environment TURBINIA_TMPDIR=$TURBINIA_TMPDIR'
ExecStart=/bin/sh -c '/usr/local/turbinia/turbiniactl -L $TURBINIA_TMPDIR/turbinia-%i.log -S -o $TURBINIA_TMPDIR %i 2>> $TURBINIA_TMPDIR/turbinia-%i.stdout.log'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this exec line will work for both server types, but if we ever want to change the flags per service, is there a good way to do that? Just curious for the future since I don't know systemd very well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's best to keep the service file simple that we do not need to change it very often.

There are several options to address your question. Will we foreseen any major additional arguments for both server types?

  1. Pass args via %I

sudo systemctl start turbinia@"server arg1 arg2 arg3"

The service file would have something like:
Environment="ARGS=%I"
ExecStart=/usr/local/foo $ARGS

  1. Create two seperate service files

  2. Create a local file that contains args:
    ARG1=-L
    ARG2=-o

The service file would have:
EnvironmentFile=/this/local/args.conf
ExecStart=/usr/local/foo $ARG1 $ARG2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 more option.

  1. Manually override command arguments for an instantied service without editing the template file directly. ExecStart would have to be cleared then set like this:

systemctl edit turbinia@psqserver

then an empty text file opens up then add:

[Service]
ExecStart=
ExecStart=/bin/sh -c '/usr/local/turbinia/turbiniactl --new-flags foobar'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for the response here. I don't have any particular flag changes that I foresee at the moment, but I think it's likely that they will change at some point, so I just wanted to understand what our options were.

@beamcodeup
Copy link
Contributor Author

PTAL

Copy link
Member

@aarontp aarontp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

RestartPreventExitStatus=100 101 102
ExecStartPre=+/bin/sh -c 'GOOGLE_APPLICATION_CREDENTIALS=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py GCE_SERVICE_ACCOUNT_KEYS_FILE) && /bin/systemctl set-environment GOOGLE_APPLICATION_CREDENTIALS=$GOOGLE_APPLICATION_CREDENTIALS'
ExecStartPre=+/bin/sh -c 'TURBINIA_TMPDIR=$(/usr/bin/python /home/turbinia/turbinia_config_parser.py TMP_DIR) && /bin/systemctl set-environment TURBINIA_TMPDIR=$TURBINIA_TMPDIR'
ExecStart=/bin/sh -c '/usr/local/turbinia/turbiniactl -L $TURBINIA_TMPDIR/turbinia-%i.log -S -o $TURBINIA_TMPDIR %i 2>> $TURBINIA_TMPDIR/turbinia-%i.stdout.log'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for the response here. I don't have any particular flag changes that I foresee at the moment, but I think it's likely that they will change at some point, so I just wanted to understand what our options were.

@aarontp aarontp merged commit ca58f58 into google:master Apr 12, 2018
aarontp pushed a commit that referenced this pull request Jun 29, 2018
* Systemd service file for Turbinia server and worker

* replace hardcoded lines with config parsers

* replacing more hardcoded lies

* add a line to prevent the turbinia service from auto restarting infinitely if the parser returned error codes

* turbina service units will be based on the template file

* Change to restart=on-abnormal

* remove custom exit codes, set restart as always, remove local vars
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.

None yet

2 participants