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

SystemLogin: Add BMC checks for OOM #480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

debmc
Copy link
Collaborator

@debmc debmc commented Apr 24, 2019

Add filtering to check the BMC dmesg's for Out of memory and Killed
process.

Signed-off-by: Deb McLemore [email protected]

@@ -95,6 +103,19 @@ def runTest(self):
self.assertEqual(r.exitcode, 1)
for i in range(2):
self.cv_BMC.run_command("dmesg")
try:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that we are running 'dmesg' inside try command, can we remove the line running dmesg twice in for loop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe the original intent of running twice was to exercise the paths, make sure the console is working, login, etc. I think we can leave. This test is run as a "smoke test" in some CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's only there to generate some console output we should do something like find / and limit the output to 1000 lines or something. If the dmesg log is cleared before op-test is run against the system then this won't be doing much for us.

@oohal
Copy link
Contributor

oohal commented Sep 10, 2019

I'd rather you made a new test case for this rather than adding it to the BMCLogin test. The BMC's dmesg containing OOM messages is interesting, but it shouldn't be reported as a failure in BMCLogin.

@debmc
Copy link
Collaborator Author

debmc commented Sep 10, 2019

I'd rather you made a new test case for this rather than adding it to the BMCLogin test. The BMC's dmesg containing OOM messages is interesting, but it shouldn't be reported as a failure in BMCLogin.

I suppose the discussion of how these tests and suites are leveraged may help position this a bit more.

During setup of CI's (FW CI for instance, e.g. Hostboot), discussion of what is the smallest set of tests/suites which can be executed to provide adequate coverage and make best use of machine time.

Four suites were identified: example, system-access, hostboot and skiroot. If these suites run successfully then we know the machine boots and a certain amount of scope/coverage is provided which can be run as a smoke test/regression test for integrating new pull requests, etc.

The system-access suite (testcases/SystemLogin.py) is a unique set of tests which I believe were to cover OOB host login (tests the OOB console and run different commands, BMC login (tests logging into the BMC via ssh and run different commands, SSH host login (tests logging into the host via ssh and run different commands) and BMC Rest API exercises.

Each of the above tests are not extensive coverage, see the other tests like testcases/Console.py, testcases/KernelLog.py, etc. The other discussion is that op-test is not intended to test BMC functions or operations per se (there is coverage to make sure regressions do not occur and other applicable scenarios to exercise skiboot/skiroot paths, etc).

Since we do not cover dmesg filtering for the BMC OS, it was a good place to add this BMC OS OOM check since consumers (CI's, etc) need a small bucket of tests to catch some of the more important issues.

Debatable yes, up to you on how to change the strategy and have others change their CI's, etc.

@oohal
Copy link
Contributor

oohal commented Sep 12, 2019

I'm not sure I fully understand the problem. Are people hard-coding a set of tests to run which forces you to add new functions to an existing test? It sounds like what we should be doing is providing a set of smoke tests that are guaranteed to run quickly and and have the hostboot CI system use that suite.

Add filtering to check the BMC dmesg's for Out of memory and Killed
process.

Signed-off-by: Deb McLemore <[email protected]>
@debmc
Copy link
Collaborator Author

debmc commented Nov 6, 2019

I'm not sure I fully understand the problem

No problem, just a matter of efficiency, split out the new filtering to its own test in the system-access suite.

@PraveenPenguin PraveenPenguin force-pushed the master branch 2 times, most recently from 4d0cb14 to b976629 Compare October 6, 2023 07:38
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