Only start rpc.statd if $NEED_STATD

Bug #1428486 reported by Martin Pitt
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
nfs-utils (Ubuntu)
Fix Released
Low
Didier Roche-Tolomelli

Bug Description

A remaining TODO item from the systemdification (bug 1312976) was to only start rpc.statd if /etc/default/nfs-common enables NEED_STATD=. This is a bit awkward with systemd and we generally avoid/move away from such "enable the unit" config files as they are redundant with update-rc.d enable/disable and systemctl enable/disable (which is the canonical way to enable/disable a unit).

We should probably just add a comment pointing to systemctl enable/disable, and make sure that this works as intended.

Martin Pitt (pitti)
Changed in nfs-utils (Ubuntu):
assignee: nobody → Martin Pitt (pitti)
status: New → Triaged
Martin Pitt (pitti)
Changed in nfs-utils (Ubuntu):
importance: Undecided → Medium
Revision history for this message
Steve Langasek (vorlon) wrote :

Rather than an advisory comment, I think the correct thing to do is to read any (non-default) value for NEED_STATD from /etc/default/nfs-common and do a one-time translation to the corresponding enable/disable operation.

Martin Pitt (pitti)
Changed in nfs-utils (Ubuntu):
assignee: Martin Pitt (pitti) → Didier Roche (didrocks)
tags: added: systemd-boot
Revision history for this message
Martin Pitt (pitti) wrote :

This is slightly complicated by the fact that rpc-statd.service (and the related rpc-statd-notify.service) don't have an [Install] and are not enabled/disabled separately, but instead are pulled in by nfs-server.service through an explicit Wants=.

This is perhaps worth a discussion with upstream: If statd isn't necessary with NFS v4, perhaps some other service (like nfs-mountd?) could check whether it needs statd and start it if needed? If it's not (reliably) possible to detect the need for it at runtime, we could also mimic the current /etc/defaults/ behaviour by dropping the Wants= from nfs-server.service, and either:

 - add a new ExecStartPre=/bin/sh -c ... which checks the value in the default file and calls "systemctl --no-block start rpc-statd.service rpc-statd-notify.service" if enabled

or

 - do an one-time migration to adding nfs-server.wants/ symlinks for rpc-statd.service and rpc-statd-notify.service iff statd is enabled. That's much harder to discover than a simple systemctl enable/disable, though.

Martin Pitt (pitti)
Changed in nfs-utils (Ubuntu):
importance: Medium → Low
Revision history for this message
Steve Langasek (vorlon) wrote :

I think the last option (one-time migration for nfs-server.wants) is the better solution in the near term. If an upstream solution can be found in the long term, that's great, but an upstream fix is unlikely for 15.04.

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

rather than preventing the user being able to systemct enable/disable, we have that option to follow that path:

1. patch to remove the Wants= in nfs-server.service and add an [Install] section to both rpc-statd.service and rpc-statd-notify.service with:
WantedBy=nfs-server.service
2. and doing the one time enablement (and sedding the config file as we did for others) + shipping an upstart override if not disabled

That way users would have the systemd tooling to enable/disable the service and we use the same thing for the one time enablement/disablement.

Making sense?

PS: I see the conffiles also have NEE_GSSD, should we handle it in a similar way?

Revision history for this message
Martin Pitt (pitti) wrote :

@Didier: Yes, that's pretty much what I had in mind. Note that "update-rc.d servicename enable|disable" already does all that (upstart override, systemctl enable/disable, etc.), so we should just call that?

> I see the conffiles also have NEED_GSSD, should we handle it in a similar way?

An empty value is "auto", and both the upstart job (shell commands in gssd-mounting.conf) and rpc-gssd.service (ConditionPathExists=/etc/krb5.keytab) do that "auto" detection. But indeed we don't currently check this value in the systemd jobs; I wonder if we need to? (e. g. RHEL/Fedora just use these systemd units). I. e. isn't the job of an init system to figure that out for you?

Steve, do you have an opinion on this? Do we need the ability to forcefully start gssd without krb, or forcefully suppress it with krb?

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

@Steve: so, as the transition is complex, in addition to my own tests of upgrade/removal/install directly the new version, I asked Martin for a review as well. Here is the patch.

I only took care about statd for now. Note that there are some subtilities from that transition is quite complex, especially as we are upgrading under upstart to enable sytemd or that some people are already on systemd and if we switched back to upstart later this cycle…). That's why you don't see me using update-rc.d on the upstart job (I have a fix on update-rc.d waiting on the debian BTS for quite some months to get reviewed, but I'm unsure it worthes it TBH), and the double lines in the conffiles to ease the transition.
This part should be dealt and works well.

However, we are quite unsure about rpc-statd versus rpc-statd-notify. Can you give a look at 27-systemd-enable-with-systemctl-statd.patch?
To mimick what we had, and the comment for nfs-client.target:
"# Note: we don't "Wants=rpc-statd.service" as "mount.nfs" will arrange to
# start that on demand if needed."
I added WantendBy=nfs-client.target in rpc-statd-notify.service.

On the nfs server side, we have rpc-statd which is WantedBy=nfs-server and Wants: rpc-statd-notify

As we both have no clue about statd on NFS, a double checking would be appreciated! That would result to enable statd:
- on the client: systemctl enable rpc-statd-notify (or pulled by rpc-statd if enabled by mount.nfs, so we can maybe remove the -notify dep?)
- on the server: systemctl enable rpc-statd

Does it make sense?

tags: added: patch
Revision history for this message
Steve Langasek (vorlon) wrote : Re: [Bug 1428486] Re: Only start rpc.statd if $NEED_STATD

On Thu, Mar 12, 2015 at 07:54:05AM -0000, Martin Pitt wrote:
> An empty value is "auto", and both the upstart job (shell commands in
> gssd-mounting.conf) and rpc-gssd.service
> (ConditionPathExists=/etc/krb5.keytab) do that "auto" detection. But
> indeed we don't currently check this value in the systemd jobs; I wonder
> if we need to? (e. g. RHEL/Fedora just use these systemd units). I. e.
> isn't the job of an init system to figure that out for you?

> Steve, do you have an opinion on this? Do we need the ability to
> forcefully start gssd without krb, or forcefully suppress it with krb?

In the previous sysvinit/upstart detection, the reason to support force
starting of gssd is to enable running gssd for an NFS mount that isn't
configured in /etc/fstab.

In the sysvinit/upstart detection - which was based on fstab entries and not
on the presence/absence of /etc/krb5.keytab - I can think of no reason to
want to suppress gssd from starting when it's started by autodetection. And
if someone wants to suppress gssd start under systemd, I think they should
use the 'update-rc.d' interface instead.

So I think these are both corner cases, and migrating the gssd setting from
/etc/default/nfs-common is a low priority.

Changed in nfs-utils (Ubuntu):
status: Triaged → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nfs-utils - 1:1.2.8-9ubuntu8

---------------
nfs-utils (1:1.2.8-9ubuntu8) vivid; urgency=medium

  * Ship missing .override files
  * Ensure we only remove the manual flag from the statd-mounting.override
    file, and potentially, remove it if empty.
 -- Didier Roche <email address hidden> Wed, 01 Apr 2015 08:22:59 +0200

Changed in nfs-utils (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.