Re: [Freeipa-devel] [PATCHES 0224-0225] Use NTP servers detected from SRV records in ntp configuration

2015-04-24 Thread Petr Vobornik 

On 04/16/2015 06:20 PM, Martin Babinsky wrote:

On 04/16/2015 05:14 PM, Martin Basti wrote:

On 16/04/15 13:53, Martin Babinsky wrote:

On 04/16/2015 01:34 PM, Martin Babinsky wrote:

On 04/15/2015 04:17 PM, Martin Basti wrote:

https://fedorahosted.org/freeipa/ticket/4981



Stupid me, thank you

Updated patches attached.


ACK



pushed to master:
* e395bdb911ebf69fbf6b3e1c9e0e148a9600bd90 ipa client: make --ntp-server 
option multivalued
* e55d8ee5d4649b2fd35aa6f29ed2a8f60088d1a8 ipa client: use NTP servers 
detected from SRV

--
Petr Vobornik

--
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] [PATCHES 0224-0225] Use NTP servers detected from SRV records in ntp configuration

2015-04-16 Thread Martin Babinsky 

On 04/16/2015 01:34 PM, Martin Babinsky wrote:

On 04/15/2015 04:17 PM, Martin Basti wrote:

https://fedorahosted.org/freeipa/ticket/4981

These patches keep usage of IPA server address as NTP server in NTP
configuration on clients, in case that  no NTP servers were specified by
user, and no NTP servers were resolved from SRV records. This will
ensure backward compatibility, as IPA does not configure NTP SRV records
for each domain automatically.

Patches attached.

Martin^2



PATCH 224 NACK
PATCH 225 ACK

Patch 224 you keep the original destination (dest=ntp_server)
for --ntp-server option, but in patch 226 the code attempts to get the
server names from options.ntpservers resulting in:

Traceback (most recent call last):
   File /sbin/ipa-client-install, line 2932, in module
 sys.exit(main())
   File /sbin/ipa-client-install, line 2913, in main
 rval = install(options, env, fstore, statestore)
   File /sbin/ipa-client-install, line 2342, in install
 if options.ntp_servers:
AttributeError: Values instance has no attribute 'ntp_servers'

So please fix this.

Naming the destination 'ntp_servers' (plural form) seems best because we
now store multiple values.

Also, if renaming option.ntp_server to option.ntp_servers, do not 
forget to change also these lines in ipa-client-install:


2852 if options.ntp_server:
2853 ntp_servers = options.ntp_server

(line numbers after applying patches 224-226)

--
Martin^3 Babinsky

--
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] [PATCHES 0224-0225] Use NTP servers detected from SRV records in ntp configuration

2015-04-16 Thread Martin Babinsky 

On 04/15/2015 04:17 PM, Martin Basti wrote:

https://fedorahosted.org/freeipa/ticket/4981

These patches keep usage of IPA server address as NTP server in NTP
configuration on clients, in case that  no NTP servers were specified by
user, and no NTP servers were resolved from SRV records. This will
ensure backward compatibility, as IPA does not configure NTP SRV records
for each domain automatically.

Patches attached.

Martin^2



PATCH 224 NACK
PATCH 225 ACK

Patch 224 you keep the original destination (dest=ntp_server)
for --ntp-server option, but in patch 226 the code attempts to get the 
server names from options.ntpservers resulting in:


Traceback (most recent call last):
  File /sbin/ipa-client-install, line 2932, in module
sys.exit(main())
  File /sbin/ipa-client-install, line 2913, in main
rval = install(options, env, fstore, statestore)
  File /sbin/ipa-client-install, line 2342, in install
if options.ntp_servers:
AttributeError: Values instance has no attribute 'ntp_servers'

So please fix this.

Naming the destination 'ntp_servers' (plural form) seems best because we 
now store multiple values.


--
Martin^3 Babinsky

--
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] [PATCHES 0224-0225] Use NTP servers detected from SRV records in ntp configuration

2015-04-16 Thread Martin Basti 

On 16/04/15 13:53, Martin Babinsky wrote:

