ome / omero-py

Python project containing Ice remoting code for OMERO
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
20 stars 33 forks source link

admin.py: optionally check for system manager environment variable #246

Closed manics closed 4 years ago

manics commented 4 years ago

Adds a config property that can be set to the name of an environment variable that is only set when running under systemd or some other service manager. omero admin start|stop will check this variable and abort if it's not set.

Example using systemd

If you have systemd v232+ the INVOCATION_ID environment variable should be set by systemd so you can check for this https://serverfault.com/a/927481

$ cat /opt/omero/server/config/10-systemd.omero
config set omero.server.servicemanager.checkenv INVOCATION_ID

If you have an older version of systemd you can define your own non-empty variable in your systemd service file:

[Service]
User=omero-server
UMask=0002
Type=forking
PIDFile=/opt/omero/server/OMERO.server/var/master/master.pid
Restart=no
RestartSec=10
+Environment=CHECK_FOR_THIS_VARIABLE=something
# Allow up to 5 min for start/stop
TimeoutSec=300
ExecStartPre=/opt/omero/server/OMERO.server/bin/omero load --glob /opt/omero/server/config/*.omero
ExecStart=/opt/omero/server/OMERO.server/bin/omero admin start
ExecStop=/opt/omero/server/OMERO.server/bin/omero admin stop
$ cat /opt/omero/server/config/10-systemd.omero
config set omero.server.servicemanager.checkenv CHECK_FOR_THIS_VARIABLE

If you then attempt to start/stop omero outside systemd:

[vagrant@omero ~]$ sudo systemctl start omero-server
[vagrant@omero ~]$ sudo -u omero-server /opt/omero/server/OMERO.server/bin/omero admin stop
ERROR: OMERO is configured to run under a service manager but CHECK_FOR_THIS_VARIABLE is not set
[vagrant@omero ~]$ sudo -u omero-server /opt/omero/server/OMERO.server/bin/omero admin restart
ERROR: OMERO is configured to run under a service manager but CHECK_FOR_THIS_VARIABLE is not set

Note since the omero configuration is updated from /opt/omero/server/config/*.omero you must've started omero at least once with systemd after updating the configuration.

I'll document this property in https://github.com/ome/openmicroscopy/blob/develop/etc/omero.properties if everyone is happy with this.

Includes a test, though I had to put it in a new file because test_admin.py contains a global autouser fixture that requires OMERODIR, which means it's not tested on Travis.

A similar property can be added to omero web. Since these are decoupled I don't think they should share the same property, instead it would be omero.web.servicemanager.checkenv.

@stick

Note this was updated so the property is now omero.server.servicemanager.checkenv, not omero.admin.servicemanager.checkenv

joshmoore commented 4 years ago

Barring the introduction of the new class name suffix (Mixin) which I may have caused, this generally looks good to me. I haven't had a change to test yet. What would be your first targets for this?

manics commented 4 years ago

I went for a mixin because it only provides a utility method for internal plugin use, it didn't seem right to construct a full Control object. Need to decide between consistency vs gradual improvement.

Once this is released I'd start by adding this to our OMERO playbooks through these role vars:

And similarly for OMERO.web

If there are no unforeseen issues we can consider making this mandatory in a future release of the roles.

will-moore commented 4 years ago

Does $ sudo systemctl start omero-server set or unset the environment variable? or neither?

Does bin/omero admin stop fail because you started via systemctl or because CHECK_FOR_THIS_VARIABLE is not set in that environment?

[vagrant@omero ~]$ sudo -u omero-server /opt/omero/server/OMERO.server/bin/omero admin stop
ERROR: OMERO is configured to run under a service manager but CHECK_FOR_THIS_VARIABLE is not set

If you did set $ export CHECK_FOR_THIS_VARIABLE=foo would $ omero admin stop then work? That's kinda how the ERROR reads.

manics commented 4 years ago

The latter. There's no easy way to tell whether omero was started by systemd (or any other service manager), so the idea of this workaround is to set a variable in the systemd config that wouldn't otherwise be set in a normal environment. You're correct that if you manually set that variable you'd bypass the check in omero admin.

will-moore commented 4 years ago

How about:

ERROR: OMERO is configured to run under a service manager which should also set CHECK_FOR_THIS_VARIABLE
will-moore commented 4 years ago

Good, thanks.

manics commented 4 years ago

If everyones happy with this it'd be nice to include it in https://github.com/ome/omero-py/pull/248 but it's not essential.

joshmoore commented 4 years ago

If everyones happy with this

WFM. :+1: