[Freeipa-devel] [PATCH 0385] replicainstall: Add possiblity to install client in one

[Tomas Babej]

On 11/26/2015 01:33 PM, Jan Cholasta wrote:
> On 25.11.2015 09:01, Jan Cholasta wrote:
>> On 24.11.2015 15:56, Tomas Babej wrote:
>>>
>>>
>>> On 11/23/2015 04:43 PM, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> On 23.11.2015 12:50, Tomas Babej wrote:
>>>>> Hi,
>>>>>
>>>>> this patch implements the single command replica promotion&enrollment
>>>>> for #5310.
>>>>>
>>>>> Tomas
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/5310
>>>>
>>>> 1) ensure_enrolled() should be called from promote_check() after the
>>>> client check is done:
>>>>
>>>>      client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
>>>>      if not client_fstore.has_files():
>>>>          ensure_enrolled(installer)
>>
>> Also it should no longer be decorated with @common_cleanup.
>>
>>>>
>>>>
>>>> 2)
>>>>
>>>> +    server_name = Knob(
>>>> +        str, None,
>>>> +        description="fully qualified name of IPA server to enrooll
>>>> to",
>>>> +        cli_name='server',
>>>> +    )
>>>>
>>>> Please use the same identifier ipa-client-install uses, i.e.
>>>> s/server_name/server/.
>>>>
>>>> Also there is typo in the description: "enrooll".
>>>
>>> Fixed.
>>
>> You don't need to set cli_name anymore, as it's the same as the default.
>>
>>>
>>>>
>>>>
>>>> 3)
>>>>
>>>> +    host_name = Knob(
>>>> +        str, None,
>>>> +        description="fully qualified name of this host",
>>>> +        cli_name='hostname',
>>>> +    )
>>>>
>>>> This knob is already defined in BaseServer, there's no need to redefine
>>>> it here (just remove the "host_name = None").
>>>>
>>>> If you want to change the description, change it in BaseServer.
>>>
>>> Yep, that was the reasoning here. Changed in BaseServer.
>>>
>>>>
>>>>
>>>> 4)
>>>>
>>>> +    keytab = Knob(
>>>> +        str, None,
>>>> +        description="path to backed up keytab from previous
>>>> enrollment",
>>>> +        cli_name='keytab',
>>>> +    )
>>>>
>>>> ipa-client-install uses the short name -k for the keytab option, it
>>>> should be used here as well.
>>>>
>>>
>>> Added.
>>
>> You don't need to set cli_name here as well.
>>
>>>
>>>>
>>>> 5) The replica file argument conflicts with the --realm, --domain,
>>>> --server, --admin-password and --principal options. This should be
>>>> checked in Replica.__init__().
>>>>
>>>> The --hostname option should either be conflicting as well or be
>>>> implemented properly for legacy replica install.
>>>>
>>>
>>> All of them now conflict --replica-file.
>>
>> Replica file is not an option but rather an argument in
>> ipa-replica-install.
>>
>> I think the error message should use the same wording which is used for
>> --forwarder vs. --no-forwarder and --reverse-zone vs. --no-reverse: "You
>> cannot specify a --something option together with replica file".
>>
>>>
>>>>
>>>> 6) I think --admin-password should be renamed to --password and the
>>>> description changed, since it now also allows OTP enrollment.
>>>>
>>>
>>> I changed the requirements to mandate --principal when --password is
>>> used, as we discussed.
>>
>> I was wrong here, sorry.
>>
>> ipa-replica-install assumes "admin" for principal by default, so we just
>> need to make sure ipa-client-install is called with --principal when
>> password is used, whether it's the provided principal or the default
>> "admin".
>>
>>>
>>> The name of the option cannot really be changed, as it is already taken
>>> by the dm_password.
>>
>> Right.
>>
> 
> One more thing I remembered:
> 
> 7) When you try replica promotion on domain level 0, ipa-replica-install
> will install the client, then do the domain level check and fail,
> leaving the client installed. The check should be done as soon as
> possible, i.e. in ipa-client-install, so that nothing is installed in
> that case.
> 
> This can be a separate patch though.
> 

Here's updated version of 385. I'll deal with (7) in a follow-up patch.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0385-4-replicainstall-Add-possiblity-to-install-client-in-o.patch
Type: text/x-patch
Size: 7461 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151126/e4e22087/attachment.bin>