Skip to content

fix(promsd): look lane tags up by name, not by a computed slug - #19

Merged
github-actions[bot] merged 5 commits into
mainfrom
fix/795-lane-tags-by-name
Nov 6, 2026
Merged

github-actions[bot] merged 5 commits into
mainfrom
fix/795-lane-tags-by-name

Conversation

@znerol2

@znerol2 znerol2 commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

promsd asked NetBox for tagged records with ?tag=<slug>, and it built that slug from the tag name by turning the colon into a hyphen. That's the rule netbox-sot follows. NetBox's own rule drops the colon, so a monitoring:scrape tag created in the NetBox UI gets the slug monitoringscrape. NetBox then answered 400, the whole refresh failed, every lane kept its last snapshot, and /readyz went stale.

  • Look up by name: NetboxClient.tag_slugs(names) resolves the lane tags with one exact-name lookup (GET /api/extras/tags/?name=…). Record queries then use the slugs NetBox returns. promsd no longer keeps its own slug rule.
  • Missing lane tag: a lane tag that the lookup doesn't return gets no record query, and its lane is empty. The other lanes are served. /healthz lists it under missing_tags, while last_error stays null and /readyz stays ready.
  • Record query errors: any error on a record query, on any page and including a 400, fails the refresh and keeps the last good snapshot with the error in last_error. That matches existing outage behavior.

The README documents missing_tags and the error behavior.

Tests

  • UI-created slug monitoringscrape, and the legacy hyphen slug.
  • A missing tag: only its lane is empty, and no query names it.
  • A device with two lane tags where one tag is missing.
  • A 400 on the first page and on a later page: the last snapshot is kept, and last_error is set.
  • A tag removed after a good snapshot: its lane empties on the next refresh.
  • scripts/check.sh passes (26 tests).

Closes dmfdeploy/dmfdeploy#795

promsd turned a tag name into a slug by replacing the colon with a hyphen.
NetBox deletes the colon when a client sends no slug, so a tag created in
the UI got a slug promsd never asked for. NetBox answered 400 and the
whole refresh failed.

Each refresh now reads the slug of every lane tag from NetBox by exact
name. A tag NetBox lacks, or a query NetBox refuses with 400, empties
that lane only. /healthz lists those tags in unavailable_tags. Other
errors still fail the refresh and keep the last snapshot.

Closes dmfdeploy/dmfdeploy#795

Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
@znerol2
znerol2 requested a review from a team as a code owner November 6, 2026 17:13
@github-actions
github-actions Bot enabled auto-merge (rebase) November 6, 2026 17:13
The previous commit treated every 400 on a record query as a lane tag
NetBox refused. A malformed request or a 400 on a later page then
replaced the last good snapshot with a partial one and cleared
last_error.

Lane tags are already resolved by exact name before any record query,
so a missing tag is known at the lookup. A lane whose tag NetBox
doesn't have is empty and /healthz lists the tag in missing_tags. No
query names it. Any error on a record query fails the refresh, keeps
the last good snapshot and sets last_error.

Refs dmfdeploy/dmfdeploy#795

Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
The tag lookup and the record queries are separate requests. A tag
created and attached between them let the records carry a tag whose
lane the lookup had marked missing, so that lane served a partial set.

build_payloads now takes the missing tags and gives those lanes an
empty payload and no skip counts. The next refresh finds the tag and
serves every record.

Refs dmfdeploy/dmfdeploy#795

Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
@znerol2
znerol2 requested a review from lkirc November 6, 2026 17:23

@lkirc lkirc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes: the new tag lookup adds an undocumented permission dependency.

  • src/dmf_promsd/netbox.py:105: /api/extras/tags/ requires the token to have tag-view access in NetBox, but README.md still says the token need only read services, devices, and virtual machines. A token provisioned as documented can receive an empty tag result, publish empty lanes, clear last_error, and remain ready. Update the documented/deployed permission prerequisite (or preserve compatibility with the documented token) before merging.

NetBox hides objects a token may not view, so a token without
extras.view_tag sees an empty tag list. That looked like "no lane tags
exist", so promsd published empty lanes, cleared last_error and stayed
ready. The refresh now fails when none of the lane tags is visible, keeps
the last good snapshot, and names the missing permission in last_error.

The README now lists tag view access among the token permissions.

Refs dmfdeploy/dmfdeploy#795

Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
@znerol2
znerol2 requested a review from lkirc November 6, 2026 18:06

@lkirc lkirc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: partial tag visibility is still treated as a healthy missing-tag state.

  • src/dmf_promsd/cache.py:77: if not slugs only fails when zero tags are visible. NetBox object permissions can constrain extras.view_tag to a subset, so a token that can see monitoring:scrape but not probe/snmp makes this refresh succeed, replaces good probe/snmp data with empty lanes, clears last_error, and leaves /readyz ready. Do not treat one returned tag as proof of full tag visibility; fail closed or explicitly require and enforce an unrestricted all-tags view permission, with a partial-visibility regression test.

Refs dmfdeploy/dmfdeploy#795

NetBox can limit tag view permission to some tags, so a hidden lane tag
looks the same as a missing one. Treating it as missing published an
empty lane and left the service ready. Fail the refresh instead when
any lane tag is not seen, keep the last snapshot, and name the unseen
tags in last_error. netbox-sot creates all three lane tags, so an
unseen one is a misconfiguration or a permission gap.

This removes the missing-tags path: the snapshot field, the
build_payloads branch and the /healthz field.

Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
@znerol2

znerol2 commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, you're right that a single visible tag proved nothing about the others. 0ebf201 fails closed instead:

  • Any lane tag promsd can't see fails the refresh, whether it's missing from NetBox or hidden by permissions. The last snapshot is kept, and last_error names every unseen tag.
  • The missing-tags path is gone: the snapshot field, the build_payloads branch and the /healthz field.
  • test_a_partly_visible_lane_tag_set_fails_the_refresh_and_keeps_every_lane covers your case: only monitoring:scrape is visible. It fails on 3e6dccc.
  • The README now says the token needs view access to all three lane tags. netbox-sot creates all three, so an unseen one is a misconfiguration or a permission gap.

Readiness still follows freshness. /readyz turns 503 once the kept snapshot is older than freshness_window_seconds, as it does for every other refresh failure, and right away when there's no snapshot yet.

The issue's acceptance criterion was updated to match (dmfdeploy/dmfdeploy#795).

@znerol2
znerol2 requested a review from lkirc November 6, 2026 18:28

@lkirc lkirc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The current head resolves the earlier tag-visibility concern: every expected lane tag must be visible before a snapshot is replaced, preserving the last good lanes and reporting the error on any missing or hidden tag.

@github-actions
github-actions Bot merged commit a40b36b into main Nov 6, 2026
11 checks passed
@znerol2
znerol2 deleted the fix/795-lane-tags-by-name branch November 8, 2026 17:44
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.

dmf-promsd fails every lane when a monitoring tag was created in the NetBox UI

2 participants