Hi,
this patches makes the sssctl commands more similar to ipa tool commands. I also think this pattern makes it easier to remember the commands.
Note that in the future, there will be more user-* group-* and netgroup-* commands (like seed for user, list of all etc.)
Comments are welcome.
Michal
On 07/25/2016 02:12 PM, Michal Židek wrote:
Hi,
this patches makes the sssctl commands more similar to ipa tool commands. I also think this pattern makes it easier to remember the commands.
Note that in the future, there will be more user-* group-* and netgroup-* commands (like seed for user, list of all etc.)
Comments are welcome.
Michal
Hi, ok, it looks like a good idea. When touching the code, can you also convert sss_override command to use the macro instead? And I think it may be nice to also add a macro for command sentinel i.e. for {NULL, ...}.
I'm not very fond of renaming local-data-* to cache-* so it doesn't imply that we backup the whole cache content. We only backup and restore data that are local to the client and not present in LDAP. Currently only local overrides, but it may include local users and groups in the future.
* cache-backup Backup local data * cache-restore Restore local data from backup
On 07/26/2016 01:19 PM, Pavel Březina wrote:
On 07/25/2016 02:12 PM, Michal Židek wrote:
Hi,
this patches makes the sssctl commands more similar to ipa tool commands. I also think this pattern makes it easier to remember the commands.
Note that in the future, there will be more user-* group-* and netgroup-* commands (like seed for user, list of all etc.)
Comments are welcome.
Michal
Hi, ok, it looks like a good idea. When touching the code, can you also convert sss_override command to use the macro instead? And I think it may be nice to also add a macro for command sentinel i.e. for {NULL, ...}.
I'm not very fond of renaming local-data-* to cache-* so it doesn't imply that we backup the whole cache content. We only backup and restore data that are local to the client and not present in LDAP. Currently only local overrides, but it may include local users and groups in the future.
- cache-backup Backup local data
- cache-restore Restore local data from backup
That was confusion on my side. I thought local data means entire cache. In that case I would propose topic name "local" for actions that work with data that is not present on remote server.
local-backup local-restore
What do you think?
New patch is attached. I also added patch for missing gettext macro (did not want to hide this change in the first patch).
Michal
On 07/26/2016 04:19 PM, Michal Židek wrote:
On 07/26/2016 01:19 PM, Pavel Březina wrote:
On 07/25/2016 02:12 PM, Michal Židek wrote:
Hi,
this patches makes the sssctl commands more similar to ipa tool commands. I also think this pattern makes it easier to remember the commands.
Note that in the future, there will be more user-* group-* and netgroup-* commands (like seed for user, list of all etc.)
Comments are welcome.
Michal
Hi, ok, it looks like a good idea. When touching the code, can you also convert sss_override command to use the macro instead? And I think it may be nice to also add a macro for command sentinel i.e. for {NULL, ...}.
I'm not very fond of renaming local-data-* to cache-* so it doesn't imply that we backup the whole cache content. We only backup and restore data that are local to the client and not present in LDAP. Currently only local overrides, but it may include local users and groups in the future.
- cache-backup Backup local data
- cache-restore Restore local data from backup
That was confusion on my side. I thought local data means entire cache. In that case I would propose topic name "local" for actions that work with data that is not present on remote server.
local-backup local-restore
What do you think?
New patch is attached. I also added patch for missing gettext macro (did not want to hide this change in the first patch).
Michal
I'd still stick with local-data-backup and local-data-restore, what do others thing? But otherwise ack.
On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote:
On 07/26/2016 04:19 PM, Michal Židek wrote:
On 07/26/2016 01:19 PM, Pavel Březina wrote:
On 07/25/2016 02:12 PM, Michal Židek wrote:
Hi,
this patches makes the sssctl commands more similar to ipa tool commands. I also think this pattern makes it easier to remember the commands.
Note that in the future, there will be more user-* group-* and netgroup-* commands (like seed for user, list of all etc.)
Comments are welcome.
Michal
Hi, ok, it looks like a good idea. When touching the code, can you also convert sss_override command to use the macro instead? And I think it may be nice to also add a macro for command sentinel i.e. for {NULL, ...}.
I'm not very fond of renaming local-data-* to cache-* so it doesn't imply that we backup the whole cache content. We only backup and restore data that are local to the client and not present in LDAP. Currently only local overrides, but it may include local users and groups in the future.
When we have the files provider there would be a cache as well. Moreover, we store secrets now. The restore command backs up all *.ldb files, right?
- cache-backup Backup local data
- cache-restore Restore local data from backup
That was confusion on my side. I thought local data means entire cache. In that case I would propose topic name "local" for actions that work with data that is not present on remote server.
local-backup local-restore
What do you think?
New patch is attached. I also added patch for missing gettext macro (did not want to hide this change in the first patch).
Michal
I'd still stick with local-data-backup and local-data-restore, what do others thing? But otherwise ack.
I'm not sure, on one hand the admins would think 'cache' but on the other hand, if we save and restore all cache data, the we operate on more than the cache..
On 07/27/2016 11:09 AM, Jakub Hrozek wrote:
On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote:
On 07/26/2016 04:19 PM, Michal Židek wrote:
On 07/26/2016 01:19 PM, Pavel Březina wrote:
On 07/25/2016 02:12 PM, Michal Židek wrote:
Hi,
this patches makes the sssctl commands more similar to ipa tool commands. I also think this pattern makes it easier to remember the commands.
Note that in the future, there will be more user-* group-* and netgroup-* commands (like seed for user, list of all etc.)
Comments are welcome.
Michal
Hi, ok, it looks like a good idea. When touching the code, can you also convert sss_override command to use the macro instead? And I think it may be nice to also add a macro for command sentinel i.e. for {NULL, ...}.
I'm not very fond of renaming local-data-* to cache-* so it doesn't imply that we backup the whole cache content. We only backup and restore data that are local to the client and not present in LDAP. Currently only local overrides, but it may include local users and groups in the future.
When we have the files provider there would be a cache as well. Moreover, we store secrets now. The restore command backs up all *.ldb files, right?
This is how I understood it at first, but the current backup and restore only work for local overrides. But as Pavel mentioned, it may work for local users and groups in the future (id_provider=local). My original confusion was that I also thought it backs-up and restores all ldb files, which is not the case.
- cache-backup Backup local data
- cache-restore Restore local data from backup
That was confusion on my side. I thought local data means entire cache. In that case I would propose topic name "local" for actions that work with data that is not present on remote server.
local-backup local-restore
What do you think?
New patch is attached. I also added patch for missing gettext macro (did not want to hide this change in the first patch).
Michal
I'd still stick with local-data-backup and local-data-restore, what do others thing? But otherwise ack.
I'm not sure, on one hand the admins would think 'cache' but on the other hand, if we save and restore all cache data, the we operate on more than the cache..
On 07/27/2016 03:28 PM, Michal Židek wrote:
On 07/27/2016 11:09 AM, Jakub Hrozek wrote:
On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote:
On 07/26/2016 04:19 PM, Michal Židek wrote:
On 07/26/2016 01:19 PM, Pavel Březina wrote:
On 07/25/2016 02:12 PM, Michal Židek wrote:
Hi,
this patches makes the sssctl commands more similar to ipa tool commands. I also think this pattern makes it easier to remember the commands.
Note that in the future, there will be more user-* group-* and netgroup-* commands (like seed for user, list of all etc.)
Comments are welcome.
Michal
Hi, ok, it looks like a good idea. When touching the code, can you also convert sss_override command to use the macro instead? And I think it may be nice to also add a macro for command sentinel i.e. for {NULL, ...}.
I'm not very fond of renaming local-data-* to cache-* so it doesn't imply that we backup the whole cache content. We only backup and restore data that are local to the client and not present in LDAP. Currently only local overrides, but it may include local users and groups in the future.
When we have the files provider there would be a cache as well. Moreover, we store secrets now. The restore command backs up all *.ldb files, right?
This is how I understood it at first, but the current backup and restore only work for local overrides. But as Pavel mentioned, it may work for local users and groups in the future (id_provider=local). My original confusion was that I also thought it backs-up and restores all ldb files, which is not the case.
No, the intention is to backup only data that are not stored on server and would be lost when the cache is removed. In the time, only local overrides were local. If secrets creates local data, the tool should be modified.
- cache-backup Backup local data
- cache-restore Restore local data from backup
That was confusion on my side. I thought local data means entire cache. In that case I would propose topic name "local" for actions that work with data that is not present on remote server.
local-backup local-restore
What do you think?
New patch is attached. I also added patch for missing gettext macro (did not want to hide this change in the first patch).
Michal
I'd still stick with local-data-backup and local-data-restore, what do others thing? But otherwise ack.
I'm not sure, on one hand the admins would think 'cache' but on the other hand, if we save and restore all cache data, the we operate on more than the cache..
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
On 07/28/2016 10:00 AM, Pavel Březina wrote:
On 07/27/2016 03:28 PM, Michal Židek wrote:
On 07/27/2016 11:09 AM, Jakub Hrozek wrote:
On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote:
On 07/26/2016 04:19 PM, Michal Židek wrote:
On 07/26/2016 01:19 PM, Pavel Březina wrote:
On 07/25/2016 02:12 PM, Michal Židek wrote: > Hi, > > this patches makes the sssctl commands more similar to > ipa tool commands. I also think this pattern makes it > easier to remember the commands. > > Note that in the future, there will be more user-* > group-* and netgroup-* commands (like seed for user, > list of all etc.) > > Comments are welcome. > > Michal
Hi, ok, it looks like a good idea. When touching the code, can you also convert sss_override command to use the macro instead? And I think it may be nice to also add a macro for command sentinel i.e. for {NULL, ...}.
I'm not very fond of renaming local-data-* to cache-* so it doesn't imply that we backup the whole cache content. We only backup and restore data that are local to the client and not present in LDAP. Currently only local overrides, but it may include local users and groups in the future.
When we have the files provider there would be a cache as well. Moreover, we store secrets now. The restore command backs up all *.ldb files, right?
This is how I understood it at first, but the current backup and restore only work for local overrides. But as Pavel mentioned, it may work for local users and groups in the future (id_provider=local). My original confusion was that I also thought it backs-up and restores all ldb files, which is not the case.
No, the intention is to backup only data that are not stored on server and would be lost when the cache is removed. In the time, only local overrides were local. If secrets creates local data, the tool should be modified.
Yes, but users and groups from local domain would also be lost if the local data is deleted. So I though we want to backup them as well in the future versions (I mean the local provider, not the files provider).
Btw. do we want to merge sss_override tool into sssctl? Because if the local-data-* commands work currently with overrides only, then we could make a new topic 'overrides' and add commands like
sssctl overrides-backup sssctl overrides-restore
and later also all the functionality of sss_overrides
sssctl overrides-user-add sssctl overrides-user-del
etc.
This way we could avoid confusion between local-data and cache. If secrets will also create some local data, we will add topic 'secrets' to deal with that separately.
sssctl secrets-backup sssctl secrets-restore
We can then keep the topic 'local' for the local provider related tools (if we want to merge them at some point). Like
sssctl local-user-add (to replace sss_useradd) sssctl local-user-del (to replace sss_userdel) etc.
What do you think?
- cache-backup Backup local data
- cache-restore Restore local data from backup
That was confusion on my side. I thought local data means entire cache. In that case I would propose topic name "local" for actions that work with data that is not present on remote server.
local-backup local-restore
What do you think?
New patch is attached. I also added patch for missing gettext macro (did not want to hide this change in the first patch).
Michal
I'd still stick with local-data-backup and local-data-restore, what do others thing? But otherwise ack.
I'm not sure, on one hand the admins would think 'cache' but on the other hand, if we save and restore all cache data, the we operate on more than the cache..
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
On Thu, Jul 28, 2016 at 12:23:24PM +0200, Michal Židek wrote:
On 07/28/2016 10:00 AM, Pavel Březina wrote:
On 07/27/2016 03:28 PM, Michal Židek wrote:
On 07/27/2016 11:09 AM, Jakub Hrozek wrote:
On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote:
On 07/26/2016 04:19 PM, Michal Židek wrote:
On 07/26/2016 01:19 PM, Pavel Březina wrote: > On 07/25/2016 02:12 PM, Michal Židek wrote: > > Hi, > > > > this patches makes the sssctl commands more similar to > > ipa tool commands. I also think this pattern makes it > > easier to remember the commands. > > > > Note that in the future, there will be more user-* > > group-* and netgroup-* commands (like seed for user, > > list of all etc.) > > > > Comments are welcome. > > > > Michal > > Hi, > ok, it looks like a good idea. When touching the code, can you also > convert sss_override command to use the macro instead? And I think it > may be nice to also add a macro for command sentinel i.e. for {NULL, > ...}. > > I'm not very fond of renaming local-data-* to cache-* so it doesn't > imply that we backup the whole cache content. We only backup and > restore > data that are local to the client and not present in LDAP. Currently > only local overrides, but it may include local users and groups in > the > future.
When we have the files provider there would be a cache as well. Moreover, we store secrets now. The restore command backs up all *.ldb files, right?
This is how I understood it at first, but the current backup and restore only work for local overrides. But as Pavel mentioned, it may work for local users and groups in the future (id_provider=local). My original confusion was that I also thought it backs-up and restores all ldb files, which is not the case.
No, the intention is to backup only data that are not stored on server and would be lost when the cache is removed. In the time, only local overrides were local. If secrets creates local data, the tool should be modified.
Yes, but users and groups from local domain would also be lost if the local data is deleted. So I though we want to backup them as well in the future versions (I mean the local provider, not the files provider).
Well, probably not in the first version, but definitely in a couple of months we want to add the capability to set extended attributes to the files provider which we'll want to back up as well. But I also think we will add the additional data into a new directory, just like we did with the secrets database.
Btw. do we want to merge sss_override tool into sssctl? Because if the local-data-* commands work currently with overrides only, then we could make a new topic 'overrides' and add commands like
I would like to merge all tools into sssctl unless there is a strong reason to keep them separate (the local domain tools should be kept separate IMO).
The reason is simply discoverability, if there is a new sssd release, the admin would just run sssctl and if there are any new tools, their topics would be displayed.
(Also I think the code duplication would be reduced as a side-effect).
sssctl overrides-backup sssctl overrides-restore
and later also all the functionality of sss_overrides
sssctl overrides-user-add sssctl overrides-user-del
Maybe, but later.
etc.
This way we could avoid confusion between local-data and cache. If secrets will also create some local data, we will add topic 'secrets' to deal with that separately.
sssctl secrets-backup sssctl secrets-restore
I think I got lost in the thread :-) What is the benefit of having more backup/restore commands than one that backs up or removes of value under the /var/lib/sss/ structure? So far I can only think of cache being more intuitive to admins than local-data.
We can then keep the topic 'local' for the local provider related tools (if we want to merge them at some point). Like
sssctl local-user-add (to replace sss_useradd) sssctl local-user-del (to replace sss_userdel) etc.
I'm not sure we want to merge id_provider=local into sssctl..
What do you think?
On 07/28/2016 01:38 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 12:23:24PM +0200, Michal Židek wrote:
On 07/28/2016 10:00 AM, Pavel Březina wrote:
On 07/27/2016 03:28 PM, Michal Židek wrote:
On 07/27/2016 11:09 AM, Jakub Hrozek wrote:
On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote:
On 07/26/2016 04:19 PM, Michal Židek wrote: > On 07/26/2016 01:19 PM, Pavel Březina wrote: >> On 07/25/2016 02:12 PM, Michal Židek wrote: >>> Hi, >>> >>> this patches makes the sssctl commands more similar to >>> ipa tool commands. I also think this pattern makes it >>> easier to remember the commands. >>> >>> Note that in the future, there will be more user-* >>> group-* and netgroup-* commands (like seed for user, >>> list of all etc.) >>> >>> Comments are welcome. >>> >>> Michal >> >> Hi, >> ok, it looks like a good idea. When touching the code, can you also >> convert sss_override command to use the macro instead? And I think it >> may be nice to also add a macro for command sentinel i.e. for {NULL, >> ...}. >> >> I'm not very fond of renaming local-data-* to cache-* so it doesn't >> imply that we backup the whole cache content. We only backup and >> restore >> data that are local to the client and not present in LDAP. Currently >> only local overrides, but it may include local users and groups in >> the >> future.
When we have the files provider there would be a cache as well. Moreover, we store secrets now. The restore command backs up all *.ldb files, right?
This is how I understood it at first, but the current backup and restore only work for local overrides. But as Pavel mentioned, it may work for local users and groups in the future (id_provider=local). My original confusion was that I also thought it backs-up and restores all ldb files, which is not the case.
No, the intention is to backup only data that are not stored on server and would be lost when the cache is removed. In the time, only local overrides were local. If secrets creates local data, the tool should be modified.
Yes, but users and groups from local domain would also be lost if the local data is deleted. So I though we want to backup them as well in the future versions (I mean the local provider, not the files provider).
Well, probably not in the first version, but definitely in a couple of months we want to add the capability to set extended attributes to the files provider which we'll want to back up as well. But I also think we will add the additional data into a new directory, just like we did with the secrets database.
Btw. do we want to merge sss_override tool into sssctl? Because if the local-data-* commands work currently with overrides only, then we could make a new topic 'overrides' and add commands like
I would like to merge all tools into sssctl unless there is a strong reason to keep them separate (the local domain tools should be kept separate IMO).
The reason is simply discoverability, if there is a new sssd release, the admin would just run sssctl and if there are any new tools, their topics would be displayed.
(Also I think the code duplication would be reduced as a side-effect).
sssctl overrides-backup sssctl overrides-restore
and later also all the functionality of sss_overrides
sssctl overrides-user-add sssctl overrides-user-del
Maybe, but later.
etc.
This way we could avoid confusion between local-data and cache. If secrets will also create some local data, we will add topic 'secrets' to deal with that separately.
sssctl secrets-backup sssctl secrets-restore
I think I got lost in the thread :-) What is the benefit of having more backup/restore commands than one that backs up or removes of value under the /var/lib/sss/ structure? So far I can only think of cache being more intuitive to admins than local-data.
How about client-data instead of local-data?
We can then keep the topic 'local' for the local provider related tools (if we want to merge them at some point). Like
sssctl local-user-add (to replace sss_useradd) sssctl local-user-del (to replace sss_userdel) etc.
I'm not sure we want to merge id_provider=local into sssctl..
What do you think?
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
On Thu, Jul 28, 2016 at 01:51:40PM +0200, Pavel Březina wrote:
On 07/28/2016 01:38 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 12:23:24PM +0200, Michal Židek wrote:
On 07/28/2016 10:00 AM, Pavel Březina wrote:
On 07/27/2016 03:28 PM, Michal Židek wrote:
On 07/27/2016 11:09 AM, Jakub Hrozek wrote:
On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote: > On 07/26/2016 04:19 PM, Michal Židek wrote: > > On 07/26/2016 01:19 PM, Pavel Březina wrote: > > > On 07/25/2016 02:12 PM, Michal Židek wrote: > > > > Hi, > > > > > > > > this patches makes the sssctl commands more similar to > > > > ipa tool commands. I also think this pattern makes it > > > > easier to remember the commands. > > > > > > > > Note that in the future, there will be more user-* > > > > group-* and netgroup-* commands (like seed for user, > > > > list of all etc.) > > > > > > > > Comments are welcome. > > > > > > > > Michal > > > > > > Hi, > > > ok, it looks like a good idea. When touching the code, can you also > > > convert sss_override command to use the macro instead? And I think it > > > may be nice to also add a macro for command sentinel i.e. for {NULL, > > > ...}. > > > > > > I'm not very fond of renaming local-data-* to cache-* so it doesn't > > > imply that we backup the whole cache content. We only backup and > > > restore > > > data that are local to the client and not present in LDAP. Currently > > > only local overrides, but it may include local users and groups in > > > the > > > future.
When we have the files provider there would be a cache as well. Moreover, we store secrets now. The restore command backs up all *.ldb files, right?
This is how I understood it at first, but the current backup and restore only work for local overrides. But as Pavel mentioned, it may work for local users and groups in the future (id_provider=local). My original confusion was that I also thought it backs-up and restores all ldb files, which is not the case.
No, the intention is to backup only data that are not stored on server and would be lost when the cache is removed. In the time, only local overrides were local. If secrets creates local data, the tool should be modified.
Yes, but users and groups from local domain would also be lost if the local data is deleted. So I though we want to backup them as well in the future versions (I mean the local provider, not the files provider).
Well, probably not in the first version, but definitely in a couple of months we want to add the capability to set extended attributes to the files provider which we'll want to back up as well. But I also think we will add the additional data into a new directory, just like we did with the secrets database.
Btw. do we want to merge sss_override tool into sssctl? Because if the local-data-* commands work currently with overrides only, then we could make a new topic 'overrides' and add commands like
I would like to merge all tools into sssctl unless there is a strong reason to keep them separate (the local domain tools should be kept separate IMO).
The reason is simply discoverability, if there is a new sssd release, the admin would just run sssctl and if there are any new tools, their topics would be displayed.
(Also I think the code duplication would be reduced as a side-effect).
sssctl overrides-backup sssctl overrides-restore
and later also all the functionality of sss_overrides
sssctl overrides-user-add sssctl overrides-user-del
Maybe, but later.
etc.
This way we could avoid confusion between local-data and cache. If secrets will also create some local data, we will add topic 'secrets' to deal with that separately.
sssctl secrets-backup sssctl secrets-restore
I think I got lost in the thread :-) What is the benefit of having more backup/restore commands than one that backs up or removes of value under the /var/lib/sss/ structure? So far I can only think of cache being more intuitive to admins than local-data.
How about client-data instead of local-data?
SGTM (sounds good to me :-))
On 07/28/2016 01:57 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 01:51:40PM +0200, Pavel Březina wrote:
On 07/28/2016 01:38 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 12:23:24PM +0200, Michal Židek wrote:
On 07/28/2016 10:00 AM, Pavel Březina wrote:
On 07/27/2016 03:28 PM, Michal Židek wrote:
On 07/27/2016 11:09 AM, Jakub Hrozek wrote: > On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote: >> On 07/26/2016 04:19 PM, Michal Židek wrote: >>> On 07/26/2016 01:19 PM, Pavel Březina wrote: >>>> On 07/25/2016 02:12 PM, Michal Židek wrote: >>>>> Hi, >>>>> >>>>> this patches makes the sssctl commands more similar to >>>>> ipa tool commands. I also think this pattern makes it >>>>> easier to remember the commands. >>>>> >>>>> Note that in the future, there will be more user-* >>>>> group-* and netgroup-* commands (like seed for user, >>>>> list of all etc.) >>>>> >>>>> Comments are welcome. >>>>> >>>>> Michal >>>> >>>> Hi, >>>> ok, it looks like a good idea. When touching the code, can you also >>>> convert sss_override command to use the macro instead? And I think it >>>> may be nice to also add a macro for command sentinel i.e. for {NULL, >>>> ...}. >>>> >>>> I'm not very fond of renaming local-data-* to cache-* so it doesn't >>>> imply that we backup the whole cache content. We only backup and >>>> restore >>>> data that are local to the client and not present in LDAP. Currently >>>> only local overrides, but it may include local users and groups in >>>> the >>>> future. > > When we have the files provider there would > be a cache as well. Moreover, we store secrets now. The restore command > backs up all *.ldb files, right?
This is how I understood it at first, but the current backup and restore only work for local overrides. But as Pavel mentioned, it may work for local users and groups in the future (id_provider=local). My original confusion was that I also thought it backs-up and restores all ldb files, which is not the case.
No, the intention is to backup only data that are not stored on server and would be lost when the cache is removed. In the time, only local overrides were local. If secrets creates local data, the tool should be modified.
Yes, but users and groups from local domain would also be lost if the local data is deleted. So I though we want to backup them as well in the future versions (I mean the local provider, not the files provider).
Well, probably not in the first version, but definitely in a couple of months we want to add the capability to set extended attributes to the files provider which we'll want to back up as well. But I also think we will add the additional data into a new directory, just like we did with the secrets database.
Btw. do we want to merge sss_override tool into sssctl? Because if the local-data-* commands work currently with overrides only, then we could make a new topic 'overrides' and add commands like
I would like to merge all tools into sssctl unless there is a strong reason to keep them separate (the local domain tools should be kept separate IMO).
The reason is simply discoverability, if there is a new sssd release, the admin would just run sssctl and if there are any new tools, their topics would be displayed.
(Also I think the code duplication would be reduced as a side-effect).
sssctl overrides-backup sssctl overrides-restore
and later also all the functionality of sss_overrides
sssctl overrides-user-add sssctl overrides-user-del
Maybe, but later.
etc.
This way we could avoid confusion between local-data and cache. If secrets will also create some local data, we will add topic 'secrets' to deal with that separately.
sssctl secrets-backup sssctl secrets-restore
I think I got lost in the thread :-) What is the benefit of having more backup/restore commands than one that backs up or removes of value under the /var/lib/sss/ structure? So far I can only think of cache being more intuitive to admins than local-data.
How about client-data instead of local-data?
SGTM (sounds good to me :-))
SGTM too :)
I will send version with client-data after the meeting.
Michal
On 07/28/2016 02:11 PM, Michal Židek wrote:
On 07/28/2016 01:57 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 01:51:40PM +0200, Pavel Březina wrote:
On 07/28/2016 01:38 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 12:23:24PM +0200, Michal Židek wrote:
On 07/28/2016 10:00 AM, Pavel Březina wrote:
On 07/27/2016 03:28 PM, Michal Židek wrote: > On 07/27/2016 11:09 AM, Jakub Hrozek wrote: >> On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote: >>> On 07/26/2016 04:19 PM, Michal Židek wrote: >>>> On 07/26/2016 01:19 PM, Pavel Březina wrote: >>>>> On 07/25/2016 02:12 PM, Michal Židek wrote: >>>>>> Hi, >>>>>> >>>>>> this patches makes the sssctl commands more similar to >>>>>> ipa tool commands. I also think this pattern makes it >>>>>> easier to remember the commands. >>>>>> >>>>>> Note that in the future, there will be more user-* >>>>>> group-* and netgroup-* commands (like seed for user, >>>>>> list of all etc.) >>>>>> >>>>>> Comments are welcome. >>>>>> >>>>>> Michal >>>>> >>>>> Hi, >>>>> ok, it looks like a good idea. When touching the code, can >>>>> you also >>>>> convert sss_override command to use the macro instead? And I >>>>> think it >>>>> may be nice to also add a macro for command sentinel i.e. for >>>>> {NULL, >>>>> ...}. >>>>> >>>>> I'm not very fond of renaming local-data-* to cache-* so it >>>>> doesn't >>>>> imply that we backup the whole cache content. We only backup and >>>>> restore >>>>> data that are local to the client and not present in LDAP. >>>>> Currently >>>>> only local overrides, but it may include local users and >>>>> groups in >>>>> the >>>>> future. >> >> When we have the files provider there would >> be a cache as well. Moreover, we store secrets now. The restore >> command >> backs up all *.ldb files, right? > > This is how I understood it at first, but the current backup > and restore only work for local overrides. But as Pavel mentioned, > it may work for local users and groups in the future > (id_provider=local). My original confusion was that I also > thought it backs-up and restores all ldb files, which is > not the case.
No, the intention is to backup only data that are not stored on server and would be lost when the cache is removed. In the time, only local overrides were local. If secrets creates local data, the tool should be modified.
Yes, but users and groups from local domain would also be lost if the local data is deleted. So I though we want to backup them as well in the future versions (I mean the local provider, not the files provider).
Well, probably not in the first version, but definitely in a couple of months we want to add the capability to set extended attributes to the files provider which we'll want to back up as well. But I also think we will add the additional data into a new directory, just like we did with the secrets database.
Btw. do we want to merge sss_override tool into sssctl? Because if the local-data-* commands work currently with overrides only, then we could make a new topic 'overrides' and add commands like
I would like to merge all tools into sssctl unless there is a strong reason to keep them separate (the local domain tools should be kept separate IMO).
The reason is simply discoverability, if there is a new sssd release, the admin would just run sssctl and if there are any new tools, their topics would be displayed.
(Also I think the code duplication would be reduced as a side-effect).
sssctl overrides-backup sssctl overrides-restore
and later also all the functionality of sss_overrides
sssctl overrides-user-add sssctl overrides-user-del
Maybe, but later.
etc.
This way we could avoid confusion between local-data and cache. If secrets will also create some local data, we will add topic 'secrets' to deal with that separately.
sssctl secrets-backup sssctl secrets-restore
I think I got lost in the thread :-) What is the benefit of having more backup/restore commands than one that backs up or removes of value under the /var/lib/sss/ structure? So far I can only think of cache being more intuitive to admins than local-data.
How about client-data instead of local-data?
SGTM (sounds good to me :-))
SGTM too :)
I will send version with client-data after the meeting.
I forgot to update this after the meeting. New patch is attached.
Michal
On (28/07/16 19:37), Michal Židek wrote:
On 07/28/2016 02:11 PM, Michal Židek wrote:
On 07/28/2016 01:57 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 01:51:40PM +0200, Pavel Březina wrote:
On 07/28/2016 01:38 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 12:23:24PM +0200, Michal Židek wrote:
On 07/28/2016 10:00 AM, Pavel Březina wrote: > On 07/27/2016 03:28 PM, Michal Židek wrote: > > On 07/27/2016 11:09 AM, Jakub Hrozek wrote: > > > On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote: > > > > On 07/26/2016 04:19 PM, Michal Židek wrote: > > > > > On 07/26/2016 01:19 PM, Pavel Březina wrote: > > > > > > On 07/25/2016 02:12 PM, Michal Židek wrote: > > > > > > > Hi, > > > > > > > > > > > > > > this patches makes the sssctl commands more similar to > > > > > > > ipa tool commands. I also think this pattern makes it > > > > > > > easier to remember the commands. > > > > > > > > > > > > > > Note that in the future, there will be more user-* > > > > > > > group-* and netgroup-* commands (like seed for user, > > > > > > > list of all etc.) > > > > > > > > > > > > > > Comments are welcome. > > > > > > > > > > > > > > Michal > > > > > > > > > > > > Hi, > > > > > > ok, it looks like a good idea. When touching the code, can > > > > > > you also > > > > > > convert sss_override command to use the macro instead? And I > > > > > > think it > > > > > > may be nice to also add a macro for command sentinel i.e. for > > > > > > {NULL, > > > > > > ...}. > > > > > > > > > > > > I'm not very fond of renaming local-data-* to cache-* so it > > > > > > doesn't > > > > > > imply that we backup the whole cache content. We only backup and > > > > > > restore > > > > > > data that are local to the client and not present in LDAP. > > > > > > Currently > > > > > > only local overrides, but it may include local users and > > > > > > groups in > > > > > > the > > > > > > future. > > > > > > When we have the files provider there would > > > be a cache as well. Moreover, we store secrets now. The restore > > > command > > > backs up all *.ldb files, right? > > > > This is how I understood it at first, but the current backup > > and restore only work for local overrides. But as Pavel mentioned, > > it may work for local users and groups in the future > > (id_provider=local). My original confusion was that I also > > thought it backs-up and restores all ldb files, which is > > not the case. > > No, the intention is to backup only data that are not stored on > server > and would be lost when the cache is removed. In the time, only local > overrides were local. If secrets creates local data, the tool > should be > modified.
Yes, but users and groups from local domain would also be lost if the local data is deleted. So I though we want to backup them as well in the future versions (I mean the local provider, not the files provider).
Well, probably not in the first version, but definitely in a couple of months we want to add the capability to set extended attributes to the files provider which we'll want to back up as well. But I also think we will add the additional data into a new directory, just like we did with the secrets database.
Btw. do we want to merge sss_override tool into sssctl? Because if the local-data-* commands work currently with overrides only, then we could make a new topic 'overrides' and add commands like
I would like to merge all tools into sssctl unless there is a strong reason to keep them separate (the local domain tools should be kept separate IMO).
The reason is simply discoverability, if there is a new sssd release, the admin would just run sssctl and if there are any new tools, their topics would be displayed.
(Also I think the code duplication would be reduced as a side-effect).
sssctl overrides-backup sssctl overrides-restore
and later also all the functionality of sss_overrides
sssctl overrides-user-add sssctl overrides-user-del
Maybe, but later.
etc.
This way we could avoid confusion between local-data and cache. If secrets will also create some local data, we will add topic 'secrets' to deal with that separately.
sssctl secrets-backup sssctl secrets-restore
I think I got lost in the thread :-) What is the benefit of having more backup/restore commands than one that backs up or removes of value under the /var/lib/sss/ structure? So far I can only think of cache being more intuitive to admins than local-data.
How about client-data instead of local-data?
SGTM (sounds good to me :-))
SGTM too :)
I will send version with client-data after the meeting.
I forgot to update this after the meeting. New patch is attached.
Michal
From 73c3e97013274ff644aa630547494fd79b1854c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Mon, 25 Jul 2016 13:50:13 +0200 Subject: [PATCH 1/3] sssctl: Consistent commands naming
Fixes: https://fedorahosted.org/sssd/ticket/3087
Use TOPIC-ACTION pattern for sssctl command names.
src/tools/common/sss_tools.c | 4 ++++ src/tools/common/sss_tools.h | 8 ++++++-- src/tools/sss_override.c | 26 ++++++++++++------------ src/tools/sssctl/sssctl.c | 33 ++++++++++++++++++++---------- src/tools/sssctl/sssctl.h | 42 +++++++++++++++++++-------------------- src/tools/sssctl/sssctl_cache.c | 18 ++++++++--------- src/tools/sssctl/sssctl_data.c | 16 +++++++-------- src/tools/sssctl/sssctl_domains.c | 6 +++--- src/tools/sssctl/sssctl_logs.c | 4 ++-- 9 files changed, 88 insertions(+), 69 deletions(-)
diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c index 686b53a..4977358 100644 --- a/src/tools/common/sss_tools.c +++ b/src/tools/common/sss_tools.c @@ -283,6 +283,10 @@ void sss_tool_usage(const char *tool_name, struct sss_route_cmd *commands) min_len = sss_tool_max_length(commands);
for (i = 0; commands[i].command != NULL; i++) {
if (commands[i].hidden) {
continue;
}
if (sss_tool_is_delimiter(&commands[i])) { fprintf(stderr, "\n%s\n", commands[i].description); continue;
diff --git a/src/tools/common/sss_tools.h b/src/tools/common/sss_tools.h index a9ebabe..b567ea4 100644 --- a/src/tools/common/sss_tools.h +++ b/src/tools/common/sss_tools.h @@ -45,14 +45,18 @@ typedef errno_t struct sss_tool_ctx *tool_ctx, void *pvt);
-#define SSS_TOOL_COMMAND(cmd, msg, err, fn) {cmd, _(msg), err, fn} -#define SSS_TOOL_DELIMITER(message) {"", (message), 0, NULL} +#define SSS_TOOL_COMMAND(cmd, msg, err, fn) {cmd, _(msg), err, fn, false} +#define SSS_TOOL_COMMAND_NOMSG(cmd, err, fn) {cmd, NULL, err, fn, false} +#define SSS_TOOL_COMMAND_OBSOLETE(cmd, err, fn) {cmd, NULL, err, fn, true} +#define SSS_TOOL_DELIMITER(message) {"", (message), 0, NULL, false} +#define SSS_TOOL_LAST {NULL, NULL, 0, NULL, false}
struct sss_route_cmd { const char *command; const char *description; errno_t handles_init_err; sss_route_fn fn;
- bool hidden; /* Do not show this command in sssctl usage */
};
void sss_tool_usage(const char *tool_name, diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c index 45a28fd..d41da52 100644 --- a/src/tools/sss_override.c +++ b/src/tools/sss_override.c @@ -1913,19 +1913,19 @@ static int override_group_export(struct sss_cmdline *cmdline, int main(int argc, const char **argv) { struct sss_route_cmd commands[] = {
{"user-add", NULL, 0, override_user_add},
{"user-del", NULL, 0, override_user_del},
{"user-find", NULL, 0, override_user_find},
{"user-show", NULL, 0, override_user_show},
{"user-import", NULL, 0, override_user_import},
{"user-export", NULL, 0, override_user_export},
{"group-add", NULL, 0, override_group_add},
{"group-del", NULL, 0, override_group_del},
{"group-find", NULL, 0, override_group_find},
{"group-show", NULL, 0, override_group_show},
{"group-import", NULL, 0, override_group_import},
{"group-export", NULL, 0, override_group_export},
{NULL, NULL, 0, NULL}
SSS_TOOL_COMMAND_NOMSG("user-add", 0, override_user_add),
SSS_TOOL_COMMAND_NOMSG("user-del", 0, override_user_del),
SSS_TOOL_COMMAND_NOMSG("user-find", 0, override_user_find),
SSS_TOOL_COMMAND_NOMSG("user-show", 0, override_user_show),
SSS_TOOL_COMMAND_NOMSG("user-import", 0, override_user_import),
SSS_TOOL_COMMAND_NOMSG("user-export", 0, override_user_export),
SSS_TOOL_COMMAND_NOMSG("group-add", 0, override_group_add),
SSS_TOOL_COMMAND_NOMSG("group-del", 0, override_group_del),
SSS_TOOL_COMMAND_NOMSG("group-find", 0, override_group_find),
SSS_TOOL_COMMAND_NOMSG("group-show", 0, override_group_show),
SSS_TOOL_COMMAND_NOMSG("group-import", 0, override_group_import),
SSS_TOOL_COMMAND_NOMSG("group-export", 0, override_group_export),
SSS_TOOL_LAST
};
return sss_tool_main(argc, argv, commands, NULL);
diff --git a/src/tools/sssctl/sssctl.c b/src/tools/sssctl/sssctl.c index 86656f1..8ce93b2 100644 --- a/src/tools/sssctl/sssctl.c +++ b/src/tools/sssctl/sssctl.c @@ -257,25 +257,36 @@ int main(int argc, const char **argv) { struct sss_route_cmd commands[] = { SSS_TOOL_DELIMITER("SSSD Status:"),
SSS_TOOL_COMMAND("list-domains", "List available domains", 0, sssctl_list_domains),
SSS_TOOL_COMMAND("domain-list", "List available domains", 0, sssctl_domain_list), SSS_TOOL_COMMAND("domain-status", "Print information about domain", 0, sssctl_domain_status), SSS_TOOL_DELIMITER("Information about cached content:"),
SSS_TOOL_COMMAND("user", "Information about cached user", 0, sssctl_user),
SSS_TOOL_COMMAND("group", "Information about cached group", 0, sssctl_group),
SSS_TOOL_COMMAND("netgroup", "Information about cached netgroup", 0, sssctl_netgroup),
SSS_TOOL_COMMAND("user-show", "Information about cached user", 0, sssctl_user_show),
SSS_TOOL_COMMAND("group-show", "Information about cached group", 0, sssctl_group_show),
SSS_TOOL_COMMAND("netgroup-show", "Information about cached netgroup", 0, sssctl_netgroup_show), SSS_TOOL_DELIMITER("Local data tools:"),
SSS_TOOL_COMMAND("backup-local-data", "Backup local data", 0, sssctl_backup_local_data),
SSS_TOOL_COMMAND("restore-local-data", "Restore local data from backup", 0, sssctl_restore_local_data),
SSS_TOOL_COMMAND("remove-cache", "Backup local data and remove cached content", 0, sssctl_remove_cache),
SSS_TOOL_COMMAND("upgrade-cache", "Perform cache upgrade", ERR_SYSDB_VERSION_TOO_OLD, sssctl_upgrade_cache),
SSS_TOOL_COMMAND("client-data-backup", "Backup local data", 0, sssctl_client_data_backup),
SSS_TOOL_COMMAND("client-data-restore", "Restore local data from backup", 0, sssctl_client_data_restore),
SSS_TOOL_COMMAND("cache-remove", "Backup local data and remove cached content", 0, sssctl_cache_remove),
SSS_TOOL_COMMAND("cache-upgrade", "Perform cache upgrade", ERR_SYSDB_VERSION_TOO_OLD, sssctl_cache_upgrade), SSS_TOOL_DELIMITER("Log files tools:"),
SSS_TOOL_COMMAND("remove-logs", "Remove existing SSSD log files", 0, sssctl_remove_logs),
SSS_TOOL_COMMAND("fetch-logs", "Archive SSSD log files in tarball", 0, sssctl_fetch_logs),
SSS_TOOL_COMMAND("logs-remove", "Remove existing SSSD log files", 0, sssctl_logs_remove),
SSS_TOOL_COMMAND("logs-fetch", "Archive SSSD log files in tarball", 0, sssctl_logs_fetch),
#ifdef HAVE_LIBINI_CONFIG_V1_3 SSS_TOOL_DELIMITER("Configuration files tools:"), SSS_TOOL_COMMAND("config-check", "Perform static analysis of SSSD configuration", 0, sssctl_config_check), #endif
{NULL, NULL, 0, NULL}
/* Obsolete commands */
SSS_TOOL_COMMAND_OBSOLETE("list-domains", 0, sssctl_domain_list),
SSS_TOOL_COMMAND_OBSOLETE("user", 0, sssctl_user_show),
SSS_TOOL_COMMAND_OBSOLETE("group", 0, sssctl_group_show),
SSS_TOOL_COMMAND_OBSOLETE("netgroup", 0, sssctl_netgroup_show),
SSS_TOOL_COMMAND_OBSOLETE("backup-local-data", 0, sssctl_client_data_backup),
SSS_TOOL_COMMAND_OBSOLETE("restore-local-data", 0, sssctl_client_data_restore),
SSS_TOOL_COMMAND_OBSOLETE("remove-cache", 0, sssctl_cache_remove),
SSS_TOOL_COMMAND_OBSOLETE("upgrade-cache", ERR_SYSDB_VERSION_TOO_OLD, sssctl_cache_upgrade),
SSS_TOOL_COMMAND_OBSOLETE("remove-logs", 0, sssctl_logs_remove),
SSS_TOOL_COMMAND_OBSOLETE("fetch-logs", 0, sssctl_logs_fetch),
Do we really need to be so "backward compatible"
a) there was a desing[1] and nobody complained b) 1.14.0 is used very rarely
BTW We should firstly change desing page. Then ask for review and then send a patch. Cond-NACK to patch till desing page will be updated and change will be accepted by others.
LS
On Fri, Jul 29, 2016 at 09:31:32AM +0200, Lukas Slebodnik wrote:
On (28/07/16 19:37), Michal Židek wrote:
On 07/28/2016 02:11 PM, Michal Židek wrote:
On 07/28/2016 01:57 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 01:51:40PM +0200, Pavel Březina wrote:
On 07/28/2016 01:38 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 12:23:24PM +0200, Michal Židek wrote: > On 07/28/2016 10:00 AM, Pavel Březina wrote: > > On 07/27/2016 03:28 PM, Michal Židek wrote: > > > On 07/27/2016 11:09 AM, Jakub Hrozek wrote: > > > > On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote: > > > > > On 07/26/2016 04:19 PM, Michal Židek wrote: > > > > > > On 07/26/2016 01:19 PM, Pavel Březina wrote: > > > > > > > On 07/25/2016 02:12 PM, Michal Židek wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > this patches makes the sssctl commands more similar to > > > > > > > > ipa tool commands. I also think this pattern makes it > > > > > > > > easier to remember the commands. > > > > > > > > > > > > > > > > Note that in the future, there will be more user-* > > > > > > > > group-* and netgroup-* commands (like seed for user, > > > > > > > > list of all etc.) > > > > > > > > > > > > > > > > Comments are welcome. > > > > > > > > > > > > > > > > Michal > > > > > > > > > > > > > > Hi, > > > > > > > ok, it looks like a good idea. When touching the code, can > > > > > > > you also > > > > > > > convert sss_override command to use the macro instead? And I > > > > > > > think it > > > > > > > may be nice to also add a macro for command sentinel i.e. for > > > > > > > {NULL, > > > > > > > ...}. > > > > > > > > > > > > > > I'm not very fond of renaming local-data-* to cache-* so it > > > > > > > doesn't > > > > > > > imply that we backup the whole cache content. We only backup and > > > > > > > restore > > > > > > > data that are local to the client and not present in LDAP. > > > > > > > Currently > > > > > > > only local overrides, but it may include local users and > > > > > > > groups in > > > > > > > the > > > > > > > future. > > > > > > > > When we have the files provider there would > > > > be a cache as well. Moreover, we store secrets now. The restore > > > > command > > > > backs up all *.ldb files, right? > > > > > > This is how I understood it at first, but the current backup > > > and restore only work for local overrides. But as Pavel mentioned, > > > it may work for local users and groups in the future > > > (id_provider=local). My original confusion was that I also > > > thought it backs-up and restores all ldb files, which is > > > not the case. > > > > No, the intention is to backup only data that are not stored on > > server > > and would be lost when the cache is removed. In the time, only local > > overrides were local. If secrets creates local data, the tool > > should be > > modified. > > Yes, but users and groups from local domain would also be > lost if the local data is deleted. So I though we want to backup > them as well in the future versions (I mean the local provider, > not the files provider).
Well, probably not in the first version, but definitely in a couple of months we want to add the capability to set extended attributes to the files provider which we'll want to back up as well. But I also think we will add the additional data into a new directory, just like we did with the secrets database.
> > Btw. do we want to merge sss_override tool into sssctl? > Because if the local-data-* commands work currently with > overrides only, then we could make a new topic 'overrides' and add > commands > like
I would like to merge all tools into sssctl unless there is a strong reason to keep them separate (the local domain tools should be kept separate IMO).
The reason is simply discoverability, if there is a new sssd release, the admin would just run sssctl and if there are any new tools, their topics would be displayed.
(Also I think the code duplication would be reduced as a side-effect).
> > sssctl overrides-backup > sssctl overrides-restore > > and later also all the functionality of sss_overrides > > sssctl overrides-user-add > sssctl overrides-user-del
Maybe, but later.
> > etc. > > This way we could avoid confusion between local-data and > cache. If secrets will also create some local data, we will > add topic 'secrets' to deal with that separately. > > sssctl secrets-backup > sssctl secrets-restore
I think I got lost in the thread :-) What is the benefit of having more backup/restore commands than one that backs up or removes of value under the /var/lib/sss/ structure? So far I can only think of cache being more intuitive to admins than local-data.
How about client-data instead of local-data?
SGTM (sounds good to me :-))
SGTM too :)
I will send version with client-data after the meeting.
I forgot to update this after the meeting. New patch is attached.
Michal
From 73c3e97013274ff644aa630547494fd79b1854c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Mon, 25 Jul 2016 13:50:13 +0200 Subject: [PATCH 1/3] sssctl: Consistent commands naming
Fixes: https://fedorahosted.org/sssd/ticket/3087
Use TOPIC-ACTION pattern for sssctl command names.
src/tools/common/sss_tools.c | 4 ++++ src/tools/common/sss_tools.h | 8 ++++++-- src/tools/sss_override.c | 26 ++++++++++++------------ src/tools/sssctl/sssctl.c | 33 ++++++++++++++++++++---------- src/tools/sssctl/sssctl.h | 42 +++++++++++++++++++-------------------- src/tools/sssctl/sssctl_cache.c | 18 ++++++++--------- src/tools/sssctl/sssctl_data.c | 16 +++++++-------- src/tools/sssctl/sssctl_domains.c | 6 +++--- src/tools/sssctl/sssctl_logs.c | 4 ++-- 9 files changed, 88 insertions(+), 69 deletions(-)
diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c index 686b53a..4977358 100644 --- a/src/tools/common/sss_tools.c +++ b/src/tools/common/sss_tools.c @@ -283,6 +283,10 @@ void sss_tool_usage(const char *tool_name, struct sss_route_cmd *commands) min_len = sss_tool_max_length(commands);
for (i = 0; commands[i].command != NULL; i++) {
if (commands[i].hidden) {
continue;
}
if (sss_tool_is_delimiter(&commands[i])) { fprintf(stderr, "\n%s\n", commands[i].description); continue;
diff --git a/src/tools/common/sss_tools.h b/src/tools/common/sss_tools.h index a9ebabe..b567ea4 100644 --- a/src/tools/common/sss_tools.h +++ b/src/tools/common/sss_tools.h @@ -45,14 +45,18 @@ typedef errno_t struct sss_tool_ctx *tool_ctx, void *pvt);
-#define SSS_TOOL_COMMAND(cmd, msg, err, fn) {cmd, _(msg), err, fn} -#define SSS_TOOL_DELIMITER(message) {"", (message), 0, NULL} +#define SSS_TOOL_COMMAND(cmd, msg, err, fn) {cmd, _(msg), err, fn, false} +#define SSS_TOOL_COMMAND_NOMSG(cmd, err, fn) {cmd, NULL, err, fn, false} +#define SSS_TOOL_COMMAND_OBSOLETE(cmd, err, fn) {cmd, NULL, err, fn, true} +#define SSS_TOOL_DELIMITER(message) {"", (message), 0, NULL, false} +#define SSS_TOOL_LAST {NULL, NULL, 0, NULL, false}
struct sss_route_cmd { const char *command; const char *description; errno_t handles_init_err; sss_route_fn fn;
- bool hidden; /* Do not show this command in sssctl usage */
};
void sss_tool_usage(const char *tool_name, diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c index 45a28fd..d41da52 100644 --- a/src/tools/sss_override.c +++ b/src/tools/sss_override.c @@ -1913,19 +1913,19 @@ static int override_group_export(struct sss_cmdline *cmdline, int main(int argc, const char **argv) { struct sss_route_cmd commands[] = {
{"user-add", NULL, 0, override_user_add},
{"user-del", NULL, 0, override_user_del},
{"user-find", NULL, 0, override_user_find},
{"user-show", NULL, 0, override_user_show},
{"user-import", NULL, 0, override_user_import},
{"user-export", NULL, 0, override_user_export},
{"group-add", NULL, 0, override_group_add},
{"group-del", NULL, 0, override_group_del},
{"group-find", NULL, 0, override_group_find},
{"group-show", NULL, 0, override_group_show},
{"group-import", NULL, 0, override_group_import},
{"group-export", NULL, 0, override_group_export},
{NULL, NULL, 0, NULL}
SSS_TOOL_COMMAND_NOMSG("user-add", 0, override_user_add),
SSS_TOOL_COMMAND_NOMSG("user-del", 0, override_user_del),
SSS_TOOL_COMMAND_NOMSG("user-find", 0, override_user_find),
SSS_TOOL_COMMAND_NOMSG("user-show", 0, override_user_show),
SSS_TOOL_COMMAND_NOMSG("user-import", 0, override_user_import),
SSS_TOOL_COMMAND_NOMSG("user-export", 0, override_user_export),
SSS_TOOL_COMMAND_NOMSG("group-add", 0, override_group_add),
SSS_TOOL_COMMAND_NOMSG("group-del", 0, override_group_del),
SSS_TOOL_COMMAND_NOMSG("group-find", 0, override_group_find),
SSS_TOOL_COMMAND_NOMSG("group-show", 0, override_group_show),
SSS_TOOL_COMMAND_NOMSG("group-import", 0, override_group_import),
SSS_TOOL_COMMAND_NOMSG("group-export", 0, override_group_export),
SSS_TOOL_LAST
};
return sss_tool_main(argc, argv, commands, NULL);
diff --git a/src/tools/sssctl/sssctl.c b/src/tools/sssctl/sssctl.c index 86656f1..8ce93b2 100644 --- a/src/tools/sssctl/sssctl.c +++ b/src/tools/sssctl/sssctl.c @@ -257,25 +257,36 @@ int main(int argc, const char **argv) { struct sss_route_cmd commands[] = { SSS_TOOL_DELIMITER("SSSD Status:"),
SSS_TOOL_COMMAND("list-domains", "List available domains", 0, sssctl_list_domains),
SSS_TOOL_COMMAND("domain-list", "List available domains", 0, sssctl_domain_list), SSS_TOOL_COMMAND("domain-status", "Print information about domain", 0, sssctl_domain_status), SSS_TOOL_DELIMITER("Information about cached content:"),
SSS_TOOL_COMMAND("user", "Information about cached user", 0, sssctl_user),
SSS_TOOL_COMMAND("group", "Information about cached group", 0, sssctl_group),
SSS_TOOL_COMMAND("netgroup", "Information about cached netgroup", 0, sssctl_netgroup),
SSS_TOOL_COMMAND("user-show", "Information about cached user", 0, sssctl_user_show),
SSS_TOOL_COMMAND("group-show", "Information about cached group", 0, sssctl_group_show),
SSS_TOOL_COMMAND("netgroup-show", "Information about cached netgroup", 0, sssctl_netgroup_show), SSS_TOOL_DELIMITER("Local data tools:"),
SSS_TOOL_COMMAND("backup-local-data", "Backup local data", 0, sssctl_backup_local_data),
SSS_TOOL_COMMAND("restore-local-data", "Restore local data from backup", 0, sssctl_restore_local_data),
SSS_TOOL_COMMAND("remove-cache", "Backup local data and remove cached content", 0, sssctl_remove_cache),
SSS_TOOL_COMMAND("upgrade-cache", "Perform cache upgrade", ERR_SYSDB_VERSION_TOO_OLD, sssctl_upgrade_cache),
SSS_TOOL_COMMAND("client-data-backup", "Backup local data", 0, sssctl_client_data_backup),
SSS_TOOL_COMMAND("client-data-restore", "Restore local data from backup", 0, sssctl_client_data_restore),
SSS_TOOL_COMMAND("cache-remove", "Backup local data and remove cached content", 0, sssctl_cache_remove),
SSS_TOOL_COMMAND("cache-upgrade", "Perform cache upgrade", ERR_SYSDB_VERSION_TOO_OLD, sssctl_cache_upgrade), SSS_TOOL_DELIMITER("Log files tools:"),
SSS_TOOL_COMMAND("remove-logs", "Remove existing SSSD log files", 0, sssctl_remove_logs),
SSS_TOOL_COMMAND("fetch-logs", "Archive SSSD log files in tarball", 0, sssctl_fetch_logs),
SSS_TOOL_COMMAND("logs-remove", "Remove existing SSSD log files", 0, sssctl_logs_remove),
SSS_TOOL_COMMAND("logs-fetch", "Archive SSSD log files in tarball", 0, sssctl_logs_fetch),
#ifdef HAVE_LIBINI_CONFIG_V1_3 SSS_TOOL_DELIMITER("Configuration files tools:"), SSS_TOOL_COMMAND("config-check", "Perform static analysis of SSSD configuration", 0, sssctl_config_check), #endif
{NULL, NULL, 0, NULL}
/* Obsolete commands */
SSS_TOOL_COMMAND_OBSOLETE("list-domains", 0, sssctl_domain_list),
SSS_TOOL_COMMAND_OBSOLETE("user", 0, sssctl_user_show),
SSS_TOOL_COMMAND_OBSOLETE("group", 0, sssctl_group_show),
SSS_TOOL_COMMAND_OBSOLETE("netgroup", 0, sssctl_netgroup_show),
SSS_TOOL_COMMAND_OBSOLETE("backup-local-data", 0, sssctl_client_data_backup),
SSS_TOOL_COMMAND_OBSOLETE("restore-local-data", 0, sssctl_client_data_restore),
SSS_TOOL_COMMAND_OBSOLETE("remove-cache", 0, sssctl_cache_remove),
SSS_TOOL_COMMAND_OBSOLETE("upgrade-cache", ERR_SYSDB_VERSION_TOO_OLD, sssctl_cache_upgrade),
SSS_TOOL_COMMAND_OBSOLETE("remove-logs", 0, sssctl_logs_remove),
SSS_TOOL_COMMAND_OBSOLETE("fetch-logs", 0, sssctl_logs_fetch),
Do we really need to be so "backward compatible"
I would say yes, we should be. We can remove the commands later, but we shouldn't break backwards compatibility if the tarball was already released.
a) there was a desing[1] and nobody complained b) 1.14.0 is used very rarely
BTW We should firstly change desing page. Then ask for review and then send a patch. Cond-NACK to patch till desing page will be updated
Fine by me.
and change will be accepted by others.
Others who? :-)
On (29/07/16 12:55), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 09:31:32AM +0200, Lukas Slebodnik wrote:
On (28/07/16 19:37), Michal Židek wrote:
On 07/28/2016 02:11 PM, Michal Židek wrote:
On 07/28/2016 01:57 PM, Jakub Hrozek wrote:
On Thu, Jul 28, 2016 at 01:51:40PM +0200, Pavel Březina wrote:
On 07/28/2016 01:38 PM, Jakub Hrozek wrote: > On Thu, Jul 28, 2016 at 12:23:24PM +0200, Michal Židek wrote: > > On 07/28/2016 10:00 AM, Pavel Březina wrote: > > > On 07/27/2016 03:28 PM, Michal Židek wrote: > > > > On 07/27/2016 11:09 AM, Jakub Hrozek wrote: > > > > > On Wed, Jul 27, 2016 at 11:03:34AM +0200, Pavel Březina wrote: > > > > > > On 07/26/2016 04:19 PM, Michal Židek wrote: > > > > > > > On 07/26/2016 01:19 PM, Pavel Březina wrote: > > > > > > > > On 07/25/2016 02:12 PM, Michal Židek wrote: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > this patches makes the sssctl commands more similar to > > > > > > > > > ipa tool commands. I also think this pattern makes it > > > > > > > > > easier to remember the commands. > > > > > > > > > > > > > > > > > > Note that in the future, there will be more user-* > > > > > > > > > group-* and netgroup-* commands (like seed for user, > > > > > > > > > list of all etc.) > > > > > > > > > > > > > > > > > > Comments are welcome. > > > > > > > > > > > > > > > > > > Michal > > > > > > > > > > > > > > > > Hi, > > > > > > > > ok, it looks like a good idea. When touching the code, can > > > > > > > > you also > > > > > > > > convert sss_override command to use the macro instead? And I > > > > > > > > think it > > > > > > > > may be nice to also add a macro for command sentinel i.e. for > > > > > > > > {NULL, > > > > > > > > ...}. > > > > > > > > > > > > > > > > I'm not very fond of renaming local-data-* to cache-* so it > > > > > > > > doesn't > > > > > > > > imply that we backup the whole cache content. We only backup and > > > > > > > > restore > > > > > > > > data that are local to the client and not present in LDAP. > > > > > > > > Currently > > > > > > > > only local overrides, but it may include local users and > > > > > > > > groups in > > > > > > > > the > > > > > > > > future. > > > > > > > > > > When we have the files provider there would > > > > > be a cache as well. Moreover, we store secrets now. The restore > > > > > command > > > > > backs up all *.ldb files, right? > > > > > > > > This is how I understood it at first, but the current backup > > > > and restore only work for local overrides. But as Pavel mentioned, > > > > it may work for local users and groups in the future > > > > (id_provider=local). My original confusion was that I also > > > > thought it backs-up and restores all ldb files, which is > > > > not the case. > > > > > > No, the intention is to backup only data that are not stored on > > > server > > > and would be lost when the cache is removed. In the time, only local > > > overrides were local. If secrets creates local data, the tool > > > should be > > > modified. > > > > Yes, but users and groups from local domain would also be > > lost if the local data is deleted. So I though we want to backup > > them as well in the future versions (I mean the local provider, > > not the files provider). > > Well, probably not in the first version, but definitely in a couple of > months we want to add the capability to set extended attributes to the > files provider which we'll want to back up as well. But I also think we > will add the additional data into a new directory, just like we did > with > the secrets database. > > > > > Btw. do we want to merge sss_override tool into sssctl? > > Because if the local-data-* commands work currently with > > overrides only, then we could make a new topic 'overrides' and add > > commands > > like > > I would like to merge all tools into sssctl unless there is a strong > reason to keep them separate (the local domain tools should be kept > separate IMO). > > The reason is simply discoverability, if there is a new sssd release, > the admin would just run sssctl and if there are any new tools, their > topics would be displayed. > > (Also I think the code duplication would be reduced as a side-effect). > > > > > sssctl overrides-backup > > sssctl overrides-restore > > > > and later also all the functionality of sss_overrides > > > > sssctl overrides-user-add > > sssctl overrides-user-del > > Maybe, but later. > > > > > etc. > > > > This way we could avoid confusion between local-data and > > cache. If secrets will also create some local data, we will > > add topic 'secrets' to deal with that separately. > > > > sssctl secrets-backup > > sssctl secrets-restore > > I think I got lost in the thread :-) What is the benefit of having more > backup/restore commands than one that backs up or removes of value > under > the /var/lib/sss/ structure? So far I can only think of cache being > more > intuitive to admins than local-data.
How about client-data instead of local-data?
SGTM (sounds good to me :-))
SGTM too :)
I will send version with client-data after the meeting.
I forgot to update this after the meeting. New patch is attached.
Michal
From 73c3e97013274ff644aa630547494fd79b1854c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Mon, 25 Jul 2016 13:50:13 +0200 Subject: [PATCH 1/3] sssctl: Consistent commands naming
Fixes: https://fedorahosted.org/sssd/ticket/3087
Use TOPIC-ACTION pattern for sssctl command names.
src/tools/common/sss_tools.c | 4 ++++ src/tools/common/sss_tools.h | 8 ++++++-- src/tools/sss_override.c | 26 ++++++++++++------------ src/tools/sssctl/sssctl.c | 33 ++++++++++++++++++++---------- src/tools/sssctl/sssctl.h | 42 +++++++++++++++++++-------------------- src/tools/sssctl/sssctl_cache.c | 18 ++++++++--------- src/tools/sssctl/sssctl_data.c | 16 +++++++-------- src/tools/sssctl/sssctl_domains.c | 6 +++--- src/tools/sssctl/sssctl_logs.c | 4 ++-- 9 files changed, 88 insertions(+), 69 deletions(-)
diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c index 686b53a..4977358 100644 --- a/src/tools/common/sss_tools.c +++ b/src/tools/common/sss_tools.c @@ -283,6 +283,10 @@ void sss_tool_usage(const char *tool_name, struct sss_route_cmd *commands) min_len = sss_tool_max_length(commands);
for (i = 0; commands[i].command != NULL; i++) {
if (commands[i].hidden) {
continue;
}
if (sss_tool_is_delimiter(&commands[i])) { fprintf(stderr, "\n%s\n", commands[i].description); continue;
diff --git a/src/tools/common/sss_tools.h b/src/tools/common/sss_tools.h index a9ebabe..b567ea4 100644 --- a/src/tools/common/sss_tools.h +++ b/src/tools/common/sss_tools.h @@ -45,14 +45,18 @@ typedef errno_t struct sss_tool_ctx *tool_ctx, void *pvt);
-#define SSS_TOOL_COMMAND(cmd, msg, err, fn) {cmd, _(msg), err, fn} -#define SSS_TOOL_DELIMITER(message) {"", (message), 0, NULL} +#define SSS_TOOL_COMMAND(cmd, msg, err, fn) {cmd, _(msg), err, fn, false} +#define SSS_TOOL_COMMAND_NOMSG(cmd, err, fn) {cmd, NULL, err, fn, false} +#define SSS_TOOL_COMMAND_OBSOLETE(cmd, err, fn) {cmd, NULL, err, fn, true} +#define SSS_TOOL_DELIMITER(message) {"", (message), 0, NULL, false} +#define SSS_TOOL_LAST {NULL, NULL, 0, NULL, false}
struct sss_route_cmd { const char *command; const char *description; errno_t handles_init_err; sss_route_fn fn;
- bool hidden; /* Do not show this command in sssctl usage */
};
void sss_tool_usage(const char *tool_name, diff --git a/src/tools/sss_override.c b/src/tools/sss_override.c index 45a28fd..d41da52 100644 --- a/src/tools/sss_override.c +++ b/src/tools/sss_override.c @@ -1913,19 +1913,19 @@ static int override_group_export(struct sss_cmdline *cmdline, int main(int argc, const char **argv) { struct sss_route_cmd commands[] = {
{"user-add", NULL, 0, override_user_add},
{"user-del", NULL, 0, override_user_del},
{"user-find", NULL, 0, override_user_find},
{"user-show", NULL, 0, override_user_show},
{"user-import", NULL, 0, override_user_import},
{"user-export", NULL, 0, override_user_export},
{"group-add", NULL, 0, override_group_add},
{"group-del", NULL, 0, override_group_del},
{"group-find", NULL, 0, override_group_find},
{"group-show", NULL, 0, override_group_show},
{"group-import", NULL, 0, override_group_import},
{"group-export", NULL, 0, override_group_export},
{NULL, NULL, 0, NULL}
SSS_TOOL_COMMAND_NOMSG("user-add", 0, override_user_add),
SSS_TOOL_COMMAND_NOMSG("user-del", 0, override_user_del),
SSS_TOOL_COMMAND_NOMSG("user-find", 0, override_user_find),
SSS_TOOL_COMMAND_NOMSG("user-show", 0, override_user_show),
SSS_TOOL_COMMAND_NOMSG("user-import", 0, override_user_import),
SSS_TOOL_COMMAND_NOMSG("user-export", 0, override_user_export),
SSS_TOOL_COMMAND_NOMSG("group-add", 0, override_group_add),
SSS_TOOL_COMMAND_NOMSG("group-del", 0, override_group_del),
SSS_TOOL_COMMAND_NOMSG("group-find", 0, override_group_find),
SSS_TOOL_COMMAND_NOMSG("group-show", 0, override_group_show),
SSS_TOOL_COMMAND_NOMSG("group-import", 0, override_group_import),
SSS_TOOL_COMMAND_NOMSG("group-export", 0, override_group_export),
SSS_TOOL_LAST
};
return sss_tool_main(argc, argv, commands, NULL);
diff --git a/src/tools/sssctl/sssctl.c b/src/tools/sssctl/sssctl.c index 86656f1..8ce93b2 100644 --- a/src/tools/sssctl/sssctl.c +++ b/src/tools/sssctl/sssctl.c @@ -257,25 +257,36 @@ int main(int argc, const char **argv) { struct sss_route_cmd commands[] = { SSS_TOOL_DELIMITER("SSSD Status:"),
SSS_TOOL_COMMAND("list-domains", "List available domains", 0, sssctl_list_domains),
SSS_TOOL_COMMAND("domain-list", "List available domains", 0, sssctl_domain_list), SSS_TOOL_COMMAND("domain-status", "Print information about domain", 0, sssctl_domain_status), SSS_TOOL_DELIMITER("Information about cached content:"),
SSS_TOOL_COMMAND("user", "Information about cached user", 0, sssctl_user),
SSS_TOOL_COMMAND("group", "Information about cached group", 0, sssctl_group),
SSS_TOOL_COMMAND("netgroup", "Information about cached netgroup", 0, sssctl_netgroup),
SSS_TOOL_COMMAND("user-show", "Information about cached user", 0, sssctl_user_show),
SSS_TOOL_COMMAND("group-show", "Information about cached group", 0, sssctl_group_show),
SSS_TOOL_COMMAND("netgroup-show", "Information about cached netgroup", 0, sssctl_netgroup_show), SSS_TOOL_DELIMITER("Local data tools:"),
SSS_TOOL_COMMAND("backup-local-data", "Backup local data", 0, sssctl_backup_local_data),
SSS_TOOL_COMMAND("restore-local-data", "Restore local data from backup", 0, sssctl_restore_local_data),
SSS_TOOL_COMMAND("remove-cache", "Backup local data and remove cached content", 0, sssctl_remove_cache),
SSS_TOOL_COMMAND("upgrade-cache", "Perform cache upgrade", ERR_SYSDB_VERSION_TOO_OLD, sssctl_upgrade_cache),
SSS_TOOL_COMMAND("client-data-backup", "Backup local data", 0, sssctl_client_data_backup),
SSS_TOOL_COMMAND("client-data-restore", "Restore local data from backup", 0, sssctl_client_data_restore),
SSS_TOOL_COMMAND("cache-remove", "Backup local data and remove cached content", 0, sssctl_cache_remove),
SSS_TOOL_COMMAND("cache-upgrade", "Perform cache upgrade", ERR_SYSDB_VERSION_TOO_OLD, sssctl_cache_upgrade), SSS_TOOL_DELIMITER("Log files tools:"),
SSS_TOOL_COMMAND("remove-logs", "Remove existing SSSD log files", 0, sssctl_remove_logs),
SSS_TOOL_COMMAND("fetch-logs", "Archive SSSD log files in tarball", 0, sssctl_fetch_logs),
SSS_TOOL_COMMAND("logs-remove", "Remove existing SSSD log files", 0, sssctl_logs_remove),
SSS_TOOL_COMMAND("logs-fetch", "Archive SSSD log files in tarball", 0, sssctl_logs_fetch),
#ifdef HAVE_LIBINI_CONFIG_V1_3 SSS_TOOL_DELIMITER("Configuration files tools:"), SSS_TOOL_COMMAND("config-check", "Perform static analysis of SSSD configuration", 0, sssctl_config_check), #endif
{NULL, NULL, 0, NULL}
/* Obsolete commands */
SSS_TOOL_COMMAND_OBSOLETE("list-domains", 0, sssctl_domain_list),
SSS_TOOL_COMMAND_OBSOLETE("user", 0, sssctl_user_show),
SSS_TOOL_COMMAND_OBSOLETE("group", 0, sssctl_group_show),
SSS_TOOL_COMMAND_OBSOLETE("netgroup", 0, sssctl_netgroup_show),
SSS_TOOL_COMMAND_OBSOLETE("backup-local-data", 0, sssctl_client_data_backup),
SSS_TOOL_COMMAND_OBSOLETE("restore-local-data", 0, sssctl_client_data_restore),
SSS_TOOL_COMMAND_OBSOLETE("remove-cache", 0, sssctl_cache_remove),
SSS_TOOL_COMMAND_OBSOLETE("upgrade-cache", ERR_SYSDB_VERSION_TOO_OLD, sssctl_cache_upgrade),
SSS_TOOL_COMMAND_OBSOLETE("remove-logs", 0, sssctl_logs_remove),
SSS_TOOL_COMMAND_OBSOLETE("fetch-logs", 0, sssctl_logs_fetch),
Do we really need to be so "backward compatible"
I would say yes, we should be. We can remove the commands later, but we shouldn't break backwards compatibility if the tarball was already released.
Nobody will comply if we remove it because sssd-1.14.0 is used very very rarely
and if somebody complain we can provide old aliases in 1.14.x IMHO, it does not worth to add complexity for unused feature because of "backward compatibility"
a) there was a desing[1] and nobody complained b) 1.14.0 is used very rarely
BTW We should firstly change desing page. Then ask for review and then send a patch. Cond-NACK to patch till desing page will be updated
Fine by me.
and change will be accepted by others.
Others who? :-)
non developers (The person who requested this change; I assume this change was not requested by developers)
In other words, the same process as with creating new design document.
LS
On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote:
Others who? :-)
non developers (The person who requested this change; I assume this change was not requested by developers)
It was (and btw I agree with the change, consistent naming is important as I wish I raised this concern when I reviewed the patches in the first place..)
On (29/07/16 13:44), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote:
Others who? :-)
non developers (The person who requested this change; I assume this change was not requested by developers)
It was (and btw I agree with the change, consistent naming is important as I wish I raised this concern when I reviewed the patches in the first place..)
I was expecting an answer for keeping backward compatibility with unused feature.
The review of changes in design document will be just a blessing the intention from requestor.
LS
On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:44), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote:
Others who? :-)
non developers (The person who requested this change; I assume this change was not requested by developers)
It was (and btw I agree with the change, consistent naming is important as I wish I raised this concern when I reviewed the patches in the first place..)
I was expecting an answer for keeping backward compatibility with unused feature.
At this point it would be only compatibility for rawhide users and anyone who compiled sssd from source or anyone who was alrady using the 1.14.0 tarball. Which is not many people, but still.
The review of changes in design document will be just a blessing the intention from requestor.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
On (29/07/16 13:59), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:44), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote:
Others who? :-)
non developers (The person who requested this change; I assume this change was not requested by developers)
It was (and btw I agree with the change, consistent naming is important as I wish I raised this concern when I reviewed the patches in the first place..)
I was expecting an answer for keeping backward compatibility with unused feature.
At this point it would be only compatibility for rawhide users and anyone who compiled sssd from source or anyone who was alrady using the 1.14.0 tarball. Which is not many people, but still.
So if you want to keep old versions then we document obsoleted version. At least with help "Deprecated alias for (new-name)"
e.g. [root@host ~]# sssctl Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status: * list-domains Deprecated alias for (domain-list) * domain-list List available domains * domain-status Print information about domain
The idea of hidding options is really terrible. a) it's not documented anywhere that it's deprecated b) users might wonder why it works.
LS
On Fri, Jul 29, 2016 at 02:09:02PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:59), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:44), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote:
Others who? :-)
non developers (The person who requested this change; I assume this change was not requested by developers)
It was (and btw I agree with the change, consistent naming is important as I wish I raised this concern when I reviewed the patches in the first place..)
I was expecting an answer for keeping backward compatibility with unused feature.
At this point it would be only compatibility for rawhide users and anyone who compiled sssd from source or anyone who was alrady using the 1.14.0 tarball. Which is not many people, but still.
So if you want to keep old versions then we document obsoleted version. At least with help "Deprecated alias for (new-name)"
e.g. [root@host ~]# sssctl Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status:
- list-domains Deprecated alias for (domain-list)
- domain-list List available domains
- domain-status Print information about domain
The idea of hidding options is really terrible. a) it's not documented anywhere that it's deprecated b) users might wonder why it works.
Fine by me, but additioanlly, what about printing the deprecation warning when a user runs that command? The point of hiding the option is to make it less discoverable.
On (29/07/16 14:27), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 02:09:02PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:59), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:44), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote:
>Others who? :-) non developers (The person who requested this change; I assume this change was not requested by developers)
It was (and btw I agree with the change, consistent naming is important as I wish I raised this concern when I reviewed the patches in the first place..)
I was expecting an answer for keeping backward compatibility with unused feature.
At this point it would be only compatibility for rawhide users and anyone who compiled sssd from source or anyone who was alrady using the 1.14.0 tarball. Which is not many people, but still.
So if you want to keep old versions then we document obsoleted version. At least with help "Deprecated alias for (new-name)"
e.g. [root@host ~]# sssctl Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status:
- list-domains Deprecated alias for (domain-list)
- domain-list List available domains
- domain-status Print information about domain
The idea of hidding options is really terrible. a) it's not documented anywhere that it's deprecated b) users might wonder why it works.
Fine by me, but additioanlly, what about printing the deprecation warning when a user runs that command?
If we really want to insist on "backward compatibility" with unused feature then it will be good addition to the the updated help output.
The point of hiding the option is to make it less discoverable.
I know what is point of hiding the option but it isn't good from user point of view if we want to keep backward compatibility. Backward compatible changes are usually well documented and not hiden
Please update design page with renamed commands and also with deprecated commands.
LS
On Fri, Jul 29, 2016 at 03:06:47PM +0200, Lukas Slebodnik wrote:
On (29/07/16 14:27), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 02:09:02PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:59), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:44), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote: > >Others who? :-) > non developers (The person who requested this change; I assume this > change was not requested by developers)
It was (and btw I agree with the change, consistent naming is important as I wish I raised this concern when I reviewed the patches in the first place..)
I was expecting an answer for keeping backward compatibility with unused feature.
At this point it would be only compatibility for rawhide users and anyone who compiled sssd from source or anyone who was alrady using the 1.14.0 tarball. Which is not many people, but still.
So if you want to keep old versions then we document obsoleted version. At least with help "Deprecated alias for (new-name)"
e.g. [root@host ~]# sssctl Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status:
- list-domains Deprecated alias for (domain-list)
- domain-list List available domains
- domain-status Print information about domain
The idea of hidding options is really terrible. a) it's not documented anywhere that it's deprecated b) users might wonder why it works.
Fine by me, but additioanlly, what about printing the deprecation warning when a user runs that command?
If we really want to insist on "backward compatibility" with unused feature then it will be good addition to the the updated help output.
The point of hiding the option is to make it less discoverable.
I know what is point of hiding the option but it isn't good from user point of view if we want to keep backward compatibility. Backward compatible changes are usually well documented and not hiden
Please update design page with renamed commands and also with deprecated commands.
OK, but this is something for the author of the patch to do :)
On 07/29/2016 03:23 PM, Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 03:06:47PM +0200, Lukas Slebodnik wrote:
On (29/07/16 14:27), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 02:09:02PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:59), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:44), Jakub Hrozek wrote: > On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote: >>> Others who? :-) >> non developers (The person who requested this change; I assume this >> change was not requested by developers) > > It was (and btw I agree with the change, consistent naming is important > as I wish I raised this concern when I reviewed the patches in the first > place..) I was expecting an answer for keeping backward compatibility with unused feature.
At this point it would be only compatibility for rawhide users and anyone who compiled sssd from source or anyone who was alrady using the 1.14.0 tarball. Which is not many people, but still.
So if you want to keep old versions then we document obsoleted version. At least with help "Deprecated alias for (new-name)"
e.g. [root@host ~]# sssctl Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status:
- list-domains Deprecated alias for (domain-list)
- domain-list List available domains
- domain-status Print information about domain
The idea of hidding options is really terrible. a) it's not documented anywhere that it's deprecated b) users might wonder why it works.
Fine by me, but additioanlly, what about printing the deprecation warning when a user runs that command?
If we really want to insist on "backward compatibility" with unused feature then it will be good addition to the the updated help output.
The point of hiding the option is to make it less discoverable.
I know what is point of hiding the option but it isn't good from user point of view if we want to keep backward compatibility. Backward compatible changes are usually well documented and not hiden
Please update design page with renamed commands and also with deprecated commands.
OK, but this is something for the author of the patch to do :)
Ok, I will update the design page, but I am little lost in this thread. I will do this:
1. update the design page 2. wrap the old command names in function that prints warning about the command being deprecated and that it will be removed in future versions. 3. Will list the old commands in the sssctl usage with note that they are deprecated.
Do all agree with the above?
Michal
On (29/07/16 15:54), Michal Židek wrote:
On 07/29/2016 03:23 PM, Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 03:06:47PM +0200, Lukas Slebodnik wrote:
On (29/07/16 14:27), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 02:09:02PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:59), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote: > On (29/07/16 13:44), Jakub Hrozek wrote: > > On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote: > > > > Others who? :-) > > > non developers (The person who requested this change; I assume this > > > change was not requested by developers) > > > > It was (and btw I agree with the change, consistent naming is important > > as I wish I raised this concern when I reviewed the patches in the first > > place..) > I was expecting an answer for keeping backward compatibility with > unused feature.
At this point it would be only compatibility for rawhide users and anyone who compiled sssd from source or anyone who was alrady using the 1.14.0 tarball. Which is not many people, but still.
So if you want to keep old versions then we document obsoleted version. At least with help "Deprecated alias for (new-name)"
e.g. [root@host ~]# sssctl Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status:
- list-domains Deprecated alias for (domain-list)
- domain-list List available domains
- domain-status Print information about domain
The idea of hidding options is really terrible. a) it's not documented anywhere that it's deprecated b) users might wonder why it works.
Fine by me, but additioanlly, what about printing the deprecation warning when a user runs that command?
If we really want to insist on "backward compatibility" with unused feature then it will be good addition to the the updated help output.
The point of hiding the option is to make it less discoverable.
I know what is point of hiding the option but it isn't good from user point of view if we want to keep backward compatibility. Backward compatible changes are usually well documented and not hiden
Please update design page with renamed commands and also with deprecated commands.
OK, but this is something for the author of the patch to do :)
Ok, I will update the design page, but I am little lost in this thread. I will do this:
- update the design page
yes + ask for blessing/review from the requester of the change
- wrap the old command names in function that prints warning
about the command being deprecated and that it will be removed in future versions. 3. Will list the old commands in the sssctl usage with note that they are deprecated.
Do all agree with the above?
If we insinst on keeping backward comaptibility with unused feature then yes for 2) and 3)
I would personally just rename them but I'm fine with 2) and 3) It would be much simpler.
LS
On 07/29/2016 09:38 PM, Lukas Slebodnik wrote:
On (29/07/16 15:54), Michal Židek wrote:
On 07/29/2016 03:23 PM, Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 03:06:47PM +0200, Lukas Slebodnik wrote:
On (29/07/16 14:27), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 02:09:02PM +0200, Lukas Slebodnik wrote:
On (29/07/16 13:59), Jakub Hrozek wrote: > On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote: >> On (29/07/16 13:44), Jakub Hrozek wrote: >>> On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote: >>>>> Others who? :-) >>>> non developers (The person who requested this change; I assume this >>>> change was not requested by developers) >>> >>> It was (and btw I agree with the change, consistent naming is important >>> as I wish I raised this concern when I reviewed the patches in the first >>> place..) >> I was expecting an answer for keeping backward compatibility with >> unused feature. > > At this point it would be only compatibility for rawhide users and anyone > who compiled sssd from source or anyone who was alrady using the 1.14.0 > tarball. Which is not many people, but still. So if you want to keep old versions then we document obsoleted version. At least with help "Deprecated alias for (new-name)"
e.g. [root@host ~]# sssctl Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status:
- list-domains Deprecated alias for (domain-list)
- domain-list List available domains
- domain-status Print information about domain
The idea of hidding options is really terrible. a) it's not documented anywhere that it's deprecated b) users might wonder why it works.
Fine by me, but additioanlly, what about printing the deprecation warning when a user runs that command?
If we really want to insist on "backward compatibility" with unused feature then it will be good addition to the the updated help output.
The point of hiding the option is to make it less discoverable.
I know what is point of hiding the option but it isn't good from user point of view if we want to keep backward compatibility. Backward compatible changes are usually well documented and not hiden
Please update design page with renamed commands and also with deprecated commands.
OK, but this is something for the author of the patch to do :)
Ok, I will update the design page, but I am little lost in this thread. I will do this:
- update the design page
yes + ask for blessing/review from the requester of the change
- wrap the old command names in function that prints warning about the command being deprecated and that it will be removed in future versions.
- Will list the old commands in the sssctl usage with note that they are deprecated.
Do all agree with the above?
If we insinst on keeping backward comaptibility with unused feature then yes for 2) and 3)
I would personally just rename them but I'm fine with 2) and 3) It would be much simpler.
LS
CCing Marc.
Marc could you give us your opinion/blessing to this change? Long story short, we though the commands will be easier to understand if they follow similar pattern as the TOPIC-ACTION that ipa tool uses. This will help us create more consistent command names as we add more functionality to the sssctl tool.
Using the obsolete commands is still possible and prints message like this:
# sssctl fetch-logs out.tgz Command 'fetch-logs' is deprecated and may be removed in future versions. Use 'logs-fetch' instead.
We can remove the obsolete commands in some later milestone o get rid of the code.
The usage now looks like this:
<USAGE>
# sssctl --help Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status: * domain-list List available domains * domain-status Print information about domain
Information about cached content: * user-show Information about cached user * group-show Information about cached group * netgroup-show Information about cached netgroup
Local data tools: * client-data-backup Backup local data * client-data-restore Restore local data from backup * cache-remove Backup local data and remove cached content * cache-upgrade Perform cache upgrade
Log files tools: * logs-remove Remove existing SSSD log files * logs-fetch Archive SSSD log files in tarball
Configuration files tools: * config-check Perform static analysis of SSSD configuration
Deprecated commands that may be removed in the future: * list-domains Obsolete alias for domain-list * user Obsolete alias for user-show * group Obsolete alias for group-show * netgroup Obsolete alias for netgroup-show * backup-local-data Obsolete alias for client-data-backup * restore-local-data Obsolete alias for client-data-restore * remove-cache Obsolete alias for cache-remove * upgrade-cache Obsolete alias for cache-upgrade * remove-logs Obsolete alias for logs-remove * fetch-logs Obsolete alias for logs-fetch
Common options: --debug=INT The debug level to run with
Help options: -?, --help Show this for a command --usage Show brief usage message for a command
</USAGE>
I also amended commands mentioned in the design page, to use the TOPIC-ACTION pattern. (For example debug-level was changed to logs-level, and belongs to the same topic as logs-remove and logs-fetch).
Link to desing page: https://fedorahosted.org/sssd/wiki/DesignDocs/sssctl
Michal
Hi Michal,
Am 02.08.2016 um 18:04 schrieb Michal Židek:
Marc could you give us your opinion/blessing to this change? Long story short, we though the commands will be easier to understand if they follow similar pattern as the TOPIC-ACTION that ipa tool uses. This will help us create more consistent command names as we add more functionality to the sssctl tool.
I vote for changing the options to the TOPIC-ACTION format.
Even if it felt different at the beginning when started using IPA, I meanwhile like it. For me the options are easier to remember and it allows to group them in the help, like in "ipa help TOPIC".
Regards, Marc
On 08/02/2016 06:04 PM, Michal Židek wrote:
On 07/29/2016 09:38 PM, Lukas Slebodnik wrote:
On (29/07/16 15:54), Michal Židek wrote:
On 07/29/2016 03:23 PM, Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 03:06:47PM +0200, Lukas Slebodnik wrote:
On (29/07/16 14:27), Jakub Hrozek wrote:
On Fri, Jul 29, 2016 at 02:09:02PM +0200, Lukas Slebodnik wrote: > On (29/07/16 13:59), Jakub Hrozek wrote: >> On Fri, Jul 29, 2016 at 01:49:41PM +0200, Lukas Slebodnik wrote: >>> On (29/07/16 13:44), Jakub Hrozek wrote: >>>> On Fri, Jul 29, 2016 at 01:07:56PM +0200, Lukas Slebodnik wrote: >>>>>> Others who? :-) >>>>> non developers (The person who requested this change; I >>>>> assume this >>>>> change was not requested by developers) >>>> >>>> It was (and btw I agree with the change, consistent naming is >>>> important >>>> as I wish I raised this concern when I reviewed the patches in >>>> the first >>>> place..) >>> I was expecting an answer for keeping backward compatibility with >>> unused feature. >> >> At this point it would be only compatibility for rawhide users >> and anyone >> who compiled sssd from source or anyone who was alrady using the >> 1.14.0 >> tarball. Which is not many people, but still. > So if you want to keep old versions then > we document obsoleted version. > At least with help "Deprecated alias for (new-name)" > > e.g. > [root@host ~]# sssctl > Usage: > sssctl COMMAND COMMAND-ARGS > > Available commands: > > SSSD Status: > * list-domains Deprecated alias for (domain-list) > * domain-list List available domains > * domain-status Print information about domain > > The idea of hidding options is really terrible. > a) it's not documented anywhere that it's deprecated > b) users might wonder why it works.
Fine by me, but additioanlly, what about printing the deprecation warning when a user runs that command?
If we really want to insist on "backward compatibility" with unused feature then it will be good addition to the the updated help output.
The point of hiding the option is to make it less discoverable.
I know what is point of hiding the option but it isn't good from user point of view if we want to keep backward compatibility. Backward compatible changes are usually well documented and not hiden
Please update design page with renamed commands and also with deprecated commands.
OK, but this is something for the author of the patch to do :)
Ok, I will update the design page, but I am little lost in this thread. I will do this:
- update the design page
yes + ask for blessing/review from the requester of the change
- wrap the old command names in function that prints warning about the command being deprecated and that it will be removed in future versions.
- Will list the old commands in the sssctl usage with note that they are deprecated.
Do all agree with the above?
If we insinst on keeping backward comaptibility with unused feature then yes for 2) and 3)
I would personally just rename them but I'm fine with 2) and 3) It would be much simpler.
LS
CCing Marc.
Marc could you give us your opinion/blessing to this change? Long story short, we though the commands will be easier to understand if they follow similar pattern as the TOPIC-ACTION that ipa tool uses. This will help us create more consistent command names as we add more functionality to the sssctl tool.
Using the obsolete commands is still possible and prints message like this:
# sssctl fetch-logs out.tgz Command 'fetch-logs' is deprecated and may be removed in future versions. Use 'logs-fetch' instead.
We can remove the obsolete commands in some later milestone o get rid of the code.
The usage now looks like this:
<USAGE>
# sssctl --help Usage: sssctl COMMAND COMMAND-ARGS
Available commands:
SSSD Status:
- domain-list List available domains
- domain-status Print information about domain
Information about cached content:
- user-show Information about cached user
- group-show Information about cached group
- netgroup-show Information about cached netgroup
Local data tools:
- client-data-backup Backup local data
- client-data-restore Restore local data from backup
- cache-remove Backup local data and remove cached content
- cache-upgrade Perform cache upgrade
Log files tools:
- logs-remove Remove existing SSSD log files
- logs-fetch Archive SSSD log files in tarball
Configuration files tools:
- config-check Perform static analysis of SSSD configuration
Deprecated commands that may be removed in the future:
- list-domains Obsolete alias for domain-list
- user Obsolete alias for user-show
- group Obsolete alias for group-show
- netgroup Obsolete alias for netgroup-show
- backup-local-data Obsolete alias for client-data-backup
- restore-local-data Obsolete alias for client-data-restore
- remove-cache Obsolete alias for cache-remove
- upgrade-cache Obsolete alias for cache-upgrade
- remove-logs Obsolete alias for logs-remove
- fetch-logs Obsolete alias for logs-fetch
Common options: --debug=INT The debug level to run with
Help options: -?, --help Show this for a command --usage Show brief usage message for a command
</USAGE>
I also amended commands mentioned in the design page, to use the TOPIC-ACTION pattern. (For example debug-level was changed to logs-level, and belongs to the same topic as logs-remove and logs-fetch).
Link to desing page: https://fedorahosted.org/sssd/wiki/DesignDocs/sssctl
Michal
Marc responded to me off list that he is also in favor of the TOPIC-ACTION like pattern.
Marc's reaction: <quote>
I vote for changing the options to the TOPIC-ACTION format.
Even if it felt different at the beginning when started using IPA, I meanwhile like it. For me the options are easier to remember and it allows to group them in the help, like in "ipa help TOPIC".
Regards, Marc
</quote>
Hi,
As was requested on devel meeting, I removed the compatibility with old commands.
New patch attached.
Michal
On (04/08/16 16:21), Michal Židek wrote:
Hi,
As was requested on devel meeting, I removed the compatibility with old commands.
New patch attached.
Michal
From 0c18c75e2b6a2e29f95b0e477369b5db766afbdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Mon, 25 Jul 2016 13:50:13 +0200 Subject: [PATCH] sssctl: Consistent commands naming
Fixes: https://fedorahosted.org/sssd/ticket/3087
Use TOPIC-ACTION pattern for sssctl command names.
It would be good to be consistent in the content of comit message.
We already have a prefered template. It would be good if you could use it. @see https://git.fedorahosted.org/cgit/sssd.git/commit/?id=3d9edb4c510028def2df41...
LS
On 08/05/2016 12:30 PM, Lukas Slebodnik wrote:
On (04/08/16 16:21), Michal Židek wrote:
Hi,
As was requested on devel meeting, I removed the compatibility with old commands.
New patch attached.
Michal
From 0c18c75e2b6a2e29f95b0e477369b5db766afbdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Mon, 25 Jul 2016 13:50:13 +0200 Subject: [PATCH] sssctl: Consistent commands naming
Fixes: https://fedorahosted.org/sssd/ticket/3087
Use TOPIC-ACTION pattern for sssctl command names.
It would be good to be consistent in the content of comit message.
We already have a prefered template. It would be good if you could use it. @see https://git.fedorahosted.org/cgit/sssd.git/commit/?id=3d9edb4c510028def2df41...
LS
Yes, sorry for that. I will also update our contribute wiki page to reflect this.
Michal
sssd-devel@lists.fedorahosted.org