openbmc-test-automation icon indicating copy to clipboard operation
openbmc-test-automation copied to clipboard

Library file inclusions produce unnecessary commands

Open Kostr opened this issue 4 years ago • 4 comments

When I was helping Vijay Khemka to write a Test Case "ipmi: add test for FRU device name" (https://gerrit.openbmc-project.xyz/c/openbmc/openbmc-test-automation/+/41817), I've noticed that besides an actual ipmitrool fru command the test framework executes series of other commands:

$ robot -v PLATFORM_ARCH_TYPE:x86 -v OPENBMC_HOST:192.168.101.130  -t "Test FRU for my device name" ipmi/test_ipmi_fru_device.robot
#(MSK) 2021/04/05 12:22:30.802988 -    0.105164 - Executing: get('/redfish/v1/')
#(MSK) 2021/04/05 12:22:30.937189 -    0.134201 - Executing: get('/redfish/v1/')
#(MSK) 2021/04/05 12:22:31.191200 -    0.254010 - Executing: get('/xyz/openbmc_project/', valid_status_codes=[200, 404])
#(MSK) 2021/04/05 12:22:31.746597 -    0.555397 - Issuing: ipmitool -I lanplus -C 17 -N 3 -p 623 -U root -P 0penBmc -H 192.168.101.130 power status
rc:                                               0x0000000000000000
==============================================================================
Test Ipmi Fru Device :: Test IPMI FRU data.
==============================================================================
Test FRU for my device name :: Search FRU for my device name          #(MSK) 2021/04/05 12:24:47.859950 -    0.219024 - Issuing: ipmitool -I lanplus -C 17 -N 3 -p 623 -U root -P 0penBmc -H 192.168.101.130 fru
....

There is no explicit Suite Setup in a robot file, so I'm wondering, should all of these starting commands be executed at all? For example is it appropriate that IPMI FRU test depends on a presence of a Redfish interface?

  • The strings
#(MSK) 2021/04/05 12:41:50.445044 -    0.106555 - Executing: get('/redfish/v1/')
#(MSK) 2021/04/05 12:41:50.579366 -    0.134322 - Executing: get('/redfish/v1/')
#(MSK) 2021/04/05 12:41:50.854746 -    0.275380 - Executing: get('/xyz/openbmc_project/', valid_status_codes=[200, 404])

are coming from the inclusion of Resource bmc_redfish_resource.robot in a lib/openbmc_ffdc_methods.robot file. I was able to get rid of them with the comment of this include string, or with the help of -v MTLS_ENABLED:True option.

  • The strings
#(MSK) 2021/04/05 12:22:31.746597 -    0.555397 - Issuing: ipmitool -I lanplus -C 17 -N 3 -p 623 -U root -P 0penBmc -H 192.168.101.130 power status
rc:                                               0x0000000000000000

are coming from a file https://github.com/openbmc/openbmc-test-automation/blob/master/lib/ipmi_client.py , particularly from this part of file:

def ipmi_setup():
    r"""
    Perform all required setup for running iptmitool commands.
    """

    verify_ipmi_user_parm_accepted()


ipmi_setup()

Is it really necessary to call this? FRU test seems to be working without it.

Kostr avatar Apr 05 '21 09:04 Kostr

There is reason why it was added, recent last year i believe the -U option support was added for OpenBMC to accept.. but the older one didn't have that option supported at the firmware.

so it give a first try with -U , and if the driver on the system accepts it , it takes the -U on all test suites executing IPMI.. if not without it.. so its more of a pre-check to support old and new method of doing it..

it may look un-necessary but it's always good to have the support till we finally do away with it..

gkeishin avatar Apr 05 '21 16:04 gkeishin

@gkeishin Thanks for the explanation! Maybe some of this information should be added to the test output? Something like Check for -U option support in BMC IPMI subsystem. Because in the other way it is confusing why the test system is making calls that it weren't asked for. Also as I understand your commit is only about ipmitool power status call. Can you say something about Redfish calls? Redfish is a completely different subsystem. bmcweb app can not be even present on the BMC, but it shouldn't affect IPMI tests.

Kostr avatar Apr 05 '21 17:04 Kostr

Maybe some of this information should be added to the test output? Something like Check for -U option support in BMC IPMI subsystem
  • [ ] Yes we can do that.. thanks for the feedback, I ll try amending ahead comment in the code.
Can you say something about Redfish calls? Redfish is a completely different subsystem. bmcweb app can not be even present on the BMC, but it shouldn't affect IPMI tests.
  • [x] Thanks for asking, yes this similar case, currently the RESTful support are both non standard /xyz/openbmc_project/ and /redfish/v1/ where redfish is recently new in the OpenBMC. So there is a mix and match of both going on , like you pointed out in the trace #(MSK) 2021/04/05 12:41:50.445044 - 0.106555 - Executing: get('/redfish/v1/') #(MSK) 2021/04/05 12:41:50.579366 - 0.134322 - Executing: get('/redfish/v1/') #(MSK) 2021/04/05 12:41:50.854746 - 0.275380 - Executing: get('/xyz/openbmc_project/', valid_status_codes=[200, 404])

where the code decides if its ONLY REST (xyz) or ONLY Redfish(/redfish/v1/) or both REST and Redfish. To support and navigate / load which path test suite needs to take, those pre-check are being made.

we would want the code to ideally detect and do its stuff, however having said that.. may be down, we would want to cleanly handle them but for now the transition is mixed RESTful. ( REST Legacy(xyz) and Redfish support) and stay that way.

gkeishin avatar Apr 06 '21 04:04 gkeishin

Thanks Kostr and gkeishin for taking up this issue

vijaykhemka avatar Apr 07 '21 23:04 vijaykhemka