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.

Sign up for GitHub

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

[pmon daemon regression test] Correct the log messages with right daemon name and fix term_and_start_status failure #3922

Merged
merged 8 commits into from
Aug 19, 2021
+49 −146
Merged

[pmon daemon regression test] Correct the log messages with right daemon name and fix term_and_start_status failure #3922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
eab7b47
Fix wrong daemon names for platform daemon regression test
sujinmkang Jul 30, 2021
7c6b588
Restart the daemon if the autorestart is set to false in case of
sujinmkang Jul 30, 2021
1e3b74f
Fix based on the autorestart enabled with buildimage PR #8326
sujinmkang Aug 4, 2021
fab9aa1
The pcied db fix is in other PR.
sujinmkang Aug 6, 2021
e2ff20c
Fix test_pmon_pcied/ledd_term_and_start_status failure
sujinmkang Aug 11, 2021
01e359e
disable only pmon auto restart
sujinmkang Aug 17, 2021
5087487
remove the disable_and_enable_autorestart_containers
sujinmkang Aug 17, 2021
352f0cb
review comments
sujinmkang Aug 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 14 additions & 45 deletions tests/platform_tests/daemon/test_fancontrol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,38 +58,6 @@ def teardown_module(duthosts, rand_one_dut_hostname):
check_critical_processes(duthost, watch_secs=10)


@pytest.fixture(scope="module", autouse=True)
def disable_and_enable_autorestart(duthosts, rand_one_dut_hostname):
duthost = duthosts[rand_one_dut_hostname]
"""Changes the autorestart of containers from `enabled` to `disabled` before testing.
and Rolls them back after testing.
Args:
duthost: Hostname of DUT.
Returns:
None.
"""
containers_autorestart_states = duthost.get_container_autorestart_states()
disabled_autorestart_containers = []

for container_name, state in containers_autorestart_states.items():
if state == "enabled":
logger.info("Disabling the autorestart of container '{}'.".format(container_name))
command_disable_autorestart = "sudo config feature autorestart {} disabled".format(container_name)
command_output = duthost.shell(command_disable_autorestart)
exit_code = command_output["rc"]
pytest_assert(exit_code == 0, "Failed to disable the autorestart of container '{}'".format(container_name))
logger.info("The autorestart of container '{}' was disabled.".format(container_name))
disabled_autorestart_containers.append(container_name)

yield

for container_name in disabled_autorestart_containers:
logger.info("Enabling the autorestart of container '{}'...".format(container_name))
command_output = duthost.shell("sudo config feature autorestart {} enabled".format(container_name))
exit_code = command_output["rc"]
pytest_assert(exit_code == 0, "Failed to enable the autorestart of container '{}'".format(container_name))
logger.info("The autorestart of container '{}' is enabled.".format(container_name))

@pytest.fixture()
def check_daemon_status(duthosts, rand_one_dut_hostname):
duthost = duthosts[rand_one_dut_hostname]
Expand All @@ -106,9 +74,9 @@ def test_pmon_fancontrol_running_status(duthosts, rand_one_dut_hostname):
daemon_status, daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
logger.info("{} daemon is {} with pid {}".format(daemon_name, daemon_status, daemon_pid))
pytest_assert(daemon_status == expected_running_status,
"Pcied expected running status is {} but is {}".format(expected_running_status, daemon_status))
"{} expected running status is {} but is {}".format(daemon_name, expected_running_status, daemon_status))
pytest_assert(daemon_pid != -1,
"Pcied expected pid is a positive integer but is {}".format(daemon_pid))
"{} expected pid is a positive integer but is {}".format(daemon_name, daemon_pid))


