Repository: osbuild/osbuild
Pull request #2068 (2025-04-08 16:56:53)

stages/chrony: support specifying refclocks (RHEL-79065)
The refclock directive can be used to specify one or more hardware reference clocks to be used as a time source.  Each refclock line must specify a driver and a mandatory parameter, in the form:

    refclock driver parameter

Drivers can have driver-specific options:

    refclock driver parameter:[driver-option,...]

General refclock options can also be specified:

    refclock driver parameter:[driver-option,...] [general-option]...

The stage options schema is written so that the "driver" property is an object that must match one of four schemas corresponding to the four drivers, each with a "name" property matching the driver name.  Each driver defines its required property and any optional driver-specific options.

There are more general refclock options supported than the ones defined in this PR, but we can add them if and when we need them in the future.

Note that the restriction on the top-level stage options schema is now lifted and any set of options can be specified.  Servers are not required.  However, at least one top-level property is required still.

Docs: https://chrony-project.org/doc/3.4/chrony.conf.html

https://github.com/osbuild/images/issues/1397



Patch:
diff --git a/stages/org.osbuild.chrony b/stages/org.osbuild.chrony
index 2274a47e..11cec065 100755
--- a/stages/org.osbuild.chrony
+++ b/stages/org.osbuild.chrony
@@ -54,6 +54,112 @@ def handle_leapsectz(chrony_conf_lines, timezone):
         chrony_conf_lines[:] = [f"leapsectz {timezone}"] + chrony_conf_lines
 
 
+def handle_refclocks(chrony_conf_lines, refclocks):
+    new_lines = []
+
+    for refclock in refclocks:
+        driver = refclock["driver"]
+        driver_name = driver["name"]
+        new_line = "refclock"
+        driver_options = ""
+        if driver_name == "PPS":
+            driver_options = handle_refclock_pps(driver)
+        elif driver_name == "SHM":
+            driver_options = handle_refclock_shm(driver)
+        elif driver_name == "SOCK":
+            driver_options = handle_refclock_sock(driver)
+        elif driver_name == "PHC":
+            driver_options = handle_refclock_phc(driver)
+        else:
+            # this should be caught by the schema
+            raise ValueError(f"unknown refclock driver {driver_name}")
+
+        new_line += " " + driver_options
+
+        # append general reflock options
+        refclock_options = []
+        poll = refclock.get("poll")
+        if poll is not None:
+            refclock_options.append(f"poll {poll}")
+
+        dpoll = refclock.get("dpoll")
+        if dpoll is not None:
+            refclock_options.append(f"dpoll {dpoll}")
+
+        offset = refclock.get("offset")
+        if offset is not None:
+            refclock_options.append(f"offset {offset}")
+
+        if refclock_options:
+            gen_options_str = " ".join(refclock_options)
+            new_line += " " + gen_options_str
+
+        new_lines.append(new_line)
+
+    chrony_conf_lines[:] = new_lines + chrony_conf_lines
+
+
+def handle_refclock_pps(driver):
+    device = driver["device"]
+    line = f"PPS {device}"
+
+    options = []
+    if driver.get("clear"):
+        options.append("clear")
+
+    if options:
+        options_str = ",".join(options)
+        line += f":{options_str}"
+
+    return line
+
+
+def handle_refclock_shm(driver):
+    segment = driver["segment"]
+    line = f"SHM {segment}"
+
+    options = []
+    perm = driver.get("perm")
+    if perm is not None:
+        options.append(f"perm={perm}")
+
+    if options:
+        options_str = ",".join(options)
+        line += f":{options_str}"
+
+    return line
+
+
+def handle_refclock_sock(driver):
+    path = driver["path"]
+    return f"SOCK {path}"
+
+
+def handle_refclock_phc(driver):
+    path = driver["path"]
+    line = f"PHC {path}"
+
+    options = []
+    if driver.get("nocrossts"):
+        options.append("nocrossts")
+    if driver.get("extpps"):
+        options.append("extpps")
+    pin = driver.get("pin")
+    if pin is not None:
+        options.append(f"pin={pin}")
+    channel = driver.get("channel")
+    if channel is not None:
+        options.append(f"channel={channel}")
+    if driver.get("clear"):
+        options.append("clear")
+
+    if options:
+        options_str = ",".join(options)
+        line += f":{options_str}"
+
+    return line
+
+
 def main(tree, options):
     timeservers = options.get("timeservers", [])
     servers = options.get("servers", [])
