With chrony-2.0 it is possible to specify a pool of NTP servers instead of individual servers. It selects four servers from the addresses that the pool name is resolved to and replaces them automatically when they are unreachable or falsetickers. This is now used in the default Fedora chrony configuration.
To avoid extending the ntp API and the date time spoke, pools are simply presented as four separate servers with the same name.
Resolves: rhbz#1193389
From: Miroslav Lichvar mlichvar@redhat.com
With chrony-2.0 it is possible to specify a pool of NTP servers instead of individual servers. It selects four servers from the addresses that the pool name is resolved to and replaces them automatically when they are unreachable or falsetickers. This is now used in the default Fedora chrony configuration.
To avoid extending the ntp API and the date time spoke, pools are simply presented as four separate servers with the same name.
Resolves: rhbz#1193389 --- pyanaconda/ntp.py | 25 +++++++++++++++++++++---- pyanaconda/ui/gui/spokes/datetime_spoke.py | 5 ----- 2 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/pyanaconda/ntp.py b/pyanaconda/ntp.py index 40263ae..4f3c9d1 100644 --- a/pyanaconda/ntp.py +++ b/pyanaconda/ntp.py @@ -39,7 +39,10 @@
#example line: #server 0.fedora.pool.ntp.org iburst -SRV_LINE_REGEXP = re.compile(r"^\s*server\s*([-a-zA-Z.0-9]+)\s*[a-zA-Z]+\s*$") +SRV_LINE_REGEXP = re.compile(r"^\s*(server|pool)\s*([-a-zA-Z.0-9]+)\s*[a-zA-Z]+\s*$") + +#treat pools as four servers with the same name +SERVERS_PER_POOL = 4
class NTPconfigError(Exception): """Exception class for NTP related problems""" @@ -90,7 +93,10 @@ def get_servers_from_config(conf_file_path=NTP_CONFIG_FILE, for line in conf_file: match = srv_regexp.match(line) if match: - ret.append(match.group(1)) + if match.group(1) == "pool": + ret.extend([match.group(2)] * SERVERS_PER_POOL) + else: + ret.append(match.group(2))
except IOError as ioerr: msg = "Cannot open config file %s for reading (%s)" % (conf_file_path, @@ -141,9 +147,20 @@ def save_servers_to_config(servers, conf_file_path=NTP_CONFIG_FILE, #write info about the origin of the following lines new_conf_file.write(heading)
- #write new servers + #write new servers, use the pool directive for each group of + #SERVERS_PER_POOL servers with the same name + servers_done = set() for server in servers: - new_conf_file.write("server " + server + " iburst\n") + if server in servers_done: + continue + num = servers.count(server) + while num >= SERVERS_PER_POOL: + new_conf_file.write("pool " + server + " iburst\n") + num -= SERVERS_PER_POOL + while num > 0: + new_conf_file.write("server " + server + " iburst\n") + num -= 1 + servers_done.add(server)
#copy non-server lines from the old config and skip our heading for line in old_conf_file: diff --git a/pyanaconda/ui/gui/spokes/datetime_spoke.py b/pyanaconda/ui/gui/spokes/datetime_spoke.py index 8c20e13..1cb897b 100644 --- a/pyanaconda/ui/gui/spokes/datetime_spoke.py +++ b/pyanaconda/ui/gui/spokes/datetime_spoke.py @@ -328,11 +328,6 @@ def _add_server(self, server):
"""
- for row in self._serversStore: - if row[0] == server: - #do not add duplicate items - return - itr = self._serversStore.append([server, SERVER_QUERY, True])
#do not block UI while starting thread (may take some time)
This looks good to me, although I think it needs some additional work from the UX perspective as if I understand it correctly now the NTP servers configuration dialog will now show the same item 4 times without any clear explanation why. I think we need to change the look of the dialog to distinguish servers from pools and do the same elsewhere. But as I said, this is definitely a good starting point. Thanks for the patch!
Added label: f22-branch.
I don't think I understand how this is supposed to work. It reads servers from the config and multiplies pool entries by 4. And then when rewriting the config it multiplies each server, whether or not it is a pool server, by 4? Maybe some examples of what the config should look like in the case of pool vs. individual vs. mixed pool+individual servers.
I don't think I understand how this is supposed to work. It reads servers from the config and multiplies pool entries by 4. And then when rewriting the config it multiplies each server, whether or not it is a pool server, by 4? Maybe some examples of what the config should look like in the case of pool vs. individual vs. mixed pool+individual servers.
If I understand the code changes correctly, this implements the support for chrony's NTP pools by interpreting pools as quadruples of servers. So instead of ``[0-4].pool.ntp.org`` anaconda would show the ``pool.ntp.org`` URL four times in the UI and write it as ``pool pool.ntp.org`` to the config. IOW, every pool instance from the config is replaced by four servers with the same URL on reading the config and vice versa when writing it out.
I don't think I understand how this is supposed to work. It reads servers from the config and multiplies pool entries by 4. And then when rewriting the config it multiplies each server, whether or not it is a pool server, by 4? Maybe some examples of what the config should look like in the case of pool vs. individual vs. mixed pool+individual servers.
The default chrony config used to have:
server 0.fedora.pool.ntp.org iburst server 1.fedora.pool.ntp.org iburst server 2.fedora.pool.ntp.org iburst server 3.fedora.pool.ntp.org iburst
Now it's just: pool 2.fedora.pool.ntp.org iburst
The "2." name is used because it's currently the only one providing IPv6 addreses. That will likely change in the future.
With the patch the user will see there are still four servers used for synchronization and will hopefully not be tempted to add more.
Ah, ok, thanks for the explanation. How about when someone uses their own ntp infrastructure? Is 4 a standard grouping for pool?
On Mon, Apr 20, 2015 at 10:17:35AM -0700, Brian C. Lane wrote:
Ah, ok, thanks for the explanation. How about when someone uses their own ntp infrastructure? Is 4 a standard grouping for pool?
Four is the maximum number of servers chronyd will use from one pool by default. If the name resolves to fewer addreses than four, it will use only that many.
The patch was meant as something quick and simple before F22 is released. Ideally, the pool/server selection would use a checkbox in the server list dialog.
I'd really like to see finished UI before merging this in, because what's going to happen is very shortly after merging this in its current state we are going to get bug reports about how we are duplicating server names.
I think I can do the UI part today or tomorrow (hopefully I still remember how the NTP server UI works). My only concern is kickstart, where ``--ntpservers`` is unfortunately an option of the ``timezone`` command and can hardly be extended to support pools somehow -- ideas are more than welcome!
I'd really like to see finished UI before merging this in, because what's going to happen is very shortly after merging this in its current state we are going to get bug reports about how we are duplicating server names.
FWIW, duplicated server lines is a valid configuration. For example
server 2.fedora.pool.ntp.org iburst server 2.fedora.pool.ntp.org iburst server 2.fedora.pool.ntp.org iburst server 2.fedora.pool.ntp.org iburst
is functionally quite close to
pool 2.fedora.pool.ntp.org iburst
The only difference is that with the former the servers will not be replaced automatically when unreachable/falseticker.
Are you guys okay with something like this?
How do you remove one?
How do you remove one?
Same as before -- uncheck the "Use" checkbox.
(1) Is it expected that the user will want to toggle the pool setting after adding a new server, or on one that anaconda adds automatically?
(2) This is just a general comment on the entire dialog, not just what you've added here. Does the word "pool" in this context mean anything to the user? We don't really explain it anywhere, except perhaps maybe in the help which you cannot reach from this dialog.
(1) Is it expected that the user will want to toggle the pool setting after adding a new server, or on one that anaconda adds automatically?
Likely for a manually added servers/pools, unlikely for automatically added ones.
(2) This is just a general comment on the entire dialog, not just what you've added here. Does the word "pool" in this context mean anything to the user? We don't really explain it anywhere, except perhaps maybe in the help which you cannot reach from this dialog.
I think it should be clear to people who enter the dialog and want to do any changes there. No stats, of course, but I think, hardly ever anybody changes the NTP server configuration maybe except for people who know they are in a network blocking some servers and providing others. But even that is usually resolved by DHCP. Having said that, I'm not entirely against completely removing the dialog, but that could lead us to troubles with NTP turned ON but not syncing and no indication of what's going wrong and no way how to fix it.
Oh I wasn't suggesting we just get rid of the dialog. I had just noticed a couple potential UI problems and wanted to make sure we had a discussion about them, and perhaps came up with some fixes if they were in fact problems. But it sounds like things are okay.
anaconda-patches@lists.fedorahosted.org
-
dashea