def test_pmon_fancontrol_stop_and_start_status(check_daemon_status, duthosts, rand_one_dut_hostname):
Expand All @@ -124,18 +92,18 @@ def test_pmon_fancontrol_stop_and_start_status(check_daemon_status, duthosts, ra

daemon_status, daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
pytest_assert(daemon_status == expected_stopped_status,
"Pcied expected stopped status is {} but is {}".format(expected_stopped_status, daemon_status))
"{} expected stopped status is {} but is {}".format(daemon_name, expected_stopped_status, daemon_status))
pytest_assert(daemon_pid == -1,
"Pcied expected pid is -1 but is {}".format(daemon_pid))
"{} expected pid is -1 but is {}".format(daemon_name, daemon_pid))

duthost.start_pmon_daemon(daemon_name)
time.sleep(10)

post_daemon_status, post_daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
pytest_assert(post_daemon_status == expected_running_status,
"Pcied expected restarted status is {} but is {}".format(expected_running_status, post_daemon_status))
"{} expected restarted status is {} but is {}".format(daemon_name, expected_running_status, post_daemon_status))
pytest_assert(post_daemon_pid != -1,
"Pcied expected pid is -1 but is {}".format(post_daemon_pid))
"{} expected pid is a positive number not {}".format(daemon_name, post_daemon_pid))
pytest_assert(post_daemon_pid > pre_daemon_pid,
"Restarted {} pid should be bigger than {} but it is {}".format(daemon_name, pre_daemon_pid, post_daemon_pid))

Expand All @@ -153,18 +121,18 @@ def test_pmon_fancontrol_term_and_start_status(check_daemon_status, duthosts, ra

daemon_status, daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
pytest_assert(daemon_status == expected_exited_status,
"Pcied expected terminated status is {} but is {}".format(expected_exited_status, daemon_status))
"{} expected terminated status is {} but is {}".format(daemon_name, expected_exited_status, daemon_status))
pytest_assert(daemon_pid == -1,
"Pcied expected pid is -1 but is {}".format(daemon_pid))
"{} expected pid is a positive number not {}".format(daemon_name, daemon_pid))

duthost.start_pmon_daemon(daemon_name)
time.sleep(10)

post_daemon_status, post_daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
pytest_assert(post_daemon_status == expected_running_status,
"Pcied expected restarted status is {} but is {}".format(expected_running_status, post_daemon_status))
"{} expected restarted status is {} but is {}".format(daemon_name, expected_running_status, post_daemon_status))
pytest_assert(post_daemon_pid != -1,
"Pcied expected pid is -1 but is {}".format(post_daemon_pid))
"{} expected pid is -1 but is {}".format(daemon_name, post_daemon_pid))
pytest_assert(post_daemon_pid > pre_daemon_pid,
"Restarted {} pid should be bigger than {} but it is {}".format(daemon_name, pre_daemon_pid, post_daemon_pid))

Expand All @@ -182,14 +150,15 @@ def test_pmon_fancontrol_kill_and_start_status(check_daemon_status, duthosts, ra
daemon_status, daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
logger.info("{} daemon got killed unexpectedly and it is {} with pid {}".format(daemon_name, daemon_status, daemon_pid))
pytest_assert(daemon_status != expected_running_status,
"Pcied unexpected killed status is not {}".format(daemon_status))
"{} unexpected killed status is not {}".format(daemon_name, daemon_status))

time.sleep(10)

post_daemon_status, post_daemon_pid = duthost.get_pmon_daemon_status(daemon_name)

pytest_assert(post_daemon_status == expected_running_status,
"Pcied expected restarted status is {} but is {}".format(expected_running_status, post_daemon_status))
"{} expected restarted status is {} but is {}".format(daemon_name, expected_running_status, post_daemon_status))
pytest_assert(post_daemon_pid != -1,
"Pcied expected pid is -1 but is {}".format(post_daemon_pid))
"{} expected pid is a positive number not {}".format(daemon_name, post_daemon_pid))
pytest_assert(post_daemon_pid > pre_daemon_pid,
"Restarted {} pid should be bigger than {} but it is {}".format(daemon_name, pre_daemon_pid, post_daemon_pid))
68 changes: 18 additions & 50 deletions tests/platform_tests/daemon/test_ledd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,38 +58,6 @@ def teardown_module(duthosts, rand_one_dut_hostname):
check_critical_processes(duthost, watch_secs=10)