On 04/16/2015 01:34 PM, Martin Babinsky wrote:

On 04/15/2015 04:17 PM, Martin Basti wrote:

https://fedorahosted.org/freeipa/ticket/4981

These patches keep usage of IPA server address as NTP server in NTP
configuration on clients, in case that  no NTP servers were 
specified by

user, and no NTP servers were resolved from SRV records. This will
ensure backward compatibility, as IPA does not configure NTP SRV 
records

for each domain automatically.

Patches attached.

Martin^2



PATCH 224 NACK
PATCH 225 ACK

Patch 224 you keep the original destination (dest=ntp_server)
for --ntp-server option, but in patch 226 the code attempts to get the
server names from options.ntpservers resulting in:

Traceback (most recent call last):
   File /sbin/ipa-client-install, line 2932, in module
 sys.exit(main())
   File /sbin/ipa-client-install, line 2913, in main
 rval = install(options, env, fstore, statestore)
   File /sbin/ipa-client-install, line 2342, in install
 if options.ntp_servers:
AttributeError: Values instance has no attribute 'ntp_servers'

So please fix this.

Naming the destination 'ntp_servers' (plural form) seems best because we
now store multiple values.

Also, if renaming option.ntp_server to option.ntp_servers, do not 
forget to change also these lines in ipa-client-install:


2852 if options.ntp_server:
2853 ntp_servers = options.ntp_server

(line numbers after applying patches 224-226)


Stupid me, thank you

Updated patches attached.

--
Martin Basti

From 73a920ad890ddbce953daf1317034f21da07c529 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Tue, 14 Apr 2015 18:56:47 +0200
Subject: [PATCH 1/2] ipa client: make --ntp-server option multivalued

There can be more ntp servers in ntp.conf

Required for ticket: https://fedorahosted.org/freeipa/ticket/4981
---
 ipa-client/ipa-install/ipa-client-install | 19 +++
 ipa-client/ipaclient/ntpconf.py   | 11 ++-
 ipa-client/man/ipa-client-install.1   |  2 +-
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 1590a08600bbb1b2fd7f4c3338b5060156d7dc38..b9c7df1485bf60c4c073cd168c8731a8130d8ac9 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -118,7 +118,9 @@ def parse_options():
 basic_group.add_option(, --force-join, dest=force_join,
   action=store_true, default=False,
   help=Force client enrollment even if already enrolled)