@@ -61,6 +167,8 @@ def main(tree, options):
     # therefore default to 'None' to distinguish these two cases.
     leapsectz = options.get("leapsectz", None)
 
+    refclocks = options.get("refclocks", [])
+
     with open(f"{tree}/etc/chrony.conf", encoding="utf8") as f:
         chrony_conf = f.read()
 
@@ -76,6 +184,8 @@ def main(tree, options):
         handle_servers(lines, servers)
     if leapsectz is not None:
         handle_leapsectz(lines, leapsectz)
+    if refclocks:
+        handle_refclocks(lines, refclocks)
 
     new_chrony_conf = "\n".join(lines)
 
diff --git a/stages/org.osbuild.chrony.meta.json b/stages/org.osbuild.chrony.meta.json
index caf72909..2229e87c 100644
--- a/stages/org.osbuild.chrony.meta.json
+++ b/stages/org.osbuild.chrony.meta.json
@@ -19,28 +19,19 @@
     "  - 'maxpoll'",
     "  - 'iburst' (defaults to true)",
     "  - 'prefer' (defaults to false)",
+    "",
     "The `leapsectz` option configures chrony behavior related to automatic checking",
     "of the next occurrence of the leap second, using the provided timezone. Its",
     "value is a string representing a timezone from the system tz database (e.g.",
     "'right/UTC'). If an empty string is provided, then all occurrences of",
     "`leapsectz` directive are removed from the configuration.",
-    "Constraints:",
-    "  - Exactly one of 'timeservers' or 'servers' options must be provided."
+    "",
+    "The refclock directive can be used to specify one or more hardware",
+    "reference clocks to be used as a time source."
   ],
   "schema": {
     "additionalProperties": false,
-    "oneOf": [
-      {
-        "required": [
-          "timeservers"
-        ]
-      },
-      {
-        "required": [
-          "servers"
-        ]
-      }
-    ],
+    "minProperties": 1,
     "properties": {
       "timeservers": {
         "type": "array",
@@ -90,6 +81,164 @@
       "leapsectz": {
         "type": "string",
         "description": "Timezone used by chronyd to determine when will the next leap second occur. Empty value will remove the option."
+      },
+      "refclocks": {
+        "type": "array",
+        "items": {
+          "type": "object",
+          "additionalProperties": false,
+          "required": [
+            "driver"
+          ],
+          "properties": {
+            "driver": {
+              "oneOf": [
+                {
+                  "$ref": "#/definitions/PPS"
+                },
+                {
+                  "$ref": "#/definitions/SHM"
+                },
+                {
+                  "$ref": "#/definitions/SOCK"
+                },
+                {
+                  "$ref": "#/definitions/PHC"
+                }
+              ]
+            },
+            "poll": {
+              "type": "integer",
+              "description": "Specifies the interval between processing times of timestamps as a power of 2 in seconds."
+            },
+            "dpoll": {
+              "type": "integer",
+              "description": "Some drivers do not listen for external events and try to produce samples in their own polling interval. This is defined as a power of 2 and can be negative to specify a sub-second interval. The default is 0 (1 second)."
+            },
+            "offset": {
+              "type": "number",
+              "description": "This option can be used to compensate for a constant error. The default is 0.0."
+            }
+          }
+        }
+      }
+    },
+    "definitions": {
+      "PPS": {
+        "description": "Driver for the kernel PPS (pulse per second) API.",
+        "type": "object",
+        "required": [
+          "name",
+          "device"
+        ],
+        "additionalProperties": false,
+        "properties": {
+          "name": {
+            "type": "string",
+            "enum": [
+              "PPS"
+            ]
+          },
+          "device": {
+            "type": "string",
+            "description": "Path to the PPS device (typically /dev/pps?).",
+            "pattern": "^\\/(?!\\.\\.)((?!\\/\\.\\.\\/).)+$"
+          },
+          "clear": {
+            "type": "boolean",
+            "description": "By default, the PPS refclock uses assert events (rising edge) for synchronisation. With this option, it will use clear events (falling edge) instead."
+          }
+        }
+      },
+      "SHM": {
+        "description": "NTP shared memory driver.",
+        "type": "object",
+        "required": [
+          "name",
+          "segment"
+        ],
+        "additionalProperties": false,
+        "properties": {
+          "name": {
+            "type": "string",
+            "enum": [
+              "SHM"
+            ]
+          },
+          "segment": {
+            "type": "integer",
+            "description": "The number of the shared memory segment."
+          },
+          "perm": {
+            "type": "string",
+            "pattern": "^[0-7]{4}$",
+            "description": "This option specifies the permissions of the shared memory segment created by chronyd. They are specified as a numeric mode. The default value is 0600 (read-write access for owner only)."
+          }
+        }
+      },
+      "SOCK": {
+        "description": "Unix domain socket driver.",
+        "type": "object",
+        "required": [
+          "name",
+          "path"
+        ],
+        "additionalProperties": false,
+        "properties": {
+          "name": {
+            "type": "string",
+            "enum": [
+              "SOCK"
+            ]
+          },
+          "path": {
+            "type": "string",
+            "pattern": "^\\/(?!\\.\\.)((?!\\/\\.\\.\\/).)+$",
+            "description": "The path to the socket."
+          }
+        }
+      },
+      "PHC": {
+        "description": "Unix domain socket driver.",
+        "type": "object",
+        "required": [
+          "name",
+          "path"
+        ],
+        "additionalProperties": false,
+        "properties": {
+          "name": {
+            "type": "string",
+            "enum": [
+              "PHC"
+            ]
+          },
+          "path": {
+            "type": "string",
+            "pattern": "^\\/(?!\\.\\.)((?!\\/\\.\\.\\/).)+$",
+            "description": "The path to the device of the PTP clock to be used as a time source."
+          },
+          "nocrossts": {
+            "type": "boolean",
+            "description": "Disable use of precise cross timestamping."
+          },
+          "extpps": {
+            "type": "boolean",
+            "description": "Enable a PPS mode in which the PTP clock is timestamping pulses of an external PPS signal connected to the clock."
+          },
+          "pin": {
+            "type": "integer",
+            "description": "The index of the pin for the PPS mode. The default value is 0."
+          },
+          "channel": {
+            "type": "integer",
+            "description": "The index of the channel for the PPS mode. The default value is 0."
+          },
+          "clear": {
+            "type": "boolean",
+            "description": "This option enables timestamping of clear events (falling edge) instead of assert events (rising edge) in the PPS mode. This may not work with some clocks."
+          }
+        }
       }
     }
   }
diff --git a/stages/org.osbuild.rhsm.meta.json b/stages/org.osbuild.rhsm.meta.json
index dbac77ba..5d9af71b 100644
--- a/stages/org.osbuild.rhsm.meta.json
+++ b/stages/org.osbuild.rhsm.meta.json
@@ -71,6 +71,10 @@
               "manage_repos": {
                 "type": "boolean",
                 "description": "Whether subscription-manager should manage DNF repos file"
+              },
+              "auto_enable_yum_plugins": {
+                "type": "boolean",
+                "description": "Whether yum/dnf plugins subscription-manager and product-id should be enabled every-time subscription-manager or subscription-manager-gui is executed"
               }
             }
           },