@pytest.fixture(scope="module", autouse=True)
def disable_and_enable_autorestart(duthosts, rand_one_dut_hostname):
duthost = duthosts[rand_one_dut_hostname]
"""Changes the autorestart of containers from `enabled` to `disabled` before testing.
and Rolls them back after testing.
Args:
duthost: Hostname of DUT.
Returns:
None.
"""
containers_autorestart_states = duthost.get_container_autorestart_states()
disabled_autorestart_containers = []

for container_name, state in containers_autorestart_states.items():
if state == "enabled":
logger.info("Disabling the autorestart of container '{}'.".format(container_name))
command_disable_autorestart = "sudo config feature autorestart {} disabled".format(container_name)
command_output = duthost.shell(command_disable_autorestart)
exit_code = command_output["rc"]
pytest_assert(exit_code == 0, "Failed to disable the autorestart of container '{}'".format(container_name))
logger.info("The autorestart of container '{}' was disabled.".format(container_name))
disabled_autorestart_containers.append(container_name)

yield

for container_name in disabled_autorestart_containers:
logger.info("Enabling the autorestart of container '{}'...".format(container_name))
command_output = duthost.shell("sudo config feature autorestart {} enabled".format(container_name))
exit_code = command_output["rc"]
pytest_assert(exit_code == 0, "Failed to enable the autorestart of container '{}'".format(container_name))
logger.info("The autorestart of container '{}' is enabled.".format(container_name))

@pytest.fixture()
def check_daemon_status(duthosts, rand_one_dut_hostname):
duthost = duthosts[rand_one_dut_hostname]
Expand All @@ -106,9 +74,9 @@ def test_pmon_ledd_running_status(duthosts, rand_one_dut_hostname):
daemon_status, daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
logger.info("{} daemon is {} with pid {}".format(daemon_name, daemon_status, daemon_pid))
pytest_assert(daemon_status == expected_running_status,
"Pcied expected running status is {} but is {}".format(expected_running_status, daemon_status))
"{} expected running status is {} but is {}".format(daemon_name, expected_running_status, daemon_status))
pytest_assert(daemon_pid != -1,
"Pcied expected pid is a positive integer but is {}".format(daemon_pid))
"{} expected pid is a positive integer but is {}".format(daemon_name, daemon_pid))


def test_pmon_ledd_stop_and_start_status(check_daemon_status, duthosts, rand_one_dut_hostname):
Expand All @@ -124,18 +92,18 @@ def test_pmon_ledd_stop_and_start_status(check_daemon_status, duthosts, rand_one

daemon_status, daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
pytest_assert(daemon_status == expected_stopped_status,
"Pcied expected stopped status is {} but is {}".format(expected_stopped_status, daemon_status))
"{} expected stopped status is {} but is {}".format(daemon_name, expected_stopped_status, daemon_status))
pytest_assert(daemon_pid == -1,
"Pcied expected pid is -1 but is {}".format(daemon_pid))
"{} expected pid is -1 but is {}".format(daemon_name, daemon_pid))

duthost.start_pmon_daemon(daemon_name)
time.sleep(10)

post_daemon_status, post_daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
pytest_assert(post_daemon_status == expected_running_status,
"Pcied expected restarted status is {} but is {}".format(expected_running_status, post_daemon_status))
"{} expected restarted status is {} but is {}".format(daemon_name, expected_running_status, post_daemon_status))
pytest_assert(post_daemon_pid != -1,
"Pcied expected pid is -1 but is {}".format(post_daemon_pid))
"{} expected pid is a positive integer but is {}".format(daemon_name, post_daemon_pid))
pytest_assert(post_daemon_pid > pre_daemon_pid,
"Restarted {} pid should be bigger than {} but it is {}".format(daemon_name, pre_daemon_pid, post_daemon_pid))