-basic_group.add_option(--ntp-server, dest=ntp_server, help=ntp server to use)
+basic_group.add_option(--ntp-server, dest=ntp_servers, action=append,
+   help=ntp server to use. This option can be used 
+multiple times)
 basic_group.add_option(-N, --no-ntp, action=store_false,
   help=do not configure ntp, default=True, dest=conf_ntp)
 basic_group.add_option(, --force-ntpd, dest=force_ntpd,
@@ -2330,10 +2332,11 @@ def install(options, env, fstore, statestore):
 # We assume that NTP servers are discoverable through SRV records in the DNS
 # If that fails, we try to sync directly with IPA server, assuming it runs NTP
 root_logger.info('Synchronizing time with KDC...')
-ntp_servers = ds.ipadns_search_srv(cli_domain, '_ntp._udp', None, break_on_first=False)
+ntp_srv_servers = ds.ipadns_search_srv(cli_domain, '_ntp._udp',
+   None, break_on_first=False)
 synced_ntp = False
-if ntp_servers:
-for s in ntp_servers:
+if ntp_srv_servers:
+for s in ntp_srv_servers:
 synced_ntp = ipaclient.ntpconf.synconce_ntp(s)
 if synced_ntp:
 break
@@ -2838,11 +2841,11 @@ def install(options, env, fstore, statestore):
 # disable other timedate services first
 if options.force_ntpd:
 ipaclient.ntpconf.force_ntpd(statestore)
-if options.ntp_server:
-ntp_server = options.ntp_server
+if options.ntp_servers:
+ntp_servers = options.ntp_servers
 else:
-ntp_server = cli_server[0]
-ipaclient.ntpconf.config_ntp(ntp_server, fstore, statestore)
+ntp_servers = cli_server
+ipaclient.ntpconf.config_ntp(ntp_servers, fstore, statestore)
 root_logger.info(NTP enabled)
 
 if options.conf_ssh:
diff --git a/ipa-client/ipaclient/ntpconf.py b/ipa-client/ipaclient/ntpconf.py
index 7d5c82a89b51f68362f12869a9234f5b69aa5ba9..c22fba401d33009b3b95d1418dc7c8a03328d569 100644
--- a/ipa-client/ipaclient/ntpconf.py
+++ b/ipa-client/ipaclient/ntpconf.py
@@ -41,7 +41,7 @@ restrict -6 ::1
 
 # Use public servers from the pool.ntp.org project.
 # Please consider

Re: [Freeipa-devel] [PATCHES 0224-0225] Use NTP servers detected from SRV records in ntp configuration

2015-04-16 Thread Martin Babinsky 

On 04/16/2015 05:14 PM, Martin Basti wrote:

On 16/04/15 13:53, Martin Babinsky wrote:

On 04/16/2015 01:34 PM, Martin Babinsky wrote:

On 04/15/2015 04:17 PM, Martin Basti wrote:

https://fedorahosted.org/freeipa/ticket/4981

These patches keep usage of IPA server address as NTP server in NTP
configuration on clients, in case that  no NTP servers were
specified by
user, and no NTP servers were resolved from SRV records. This will
ensure backward compatibility, as IPA does not configure NTP SRV
records
for each domain automatically.

Patches attached.

Martin^2



PATCH 224 NACK
PATCH 225 ACK

Patch 224 you keep the original destination (dest=ntp_server)
for --ntp-server option, but in patch 226 the code attempts to get the
server names from options.ntpservers resulting in:

Traceback (most recent call last):
   File /sbin/ipa-client-install, line 2932, in module
 sys.exit(main())
   File /sbin/ipa-client-install, line 2913, in main
 rval = install(options, env, fstore, statestore)
   File /sbin/ipa-client-install, line 2342, in install
 if options.ntp_servers:
AttributeError: Values instance has no attribute 'ntp_servers'

So please fix this.

Naming the destination 'ntp_servers' (plural form) seems best because we
now store multiple values.


Also, if renaming option.ntp_server to option.ntp_servers, do not
forget to change also these lines in ipa-client-install:

2852 if options.ntp_server:
2853 ntp_servers = options.ntp_server

(line numbers after applying patches 224-226)


Stupid me, thank you

Updated patches attached.


ACK

--
Martin^3 Babinsky

--
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] [PATCHES 0224-0225] Use NTP servers detected from SRV records in ntp configuration

2015-04-15 Thread Martin Basti 

https://fedorahosted.org/freeipa/ticket/4981

These patches keep usage of IPA server address as NTP server in NTP 
configuration on clients, in case that  no NTP servers were specified by 
user, and no NTP servers were resolved from SRV records. This will 
ensure backward compatibility, as IPA does not configure NTP SRV records 
for each domain automatically.


Patches attached.

Martin^2
From 55f7717b37b205bd5f9a6d068868fd370ee9c539 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Tue, 14 Apr 2015 18:56:47 +0200
Subject: [PATCH 1/2] ipa client: make --ntp-server option multivalued

There can be more ntp servers in ntp.conf

Required for ticket: https://fedorahosted.org/freeipa/ticket/4981
---
 ipa-client/ipa-install/ipa-client-install | 17 ++---
 ipa-client/ipaclient/ntpconf.py   | 11 ++-
 ipa-client/man/ipa-client-install.1   |  2 +-
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 1ab6cc37bb172fb9d9ca4273567a9e38687c763f..6ff63e505a0dc627a3bdf1ab3445d156149fec73 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -118,7 +118,9 @@ def parse_options():
 basic_group.add_option(, --force-join, dest=force_join,
   action=store_true, default=False,
   help=Force client enrollment even if already enrolled)
