Frzk / ansible-role-chrony

Ansible role to manage chrony.
Apache License 2.0
14 stars 8 forks source link

Allow users to specify options to apply to server / pools / peers #3

Open JuddTracy-DAS opened 3 years ago

JuddTracy-DAS commented 3 years ago

I would like to add the ability to separate the server list from the options applied to each server in the list similarly to the way the openstack chrony role works. For my use and hopefully others the separation makes the configuration more expandable.

The way it is done in the openstack version is they have a global options variable like the following with an empty default set:

chrony_ntp_server_options: "iburst maxpoll 10"

Then in the template that value can be applied to each server like:

{% if chrony_ntp_servers|length > 0 %}
# List of NTP servers to use
{% for server in chrony_ntp_servers %}
server {{ server }} {{ chrony_ntp_server_options }}
{% endfor %}
{% endif %}

This allows me to use inventory groups to specify the servers like:

[STRATUM1_TIME_SERVERS]
time0s1.example.com
time1s1.example.com
time2s1.example.com

[PROD_TIME_SERVERS]
prod-time0.example.com
prod-time1.example.com
prod-time2.example.com

So when I want to setup the PROD_TIME_SERVERS with STRATUM1_TIME_SERVERS as their servers I can do the following


chrony_ntp_server_options: "iburst maxpoll 10"
chrony_ntp_servers: "{{ groups.STRATUM1_TIME_SERVERS }}"

Without defining chrony_ntp_server_options the role should work as before.

Frzk commented 3 years ago

Hi,

Thanks for this idea. I've been thinking about it those last days. Your use-case sounds legitimate.

The major drawback with the suggested approach is that you can't specify different options for each server. I don't know if it's something people do/want, but I wanted to keep the ability to do it.

Now, I guess we could have a chrony_ntp_servers_common_options (as suggested) provided that we change the template a little bit in this (more or less) way, to make things clear:

{% if chrony_ntp_servers|length > 0 %}
# List of NTP servers to use
{% for server in chrony_ntp_servers %}
{%- set parts = server.split(" ", 1) -%}
server {{ parts[0] }} {{ chrony_ntp_servers_common_options | default("") }} {{ parts[1] | default("") }}
{% endfor %}
{% endif %}

I still need to figure out how to deal with the extra spaces it generates when chrony_ntp_servers_common_options and/or parts[1] are not set.

I also would like to keep perfectly clear (in the doc) that chrony_ntp_servers values can still come with their own options.

What do you think about it ?

JuddTracy-DAS commented 3 years ago

Honestly I am not a fan of parsing strings to do that. What if we made a server lookup table that the template would query for different options:

chrony_ntp_servers_common_options: "iburst maxpoll 10"
chrony_ntp_server_options:
  "prod-time0.example.com": "key 10"
  "prod-time0.example.com": "key 20"

The template could be something like this and the join would help deal with the spaces

{% if chrony_ntp_servers|length > 0 %}
# List of NTP servers to use
{% for server in chrony_ntp_servers %}
server {{ server }} {{ [chrony_ntp_servers_common_options] + [chrony_ntp_server_options[server] | default("")] | join(" ") }}
{% endfor %}
{% endif %}
Frzk commented 3 years ago

I have to admit that I'm not really fan of using a Python method in a Jinja template either. But that's the best I could come with.

I'm sorry, but from my point of view, repeating the servers hostnames to set their options is a no-go.

Something like this would be way more elegant:

chrony_ntp_servers_common_options: "whatever here"
chrony_ntp_servers:
  - server: "ntp1.example.com"
    options: "opt1 value opt2"
  - server: "ntp2.example.com"
    options: "opt3"

but that's not going to help you :-/

I'll try to come up with something else. Can't give an ETA though.