[Freeipa-devel] [PATCHES 306-316] Automated migration tool from Winsync

Tue Jun 30 15:55:51 UTC 2015

On 06/16/2015 01:01 PM, Jan Cholasta wrote:
> Dne 16.6.2015 v 10:14 Martin Babinsky napsal(a):
>> On 05/06/2015 10:12 AM, Tomas Babej wrote:
>>>
>>>
>>> On 05/05/2015 02:02 PM, Tomas Babej wrote:
>>>>
>>>>
>>>> On 04/29/2015 12:28 PM, Tomas Babej wrote:
>>>>>
>>>>>
>>>>> On 03/11/2015 04:20 PM, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Dne 10.3.2015 v 16:35 Tomas Babej napsal(a):
>>>>>>>
>>>>>>> On 03/09/2015 12:26 PM, Tomas Babej wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> this couple of patches provides a initial implementation of the
>>>>>>>> winsync migration tool:
>>>>>>>>
>>>>>>>> https://fedorahosted.org/freeipa/ticket/4524
>>>>>>>>
>>>>>>>> Some parts could use some polishing, but this is a sound
>>>>>>>> foundation.
>>>>>>>>
>>>>>>>> Tomas
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> Attaching one more patch to the bundle. This one should make the
>>>>>>> winsync
>>>>>>> tool readily available after install.
>>>>>>>
>>>>>>> Tomas
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Nitpicks:
>>>>>>
>>>>>> The winsync_migrate module should be in ipaserver.install. Also I
>>>>>> don't see why it has to be a package when there is just one short
>>>>>> file in it.
>>>>>>
>>>>>> By convention, the AdminTool subclass should be named
>>>>>> WinsyncMigrate, or the tool should be named ipa-migrate-winsync.
>>>>>>
>>>>>> Honza
>>>>>>
>>>>>
>>>>> Updated patches attached.
>>>>>
>>>>> Tomas
>>>>
>>>> Rebased patches with cleaned membership bits.
>>>>
>>>> Tomas
>>>
>>> I did some self-review, updated patches attached.
>>>
>>>
>> Hi Tomas,
>>
>> patches look good and seem to work as expected. I have some comments:
>>
>> 1.) When running the tool I get a number of warnings about users not
>> found (https://paste.fedoraproject.org/232251/43884831/), but in the end
>> everything seems to be fine and users are migrated in the external
>> groups just fine. Is this behavior normal?
>>

In that case, yes. What happened here is that SSSD in POSIX trust will
not resolve users that do not have POSIX attributes set. Winsync
synchornizes all the users, hence the discrepancy.

>> 2.) Since both "--realm" and "--server" options are mandatory, I was
>> thinking if it would be better to use positional arguments, since you
>> always have to specify them. What are your thought on this?
> 
> I would rather stay consistent with ipa-server-install and friends and
> keep them as options.
> 
>>
>> 3.) Patches 317-318 seem to just just rename/move things and could be
>> squashed in the previous ones. But that is just a minor thing and I
>> leave that to your discretion.
>>
>> 4.) After all the renaming and moving around the WinsyncMigrate class
>> (see previous point) there is an unused file
>> "ipaserver/winsync_migrate/__init__.py" left. You should remove it in
>> some patch (e.g. in patch 318 if you decide to keep it).

I removed the file and squashed the change into 318.

> 
> Also please rename the class to "MigrateWinsync", for consistency.
> 

Naming is consistent, the tool is called ipa-winsync-migrate, class is
called WinsyncMigrate. This is consistent with other IPA tools.

>>
>> 5.) Option "--log-file" seems to be broken. When specified on CLI the
>> log is created but empty, the program prints out nothing and then exits
>> without doing anything. However, I suspect that this is AdminTool's
>> problem, not yours.
>>

Yep. Please, file a ticket for this more generic issue.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0306-5-winsync-migrate-Add-initial-plumbing.patch
Type: text/x-patch
Size: 5628 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0307-5-winsync-migrate-Add-a-way-to-find-all-winsync-users.patch
Type: text/x-patch
Size: 2205 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0308-5-migrate-winsync-Create-user-ID-overrides-in-place-of.patch
Type: text/x-patch
Size: 2449 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0309-5-migrate-winsync-Add-option-validation-and-handling.patch
Type: text/x-patch
Size: 2523 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0310-5-winsync-migrate-Move-the-api-initalization-and-LDAP-.patch
Type: text/x-patch
Size: 1973 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0311-5-dcerpc-Change-logging-level-for-debug-information.patch
Type: text/x-patch
Size: 1344 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0312-5-dcerpc-Add-debugging-message-to-failing-kinit-as-htt.patch
Type: text/x-patch
Size: 911 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0313-5-winsync-migrate-Require-root-privileges.patch
Type: text/x-patch
Size: 1009 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0314-5-idviews-Do-not-abort-the-find-show-commands-on-conve.patch
Type: text/x-patch
Size: 1784 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0315-5-winsync-migrate-Require-explicit-specification-of-th.patch
Type: text/x-patch
Size: 3131 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0316-5-winsync-migrate-Delete-winsync-agreement-prior-to-mi.patch
Type: text/x-patch
Size: 2852 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0317-5-winsync-migrate-Rename-to-tool-to-achive-consistency.patch
Type: text/x-patch
Size: 3957 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0011.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0318-5-winsync-migrate-Move-the-tool-under-ipaserver.instal.patch
Type: text/x-patch
Size: 1293 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0012.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0319-5-winsync-migrate-Include-the-tool-parts-in-Makefile-a.patch
Type: text/x-patch
Size: 1222 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0013.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0320-5-idviews-Fallback-to-AD-DC-LDAP-only-if-specifically-.patch
Type: text/x-patch
Size: 15669 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0014.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0321-5-man-Add-manpage-for-ipa-winsync-migrate.patch
Type: text/x-patch
Size: 2846 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0015.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0322-5-winsync_migrate-Migrate-memberships-of-the-winsynced.patch
Type: text/x-patch
Size: 3192 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0016.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0323-5-winsync_migrate-Generalize-membership-migration.patch
Type: text/x-patch
Size: 7164 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150630/02ab9071/attachment-0017.bin>