fix(tracking): delete unused members before the nodes they hold

FR
3d ago
+656-1

Why

A generator, or any tracking run with delete_unused_nodes=True, creates nodes in dependency order: the node that is held first, then the node that holds it through a mandatory relationship. For example, two IP addresses and then a BGP session whose mandatory src_ip and dst_ip point at them. When a later run stops building that session, the cleanup deleted the unused members in the order the group returns them, which is creation order. The server refused to delete the IP addresses while the session still held them, so the run failed with TrackingGroupCleanupError, and only the next run deleted them.

Goal: one run deletes every unused node, including unused nodes that hold each other through mandatory relationships, without sending a delete that fails only because of the order.

Non-goals:

  • Backporting to develop and stable. They do not have the refusal handling from #1278, and this PR depends on it.
  • Changing how a member is handled when a node outside the group still holds it. It is still refused, kept in the group and reported.

Closes #1477

What changed

Behavioral changes:

  • The cleanup deletes a node before the nodes it holds through a mandatory relationship, so one run removes nodes that depend on each other.
  • If the order cannot be worked out (the schema or the peers cannot be read, or computing the order raises any other error), nothing is deleted. The failure stops the cleanup like any other request failure: every unused member stays in the group, this run's group is written, and the error is raised. The next run tries again.
  • A member that a node outside the group holds is still refused, kept in the group and reported in TrackingGroupCleanupError.

Implementation notes:

  • DeletionOrder in infrahub_sdk/query_groups.py builds a dependency graph between the candidate kinds from their relationships with optional: false. A peer that is a generic matches every candidate kind that inherits from it. The kinds are sorted with the existing topological_sort().
  • The schema is read with client.schema.all(), which returns the cached branch schema. The cache is already filled by the group lookup, so the order between kinds costs no request. schema.get() is not used per kind, because it downloads the whole branch schema again for every kind that is not in the cache.
  • Kinds that hold nodes of their own kind, or kinds that hold each other, cannot be ordered from the schema alone. For those kinds only, one filters(..., only=[mandatory relationships]) query per kind reads the peers, and the nodes are sorted by them. In every other case no query is sent.
  • A kind that is not in the schema has no known dependencies and goes last. Its delete fails with SchemaNotFoundError and is recorded as a refusal, as before.
  • The delete loop, _record_failure() and ReapResult are unchanged. They work on the ordered list.

What stayed the same:

  • No public API change. delete_unused() and update_group() keep their signatures.
  • When the order is worked out, the same members are deleted as before, only in a different order.

How to review

  1. DeletionOrder and the helpers above it in infrahub_sdk/query_groups.py: the graph, the handling of cycles and the order inside a kind.
  2. _order_for_deletion() and delete_unused() (async and sync): the ordering failure path that keeps every member.
  3. Tests in tests/unit/sdk/test_group_context.py and tests/integration/test_tracking_zero_members.py.

Worth extra scrutiny: whether every relationship with optional: false is one the server enforces when it deletes the peer.

How to test

uv run pytest tests/unit/sdk/test_group_context.py
uv run pytest tests/integration/test_tracking_zero_members.py

The new integration test test_unused_owner_and_animal_are_deleted_in_one_run (async and sync) creates a person and then a cat that holds them through the mandatory owner relationship, then runs an empty tracking session. With the fix, both are deleted in one run. With the ordering switched off, it fails with the error from the issue: "Cannot delete TestingPerson … It is linked to mandatory relationship owner on node TestingCat".

Impact & rollout

  • Backward compatibility: no breaking change.
  • Performance: no extra request in the common case. One peer query per kind that holds nodes of its own kind or of a kind that holds it back, plus one more request per page when that query has more members than the client's page size.
  • Infrahub CI: query_groups.py runs inside Infrahub generators. Running Infrahub's CI against this branch before merging is worth considering.

Checklist

  • Tests added/updated
  • Changelog entry added (changelog/1477.fixed.md)
  • External docs updated (not needed: the tracking guide does not describe the cleanup order or refusals)
  • Internal .md docs updated (none cover this area)

🤖 Generated with Claude Code