[sssd PR#132][opened] Add "Wants=" to sssd unit and avoid PAC responder to be always running

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit and avoid PAC responder to be always running
Action: opened

PR body:
"""
The first patch changes the current logic of having the services' sockets disabled by default as it adds a "Wants=" to the sssd unit file, making all the services' sockets enabled by the moment sssd service is enabled.

The second patch takes advantage of the first patch and avoids running PAC responder in case its socket is active, leaving the service to be socket-activated when needed.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][synchronized] Add "Wants=" to sssd unit and avoid PAC responder to be always running

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit and avoid PAC responder to be always running
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][comment] Add "Wants=" to sssd unit and avoid PAC responder to be always running

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit and avoid PAC responder to be always running

lslebodn commented:
"""
I am sorry I do not agree with such solution.
As I wrote few times on IRC. We should simplify monitor code and not add new code there.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-274771546

[sssd PR#132][comment] Add "Wants=" to sssd unit and avoid PAC responder to be always running

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit and avoid PAC responder to be always running

fidencio commented:
"""
@lslebodn:
Okay, you don't agree with the second patch and I'm totally fine about that (in case other developers don't agree as well). So, we can just drop it without any problem.

However, this PR also inclyudes the first patch, which is important and I'd like to have it reviewed.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-274772147

[sssd PR#132][comment] Add "Wants=" to sssd unit and avoid PAC responder to be always running

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit and avoid PAC responder to be always running

lslebodn commented:
"""
On (24/01/17 02:58), fidencio wrote:
> @lslebodn:
> Okay, you don't agree with the second patch and I'm totally fine about that (in case other developers don't agree as well). So, we can just drop it without any problem.
>
> However, this PR also inclyudes the first patch, which is important and I'd like to have it reviewed.

PR should be reviewd as whole patch set and not just some parts.
We can continue with review after droping problematic patch.

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-274774278

[sssd PR#132][+Changes requested] Add "Wants=" to sssd unit and avoid PAC responder to be always running

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit and avoid PAC responder to be always running

Label: +Changes requested

[sssd PR#132][synchronized] Add "Wants=" to sssd unit and avoid PAC responder to be always running

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit and avoid PAC responder to be always running
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][edited] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: edited

Changed field: title
Original value:
"""
Add "Wants=" to sssd unit and avoid PAC responder to be always running
"""

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
The patch changes the current logic of having the services' sockets disabled by default as it adds a "Wants=" to the sssd unit file, making all the services' sockets enabled by the moment sssd service is enabled.

The second patch was dropped for now and I will send an email to SSSD ML asking for opinions and trying to expose both mine and @lslebodn ideas.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-274781056

[sssd PR#132][-Changes requested] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

Label: -Changes requested

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

lslebodn commented:
"""
BTW the second patch had to be also removed because required functions were in newer versions of systemd which are not in CentOS
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-274812500

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
CI: http://sssd-ci.duckdns.org/logs/job/61/27/summary.html
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-274844018

[sssd PR#132][synchronized] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][+postponed until sssd 2.0] Add "Wants=" to sssd unit

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

Label: +postponed until sssd 2.0

[sssd PR#132][synchronized] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
Just rebased atop of git master.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-411114188

[sssd PR#132][comment] Add "Wants=" to sssd unit

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

pbrezina commented:
"""
Sockets for secrets and kcm responders are avoided on purpose?
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-411698413

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
Yes, they are. Theoretically sssd and kcm responders' sockets are (or should be) enabled by default by the distro.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-411701824

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
Yes, they are. Theoretically secrets and kcm responders' sockets are (or should be) enabled by default by the distro.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-411701824

[sssd PR#132][comment] Add "Wants=" to sssd unit

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

pbrezina commented:
"""
OK. In that case the manpage change should be rephrase because the term `all services` is misleading.
```
                                    By default, all services are enabled.
                                    In case the Administrator wants to persistently disable
                                    one of them, it can be done by running:
                                    "systemctl mask sssd-@service@.socket"
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-411714943

[sssd PR#132][synchronized] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
Patch set has been updated and is ready for review.
I'm also removing the "postponing until sssd 2.0" label.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-413358685

[sssd PR#132][-postponed until sssd 2.0] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

Label: -postponed until sssd 2.0

[sssd PR#132][synchronized] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][comment] Add "Wants=" to sssd unit

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

pbrezina commented:
"""
I don't really understand this line:

```
+                "However, SSSD automatically pulls in the "%s" socket(s) and "
+                "relies on systemd to start and manage the responder's "
+                "process.\n"
```

Otherwise patches looks good. As you mentioned on IRC, systemd will not start the socket if reposonder is already enabled in sssd.conf per 9c0c83eecf, so we are good here.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-413486215

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
@pbrezina: what I tried to say is that sssd.service has a Wants=[e-mail], which will make the [e-mail] to be started as soon as sssd.service is started ... relying then on systemd.
I'm totally open for suggestions on how to rewrite this part to make ir more clear for our users.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-413491333

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
```
[ffidenci@pessoa sssd]$ git diff
diff --git a/src/tools/sssd_check_socket_activated_responders.c b/src/tools/sssd_check_socket_activated_responders.c
index e83de622b..0b1a4df94 100644
--- a/src/tools/sssd_check_socket_activated_responders.c
+++ b/src/tools/sssd_check_socket_activated_responders.c
@@ -192,9 +192,9 @@ int main(int argc, const char *argv[])
                 "The "services" line contains "%s", meaning that the "
                 "responder's process will be started and managed by SSSD's "
                 "monitor. "
-                "However, SSSD automatically pulls in the "%s" socket(s) and "
-                "relies on systemd to start and manage the responder's "
-                "process.\n"
+                "However, SSSD relies on systemd to start "
+                "sssd-%s.socket and then manage the responder's process, "
+                "causing then a configuration conflict.\n"
                 "In order to solve this misconfiguration, please, either "
                 "remove "%s" from the "services" line in "%s" or call "
                 "`systemctl mask sssd-%s.socket`\n"
```

@pbrezina, does it look better?
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-413800580

[sssd PR#132][synchronized] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][comment] Add "Wants=" to sssd unit

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

pbrezina commented:
"""
This sounds better. I'm having some issues when testing this patch, but it may be unrelated. I will let you know later.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-414258216

[sssd PR#132][comment] Add "Wants=" to sssd unit

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

pbrezina commented:
"""
OK. It was not related. Patches works as expected so ACK to the patches but...

Services that are enabled through sssd.conf yields the following warning, which is correct:
```
[root /var/lib/sss/db]# systemctl status sssd-nss.socket
● sssd-nss.socket - SSSD NSS Service responder socket
   Loaded: loaded (/usr/lib/systemd/system/sssd-nss.socket; disabled; vendor preset: disabled)
   Active: failed (Result: exit-code) since Mon 2018-08-20 11:57:03 CEST; 6min ago
     Docs: man:sssd.conf(5)
   Listen: /var/lib/sss/pipes/nss (Stream)

Aug 20 11:57:03 pbrezina.my systemd[1]: Starting SSSD NSS Service responder socket.
Aug 20 11:57:03 pbrezina.my sssd[9458]: There's a misconfiguration in the "services" line of "/etc/sssd/sssd.conf"!
                                        The "services" line contains "nss", meaning that the responder's process will be started and managed by SSSD's monitor. However, SSSD relies on s
                                        In order to solve this misconfiguration, please, either remove "nss" from the "services" line in "/etc/sssd/sssd.conf" or call `systemctl mask ss
                                        Please, refer to "sssd.conf" man page for more info and mind that the recommended way to go is to take advantage of systemd, as much as possible,
Aug 20 11:57:03 pbrezina.my systemd[1]: sssd-nss.socket: Control process exited, code=exited status=17
Aug 20 11:57:03 pbrezina.my systemd[1]: Failed to listen on SSSD NSS Service responder socket.
Aug 20 11:57:03 pbrezina.my systemd[1]: sssd-nss.socket: Unit entered failed state.
```
However, I'm not sure if this is something desirable for all responders. Everyone have `nss` and `pam` services enabled through `sssd.conf` from its beginning and socket activation use case is not really desirable for them as you want to authenticate and id fast very often.

I already mentioned it on the meeting and I will mention it again just so the idea gets proper thoughts. If we are going to enable all responders/sockets and rely on systemd by default maybe it is time to drop this functionality from monitor (also we are now in 2.0 so we can do such change).

* We can rely completely on systemd to manage socket and services start and monitor.
* If a service is present in sssd.conf, it will have no idle timeout and it will run indefinitely.
* If a service is not present in sssd.conf, it will terminated itself after some time as it is now.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-414268841

[sssd PR#132][comment] Add "Wants=" to sssd unit

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

pbrezina commented:
"""
That being said, I do not oppose your work Fabiano, it would be used in that case as well.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-414269114

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
@pbrezina, I totally agree and the steps described can be taken as a continuation of my patch.
I'd like to:
- Have my patch merged;
- Have a bug opened with your proposal;

So, any contributor could take the issue to work on.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-414285015

[sssd PR#132][comment] Add "Wants=" to sssd unit

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

pbrezina commented:
"""
Fair enough.  I'm adding accepted label.
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-414299547

[sssd PR#132][+Accepted] Add "Wants=" to sssd unit

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

Label: +Accepted

[sssd PR#132][synchronized] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][comment] Add "Wants=" to sssd unit

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

jhrozek commented:
"""
I still found two issues:
 - with no sssd.conf, the nss service is still enabled in confdb, so it's started by the monitor and then also the socket-activated service is started
 - for some reason, the autofs (I picked this service arbitrarily) service didn't start for me with sssd.conf which didn't enable the autofs service
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-415761243

[sssd PR#132][-Accepted] Add "Wants=" to sssd unit

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

Label: -Accepted

[sssd PR#132][synchronized] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
@jhrozek, patch set has been updated.

Now it also checks the confdb in order to now what we should do.

Please, give it a try, all the issues you have seen should be resolved.

In case something is still missing, I'd like to leave this patches here and someone else can keep working on this as I'm not part of SSSD team anymore. Of course, please,  **keep my authorship on my patches** or at least mention your work has been based on them.

Unfortunately this PR took way too long to get the attention it needed. :-(
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-415790746

[sssd PR#132][comment] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Title: #132: Add "Wants=" to sssd unit

fidencio commented:
"""
@jhrozek, patch set has been updated.

Now it also checks the confdb in order to now what we should do.

Please, give it a try, all the issues you have seen should be resolved.

In case something is still missing, I'd like to leave these patches here and someone else can keep working on this as I'm not part of SSSD team anymore. Of course, please,  **keep my authorship on my patches** or at least mention your work has been based on them.

Unfortunately this PR took way too long to get the attention it needed. :-(
"""

See the full comment at https://github.com/SSSD/sssd/pull/132#issuecomment-415790746

[sssd PR#132][closed] Add "Wants=" to sssd unit

[fidencio]

URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132