Fix ACCTZ clock skew by using DUT clock for RecordSubscribe timestamps#5347
Fix ACCTZ clock skew by using DUT clock for RecordSubscribe timestamps#5347pjacakArista wants to merge 1 commit into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses deterministic test failures in the ACCTZ suite caused by clock skew between the test host and the device under test (DUT). By standardizing the timestamp generation to use the DUT's own clock via gNMI, the tests become more robust and reliable across different environments. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function, acctz.StartTimestamp, which retrieves the current time from the Device Under Test (DUT) via gNMI to prevent clock skew issues in accounting tests. This helper is integrated into several test files to replace local time capturing. A critical issue was identified in record_subscribe_partial_test.go where the request timestamp is captured after the RPCs are sent, which would result in missing the expected records in the subscription stream.
pjacakArista
force-pushed
the
arista-ACCTZ4.2-timestamp
branch
from
5c59677 to
3825208
Compare
|
PR #5330 added Scope of this PR is now limited to those 2 tests only |
pjacakArista
force-pushed
the
arista-ACCTZ4.2-timestamp
branch
from
3825208 to
efa5969
Compare
pjacakArista
force-pushed
the
arista-ACCTZ4.2-timestamp
branch
from
b344267 to
dbe6880
Compare
The record_payload_truncation and record_subscribe_full tests use time.Now() from the test host as the RecordSubscribe request timestamp. Clock skew between the test host and DUT (~300-825ms observed) causes the acctz service to reject the timestamp as "in the future", resulting in deterministic test failures. Replace time.Now() with helpers.GetRouterTime() to source the timestamp from the DUT's own clock, matching the pattern already used in record_subscribe_non_grpc and record_subscribe_partial.
pjacakArista
force-pushed
the
arista-ACCTZ4.2-timestamp
branch
from
dbe6880 to
45024db
Compare
3 participants
ACCTZ tests (ACCTZ-1.1, 2.1, 3.1, 4.2) use time.Now() from the test host for RecordSubscribe request timestamps. Clock skew between the test host and DUT (~300-825ms observed) causes the acctz service to reject the timestamp as "in the future", resulting in deterministic test failures.
Add a shared acctz.StartTimestamp() helper that fetches the DUT's own clock via gNMI /system/state/current-datetime and use it across all four acctz tests instead of the various per-test workarounds (Nanos: 0, time.Sleep, time.Now().Add(-10s)).
With this change ACCTZ-4.2 passes on Arista's EOS