Test patch:
diff --git a/stages/test/test_chrony.py b/stages/test/test_chrony.py
new file mode 100644
index 00000000..0f7de869
--- /dev/null
+++ b/stages/test/test_chrony.py
@@ -0,0 +1,293 @@
+#!/usr/bin/python3
+
+import os
+import re
+
+import pytest
+
+from osbuild import testutil
+
+STAGE_NAME = "org.osbuild.chrony"
+
+
+default_config = """\
+pool 2.fedora.pool.ntp.org iburst
+sourcedir /run/chrony-dhcp
+driftfile /var/lib/chrony/drift
+makestep 1.0 3
+rtcsync
+ntsdumpdir /var/lib/chrony
+leapseclist /usr/share/zoneinfo/leap-seconds.list
+logdir /var/log/chrony
+leapsectz right/UTC"""
+
+
+# stage options where all properties are used
+all_options = {
+    "timeservers": [
+        "ntp1.example.com",
+        "ntp2.example.com"
+    ],
+    "servers": [
+        {
+            "hostname": "ntp3.example.com",
+            "prefer": True,
+            "minpoll": 4,
+            "maxpoll": 4
+        },
+        {
+            "hostname": "ntp4.example.com",
+            "iburst": False,
+            "minpoll": 5,
+            "maxpoll": 5
+        }
+    ],
+    "leapsectz": "",
+    "refclocks": [
+        {
+            "driver": {
+                "name": "PPS",
+                "device": "/dev/pps42",
+                "clear": True
+            },
+            "poll": 1,
+            "dpoll": 2,
+            "offset": 0.3
+        },
+        {
+            "driver": {
+                "name": "SHM",
+                "segment": 42,
+                "perm": "0660"
+            },
+            "poll": 3,
+            "dpoll": 4,
+            "offset": 0.4
+        },
+        {
+            "driver": {
+                "name": "SOCK",
+                "path": "/run/time/thingie.socket"
+            },
+            "poll": 5,
+            "dpoll": 7,
+            "offset": 0.1
+        },
+        {
+            "driver": {
+                "name": "PHC",
+                "path": "/dev/ptp11",
+                "nocrossts": True,
+                "extpps": True,
+                "pin": 3,
+                "channel": 4,
+                "clear": True
+            },
+            "poll": 9,
+            "dpoll": 10,
+            "offset": 0.2
+        }
+    ]
+}
+
+all_options_conf = {  # we don't really care about the order of the lines
+    # timeservers
+    "server ntp1.example.com iburst",
+    "server ntp2.example.com iburst",
+
+    # servers
+    "server ntp3.example.com prefer iburst minpoll 4 maxpoll 4",
+    "server ntp4.example.com minpoll 5 maxpoll 5",
+
+    "refclock PPS /dev/pps42:clear poll 1 dpoll 2 offset 0.3",
+    # refclocks
+    "refclock SHM 42:perm=0660 poll 3 dpoll 4 offset 0.4",
+    "refclock SOCK /run/time/thingie.socket poll 5 dpoll 7 offset 0.1",
+    "refclock PHC /dev/ptp11:nocrossts,extpps,pin=3,channel=4,clear poll 9 dpoll 10 offset 0.2",
+
+    # the original config lines that weren't overwritten or removed
+    # note that leapsectz is removed
+    "sourcedir /run/chrony-dhcp",
+    "driftfile /var/lib/chrony/drift",
+    "makestep 1.0 3",
+    "rtcsync",
+    "ntsdumpdir /var/lib/chrony",
+    "leapseclist /usr/share/zoneinfo/leap-seconds.list",
+    "logdir /var/log/chrony",
+}
+
+
+timeservers = {
+    "timeservers": [
+        "ntp1.example.com",
+        "ntp2.example.com"
+    ],
+}
+
+
+servers_and_leap = {
+    "servers": [
+        {
+            "hostname": "ntp3.example.com",
+            "prefer": True,
+            "minpoll": 4,
+            "maxpoll": 4
+        },
+        {
+            "hostname": "ntp4.example.com",
+            "iburst": False,
+            "minpoll": 5,
+            "maxpoll": 6
+        }
+    ],
+    "leapsectz": ""
+}
+
+
+@pytest.mark.parametrize("test_data,expected_errs", [
+    # everything
+    (
+        all_options,
+        "",
+    ),
+
+    # only timeservers
+    (
+        timeservers,
+        "",
+    ),
+
+    # servers + leapsectz
+    (
+        servers_and_leap,
+        ""
+    ),
+
+    # bad refclock driver
+    (
+        {
+            "refclocks": [
+                {
+                    "driver": {
+                        "name": "invalid",
+                        "device": "/dev/pps42",
+                        "clear": True
+                    },
+                    "poll": 1,
+                    "dpoll": 2,
+                    "offset": 0.3
+                },
+            ],
+        },
+        ["is not valid under any of the given schemas"]
+    ),
+
+    # nothing (bad)
+    (
+        {},
+        # under py3.6 the message is
+        #   {} does not have enough properties
+        # under other versions the message is
+        #   {} should be non-empty
+        [re.compile(r"{} should be non-empty|{} does not have enough properties")]
+    ),
+
+    # pattern violations
+    (
+        {
+            "refclocks": [
+                {
+                    "driver": {
+                        "name": "PPS",
+                        "device": "not-a-path",
+                    },
+                },
+                {
+                    "driver": {
+                        "name": "SHM",
+                        "segment": 41,
+                        "perm": "a660"
+                    },
+                },
+                {
+                    "driver": {
+                        "name": "SOCK",
+                        "path": "/../bad"
+                    },
+                },
+                {
+                    "driver": {
+                        "name": "PHC",
+                        "path": "/dots/../root",
+                    },
+                }
+            ]
+        },
+        [
+            "{'name': 'PPS', 'device': 'not-a-path'} is not valid under any of the given schemas",
+            "{'name': 'SHM', 'segment': 41, 'perm': 'a660'} is not valid under any of the given schemas",
+            "{'name': 'SOCK', 'path': '/../bad'} is not valid under any of the given schemas",
+            "{'name': 'PHC', 'path': '/dots/../root'} is not valid under any of the given schemas",
+        ]
+    )
+])
+@pytest.mark.parametrize("stage_schema", ["1"], indirect=True)
+def test_schema_validation(stage_schema, test_data, expected_errs):
+    test_input = {
+        "name": STAGE_NAME,
+        "options": test_data,
+    }
+    res = stage_schema.validate(test_input)
+    for expected_err in expected_errs:
+        if expected_err == "":
+            assert res.valid is True, f"err: {[e.as_dict() for e in res.errors]}"
+        else:
+            assert res.valid is False
+            testutil.assert_jsonschema_error_contains(res, expected_err, expected_num_errs=len(expected_errs))
+
+
+@pytest.mark.parametrize("options,expected_contents", [
+    (all_options, all_options_conf),
+    (
+        timeservers,
+        {
+            "server ntp1.example.com iburst",
+            "server ntp2.example.com iburst",
+
+            # the original config lines that weren't overwritten or removed
+            "sourcedir /run/chrony-dhcp",
+            "driftfile /var/lib/chrony/drift",
+            "makestep 1.0 3",
+            "rtcsync",
+            "ntsdumpdir /var/lib/chrony",
+            "leapsectz right/UTC",
+            "leapseclist /usr/share/zoneinfo/leap-seconds.list",
+            "logdir /var/log/chrony",
+        },
+    ),
+    (
+        servers_and_leap,
+        {
+            "server ntp3.example.com prefer iburst minpoll 4 maxpoll 4",
+            "server ntp4.example.com minpoll 5 maxpoll 6",
+
+            # the original config lines that weren't overwritten or removed
+            "sourcedir /run/chrony-dhcp",
+            "driftfile /var/lib/chrony/drift",
+            "makestep 1.0 3",
+            "rtcsync",
+            "ntsdumpdir /var/lib/chrony",
+            "leapseclist /usr/share/zoneinfo/leap-seconds.list",
+            "logdir /var/log/chrony",
+        },
+    ),
+])
+def test_chrony_conf_contents(tmp_path, stage_module, options, expected_contents):
+    chrony_conf_path = "etc/chrony.conf"
+    testutil.make_fake_tree(tmp_path, {
+        chrony_conf_path: default_config,
+    })
+
+    stage_module.main(tmp_path, options)
+    with open(os.path.join(tmp_path, chrony_conf_path), encoding="utf-8") as chrony_conf:
+        assert set(chrony_conf.read().split("\n")) == expected_contents
diff --git a/test/data/stages/chrony/b.json b/test/data/stages/chrony/b.json
index a514b84d..d3577545 100644
--- a/test/data/stages/chrony/b.json
+++ b/test/data/stages/chrony/b.json
@@ -657,6 +657,51 @@
             "timeservers": [
               "ntp.example.com",
               "ntp2.example.com"
+            ],
+            "refclocks": [
+              {
+                "driver": {
+                  "name": "PPS",
+                  "device": "/dev/pps42",
+                  "clear": true
+                },
+                "poll": 1,
+                "dpoll": 2,
+                "offset": 0.3
+              },
+              {
+                "driver": {
+                  "name": "SHM",
+                  "segment": 42,
+                  "perm": "0660"
+                },
+                "poll": 1,
+                "dpoll": 2,
+                "offset": 0.3
+              },
+              {
+                "driver": {
+                  "name": "SOCK",
+                  "path": "/run/time/thingie.socket"
+                },
+                "poll": 1,
+                "dpoll": 2,
+                "offset": 0.3
+              },
+              {
+                "driver": {
+                  "name": "PHC",
+                  "path": "/dev/ptp11",
+                  "nocrossts": true,
+                  "extpps": true,
+                  "pin": 3,
+                  "channel": 4,
+                  "clear": true
+                },
+                "poll": 1,
+                "dpoll": 2,
+                "offset": 0.3
+              }
             ]
           }
         }
