On 06/21/2016 04:40 PM, Jakub Hrozek wrote:
On Tue, Jun 21, 2016 at 04:10:41PM +0200, Jakub Hrozek wrote:
On Tue, Jun 21, 2016 at 03:50:17PM +0200, Jakub Hrozek wrote:
On Mon, Jun 20, 2016 at 07:13:10PM +0200, Jakub Hrozek wrote:
Hi,

Pavel's sssctl tool is at:
     https://fedorapeople.org/cgit/pbrezina/public_git/sssd.git/log/?h=sssctl

I'll try to have a first pass at review today in the evening, but I
thought I'll pass on the branch in case anyone else is interested..

Hi, I went through the patches today. I pushed some of my review
comments to my branch:
     https://github.com/jhrozek/sssd/tree/sssctl
these are mostly trivial. The only important part is that sss_simpleifp
changes are no longer backwards-incompatible to make enterprise distros
happy.

Other comments:
     1) The sssctl tool restarts sssd with a command and only supports
     systemctl and service. For Beta, I would propose:
         - if sssd is built with systemd, use systemctl
         - otherwise, detect /sbin/service during configure time. If it's
           available, use /sbin/service
         - otherwise, don't support restarting sssd at all, but tell the
           admin to restart it

     For the next release, I would prefer to call systemd D-Bus
     interface to restart the service instead. This is cross-platform
     enough. Let other distributions submit patches for restarting the
     service via another service manager. This would be handled with a
     1.14.0 ticket.

     2) We should open a ticket to merge sss_cache, sss_debuglevel and
     maybe others under sssctl. sss_cache would just be a shell wrapper
     that calls sssctl for some time and eventually removed (in 2.0?)

     3) the commands that remove the cache should remove the timestamps
     cache as well, if it exists (depends on which patches get to master
     first)

     4) removing logs -- is there a reason to not remove *.log but
     special case child.log as well?

     5) CI fails. I haven't ran Coverity yet either.

6) we also need integration tests but I'm fine with manual testing for
now. Again, I will file a ticket for 1.14.0

7) The tool must be added to specfile. This blocks the patches.

Done.

8) The tool should get a manpage. This can be added later (although it
probably means we have another Beta release to have at least some short
string freeze)

9) We should investigate socket-activation for the IFP interface. Again,
a ticket, but not something we should forget about.

10) sssctl user doesn't seem to work with users from subdomains. I
wonder if this would better be solved when we unify the sysdb cache
format?

Yes, I should write a comment on that. Since the way how the names will be handled are going to change, I didn't want to add a glue code to create a fully qualified name in case of subdomains.


[root@unidirect ~]# id administra...@win.trust.test
uid=300400500(ad_default_ad...@win.trust.test)
gid=300400500(ad_default_ad...@win.trust.test)
groups=300400500(ad_default_ad...@win.trust.test),300400520(group policy
creator own...@win.trust.test),300400519(enterprise
adm...@win.trust.test),300400512(domain
adm...@win.trust.test),300400518(schema
adm...@win.trust.test),300400513(domain
us...@win.trust.test),713600009(ad_admins)

[root@unidirect ~]# sssctl user administra...@win.trust.test
User administrator is not present in cache.

Nonetheless, I wouldn't block the inclusion of these patches at this
point over this bug. It's not a regression, just a bug in a new feature.

11) If a backup of database exists, removing database seems to fail:
[root@client ~]# sssctl backup-local-data
SSSD backup of local data already exist, override? (yes/no) [no] asd
Invalid input, please provide either 'yes' or 'no'.
SSSD backup of local data already exist, override? (yes/no) [no] yes
/usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header
[root@client ~]# sssctl remove-cache
SSSD must not be running. Stop SSSD now? (yes/no) [yes] yes
Warning: sssd.service changed on disk. Run 'systemctl daemon-reload' to
reload units.
Creating backup of local data...
Unable to create backup of local data, can not remove the cache.
SSSD backup of local data already exist, override? (yes/no) [no]


Fixed, thanks.

It is also capable of printing domain online/offline status.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to