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.

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to