Skip to content

fix: stops orphan tagging redfish inventory items the source still manages - #545

Open
marcinpsk wants to merge 1 commit into
bb-Ricardo:developmentfrom
marcinpsk:fix/check-redfish-orphan-tagging
Open

fix: stops orphan tagging redfish inventory items the source still manages#545
marcinpsk wants to merge 1 commit into
bb-Ricardo:developmentfrom
marcinpsk:fix/check-redfish-orphan-tagging

Conversation

@marcinpsk

Copy link
Copy Markdown

Problem

tag_all_the_things() adds the orphaned tag to every object which carries the primary tag and which a run does not register with its source. Two paths in the check_redfish source left inventory items untouched, so they were tagged orphaned while the source was still managing them.

1. An empty batch said nothing. update_all_items() read the component type back from the first item in the batch and returned early on an empty list:

if len(items) == 0:
    return

inventory_type = grab(items, "0.inventory_type")

A scan which reports no component of a type therefore left every one of that device's components of that type untouched, instead of marking them absent. A server whose fans stop being reported is the case which shows it.

2. Already absent items were never touched again. Marking an item absent was conditional on its health actually changing:

elif grab(nb_inventory_item, "data.custom_fields.health") != "Absent":

So an item already at absent was skipped on every later run, and tagged orphaned from then on.

Fix

The component type is passed in by the caller, which knows it whether or not the batch is empty, and is stamped onto the items in update_all_items(), so the value used for the lookup and the value stored on each item cannot drift apart.

Registering the item with the source is now unconditional. update() still writes nothing when the value is unchanged, so this adds no NetBox requests.

Notes

  • Changes the update_all_items() signature and drops the per-item inventory_type key at the nine call sites.
  • Adds [tool.pytest.ini_options] to pyproject.toml
  • The make_source() helper - should be moved to tests/conftest.py when available (I see it in adds a vcsim based test suite and runs it in CI #541)

…anages

tag_all_the_things() adds the orphaned tag to every object which carries the primary
tag and which a run does not register with its source. Two paths left inventory items
untouched, so they were tagged orphaned while the source was still managing them.

update_all_items() read the component type back from the first item in the batch and
returned early on an empty list. A scan which reports no component of a type therefore
left every one of that device's components of that type untouched, instead of marking
them absent. A server whose fans stop being reported is the case which shows it. The
type is now passed in by the caller, which knows it whether or not the batch is empty,
and is stamped onto the items, so the value used for the lookup and the value stored on
each item cannot drift apart.

Marking an item absent was also conditional on its health actually changing, so an item
already at absent was never touched again on any later run and was tagged orphaned from
then on. Registering the item with the source is now unconditional. update() still writes
nothing when the value is unchanged, so this adds no NetBox requests.

The tests seed items the way query_current_data() does, then drive the real update_fan()
and the real tag_all_the_things(). Both fail before this change.
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.

1 participant