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

[tests/generic_config_updater] implement GCU test NTP_SERVER #5803

Merged
merged 6 commits into from
Jul 8, 2022

Conversation

wen587
Copy link
Contributor

@wen587 wen587 commented Jun 13, 2022

Description of PR

Summary: Testcase of ntp server for generic updater apply-patch
[Done] sonic-net/sonic-utilities#2212 NTP restarter
[Done] submodule update
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012

Approach

What is the motivation for this PR?

End to End test support for Generic Updater apply-patch

How did you do it?

Modify ntp server config to dut and check if config change as expected

How did you verify/test it?

Run test of sonic-mgmt/tests/generic_config_updater/test_ntp.py on KVM

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@wen587 wen587 marked this pull request as ready for review June 13, 2022 08:22
@wen587 wen587 requested a review from a team as a code owner June 13, 2022 08:22
@wen587
Copy link
Contributor Author

wen587 commented Jun 24, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wen587
Copy link
Contributor Author

wen587 commented Jun 27, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

if output["stdout"] != "active":
return False
output = duthost.shell("ps -o etimes -p $(systemctl show ntp.service --property ExecMainPID --value) | sed '1d'")
if int(output['stdout'].strip()) < 10:
Copy link
Contributor Author

@wen587 wen587 Jun 29, 2022

Choose a reason for hiding this comment

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

I treat ntp service as restart when its starting time is less than 10sec.
Correct me if it is not reasonable. #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure how stable is the 10s hard-coded value. If you know exactly when the service is restarted (by your test script), you can take a timestamp before the restarting and use it to check the etimes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The restart is embedded into GCU post-check. Seems hard to find the exact time.
10s should be enough. Usually, the elapsed time is just 1 sec.
I am a little worried that the service process is not up if the check is too soon.

@wen587
Copy link
Contributor Author

wen587 commented Jul 7, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wen587 wen587 merged commit d4decc6 into sonic-net:master Jul 8, 2022
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.

2 participants