diff --git a/test/data/stages/chrony/b.mpp.yaml b/test/data/stages/chrony/b.mpp.yaml
index 39ae20a4..801c2b2a 100644
--- a/test/data/stages/chrony/b.mpp.yaml
+++ b/test/data/stages/chrony/b.mpp.yaml
@@ -32,3 +32,35 @@ pipelines:
           timeservers:
             - ntp.example.com
             - ntp2.example.com
+          refclocks:
+            - driver:
+                name: PPS
+                device: "/dev/pps42"
+                clear: true
+              poll: 1
+              dpoll: 2
+              offset: 0.3
+            - driver:
+                name: SHM
+                segment: 42
+                perm: "0660"
+              poll: 1
+              dpoll: 2
+              offset: 0.3
+            - driver:
+                name: SOCK
+                path: "/run/time/thingie.socket"
+              poll: 1
+              dpoll: 2
+              offset: 0.3
+            - driver:
+                name: PHC
+                path: "/dev/ptp11"
+                nocrossts: true
+                extpps: true
+                pin: 3
+                channel: 4
+                clear: true
+              poll: 1
+              dpoll: 2
+              offset: 0.3
diff --git a/test/data/stages/chrony/diff.json b/test/data/stages/chrony/diff.json
index e3872a35..a0cd450e 100644
--- a/test/data/stages/chrony/diff.json
+++ b/test/data/stages/chrony/diff.json
@@ -5,7 +5,7 @@
     "/etc/chrony.conf": {
       "content": [
         "sha256:204145d4e6cdbb720012175d1ad0209d5f618055d906ed651f883370cce915b0",
-        "sha256:9afbf4d52438f25af8fd74e6a9ee41c61e266505e83063ad894376b710496746"
+        "sha256:6608a8dd24405bf8dfdfdb4eba8a5a3340b1700fe09cf63111c529df0afb7ae3"
       ]
     }
   }