Expand All @@ -145,26 +113,26 @@ def test_pmon_ledd_term_and_start_status(check_daemon_status, duthosts, rand_one
@summary: This test case is to check the ledd terminated and restarted status
"""
duthost = duthosts[rand_one_dut_hostname]

if "201811" in duthost.os_version or "201911" in duthost.os_version:
Copy link
Contributor

Choose a reason for hiding this comment

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

@sujinmkang what is the difference in behavior for SIGTERM across different version?

Copy link
Contributor Author

@sujinmkang sujinmkang Aug 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@prgeor master and 202012 daemon has the signal_handler implemented to return the exitcode (128 + signal_number) but previous version just return exit code 0 for sigterm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really want to skip this test for 201811 and 201911? If not, it would be better we handle them differently here or make the exitcode same across 201811, 201911. What about 202012?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prgeor this change was based on the discussion during 202012 issue triage. If there would be any back port happening, then we will need another changes for this.

pytest.skip("Skip: SIG_TERM behaves differnetly in {} on {}".format(daemon_name, duthost.os_version))

pre_daemon_status, pre_daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
logger.info("{} daemon is {} with pid {}".format(daemon_name, pre_daemon_status, pre_daemon_pid))

duthost.stop_pmon_daemon(daemon_name, SIG_TERM, pre_daemon_pid)
time.sleep(2)

daemon_status, daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
pytest_assert(daemon_status == expected_exited_status,
"Pcied expected terminated status is {} but is {}".format(expected_exited_status, daemon_status))
pytest_assert(daemon_pid == -1,
"Pcied expected pid is -1 but is {}".format(daemon_pid))
pytest_assert(daemon_status != expected_running_status and pre_daemon_pid != daemon_pid,
"{} status for SIG_TERM should not be {} with pid:{}!".format(daemon_name, daemon_status, daemon_pid))

duthost.start_pmon_daemon(daemon_name)
time.sleep(10)

post_daemon_status, post_daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
pytest_assert(post_daemon_status == expected_running_status,
"Pcied expected restarted status is {} but is {}".format(expected_running_status, post_daemon_status))
"{} expected restarted status is {} but is {}".format(daemon_name, expected_running_status, post_daemon_status))
pytest_assert(post_daemon_pid != -1,
"Pcied expected pid is -1 but is {}".format(post_daemon_pid))
"{} expected pid is a positive integer but is {}".format(daemon_name, post_daemon_pid))
pytest_assert(post_daemon_pid > pre_daemon_pid,
"Restarted {} pid should be bigger than {} but it is {}".format(daemon_name, pre_daemon_pid, post_daemon_pid))

Expand All @@ -180,16 +148,16 @@ def test_pmon_ledd_kill_and_start_status(check_daemon_status, duthosts, rand_one
duthost.stop_pmon_daemon(daemon_name, SIG_KILL, pre_daemon_pid)

daemon_status, daemon_pid = duthost.get_pmon_daemon_status(daemon_name)
logger.info("{} daemon got killed unexpectedly and it is {} with pid {}".format(daemon_name, daemon_status, daemon_pid))
pytest_assert(daemon_status != expected_running_status,
"Pcied unexpected killed status is not {}".format(daemon_status))
"{} unexpected killed status is not {}".format(daemon_name, daemon_status))

time.sleep(10)

post_daemon_status, post_daemon_pid = duthost.get_pmon_daemon_status(daemon_name)

pytest_assert(post_daemon_status == expected_running_status,
"Pcied expected restarted status is {} but is {}".format(expected_running_status, post_daemon_status))
"{} expected restarted status is {} but is {}".format(daemon_name, expected_running_status, post_daemon_status))
pytest_assert(post_daemon_pid != -1,
"Pcied expected pid is -1 but is {}".format(post_daemon_pid))
"{} expected pid is a positive integer but is {}".format(daemon_name, post_daemon_pid))
pytest_assert(post_daemon_pid > pre_daemon_pid,
"Restarted {} pid should be bigger than {} but it is {}".format(daemon_name, pre_daemon_pid, post_daemon_pid))
Loading