-basic_group.add_option(--ntp-server, dest=ntp_server, help=ntp server to use)
+basic_group.add_option(--ntp-server, dest=ntp_server, action=append,
+   help=ntp server to use. This option can be used 
+multiple times)
 basic_group.add_option(-N, --no-ntp, action=store_false,
   help=do not configure ntp, default=True, dest=conf_ntp)
 basic_group.add_option(, --force-ntpd, dest=force_ntpd,
@@ -2330,10 +2332,11 @@ def install(options, env, fstore, statestore):
 # We assume that NTP servers are discoverable through SRV records in the DNS
 # If that fails, we try to sync directly with IPA server, assuming it runs NTP
 root_logger.info('Synchronizing time with KDC...')
-ntp_servers = ds.ipadns_search_srv(cli_domain, '_ntp._udp', None, break_on_first=False)
+ntp_srv_servers = ds.ipadns_search_srv(cli_domain, '_ntp._udp',
+   None, break_on_first=False)
 synced_ntp = False
-if ntp_servers:
-for s in ntp_servers:
+if ntp_srv_servers:
+for s in ntp_srv_servers:
 synced_ntp = ipaclient.ntpconf.synconce_ntp(s)
 if synced_ntp:
 break
@@ -2839,10 +2842,10 @@ def install(options, env, fstore, statestore):
 if options.force_ntpd:
 ipaclient.ntpconf.force_ntpd(statestore)
 if options.ntp_server:
-ntp_server = options.ntp_server
+ntp_servers = options.ntp_server
 else:
-ntp_server = cli_server[0]
-ipaclient.ntpconf.config_ntp(ntp_server, fstore, statestore)
+ntp_servers = cli_server
+ipaclient.ntpconf.config_ntp(ntp_servers, fstore, statestore)
 root_logger.info(NTP enabled)
 
 if options.conf_ssh:
diff --git a/ipa-client/ipaclient/ntpconf.py b/ipa-client/ipaclient/ntpconf.py
index 7d5c82a89b51f68362f12869a9234f5b69aa5ba9..c22fba401d33009b3b95d1418dc7c8a03328d569 100644
--- a/ipa-client/ipaclient/ntpconf.py
+++ b/ipa-client/ipaclient/ntpconf.py
@@ -41,7 +41,7 @@ restrict -6 ::1
 
 # Use public servers from the pool.ntp.org project.
 # Please consider joining the pool (http://www.pool.ntp.org/join.html).
-server $SERVER
+$SERVERS_BLOCK
 
 #broadcast 192.168.1.255 key 42		# broadcast server
 #broadcastclient			# broadcast client
@@ -84,7 +84,7 @@ SYNC_HWCLOCK=yes
 NTPDATE_OPTIONS=
 
 ntp_step_tickers = # Use IPA-provided NTP server for initial time
-$SERVER
+$TICKER_SERVERS_BLOCK
 
 def __backup_config(path, fstore = None):
 if fstore:
@@ -97,12 +97,13 @@ def __write_config(path, content):
 fd.write(content)
 fd.close()
 
-def config_ntp(server_fqdn, fstore = None, sysstore = None):
+def config_ntp(ntp_servers, fstore = None, sysstore = None):
 path_step_tickers = paths.NTP_STEP_TICKERS
 path_ntp_conf = paths.NTP_CONF
 path_ntp_sysconfig = paths.SYSCONFIG_NTPD
-sub_dict = { }
-sub_dict[SERVER] = server_fqdn
+sub_dict = {}
+sub_dict[SERVERS_BLOCK] = \n.join(server %s % s for s in ntp_servers)
+sub_dict[TICKER_SERVERS_BLOCK] = \n.join(ntp_servers)
 
 nc = ipautil.template_str(ntp_conf, sub_dict)
 config_step_tickers = False
diff --git a/ipa-client/man/ipa-client-install.1 b/ipa-client/man/ipa-client-install.1
index 726a6c133132dd2e3ba2fde43d8a2ec0549bfcef..978ac38c09567c101f9ad36598bb6d286284daa1 100644
---