Provide flexibility when retrieving facts

[jose-delarosa]

SUMMARY

Provides flexibility when retrieving facts by not assuming that certains keys exist. Checks first if key exists before attempting to read from it.

Fixes #45248

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

redfish_utils

ANSIBLE VERSION

ansible 2.8.0.dev0 (fix-key-not-found a233088725) last updated 2018/09/25 22:22:07 (GMT -500)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /share1/git/ansible/lib/ansible
  executable location = /share1/git/ansible/bin/ansible
  python version = 2.7.5 (default, May 31 2018, 09:41:32) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]

ADDITIONAL INFORMATION

Cleaner code

[ansibot]

cc @billdodd @mraineri @tomasg2012
click here for bot help

[billdodd]

Thanks, Jose. I reviewed the code changes, and they look good.

I also tested the changes against an HPE iLO system. The results were better, but there were still some KeyError exceptions raised. I'll describe them in a separate comment. (It might take a while; that test system seems to be in a bad state now and I can't get it to respond to any Redfish APIs.)

[billdodd]

Here are the places where I still see KeyError exceptions when running against an HPE iLO system.

1. get_cpu_inventory(), line 766, KeyError on "@odata.id":

    key = "Processors"
    ...
    processors_uri = data[key]["@odata.id"]

On the iLO, the "Processors" property does not have an "@odata.id".

Instead of this:

    "Processors": {
        "@odata.id": "/redfish/v1/Systems/1/Processors/"
    },

it looks like this:

    "Processors": {
        "Count": 2,
        "ProcessorFamily": "Intel(R) Xeon(R) CPU E5-2660 v3 @ 2.60GHz",
        "Status": {
            "HealthRollUp": "OK"
        }
    },

2. get_nic_inventory(), line 811, KeyError on "EthernetInterfaces":

    key = "EthernetInterfaces"
    ...
    ethernetinterfaces_uri = data[key]["@odata.id"]

No "EthernetInterfaces" property on this system (though it does have an EthernetInterfaces link nested in an Oem property).

3. get_psu_inventory(), line 855, KeyError on "Links":

    for psu in data[u'Links'][u'PoweredBy']:

No "Links" property on this system. It does have "links" property, but no "PoweredBy" property under "links".

4. get_bios_attributes(), line 533, KeyError on "@odata.id":

   key = "Bios"
    ...
    bios_uri = data[key]["@odata.id"]

On this system, the "Bios" property does not have an "@odata.id".

Instead of this:

    "Bios": {
        "@odata.id": "/redfish/v1/Systems/1/Bios"
    },

it looks like this:

    "Bios": {
        "Current": {
            "VersionString": "U15 v2.40 (02/17/2017)"
        }
    },

5. get_bios_boot_order(), line 560, KeyError on "@odata.id":

    key = "Bios"
    ...
    bios_uri = data[key]["@odata.id"]

Same issue as # 4 above (just in a different method)

So I guess the bottom line is that some or all of the other data[key] and data[key1][key2] references that are not checked need to be.

[jose-delarosa]

@billdodd Open to suggestions on how to generalize.

[mraineri]

At least for numbers 1, 4, and 5, those seem to be implementation issues. If the Processors or Bios property is supported on a service, it's supposed to be a JSON object containing an @odata.id property.

[billdodd]

Thanks, Mike. Yes, it does look like 1, 4 and 5 are implementation issues. I checked a mockup from a newer release from the vendor and those items are corrected. I'm working on updating the firmware of our test system.

[jose-delarosa]

Bill,

For 2, the schema specifies either EthernetInterface (what I use), NetworkInterface and NetworkAdapter, do you agree? If so, I can change key=EthernetInterfaces to a list that contains all schema names.

Not sure what to do for 3, I don't see anything about PSUs in the mockup you sent me, so maybe just add code to gracefully bypass it if the schema name is not found?

[billdodd]

Jose,

For 2, the set of properties that would be found in "EthernetInterfaces", "NetworkInterfaces" and "NetworkAdapters" resources would all be different. So I'd say just stick with "EthernetInterfaces" for the "GetNicInventory" command. If any users in future have a need to get the inventory of those other resources, a new command or commands could be added.

Note that I got the KeyError originally on the iLO system with an older firmware. After I updated the firmware to the latest, the "EthernetInterfaces" property was there so it did not get a KeyError. But it still makes sense to check for the presence of "EthernetInterfaces" and if missing return an empty inventory payload (a graceful bypass as you mentioned for 3).

For 3, yes I agree with bypassing the inventory if "Links" or "PoweredBy" are not present.

[jose-delarosa]

@billdodd check last commit, added checks in several functions

[billdodd]

Thanks. I'll check it out and report back.

[billdodd]

Numbers 2 and 3 from my Sept 26 comment above now pass on the iLO. Numbers 1, 4, and 5 still fail, but those are service issues as pointed out above by Mike R.

[jose-delarosa]

shipit

[billdodd]

Reviewed code. Looks good.

shipit

[jose-delarosa]

Patch is not merging (again), checked BOTMETA.yml, we're all still there. Will ask in IRC to see what's going on.

[jose-delarosa]

bot_status

[jose-delarosa]

Discussed in IRC. User Pilou mentioned that modules in module_utils do not get shipit labels. He made a reference to ansible/ansibullbot#1046. Added this PR to core team meeting agenda ansible/community#375 to have it merged,

[ansibot]

Components

lib/ansible/module_utils/redfish_utils.py
support: community
maintainers: billdodd jose-delarosa mraineri tomasg2012

Metadata

waiting_on: maintainer
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): False
community_shipits (namespace maintainers): False
ansible_shipits (core team members): False
shipit_actors (maintainer or core team member): None
shipit_actors_other:
automerge: automerge shipit test failed

click here for bot help

[ryansb]

rebuild_merge