ntp: T4909 rewrite NTP op mode in the new format

[Giggum]

Change Summary

Op commands are rewritten in Python and encompasses the existing chronyc calls that were in show_ntp.sh.

Have also added two additional functions. Op-mode definition has been updated accordingly and follows the chronyc command language so that there's alignment between xml and python source.

Op commands consist of:
show ntp
show ntp activity
show ntp sources
show ntp tracking
note: show ntp activity and sources are new additions, while tracking has been renamed from previous system.

See task in Maniphest for additional info,

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

https://vyos.dev/T4909

Related PR(s)

Component(s) name

Proposed changes

How to test

Smoketest result

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly -> will do as subsequent step

[sever-sever]

There is no check if the service is not configured

vyos@r4# delete service ntp 
[edit]
vyos@r4# commit
[edit]
vyos@r4# sudo su
root@r4:/home/vyos# sudo ./ntp.py show_summary --command activity
Traceback (most recent call last):
  File "/home/vyos/./ntp.py", line 45, in <module>
    res = vyos.opmode.run(sys.modules[__name__])
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/opmode.py", line 263, in run
    res = func(**args)
          ^^^^^^^^^^^^
  File "/home/vyos/./ntp.py", line 24, in show_summary
    output = cmd(f"chronyc '{command}'")
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/utils/process.py", line 155, in cmd
    raise OSError(code, feedback)
PermissionError: [Errno 1] failed to run command: chronyc 'activity'
returned: 506 Cannot talk to daemon
exit code: 1
root@r4:/home/vyos# 

[sever-sever]

The commit message does not include the task number, but you marked it as done.

For future use please git commit --amend and push --force to override the previous commit

[Giggum]

There is no check if the service is not configured

vyos@r4# delete service ntp 
[edit]
vyos@r4# commit
[edit]
vyos@r4# sudo su
root@r4:/home/vyos# sudo ./ntp.py show_summary --command activity
Traceback (most recent call last):
  File "/home/vyos/./ntp.py", line 45, in <module>
    res = vyos.opmode.run(sys.modules[__name__])
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/opmode.py", line 263, in run
    res = func(**args)
          ^^^^^^^^^^^^
  File "/home/vyos/./ntp.py", line 24, in show_summary
    output = cmd(f"chronyc '{command}'")
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/utils/process.py", line 155, in cmd
    raise OSError(code, feedback)
PermissionError: [Errno 1] failed to run command: chronyc 'activity'
returned: 506 Cannot talk to daemon
exit code: 1
root@r4:/home/vyos# 

Added this function to ntp.py
def is_configured(): # Check if ntp is configured config = ConfigTreeQuery() if not config.exists("service ntp"): raise vyos.opmode.UnconfiguredSubsystem("NTP service is not enabled")

[Giggum]

@dmbaturin, @sever-sever I've posted a question here seeking feedback https://vyos.dev/T4909 would appreciate you input.

[Giggum]

Feedback implemented (1) add raw mode, (2) check if ntp service is configured

[Giggum]

Feedback has been incorporated and changes applied as result.

[sever-sever]

The commit message does not include the task number, but you marked it as done.

For future use please git commit --amend and push --force to override the previous commit

Use please this in the future
This way you’ll have one commit instead of 10 commits

it will be difficult to revert the changes for example

It is a good idea to have just one commit.
Thanks.

[GurliGebis]

@Giggum a way to fix it would be to git rebase current and then mark all the fix commit as that, so they just gets merged into the main commit, and then force push.

[Giggum]

@sever-sever, @GurliGebis thank you both was abled to squash those commits into one.

[GurliGebis]

@sever-sever, @GurliGebis thank you both was abled to squash those commits into one.

You're welcome 🙂

[c-po]

@Mergifyio backport sagitta

[mergify]

backport sagitta

✅ Backports have been created

• #3395 ntp: T4909 rewrite NTP op mode in the new format (backport #3307) has been created for branch sagitta