The old script is a shell script, so it needs a complete rewrite rather than refactoring.
Description
Description
Details
Details
- Version
- -
- Is it a breaking change?
- Perfectly compatible
- Issue type
- Internal change (not visible to end users)
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | FEATURE REQUEST | None | T4564 Root task for rewriting [op-mode] to vyos.opmode format | ||
| Resolved | dmbaturin | T4909 Rewrite the NTP op mode in the new format |
Event Timeline
Comment Actions
@dmbaturin hope you don't mind but this looked like a good task for a beginner and with it being low priority thought I'd give it a go.
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.
I haven't deleted show_ntp.sh in the PR in case a reversion is needed (though unlikely)
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.
output of show ntp:
.- Number of sample points in measurement set.
/ .- Number of residual runs with same sign.
| / .- Length of measurement set (time).
| | / .- Est. clock freq error (ppm).
| | | / .- Est. error in freq.
| | | | / .- Est. offset.
| | | | | | On the -.
| | | | | | samples. \
| | | | | | |
Name/IP Address NP NR Span Frequency Freq Skew Offset Std Dev
==============================================================================
ec2-34-206-168-146.compu> 4 3 6 -930.574 9430.514 -55ms 1385us
ec2-18-193-41-138.eu-cen> 4 3 6 -1063.710 12223.605 -65ms 1890us
ec2-122-248-201-177.ap-s> 4 3 7 -1605.161 71816.508 -56ms 12msoutput of show ntp activity
200 OK 3 sources online 0 sources offline 0 sources doing burst (return to online) 0 sources doing burst (return to offline) 0 sources with unknown address
output of show ntp sources
.-- Source mode '^' = server, '=' = peer, '#' = local clock. / .- Source state '*' = current best, '+' = combined, '-' = not combined, | / 'x' = may be in error, '~' = too variable, '?' = unusable. || .- xxxx [ yyyy ] +/- zzzz || Reachability register (octal) -. | xxxx = adjusted offset, || Log2(Polling interval) --. | | yyyy = measured offset, || \ | | zzzz = estimated error. || | | \ MS Name/IP address Stratum Poll Reach LastRx Last sample =============================================================================== ^+ ec2-34-206-168-146.compu> 3 6 37 27 +509us[-1220us] +/- 99ms ^* ec2-18-193-41-138.eu-cen> 3 6 37 26 +98us[-1638us] +/- 110ms ^- ec2-122-248-201-177.ap-s> 3 6 37 25 +40ms[ +40ms] +/- 280ms
output of show ntp tracking
Reference ID : 12C1298A (ec2-18-193-41-138.eu-central-1.compute.amazonaws.com) Stratum : 4 Ref time (UTC) : Sun Apr 14 07:23:34 2024 System time : 0.001320436 seconds slow of NTP time Last offset : -0.001735987 seconds RMS offset : 0.001735987 seconds Frequency : 7.839 ppm slow Residual freq : -0.000 ppm Skew : 415.036 ppm Root delay : 0.128031909 seconds Root dispersion : 0.063434333 seconds Update interval : 63.6 seconds Leap status : Normal
I'll updated the documentation as follow up to this PR pending okay by all.
Comment Actions
Status update:
- "Like for like" functionality between .sh script and .py script is complete and working (can be viewed in PR)
- Raw output capability -> in progress
Comment Actions
@dmbaturin, @sever
Would love your input regarding the lack of headers when using the -c option. I've created a PoC around "chronyc -c activity" as it was the most straight forward command to start with.
Questions:
- Are dictionary keys matching the chronyc -c stdout headers wanted? A lot easier to implement if the answer is No.
If the answer to 1 is Yes then two more questions:
- Are you okay with naming the dict.keys() the same as the header labels in console output? For example for "chronyc tracking" they would be Reference ID, stratum, etc...
- For the four commands I'd implement, the approach I'm thinking is to preformat the dictionary keys within the function to align with each of the console commands and use a switch-statement to select the correct key list depending on the command being called e.g. chrony -c tracking, chrony -c sourcestats etc... From a maintainability perspective, should new commands be added in the future, it would require adding the preformatted key list to the function. This okay with you?
$ chronyc activity
200 OK
3 sources online
0 sources offline
0 sources doing burst (return to online)
0 sources doing burst (return to offline)
0 sources with unknown address
$ chronyc -c activity (no headers are given with -c option)
3,0,0,0,0
$ python3 ntp.py show --raw --command activity
{
"sourcesonline": "3", "sourcesoffline": "0", "sourcesdoingburst_returntoonline": "0", "sourcesdoingburst_returntooffline": "0", "sourceswithunknownaddress": "0"
}
$ chronyc tracking (as example of the headers mentioned in point 2 above)
Reference ID : 32CD3926 (50.205.57.38)
Stratum : 2
Ref time (UTC) : Tue Apr 16 18:53:38 2024
System time : 0.000114431 seconds slow of NTP time
Last offset : -0.000246494 seconds
RMS offset : 0.000139090 seconds
Frequency : 1.716 ppm slow
Residual freq : -0.002 ppm
Skew : 0.031 ppm
Root delay : 0.014382127 seconds
Root dispersion : 0.001521245 seconds
Update interval : 1040.1 seconds
Leap status : Normal
Comment Actions
From initial PR these two feedback points are now implemented. PR has been amended see https://github.com/vyos/vyos-1x/pull/3307
- Include a check to see if ntp service is enabled
# delete service ntp # commit $ python3 ntp.py show --raw --command sources NTP service is not enabled. $
- Include raw mode output for implemented commands:
$ python3 ntp.py show --raw --command activity { "sources_online": "3", "sources_offline": "0", "sources_doing_burst_return_to_online": "0", "sources_doing_burst_return_to_offline": "0", "sources_with_unknown_address": "0" } ---------------------- $ python3 ntp.py show --raw --command sources { "m": [ "^", "^", "^" ], "s": [ "-", "*", "-" ], "name/_ip_address": [ "23.131.160.7", "5.161.184.148", "74.6.168.73" ], "stratum": [ "2", "2", "2" ], "poll": [ "10", "10", "10" ], "reach": [ "377", "377", "377" ], "last_rx": [ "856", "862", "323" ], "last_sample_adj_offset": [ "-0.001282255", "0.001506826", "-0.006494109" ], "last_sample_mes_offset": [ "-0.001282255", "0.001658859", "-0.006494109" ], "last_sample_est_error": [ "0.047297850", "0.013728894", "0.036012821" ] } ---------------------- $ python3 ntp.py show --raw --command sourcestats { "name/_ip_address": [ "23.131.160.7", "5.161.184.148", "74.6.168.73" ], "np": [ "6", "64", "48" ], "nr": [ "3", "33", "22" ], "span": [ "7269", "42411", "40681" ], "frequency": [ "-0.249", "0.001", "-0.005" ], "freq_skew": [ "0.674", "0.039", "0.034" ], "offset": [ "-0.001139544", "0.000000704", "-0.007225735" ], "std_dev": [ "0.000479950", "0.001237294", "0.000862982" ] } ---------------------- $ python3 ntp.py show --raw --command tracking { "ref_id": "05A1B894", "ref_id_name": "5.161.184.148", "stratum": "3", "ref_time": "1713367837.312229155", "system_time": "-0.000149552", "last_offset": "0.000152033", "rms_offset": "0.000149490", "frequency": "-1.268", "residual_freq": "0.001", "skew": "0.041", "root_delay": "0.019489409", "root_dispersion": "0.002548643", "update_interval": "1038.4", "leap_status": "Normal" }
Comment Actions
Believe this task can be closed the PRs for Sagitta and Current were merged:
Merged PR in Current: https://github.com/vyos/vyos-1x/pull/3307
Merged PR in Sagitta: https://github.com/vyos/vyos-1x/pull/3395