URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: Do not build the local provider by default Action: opened
PR body: """ Folks,
This series is the first attempt to avoid building the local provider by default. With we only build it conditionally and the default is set to not build it. The problems: - integration tests depend on the local provider: For this, I've changed our configure line, specifying to enable the local provider; - some unit tests depend on local provider: I've changed some tests (please, take a careful look at those in order to be sure I'm not invalidating the tests) and worked them around so they're still valid with both scenarios (building or not building the local provider);
My **personal** preference would be to start using the **files** provider in our tests instead of using the "local" one. However, I've faced some issues related to the amount of users being find in some sysdb tests (ping me, we can discuss this either here or in the #sssd channel) (maybe because it also finds the local users/groups created on my machine?).
Again, this is a first attempt, let's discuss improvements needed :-) """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/611/head:pr611 git checkout pr611
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
pbrezina commented: """
My personal preference would be to start using the files provider in our tests instead of using the "local" one. However, I've faced some issues related to the amount of users being find in some sysdb tests (ping me, we can discuss this either here or in the #sssd channel) (maybe because it also finds the local users/groups created on my machine?).
Good idea. How did you configure the files provider? You can you `passwd_files` and `group_files` to set the files you want to read, also you can use `SSS_FILES_PASSWD` and `SSS_FILES_GROUP` environment variable to overwrite standard file names. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-401785637
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ Thanks, this is a good start. We also want to remove the whole pysss.local interface and don't build the `sss_*` tools like `sss_useradd` and so on.
If you want to remove the usage of the local provider from tests, does it make sense to keep the code at all using the is_local_provider functions? Remember the code is not lost forever, we can always ressurect it from git history. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-403766316
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ btw I'm not sold either way myself, on one hand the local provider might be handy for tests, on the other hand I haven't used it in a long time myself. So I would not be completely mad if it goes away. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-403766637
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ About the sysdb tests, can you paste what issues you faced here? Or push a branch? I could see issues with not matching number of groups returned because the local provider is mpg-enabled by default, but I don't know what the issue with users could be. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-403767914
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
pbrezina commented: """
Thanks, this is a good start. We also want to remove the whole pysss.local interface and don't build the sss_* tools like sss_useradd and so on.
If you want to remove the usage of the local provider from tests, does it make sense to keep the code at all using the is_local_provider functions? Remember the code is not lost forever, we can always ressurect it from git history.
btw I'm not sold either way myself, on one hand the local provider might be handy for tests, on the other hand I haven't used it in a long time myself. So I would not be completely mad if it goes away.
Definitely drop all the code related to local provider. It makes the code complicated since it is a special case. It is also very easy to forget it.
If it will be needed again, we can implement a simple backend for local provider just returning EOK from the dbus handlers. But I have literally never used it for testing ever. In addition, it does not really tests anything today since it never goes to backend and requires special casing in responders. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-403879182
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: Do not build the local provider by default Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/611/head:pr611 git checkout pr611
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: WIP: Do not build the local provider by default Action: edited
Changed field: title Original value: """ Do not build the local provider by default """
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: WIP: Do not build the local provider by default
fidencio commented: """ @jhrozek, @pbrezina,
I haven't addressed any of the comments made, yet. I've pushed the patchset containing the most of the changes **theoretically** needed.
We can discuss test by test in our phone meeting Tomorrow. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-405971187
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: WIP: Do not build the local provider by default
fidencio commented: """ It's been discussed on IRC a few times already and let me try to make it explicit here. I do need some help in order to make the tests that are failing to pass. Those tests **theoretically** only test APIs but there seem to be some changes depending on whether they are or are not used "local" provider.
The help needed here and asked on IRC a few times before is in order to figure out the changes that have to be done. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-409196463
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: WIP: Do not build the local provider by default Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/611/head:pr611 git checkout pr611
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: WIP: Do not build the local provider by default
fidencio commented: """ I've updated the patches, @pbrezina helped me with the most part of the issues I was facing.
There are still an issue in the ldap_id_cleanup() test as the test fails when ran by the first time but passes if ran again after a minute or two.
Summing up, there's still work to be done in this PR, thus i"m adding the "Changes requested" label. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-409930406
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: WIP: Do not build the local provider by default
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: WIP: Do not build the local provider by default Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/611/head:pr611 git checkout pr611
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: WIP: Do not build the local provider by default
jhrozek commented: """ Do you need help with triaging the failure? """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-410182402
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: WIP: Do not build the local provider by default Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/611/head:pr611 git checkout pr611
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: WIP: Do not build the local provider by default
fidencio commented: """ @jhrozek, that would be really appreciated.
What I've found so far is that when not building the local provider the ldap_id_cleanup test is bailing out because when trying to cleanup the groups the search done doesn't return any records from the cache.
I've checked that those records are present in the timestamp cache and if searched there we'd bea ble to move forward. However, the search is done in the cache itself.
So, what I'm trying to understand here is why the cache has not been updated (following the timestamp cache) when not using the local provider. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-410268713
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: WIP: Do not build the local provider by default Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/611/head:pr611 git checkout pr611
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: WIP: Do not build the local provider by default
fidencio commented: """ So, thanks to both @jhrozek and @pbrezina the tests are now passing!
I've decided to re-order/re-organize the patches and all comments made should be addressed.
The series now contain: - A bunch of commits changing the tests to work with the "files" provider - It includes a few fixes needed in order to make ldap_id_cleanup test to pass; - One commit making the build of the "local" provider conditional - One commit removing pysss.local interface. """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-410786778
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: WIP: Do not build the local provider by default
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: Do not build the local provider by default Action: edited
Changed field: title Original value: """ WIP: Do not build the local provider by default """
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
Label: +postponed until sssd 2.0
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ I wish we had more time to nuke the code completely, but not building it by default is a good start. We can remove the stuff completely in 2.1 """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-412369229
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ I'll ack once coverity finishes """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-412369263
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ Coverity and CI were OK """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-412460335
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: Do not build the local provider by default Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/611/head:pr611 git checkout pr611
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-412564711
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-412574313
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
jhrozek commented: """ * master: 0e211b8ba30c3adcdeef21ca1339b194cbfffb04 82d51b7feba8df49df3f7a023d2b16aba26a858e b8946c46e04e8b5329e4d95f9a67b4a76f883bb2 c075e2865b2fbedb6a7eeb6a90780c6dbffeb5fd 15342ebe8c22738e947728f06c6833b5be09eca8 5a87af9128c5468ce1f2e55602c9d3ec05987b8d a24f0c202ed7ea159ad04eb19b506b7f21ad41cc 2e8fe6a3dd022721e1ff476c8fd7af317a535d63 728e4be10fc7a7b3173b74b1f597e835914c9abf 99b5bb54447287da5031dcd2bfebeef8b47d52d7 a8a9e66a852ba8aa1371e2e1897d49f3feee76da 064ca0b46eed06977dbd32c2bd894f71dc43be2e 6ebcc59b9458c5a118cd182c07a840fa6756a43e 35a200d5bd84d6a6742e77ffda9945e37134d721 2243b348976705ca81da954cb1ead1f406d01918 7d483737f3763947a5b8a43efbfaf31f54f956a4 """
See the full comment at https://github.com/SSSD/sssd/pull/611#issuecomment-412580797
URL: https://github.com/SSSD/sssd/pull/611 Author: fidencio Title: #611: Do not build the local provider by default Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/611/head:pr611 git checkout pr611
URL: https://github.com/SSSD/sssd/pull/611 Title: #611: Do not build the local provider by default
Label: +Pushed
sssd-devel@lists.fedorahosted.org
-
fidencio -
jhrozek
-
pbrezina