openbmc / openbmc-test-automation

Apache License 2.0
100 stars 92 forks source link

Library file inclusions produce unnecessary commands #2113

Open Kostr opened 3 years ago

Kostr commented 3 years ago

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?

#(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.

#(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.

gkeishin commented 3 years ago

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..

Kostr commented 3 years ago

@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.

gkeishin commented 3 years ago
Maybe some of this information should be added to the test output? Something like Check for -U option support in BMC IPMI subsystem
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.

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.

vijaykhemka commented 3 years ago

Thanks Kostr and gkeishin for taking up this issue