diff --git a/test/data/stages/rhsm/b.json b/test/data/stages/rhsm/b.json
index 19e5a973..53be3825 100644
--- a/test/data/stages/rhsm/b.json
+++ b/test/data/stages/rhsm/b.json
@@ -755,7 +755,8 @@
             },
             "subscription-manager": {
               "rhsm": {
-                "manage_repos": false
+                "manage_repos": false,
+                "auto_enable_yum_plugins": false
               },
               "rhsmcertd": {
                 "auto_registration": true
diff --git a/test/data/stages/rhsm/b.mpp.yaml b/test/data/stages/rhsm/b.mpp.yaml
index 278a92e8..6b8b4570 100644
--- a/test/data/stages/rhsm/b.mpp.yaml
+++ b/test/data/stages/rhsm/b.mpp.yaml
@@ -38,5 +38,6 @@ pipelines:
           subscription-manager:
             rhsm:
               manage_repos: false
+              auto_enable_yum_plugins: false
             rhsmcertd:
               auto_registration: true
diff --git a/test/data/stages/rhsm/diff.json b/test/data/stages/rhsm/diff.json
index 63ba55b3..709af2a1 100644
--- a/test/data/stages/rhsm/diff.json
+++ b/test/data/stages/rhsm/diff.json
@@ -17,7 +17,7 @@
     "/etc/rhsm/rhsm.conf": {
       "content": [
         "sha256:57e2bee5a30283ee95f3da2d5945f1d5852f1a4ed6900d22afd8f6a5ee5e5a11",
-        "sha256:aeb3eca521d423823acc5dc0e7d3409b6570e71174306871c1a861e387369933"
+        "sha256:881061a0cbca545ba30b76f84c47d623e086e0b5dadec91fb5d53a8292681652"
       ]
     }
   }