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

Introduce new ZInfoNTPSource message in proto/info/ntpsource.proto #59

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

rouming
Copy link
Contributor

@rouming rouming commented Jun 6, 2024

ZInfoNTPSource message contains a list of peers describing to which NTP daemon (e.g. chronyd) is synchronized on EVE side. This message will be sent periodically from EVE to the /info controller endpoint.

CC: @zed-rishabh
CC: @eriknordmark

@rouming rouming requested a review from eriknordmark as a code owner June 6, 2024 13:41
@rouming rouming force-pushed the ntpsource-info-proto branch 3 times, most recently from 95e3a81 to c785ecb Compare June 6, 2024 14:09
@rouming
Copy link
Contributor Author

rouming commented Jun 6, 2024

There is one yetus complain

buflint:Enum zero value name "NTP_PEER_STATE_SYNC" should be suffixed with "_UNSPECIFIED".

which I can't fix, because 0 state means "SYNC" and not UNSPECIFIED.

@rouming rouming force-pushed the ntpsource-info-proto branch 3 times, most recently from 965f12b to f7b69fb Compare June 6, 2024 16:11
@eriknordmark eriknordmark changed the title Introduuce new ZInfoNTPSource message in proto/info/ntpsource.proto Introduce new ZInfoNTPSource message in proto/info/ntpsource.proto Jun 6, 2024
@eriknordmark
Copy link
Contributor

There is one yetus complain

buflint:Enum zero value name "NTP_PEER_STATE_SYNC" should be suffixed with "_UNSPECIFIED".

which I can't fix, because 0 state means "SYNC" and not UNSPECIFIED.

Is NTPSourceState and NTPSourceMode from some external/standard document? If so it would be good to put a reference to such a document.
And add whatever comment buflint needs to disable the check for that line. See https://buf.build/docs/configuration/v1/buf-yaml#allow_comment_ignores

@rouming rouming force-pushed the ntpsource-info-proto branch from f7b69fb to 55bd848 Compare June 7, 2024 09:55
rouming added 2 commits June 7, 2024 12:01
This will be published from EVE.

Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
This is autogenerated output, nothing to look at.

Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
@rouming rouming force-pushed the ntpsource-info-proto branch from 55bd848 to ed39d66 Compare June 7, 2024 10:01
@rouming
Copy link
Contributor Author

rouming commented Jun 7, 2024

which I can't fix, because 0 state means "SYNC" and not UNSPECIFIED.

Is NTPSourceState and NTPSourceMode from some external/standard document? If so it would be good to put a reference to such a document. And add whatever comment buflint needs to disable the check for that line.

mode and states are from the chrony itself. I added explanation and link to the sources.

I fixed the yetus for _UNSPECIFIED part by keeping state and mode +1, but there are more not related to that. I do not want to spoil each line with some yetus-disabling comment. This makes code looks ugly and the overall impression is that not a tool for development, but development for the tool making it happy. As usual 90% useless.

@rouming
Copy link
Contributor Author

rouming commented Jun 7, 2024

@eriknordmark could you please take a look and approve/merge if no objections. Want this changes to be taken by the other teams. Thanks.

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eriknordmark eriknordmark merged commit ab08ae4 into lf-edge:main Jun 7, 2024
3 of 4 checks passed
Comment on lines +52 to +76
message NTPSource {
bool authenticated = 1;
bool reachable = 2; // (reachability == 0xff), i.e. 8 attempts
uint32 reachability = 3;
NTPSourceState state = 4;
NTPSourceMode mode = 5;
string hostname = 6;
string src_addr = 7;
uint32 src_port = 8;
string dst_addr = 9;
uint32 dst_port = 10;
uint32 leap = 11;
uint32 stratum = 12;
uint32 precision = 13;
string ref_id = 14;
google.protobuf.Timestamp ref_time = 15;
sint32 poll = 16;
uint32 flags = 17; // bitmap from NTPSourceBitmap
double offset = 18;
double delay = 19;
double dispersion = 20;
double jitter = 21;
double root_delay = 22;
double root_disp = 23;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will be the unique identifier?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants