Description
SbomManager (pkg/sbommanager/v1/sbom_manager.go) tracks retries for a given image via two separate counters, keyed independently by sbomName:
scanRetries (plain map[string]int) — incremented only in handleScannerCrash, counting consecutive sidecar OOM crashes.failureRetries (*expirable.LRU[string, int]) — incremented only in handleGenericFailure, counting consecutive generic SBOM-generation failures (source construction errors, syft cataloging errors, sidecar scan errors, and content-free ErrImageTooLarge).
Both are compared against the same maxScanRetries constant (currently 3) to decide when to pin an image to a terminal status (Incomplete or TooLarge). Because the two counters are independent, an image whose failures alternate between the two categories can take more than maxScanRetries total attempts before ever reaching a terminal status.
Example
- Container starts 2 times, each hitting a generic scan/source/syft error →
failureRetries[sbomName] reaches 2 (below the maxScanRetries=3 threshold, no terminal status yet).
- Container starts a 3rd time, but this time the sidecar OOM-crashes 3 consecutive times →
scanRetries[sbomName] independently counts 1, 2, 3 and pins the image at its own threshold.
Total attempts before a terminal status: 5 (2 generic + 3 crash), not 3 — because handleScannerCrash's crash-loop budget is layered on top of, rather than sharing, the generic-failure budget.
Why this matters
The whole point of maxScanRetries (introduced in #855, alongside failureRetries and the annotation-only PatchSBOMAnnotations marking) is to bound the number of times node-agent will silently reprocess a permanently-broken image before giving up and marking it terminal. With two independent counters, that bound is only enforced per-failure-category, not per-image — a real image experiencing a mix of failure modes gets a larger effective budget than maxScanRetries implies, with correspondingly more redundant scan attempts, error logs, and backend failure reports before the loop is finally broken.
This is a pre-existing inconsistency, not a regression from #855 — scanRetries already existed independently before that PR; #855 just added a second, similarly-shaped counter (failureRetries) alongside it rather than unifying them.
Suggested fix
Unify the two counters into a single per-sbomName failure budget that both handleGenericFailure and handleScannerCrash increment against, so maxScanRetries genuinely caps total attempts regardless of failure category. This likely means:
- Sharing one counter (e.g. keep
failureRetries as the single source of truth, and have handleScannerCrash increment/check it too instead of its own scanRetries map), or
- Deciding scanner-crash and generic failures deserve genuinely separate budgets (in which case this should be a documented design decision rather than an accidental side effect, and the total combined budget should be made explicit).
References
Description
SbomManager(pkg/sbommanager/v1/sbom_manager.go) tracks retries for a given image via two separate counters, keyed independently bysbomName:scanRetries(plainmap[string]int) — incremented only inhandleScannerCrash, counting consecutive sidecar OOM crashes.failureRetries(*expirable.LRU[string, int]) — incremented only inhandleGenericFailure, counting consecutive generic SBOM-generation failures (source construction errors, syft cataloging errors, sidecar scan errors, and content-freeErrImageTooLarge).Both are compared against the same
maxScanRetriesconstant (currently3) to decide when to pin an image to a terminal status (IncompleteorTooLarge). Because the two counters are independent, an image whose failures alternate between the two categories can take more thanmaxScanRetriestotal attempts before ever reaching a terminal status.Example
failureRetries[sbomName]reaches 2 (below themaxScanRetries=3threshold, no terminal status yet).scanRetries[sbomName]independently counts 1, 2, 3 and pins the image at its own threshold.Total attempts before a terminal status: 5 (2 generic + 3 crash), not 3 — because
handleScannerCrash's crash-loop budget is layered on top of, rather than sharing, the generic-failure budget.Why this matters
The whole point of
maxScanRetries(introduced in #855, alongsidefailureRetriesand the annotation-onlyPatchSBOMAnnotationsmarking) is to bound the number of times node-agent will silently reprocess a permanently-broken image before giving up and marking it terminal. With two independent counters, that bound is only enforced per-failure-category, not per-image — a real image experiencing a mix of failure modes gets a larger effective budget thanmaxScanRetriesimplies, with correspondingly more redundant scan attempts, error logs, and backend failure reports before the loop is finally broken.This is a pre-existing inconsistency, not a regression from #855 —
scanRetriesalready existed independently before that PR; #855 just added a second, similarly-shaped counter (failureRetries) alongside it rather than unifying them.Suggested fix
Unify the two counters into a single per-
sbomNamefailure budget that bothhandleGenericFailureandhandleScannerCrashincrement against, somaxScanRetriesgenuinely caps total attempts regardless of failure category. This likely means:failureRetriesas the single source of truth, and havehandleScannerCrashincrement/check it too instead of its ownscanRetriesmap), orReferences
handleGenericFailure:pkg/sbommanager/v1/sbom_manager.gohandleScannerCrash:pkg/sbommanager/v1/sbom_manager.go