Re: [Freeipa-devel] [PATCH] 0002 Add client install option to set ipa_backup_server

[Petr Spacek]

On 27.7.2016 20:03, Martin Basti wrote:
> 
> 
> On 26.07.2016 17:01, Ariel Barria wrote:
>> Hello everyone.
>>
>> I send patch for review.
>>
>> Regards,
>>
>>
> Hello, thank you for the patch, but I have a few comments:
> 
> 1)
> can you please use option --backup-server instead of --ipa-backup-server to be
> consistent with --server (as we don't have option --ipa-server)
> 
> 2)
> values passed by --server option are validated if it is IPA server or not,
> this should happen for backup server(s) too.
> 
> But looking to current ipa-client-install, it may be challenging to achieve
> this goal. I'm afraid that you might rather wait until we refactor the client
> code (next release hopefully). But in case you are brave enough, I can provide
> advises, but it will be hell.
> 
> 3)
> There is a question, if the backup server should be used also for krb5.conf or
> other configs where multiple servers can be specified. Probably not. But at
> least this should be mentioned in manpage that this option is used only for
> SSSD (probably there should be check to prevent using --backup-server together
> with --no-sssd option)

I would use backup_servers even in krb5.conf. Quick testing indicates that
krb5 lib respects order of KDCs in krb5.conf so simply list backup servers at
the end of the list.

That would remove mutual exclusivity of --no-sssd and --backup-server options.

Petr^2 Spacek

> 
> 4)
> 'man ipa-client-install' should be updated with the new option
> 
> 5)
> ipa_backup_server allows to specify multiple servers, so the new option should
> be multivalued (and then joined to coma separated list into SSSD config)
> 
> regards,
> Martin

-- 
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

Re: [Freeipa-devel] [PATCH] 0002 Add client install option to set ipa_backup_server

[Martin Basti]

On 26.07.2016 17:01, Ariel Barria wrote:

Hello everyone.

I send patch for review.

Regards,



Hello, thank you for the patch, but I have a few comments:

1)
can you please use option --backup-server instead of --ipa-backup-server 
to be consistent with --server (as we don't have option --ipa-server)


2)
values passed by --server option are validated if it is IPA server or 
not, this should happen for backup server(s) too.


But looking to current ipa-client-install, it may be challenging to 
achieve this goal. I'm afraid that you might rather wait until we 
refactor the client code (next release hopefully). But in case you are 
brave enough, I can provide advises, but it will be hell.


3)
There is a question, if the backup server should be used also for 
krb5.conf or other configs where multiple servers can be specified. 
Probably not. But at least this should be mentioned in manpage that this 
option is used only for SSSD (probably there should be check to prevent 
using --backup-server together with --no-sssd option)


4)
'man ipa-client-install' should be updated with the new option

5)
ipa_backup_server allows to specify multiple servers, so the new option 
should be multivalued (and then joined to coma separated list into SSSD 
config)


regards,
Martin

-- 
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

[Freeipa-devel] [PATCH] 0002 Add client install option to set ipa_backup_server

[Ariel Barria]

Hello everyone.

I send patch for review.

Regards,
From 3a27ef5bf3001f5f5ad2e71a4fc76a7a5c104e88 Mon Sep 17 00:00:00 2001
From: "Ariel O. Barria" 
Date: Tue, 26 Jul 2016 09:32:26 -0500
Subject: [PATCH] freeipa arielb 0002 Add client install option to set
 ipa_backup_server

Add ipa backup_server on sssd.conf using ipa-client-install

https://fedorahosted.org/freeipa/ticket/4016
---
 client/ipa-client-install | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 05b6b6e0da07353750d0dca4e6df9d1f58d69c35..af5d72a5d9e0b89edbb1a5bb974621ae2f02b85a 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -115,6 +115,10 @@ def parse_options():
 basic_group = OptionGroup(parser, "basic options")
 basic_group.add_option("--domain", dest="domain", help="domain name")
 basic_group.add_option("--server", dest="server", help="IPA server", action="append")
+basic_group.add_option("--ipa-backup-server", dest="ipa_backup_server",
+  default=False,
+  help="Configure sssd to use backup server if no primary"
+   " servers can be reached.")
 basic_group.add_option("--realm", dest="realm_name", help="realm name")
 basic_group.add_option("--fixed-primary", dest="primary", action="store_true",
   default=False, help="Configure sssd to use fixed server as primary IPA server")
@@ -231,6 +235,13 @@ def parse_options():
 if (options.server and not options.domain):
 parser.error("--server cannot be used without providing --domain")
 
+if (options.server and not options.domain):
+parser.error("--server cannot be used without providing --domain")
+
+if (options.ipa_backup_server and not options.primary):
+parser.error("--ipa-backup-server cannot be used without providing "
+ "--fixed-primary")
+
 if options.domain:
 try:
 validate_domain_name(options.domain)
@@ -1273,6 +1284,9 @@ def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options, clie
 if not options.on_master:
 if options.primary:
 domain.set_option('ipa_server', ', '.join(cli_server))
+if options.ipa_backup_server:
+domain.set_option('ipa_backup_server',
+   options.ipa_backup_server)
 else:
 domain.set_option('ipa_server', '_srv_, %s' % ', '.join(cli_server))
 else:
-- 
2.7.4

-- 
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