Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Platform chrony configuration intolerant of user modified config #1449

Closed
fifofonix opened this issue Mar 27, 2023 · 8 comments · Fixed by coreos/fedora-coreos-config#2333
Closed
Labels

Comments

@fifofonix
Copy link

Describe the bug

Prior to this FCOS version it was possible to configure a custom NTP server via ignition by writing to /etc/sysconfig/chronyd. Note machines previously commissioned with this ignition snippet upgrade fine to this testing version. This issue occurs also on latest next.

    - path: /etc/sysconfig/chronyd
      mode: 0644
      overwrite: true
      contents:
        inline: |
          OPTIONS="'server ntp.server.acme.edu'
          "

It is not clear to me what the simple way should be to configure a custom ntp server. If there is a simple more canonical way than above I'm open to adopting it instead.

Reproduction steps

  1. Commission a new machine for the specified testing stream version with ignition snippet provided.
  2. Chronyd.service fails to start.

Expected behavior

  1. Chronyd.service should start normally.

Actual behavior

Chrony.service fails:

Could not parse server directive at line 1

This seems to be because a FCOS chronyd config is now created (this attempts to merge in the ignition-specified options but fails on quoting issues, yielding something like:

OPTIONS=''server ntp.server.acme.edu'
 -f /run/coreos/platform-chrony.conf'

System details

  • vSphere & AWS

Butane or Ignition config

No response

Additional information

Manually amending the auto-generated /run/coreos/sysconfig-chrony to below results in a happy chronyd.

OPTIONS="'server ntp.nyp.org'
 '-f /run/coreos/platform-chrony.conf'
"
@fifofonix
Copy link
Author

Output of sudo journalctl -u coreos-platform-chrony-config.service is:

Mar 28 10:53:42 t-xxx-g1-1 systemd[1]: Starting coreos-platform-chrony-config.service - CoreOS Configure Chrony Based On The Platform...
Mar 28 10:53:42 t-xxx-g1-1 coreos-platform-chrony-config[1361]: coreos-platform-chrony-config: Updated chrony to use aws configuration /run/coreos/platform-chrony.conf
Mar 28 10:53:42 t-xxx-g1-1 systemd[1]: Finished coreos-platform-chrony-config.service - CoreOS Configure Chrony Based On The Platform.
Mar 28 10:57:15 t-xxx-g1-1 systemd[1]: coreos-platform-chrony-config.service: Deactivated successfully.
Mar 28 10:57:15 t-xxx-g1-1 systemd[1]: Stopped coreos-platform-chrony-config.service - CoreOS Configure Chrony Based On The Platform.

@jlebon
Copy link
Member

jlebon commented Mar 28, 2023

Thanks for the details!

    - path: /etc/sysconfig/chronyd
      mode: 0644
      overwrite: true
      contents:
        inline: |
         OPTIONS="'server ntp.server.acme.edu'
         "

Interesting, I wouldn't have thought this worked. But indeed, the synopsys in chronyd(8) is:

chronyd [OPTION]... [DIRECTIVE]...

And the systemd service just does a pure string substitution:

EnvironmentFile=-/etc/sysconfig/chronyd
ExecStart=/usr/sbin/chronyd $OPTIONS

I could be wrong, but I think the intention with /etc/sysconfig/chronyd was more for specifying option flags rather than overriding the directives from /etc/chrony.conf. (And if you actually want to change the chrony configuration, you'd directly modify /etc/chrony.conf instead; unfortunately there is no /etc/chrony.conf.d/ by default.)

That said, if we want the new code in coreos/fedora-coreos-config#2271 to be tolerant of this, probably the simplest would be to not do anything if /etc/sysconfig/chronyd was modified (i.e. applying the same diff check we currently do for /etc/chrony.conf). In such cases, we consider the chrony configuration as owned by the user.

@dustymabe
Copy link
Member

dustymabe commented Mar 28, 2023

That said, if we want the new code in coreos/fedora-coreos-config#2271 to be tolerant of this, probably the simplest would be to not do anything if /etc/sysconfig/chronyd was modified (i.e. applying the same diff check we currently do for /etc/chrony.conf). In such cases, we consider the chrony configuration as owned by the user.

yeah. Feels a bit like if you'd want to sepcify your own server and stuff you'd supply your own chrony.conf, but I agree the simplest is probably just say if the user changes anything then let them own it.

dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this issue Mar 28, 2023
Typically we'd think the user would only pass in optional arguments
to chrony via OPTIONS in /etc/sysconfig/chrony but we found a case
where a user was providing actual config directives. Let's also
exit early out of coreos-platform-chrony-config if /etc/sysconfig/chrony
has been touched. If either /etc/chrony.conf or /etc/sysconfig/chrony
has been touched we just consider the user to own the config and not
do anything special based on the platform.

Fixes coreos/fedora-coreos-tracker#1449
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this issue Mar 28, 2023
Typically we'd think the user would only pass in optional arguments
to chrony via OPTIONS in /etc/sysconfig/chronyd but we found a case
where a user was providing actual config directives. Let's also
exit early out of coreos-platform-chrony-config if /etc/sysconfig/chronyd
has been touched. If either /etc/chrony.conf or /etc/sysconfig/chronyd
has been touched we just consider the user to own the config and not
do anything special based on the platform.

Fixes coreos/fedora-coreos-tracker#1449
@dustymabe
Copy link
Member

PR for the proposed change: coreos/fedora-coreos-config#2333

jlebon pushed a commit to coreos/fedora-coreos-config that referenced this issue Mar 28, 2023
Typically we'd think the user would only pass in optional arguments
to chrony via OPTIONS in /etc/sysconfig/chronyd but we found a case
where a user was providing actual config directives. Let's also
exit early out of coreos-platform-chrony-config if /etc/sysconfig/chronyd
has been touched. If either /etc/chrony.conf or /etc/sysconfig/chronyd
has been touched we just consider the user to own the config and not
do anything special based on the platform.

Fixes coreos/fedora-coreos-tracker#1449
@dustymabe dustymabe added status/pending-testing-release Fixed upstream. Waiting on a testing release. status/pending-next-release Fixed upstream. Waiting on a next release. labels Mar 28, 2023
@fifofonix
Copy link
Author

Tested successfully on newly provisioned node using testing-devel AMI.

@dustymabe dustymabe changed the title Chronyd Configuration On Newly Provisioned Testing (37.20230322.2.0) Nodes Failing Platform chrony configuration intolerant of user modified config Apr 6, 2023
@dustymabe
Copy link
Member

The fix for this went into next stream release 38.20230402.1.0. Please try out the new release and report issues.

@dustymabe
Copy link
Member

The fix for this went into testing stream release 37.20230401.2.0. Please try out the new release and report issues.

@dustymabe dustymabe added status/pending-stable-release Fixed upstream and in testing. Waiting on stable release. and removed status/pending-testing-release Fixed upstream. Waiting on a testing release. status/pending-next-release Fixed upstream. Waiting on a next release. labels Apr 6, 2023
@dustymabe
Copy link
Member

The fix for this went into stable stream release 37.20230401.3.0.

@dustymabe dustymabe removed the status/pending-stable-release Fixed upstream and in testing. Waiting on stable release. label Apr 18, 2023
c4rt0 pushed a commit to c4rt0/fedora-coreos-config that referenced this issue May 17, 2023
Typically we'd think the user would only pass in optional arguments
to chrony via OPTIONS in /etc/sysconfig/chronyd but we found a case
where a user was providing actual config directives. Let's also
exit early out of coreos-platform-chrony-config if /etc/sysconfig/chronyd
has been touched. If either /etc/chrony.conf or /etc/sysconfig/chronyd
has been touched we just consider the user to own the config and not
do anything special based on the platform.

Fixes coreos/fedora-coreos-tracker#1449
HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this issue Oct 10, 2023
Typically we'd think the user would only pass in optional arguments
to chrony via OPTIONS in /etc/sysconfig/chronyd but we found a case
where a user was providing actual config directives. Let's also
exit early out of coreos-platform-chrony-config if /etc/sysconfig/chronyd
has been touched. If either /etc/chrony.conf or /etc/sysconfig/chronyd
has been touched we just consider the user to own the config and not
do anything special based on the platform.

Fixes coreos/fedora-coreos-tracker#1449
HuijingHei pushed a commit to HuijingHei/fedora-coreos-config that referenced this issue Oct 10, 2023
Typically we'd think the user would only pass in optional arguments
to chrony via OPTIONS in /etc/sysconfig/chronyd but we found a case
where a user was providing actual config directives. Let's also
exit early out of coreos-platform-chrony-config if /etc/sysconfig/chronyd
has been touched. If either /etc/chrony.conf or /etc/sysconfig/chronyd
has been touched we just consider the user to own the config and not
do anything special based on the platform.

Fixes coreos/fedora-coreos-tracker#1449
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants