Re: [PATCH 1/2] nfs-service: Added the starting of gssproxy

[Steve Dickson <SteveD@redhat.com>]

On 09/22/2014 09:55 PM, J. Bruce Fields wrote:
> On Mon, Sep 22, 2014 at 08:26:55PM -0400, Simo Sorce wrote:
>> On Mon, 22 Sep 2014 19:58:05 -0400
>> Steve Dickson <SteveD@redhat.com> wrote:
>>
>>>
>>>
>>> On 09/22/2014 06:34 PM, J. Bruce Fields wrote:
>>>> On Mon, Sep 22, 2014 at 05:14:05PM -0400, Steve Dickson wrote:
>>>>>
>>>>>
>>>>> On 09/22/2014 04:44 PM, J. Bruce Fields wrote:
>>>>>> On Mon, Sep 22, 2014 at 03:43:09PM -0400, Steve Dickson wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 09/22/2014 03:26 PM, Simo Sorce wrote:
>>>>>>>> On Mon, 22 Sep 2014 15:20:07 -0400
>>>>>>>> Steve Dickson <steved@redhat.com> wrote:
>>>>>>>>
>>>>>>>>> Added the gssproxy.service to both the Wants= and
>>>>>>>>> Atfers= lines, before the rpc-svcgssd.service. There
>>>>>>>>> are  ConditionPathExists= lines in the rpc-svcgssd.service
>>>>>>>>> unit which will stop the rpc.svcgssd daemon from
>>>>>>>>> starting when the gssproxy daemon is already running.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Steve Dickson <steved@redhat.com>
>>>>>>>>> ---
>>>>>>>>>  systemd/nfs-server.service | 5 +++--
>>>>>>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/systemd/nfs-server.service
>>>>>>>>> b/systemd/nfs-server.service index 2fa7387..c740fa2 100644
>>>>>>>>> --- a/systemd/nfs-server.service
>>>>>>>>> +++ b/systemd/nfs-server.service
>>>>>>>>> @@ -2,12 +2,13 @@
>>>>>>>>>  Description=NFS server and services
>>>>>>>>>  Requires= network.target proc-fs-nfsd.mount rpcbind.target
>>>>>>>>>  Requires= nfs-mountd.service
>>>>>>>>> -Wants=rpc-statd.service nfs-idmapd.service rpc-gssd.service
>>>>>>>>> rpc-svcgssd.service +Wants=rpc-statd.service
>>>>>>>>> nfs-idmapd.service +Wants=rpc-gssd.service  
>>>>>>>>>  Wants=rpc-statd-notify.service
>>>>>>>>>  
>>>>>>>>>  After= network.target proc-fs-nfsd.mount rpcbind.target
>>>>>>>>> nfs-mountd.service After= nfs-idmapd.service rpc-statd.service
>>>>>>>>> -After= rpc-gssd.service rpc-svcgssd.service
>>>>>>>>> +After= rpc-gssd.service gssproxy.service rpc-svcgssd.service
>>>>>>>>>  Before= rpc-statd-notify.service
>>>>>>>>>  
>>>>>>>>>  Wants=nfs-config.service
>>>>>>>>
>>>>>>>> I think you really need to insure that the modules are loaded
>>>>>>>> before any of the server services are started, perhaps adding a
>>>>>>>> unit file that exec's modprobe and has "Before:
>>>>>>>> gssproxy.service rpc-svcgssd.service" in it ?
>>>>>>> I really don't think its needed... From my testing it appears 
>>>>>>> gssproxy is always being started and rpc.svcgssd is not... 
>>>>>>
>>>>>> Huh.  Well rpc-svcgssd.service has var-lib-nfs-rpc_pipefs.mount
>>>>>> as both "Requires=" and "After=", so rpc-svcgssd.service will
>>>>>> never run without first running var-lib-nfs-rpc_pipefs.mount,
>>>>>> which will load sunrpc.  But I don't see where auth_rpcgss is
>>>>>> getting loaded.  And I don't see what ensures anything happening
>>>>>> before gssproxy runs.
>>>>> It happens during the mount on the client and when the server
>>>>> is started. 
>>>>>
>>>>>>
>>>>>> We want to make sure your testing's not just getting lucky on the
>>>>>> startup order.
>>>>> The reason it working is because rpc.gssd is being started on the
>>>>> server these days for callbacks and the After= line in
>>>>> rpc-svcgssd.service is being executed before the
>>>>> ConditionPathExists which cause rpc.svcgssd not to start.
>>>>
>>>> nfs-utils$ grep After systemd/rpc-svcgssd.service 
>>>> After=var-lib-nfs-rpc_pipefs.mount
>>>> After=gssproxy.service
>>>> After=nfs-config.service
>>>>
>>>> There doesn't seem to be an After= line referring to rpc.gssd.
>>> No, why should there be? There is After= line referring to rpc.gssd
>>> in nfs-server.service
>>>
>>> grep After systemd/nfs-server.service 
>>> After= network.target proc-fs-nfsd.mount rpcbind.target
>>> nfs-mountd.service After= nfs-idmapd.service rpc-statd.service
>>> After= rpc-gssd.service rpc-svcgssd.service
>>> After=nfs-config.service
>>>
>>> So when the server starts,rpc.gssd will start and rpc.svcgssd will
>>> start if gssproxy is not enable and there is a key tab. 
>>
>> They can start in parallel, there is nothing in that line that makes
>> one start before the other.
>>
>> If you are relying on this you are relying on luck.
>>
>>>>> So when gssproxy.service does it's "Before=nfs-secure.service
>>>>> nfs-secure-server.service" line everything is loaded before
>>>>> gssproxy start... 
>>>>
>>>> That line only makes gss-proxy start before those other things.
>>> Right, which will load the sunrpc modules.
>>
>> No, starting before those service in itself achieves nothing.\
>> I think what may cause the module to load maybe the fact
>> gssproxy.service includes:
>> Requires=proc-fs-nfsd.mount
> 
> I'd expect that to only load the nfsd module.
> 
> Hm, I think nfsd actually has a dependency on auth_rpcgss.  I wonder if
> that's correct.  Maybe that's what's doing it.
It is... In another thread I clearly showed that.... So I really
don't think another unit file is necessary...

steved.
 
> 
>> But to be honest this was a hack to deal with broken nfs service files,
>> gss-proxy should not require nfsd, the dependency should be the other
>> way around, as gss-proxy can run on machines where there is no nfs
>> service whatsoever, as it stand this is a bug in gssproxy.service and
>> I'd like to fix it.
> 
> So, something like this?  (Untested, no idea if I'm doing this right.)
> 
> --b.
> 
> diff --git a/systemd/auth-rpcgss-module.service b/systemd/auth-rpcgss-module.service
> new file mode 100644
> index 000000000000..252545b458fd
> --- /dev/null
> +++ b/systemd/auth-rpcgss-module.service
> @@ -0,0 +1,15 @@
> +# We want to start gss-proxy on kernels that support it and rpc.svcgssd
> +# on those that don't.  Those services check for support by checking
> +# for existence of the path /proc/net/rpc/use-gss-proxy.  Before they
> +# can perform that check, they need this module loaded.  (Unless
> +# rpcsec_gss is built directly into the kernel, in which case this unit
> +# will fail.  But that's OK.)
> +[Unit]
> +Description=Kernel Module supporting RPCSEC_GSS
> +Before=gssproxy.service rpc-svcgssd.service
> +
> +[Service]
> +ExecStart=/sbin/modprobe -q auth_rpcgss
> +
> +[Install]
> +WantedBy=gssproxy.service rpc-svcgssd.service
>