URL: https://github.com/SSSD/sssd/pull/32 Author: fidencio Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138 Action: opened
PR body: """ This patch series is intended to solve #3138 by adding a new service that updates the confdb. As part of the series this service is used by secrets service.
I only ran CI locally and the two secrets tests have been failing. /o\
Also, I've noticed some weird behavior, where the sssd-update-confdb service starts for apparently no reason, when upgrading fedora packages.
Anyways, these pieces of code really need some detailed review as it was the first time I've been "seriously" playing with TEvent requests. So, please, consider it more like an RFC than a well finished and polished code.
Best Regards, """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/32/head:pr32 git checkout pr32
URL: https://github.com/SSSD/sssd/pull/32 Author: fidencio Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138 Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/32/head:pr32 git checkout pr32
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
fidencio commented: """ Okay, with the updated version of the patches both issues that I had are fixed. Here is the link for the CI (that has passed): http://sssd-ci.duckdns.org/logs/job/53/90/summary.html """
See the full comment at https://github.com/SSSD/sssd/pull/32#issuecomment-249112674
URL: https://github.com/SSSD/sssd/pull/32 Author: fidencio Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138 Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/32/head:pr32 git checkout pr32
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
jhrozek commented: """ Hi, thank you for working on this. I think there are two "architectural" questions we should answer.
1) Lukas raised the suggestion of using PreExec with `sssd --genconf` on #sssd on IRC. Did you explore this or did you not like this solution because of race-conditions? 2) If we don't use `sssd --genconf`, I wonder if it would be actually better to add the new socket to the monitor process rather than a new service (sorry, I realize it was me who lead you down this path of adding a new service...I only remembered this idea now once @simo5 mentioned it on IRC). IMO this would have one advantage which is that the sssd process is already permitted by SELinux to read sssd.conf and write to confdb. And long-term we wanted to make the sssd process only initialize sssd and then exit since the services would (in the ideal case of a modern Linux system) self-monitor themselves. """
See the full comment at https://github.com/SSSD/sssd/pull/32#issuecomment-250105213
On Wed, Sep 28, 2016 at 10:32 AM, jhrozek sssd-github-notification@fedorahosted.org wrote:
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
jhrozek commented: """ Hi, thank you for working on this. I think there are two "architectural" questions we should answer.
- Lukas raised the suggestion of using PreExec with `sssd --genconf` on #sssd on IRC. Did you explore this or did you not like this solution because of race-conditions?
I do believe it may work, but I would have to adapt sssd --genconf logic, in order to avoid race-conditions.Shall I go for it? I was thinking about waiting a little bit before doing any changes to let people raise their questions/comments so I won't spend one week on something that's not going to be used at all.
There are still the suggestions/questions raised by Pavel in the bug report, where Dmitri jumped in as well, that just makes me think that people are not in sync about the confdb itself and any kind of effort on this area must have some agreement beforehand. What do you think about this?
- If we don't use `sssd --genconf`, I wonder if it would be actually better to add the new socket to the monitor process rather than a new service (sorry, I realize it was me who lead you down this path of adding a new service...I only remembered this idea now once @simo5 mentioned it on IRC). IMO this would have one advantage which is that the sssd process is already permitted by SELinux to read sssd.conf and write to confdb. And long-term we wanted to make the sssd process only initialize sssd and then exit since the services would (in the ideal case of a modern Linux system) self-monitor themselves.
I really don't know what's the best approach and I completely opened for suggestions.
"""
See the full comment at https://github.com/SSSD/sssd/pull/32#issuecomment-250105213
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-leave@lists.fedorahosted.org
Best Regards,
On Wed, Sep 28, 2016 at 10:54:47AM +0200, Fabiano Fidêncio wrote:
On Wed, Sep 28, 2016 at 10:32 AM, jhrozek sssd-github-notification@fedorahosted.org wrote:
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
jhrozek commented: """ Hi, thank you for working on this. I think there are two "architectural" questions we should answer.
- Lukas raised the suggestion of using PreExec with `sssd --genconf` on #sssd on IRC. Did you explore this or did you not like this solution because of race-conditions?
I do believe it may work, but I would have to adapt sssd --genconf logic, in order to avoid race-conditions.Shall I go for it? I was thinking about waiting a little bit before doing any changes to let people raise their questions/comments so I won't spend one week on something that's not going to be used at all.
There are still the suggestions/questions raised by Pavel in the bug report, where Dmitri jumped in as well, that just makes me think that people are not in sync about the confdb itself and any kind of effort on this area must have some agreement beforehand. What do you think about this?
- If we don't use `sssd --genconf`, I wonder if it would be actually better to add the new socket to the monitor process rather than a new service (sorry, I realize it was me who lead you down this path of adding a new service...I only remembered this idea now once @simo5 mentioned it on IRC). IMO this would have one advantage which is that the sssd process is already permitted by SELinux to read sssd.conf and write to confdb. And long-term we wanted to make the sssd process only initialize sssd and then exit since the services would (in the ideal case of a modern Linux system) self-monitor themselves.
I really don't know what's the best approach and I completely opened for suggestions.
Could you reply to the github PR, please?
(This reminds me to implement Sumit's suggestion to note that replying to the github notification e-mails doesn't reach github unless you reply to the message from github..)
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
fidencio commented: """ @jhrozek
Answer for 1) I do believe it may work, but I would have to adapt `sssd --genconf` logic, in order to avoid race-conditions. Shall I go for it? I was thinking about waiting a little bit before doing any changes to let people raise their questions/comments so I won't spend one week on something that's not going to be used at all.
There are still the suggestions/questions raised by Pavel in the bug report, where Dmitri jumped in as well, that just makes me think that people are not in sync about the confdb itself and any kind of effort on this area must have some agreement beforehand. What do you think about this?
Answer for 2) I really don't know what's the best approach and I completely opened for suggestions. """
See the full comment at https://github.com/SSSD/sssd/pull/32#issuecomment-250121090
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
simo5 commented: """ The problem of using genconf is race conditions and the fact it won't be possible to call it again after the service is started. I am for adding socket activation to the monitor and have the moinitor only regen the config (and exit ?) when socket activated. """
See the full comment at https://github.com/SSSD/sssd/pull/32#issuecomment-250167784
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
jhrozek commented: """ Yes, as discussed, moving this functionality to the monitor is something we'll go with. Therefore I'm adding the Changes requested label. """
See the full comment at https://github.com/SSSD/sssd/pull/32#issuecomment-251108410
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
fidencio commented: """ This PR doesn't make sense anymore in the state that this branch is. I'm closing the PR and a new one will be re-open by whoever ends up working on this in the future. """
See the full comment at https://github.com/SSSD/sssd/pull/32#issuecomment-318267955
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/32 Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138
Label: +Rejected
URL: https://github.com/SSSD/sssd/pull/32 Author: fidencio Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138 Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/32/head:pr32 git checkout pr32
sssd-devel@lists.fedorahosted.org
-
Fabiano Fidêncio -
fidencio -
Jakub Hrozek