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 2 test for System Health: System health fan direction and system health led #16223

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

PJHsieh
Copy link
Contributor

@PJHsieh PJHsieh commented Dec 25, 2024

Description of PR

Summary:
Introduce 2 test for System Health: System health fan direction and system health led

test_system_health_fan_direction.py::

  • Validate healthd's capability to check fan direction.

test_system_health_led.py::

  • "Verify whether the fault LED color matches the system_health_monitoring_config.

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

Introduce 2 new test cases for system-health feature.

How did you do it?

Implement the 2 test cases.

test_system_health_fan_direction::

  • Stop thermalctld, and the mock the one fan direction to the other direction. Then, check the "show system-health summary" output.

test_system_health_led::

  • Stop pmon, and then check the led color from the output of "show system-health summary".

How did you verify/test it?

Run the 2 test cases.

Any platform specific information?

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

Documentation

… led

* test_fan_direction_alarm
  Add test case to test the abiliby for system-health's detection fan direction

* test_system_health_led
  Add test case to test the led color is as expected as defined in system_health configuration file.
@PJHsieh PJHsieh requested a review from prgeor as a code owner December 25, 2024 01:29
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@PJHsieh
Copy link
Contributor Author

PJHsieh commented Dec 25, 2024

@prgeor , all the check are passed. Please review and merge this PR, thanks.

@yutongzhang-microsoft
Copy link
Contributor

Can you verfiy that if these two added scripts can run on kvm testbed?

@PJHsieh
Copy link
Contributor Author

PJHsieh commented Dec 25, 2024

Hi, @yutongzhang-microsoft ,

I am sorry that I have no such environment for verification.

@yutongzhang-microsoft
Copy link
Contributor

yutongzhang-microsoft commented Dec 25, 2024

Hi, @yutongzhang-microsoft ,

I am sorry that I have no such environment for verification.

As the topology mark is any in these two scripts, can you add them into onboarding-t0 and onboarding-t1 PR checker and let the pipeline to check?
https://github.com/sonic-net/sonic-mgmt/blob/master/.azure-pipelines/pr_test_scripts.yaml#L473
https://github.com/sonic-net/sonic-mgmt/blob/master/.azure-pipelines/pr_test_scripts.yaml#L485

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@PJHsieh
Copy link
Contributor Author

PJHsieh commented Dec 25, 2024

Hi, @yutongzhang-microsoft ,

the 2 test cases are skipped on the KVM platform.

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@PJHsieh
Copy link
Contributor Author

PJHsieh commented Dec 25, 2024

Hi, @yutongzhang-microsoft ,
I am sorry that I have no such environment for verification.

As the topology mark is any in these two scripts, can you add them into onboarding-t0 and onboarding-t1 PR checker and let the pipeline to check? https://github.com/sonic-net/sonic-mgmt/blob/master/.azure-pipelines/pr_test_scripts.yaml#L473 https://github.com/sonic-net/sonic-mgmt/blob/master/.azure-pipelines/pr_test_scripts.yaml#L485

Hi, @yutongzhang-microsoft,

the 2 test cases are add into onboarding-t0 and onboarding-t1 PR checker list.

@yutongzhang-microsoft
Copy link
Contributor

Hi, @yutongzhang-microsoft ,
I am sorry that I have no such environment for verification.

As the topology mark is any in these two scripts, can you add them into onboarding-t0 and onboarding-t1 PR checker and let the pipeline to check? https://github.com/sonic-net/sonic-mgmt/blob/master/.azure-pipelines/pr_test_scripts.yaml#L473 https://github.com/sonic-net/sonic-mgmt/blob/master/.azure-pipelines/pr_test_scripts.yaml#L485

Hi, @yutongzhang-microsoft,

the 2 test cases are add into onboarding-t0 and onboarding-t1 PR checker list.

Great! Thanks!

system_health/test_system_health_fan_direction.py:
skip:
reason: "There is no table SYSTEM_HEALTH_INFO in STATE_DB on kvm testbed, skip in PR testing"
conditions:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you describe the skip reason correctly? It seems that we don't use the table SYSTEM_HEALTH_INFO in these two scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you describe the skip reason correctly? It seems that we don't use the table SYSTEM_HEALTH_INFO in these two scripts.
Hi, @yutongzhang-microsoft ,

The skip reason is revised.


system_health/test_system_health_led.py:
skip:
reason: "There is no table SYSTEM_HEALTH_INFO in STATE_DB on kvm testbed, skip in PR testing"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

Hi, @yutongzhang-microsoft ,

The skip reason is revised.

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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