[sssd PR#146][opened] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: opened

PR body:
"""
Those two patches together are responsible for avoiding running two "instances" of the same service.

The commit message of each patch has quite a lot of explanation.
"""

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

[sssd PR#146][comment] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

fidencio commented:
"""
Ah, an important and obvious note ... of course I'm okay re-writing the second patch to use DBus instead of sd-bus, in case it's required by the reviewers.
"""

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

[sssd PR#146][comment] Avoid running two instances of the same service

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

pbrezina commented:
"""
Please, stick with sbus code and do not use sdbus. We are at a point where we have internal higher level api to create send and parse dbus message, all talloc compliant, please use this. In the future we may change sbus code to use sdbus instead of libdbus.

As per design, I will start/continue discussion on mailing list since I deem it better than this page.
"""

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

[sssd PR#146][+Changes requested] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: +Changes requested

[sssd PR#146][comment] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

fidencio commented:
"""
@pbrezina: sticking with sbus code is not something that simple to do here and I'll explain you the reasons.

The code flow is: 
 - for each of the responders
   - talk to systemd in order to know whether the responder has its socket activated

"Talk to systemd" here means a sbus call to systemd and each of those calls would be an async call ... I can see a quite spaghetti code as a result of those. :-\

sgallagher mentioned that we can do sync calls (although should be avoided), but does this worth the trouble? Mainly considering we would like to drop the monitor in the near future?

I've also checked a few (not so) different options like using the "ListUnitsByNames" API. Using it we could do just one call to systemd, parse the result and that's it, which would make things way easier. However, this API is really recent and would only work for fc25+ (which makes it sound a less attractive solution than using the sd-bus code).

Opinions? Suggestions?
"""

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

[sssd PR#146][comment] Avoid running two instances of the same service

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

pbrezina commented:
"""
My opinion is do not use sync code when use can use async.
"""

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

[sssd PR#146][synchronized] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: synchronized

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

[sssd PR#146][comment] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

fidencio commented:
"""
This patch set has been updated following @sumit-bose's suggestion.
"""

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

[sssd PR#146][-Changes requested] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: -Changes requested

[sssd PR#146][synchronized] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: synchronized

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

[sssd PR#146][+Changes requested] Avoid running two instances of the same service

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: +Changes requested

[sssd PR#146][synchronized] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: synchronized

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

[sssd PR#146][comment] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

fidencio commented:
"""
As required by @sgallagher, the python script is gone and a new binary has been introduced to check if the responders' socket is enabled but the responder is still part of the services' line.
"""

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

[sssd PR#146][-Changes requested] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: -Changes requested

[sssd PR#146][synchronized] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: synchronized

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

[sssd PR#146][comment] Avoid running two instances of the same service

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

jhrozek commented:
"""
I will test the patches once I reinstall my test VM :) but in the meantime I provided two general code comments.
"""

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

[sssd PR#146][comment] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

fidencio commented:
"""
On Wed, Mar 1, 2017 at 12:24 PM, Jakub Hrozek 
wrote:
> *@jhrozek* commented on this pull request.
>
> In src/tools/sssd_check_socket_activated_responders.c
> https://github.com/SSSD/sssd/pull/146#discussion_r103662581:
> >
> ret = ini_config_create(&ini_config);
> if (ret != 0) {
>    DEBUG(SSSDBG_CRIT_FAILURE, "ini_config_create() failed [%d][%s]\n",
>          ret, sss_strerror(ret));
>    goto done;
> }
> ret = ini_config_file_open(SSSD_CONFIG_FILE, 0, &file_ctx);
> if (ret != 0) {
>    DEBUG(SSSDBG_CRIT_FAILURE, "ini_config_file_open() failed [%d][%s]\n",
>          ret, sss_strerror(ret));
>    goto done;
> }
> ret = ini_config_parse(file_ctx, INI_STOP_ON_ANY, 0, 0, ini_config);
>
> Would it make sense to use the same flags as sssd uses when it loads the
> config file? (Or even move them to a #define and reuse)

Okay.
> In src/tools/sssd_check_socket_activated_responders.c
> https://github.com/SSSD/sssd/pull/146#discussion_r103662680:
> > ret = ini_config_file_open(SSSD_CONFIG_FILE, 0, &file_ctx);
> if (ret != 0) {
>    DEBUG(SSSDBG_CRIT_FAILURE, "ini_config_file_open() failed [%d][%s]\n",
>          ret, sss_strerror(ret));
>    goto done;
> }
> ret = ini_config_parse(file_ctx, INI_STOP_ON_ANY, 0, 0, ini_config);
> if (ret != 0) {
>    DEBUG(SSSDBG_CRIT_FAILURE, "ini_config_parse() failed [%d][%s]\n",
>           ret, sss_strerror(ret));
>    goto done;
> }
> ret = ini_get_config_valueobj("sssd", "services", ini_config,
>                             INI_GET_FIRST_VALUE, &vobj);
>
> Did you test this with no services line at all? (Sorry, I managed to break
> my test env, so I will only test the patches later..)

Oh! /o\

I'll update new patch set soon.
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> https://github.com/SSSD/sssd/pull/146#pullrequestreview-24455038, or mute
> the thread
> https://github.com/notifications/unsubscribe-auth/AAG4ejiYOZhSZGHNaPBiezfya_ETICS6ks5rhVVhgaJpZM4L4wZO
> .

"""

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

[sssd PR#146][synchronized] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: synchronized

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

[sssd PR#146][comment] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

fidencio commented:
"""
New patch set has been pushed!
"""

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

[sssd PR#146][comment] Avoid running two instances of the same service

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

jhrozek commented:
"""
The last version works for me. I tested:
 * no services line at all - I was allowed to start the socket
 * service present in the list - starting the socket failed
 * service not present in the list - the socket was started

So for now ACK, and I will finally add the accepted label and push the patches when CI and Coverity finish.
"""

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

[sssd PR#146][comment] Avoid running two instances of the same service

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

jhrozek commented:
"""
CI: http://sssd-ci.duckdns.org/logs/job/63/63/summary.html

ACK
"""

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

[sssd PR#146][+Accepted] Avoid running two instances of the same service

[jhrozek]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: +Accepted

[sssd PR#146][+Changes requested] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: +Changes requested

[sssd PR#146][-Accepted] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: -Accepted

[sssd PR#146][comment] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

fidencio commented:
"""
On Thu, Mar 2, 2017 at 1:32 PM, lslebodn  wrote:
> *@lslebodn* commented on this pull request.
>
> In contrib/sssd.spec.in
> https://github.com/SSSD/sssd/pull/146#discussion_r103913183:
> > @@ -815,6 +815,7 @@ done
>
> %{_unitdir}/sssd-sudo.service
>  %{_unitdir}/sssd-secrets.socket
>  %{_unitdir}/sssd-secrets.service
> +%{_libexecdir}/%{servicename}/sssd_check_socket_activated_responders
>  %else
>
> It will fail on el6

It's inside the %if (0%{?use_systemd} == 1) ... not sure why it will fail
on el6.
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> https://github.com/SSSD/sssd/pull/146#pullrequestreview-24718606, or mute
> the thread
> https://github.com/notifications/unsubscribe-auth/AAG4eqaM5ghpYd2Hg7oXxKCWHJ0387XOks5rhrbzgaJpZM4L4wZO
> .

"""

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

[sssd PR#146][comment] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

lslebodn commented:
"""
This version do not read config snippets. It was ok for python-sssdconfig because we decided to not support it there.

Previously we plan to use `ExecStartPre` also for services. `RefuseManualStart` might do the same. But on my system it is used just for target units. Did you consult it with systemd guys?
"""

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

[sssd PR#146][synchronized] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: synchronized

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

[sssd PR#146][comment] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

lslebodn commented:
"""
On (02/03/17 04:41), fidencio wrote:
> On Thu, Mar 2, 2017 at 1:32 PM, lslebodn  wrote:
> > *@lslebodn* commented on this pull request.
> >
> > In contrib/sssd.spec.in
> > https://github.com/SSSD/sssd/pull/146#discussion_r103913183:
> > > @@ -815,6 +815,7 @@ done
> >
> > %{_unitdir}/sssd-sudo.service
> >  %{_unitdir}/sssd-secrets.socket
> >  %{_unitdir}/sssd-secrets.service
> > +%{_libexecdir}/%{servicename}/sssd_check_socket_activated_responders
> >  %else
> >
> > It will fail on el6
>
> It's inside the %if (0%{?use_systemd} == 1) ... not sure why it will fail
> on el6.

Ahh; you are right.

But we have other files from directory "%{_libexecdir}/%{servicename}"
on ther place after "%dir %{_libexecdir}/%{servicename}"
I would prefer to keep them together rather then mix with "%unitdir"

LS

"""

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

[sssd PR#146][synchronized] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: synchronized

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

[sssd PR#146][comment] Avoid running two instances of the same service

[fidencio]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

fidencio commented:
"""
@lslebodn: new patch set updated. All your comments should've been addressed now.
"""

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

[sssd PR#146][comment] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

lslebodn commented:
"""
master:

* e0ca21d9f899c60cc50030c6ae793c48e92b5b7f
* 9c0c83eecf963416effee67dab55711234373fde
"""

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

[sssd PR#146][+Pushed] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: +Pushed

[sssd PR#146][-Changes requested] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

Label: -Changes requested

[sssd PR#146][closed] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Author: fidencio
 Title: #146: Avoid running two instances of the same service
Action: closed

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

[sssd PR#146][comment] Avoid running two instances of the same service

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/146
Title: #146: Avoid running two instances of the same service

lslebodn commented:
"""
I did small change before push because there were hardcoded paths in socket files.
@fidencio ACKed them :-)

`sed -e 's!/usr/libexec!@libexecdir@!' -i src/sysv/systemd/*.in`
"""

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