Skip to content

allow 'systemctl enable' and ensure start after local and remote fs #121

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.

Sign up for GitHub

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

Open
aflyhorse wants to merge 2 commits into
base: master
Choose a base branch
from
+16 −0
Open

allow 'systemctl enable' and ensure start after local and remote fs #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9c6b73
allow 'systemctl enable' and ensure start after local and remote fs
aflyhorse Mar 3, 2022
0baa347
add comments to indicate alternative config options
aflyhorse Mar 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions scripts/robinhood.service.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
[Unit]
Description=Robinhood server
#only works if config file is unique
After=local-fs.target remote-fs.target
Copy link
Copy Markdown
Collaborator

@martinetd martinetd Mar 4, 2022

Choose a reason for hiding this comment

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

Hmm, this is a tough one.
On one hand it's too broad - we'd to better with a "RequiresMountsFor" with the filesystem paths required, so filesystems we don't care about being slow or fickle don't impact robinhood startup.
On the other hand just after isn't strong enough: if the remote filesystems failed to mount, robinhood will be started anyway, but with a broad "all filesystems" requirement we don't want to stop robinhood from starting if some useless filesystem had a problem.

I don't think we can have a solution for everyone here honestly -- I guess this is "good enough" as a middle ground, with perhaps a comment saying it can be replaced in a drop-in config by RequiresMountsfor=/mnt/lustre ?

Copy link
Copy Markdown
Author

@aflyhorse aflyhorse Mar 7, 2022

Choose a reason for hiding this comment

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

I installed via the RPMs so normally I won't modify installed service file. I added these lines to ensure robinhood won't start before the mount and terminate itself.
I'll append a suggestion comment.

Copy link
Copy Markdown
Collaborator

@martinetd martinetd Mar 7, 2022

Choose a reason for hiding this comment

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

Thanks for the comment!

Let's not suggest something as barbaric as modifying this file in place: suggest systemctl edit robinhood.service (or robinhood@smth.service) with the following content:

[Unit]
After=
After=local-fs.target
RequiresMountsFor=/path/to/scan

The first After resets previous value, the second restores local-fs which is probably needed anyway, and the last adds proper requirements for only the fs we care about.

(If I was true to myself I'd say we should make this into a systemd generator so conditons could be rewritten dynamically based on the EnvironmentFiles, but that would be overdoing it...)

Requires=local-fs.target
#to speedup the starting, remove the above two lines,
# then uncomment and modify the following 'RequiresMountsFor' directive
#RequiresMountsFor=/path/to/scan

[Service]
Type=simple
Expand All @@ -9,3 +14,6 @@ EnvironmentFile=-@CONFDIR@/sysconfig/robinhood
LimitNOFILE=8096
ExecStart=@SBINDIR@/robinhood $RBH_OPT
ExecReload=/bin/kill -HUP $MAINPID

[Install]
WantedBy=multi-user.target
8 changes: 8 additions & 0 deletions scripts/robinhood@.service.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
[Unit]
Description=Robinhood server for %I
After=local-fs.target remote-fs.target
Requires=local-fs.target
#to speedup the starting, remove the above two lines,
# then uncomment and modify the following 'RequiresMountsFor' directive
#RequiresMountsFor=/path/to/scan

[Service]
Type=simple
Expand All @@ -9,3 +14,6 @@ EnvironmentFile=-@CONFDIR@/sysconfig/robinhood.%I
LimitNOFILE=8096
ExecStart=@SBINDIR@/robinhood $RBH_OPT -f @CONFDIR@/robinhood.d/%I.conf
ExecReload=/bin/kill -HUP $MAINPID

[Install]
WantedBy=multi-user.target