Skip to content

baseosmgr: add Phase-1 unit tests + pathConfig seam#5922

Draft
eriknordmark wants to merge 3 commits into
lf-edge:masterfrom
eriknordmark:baseosmgr-unit-tests-phase1
Draft

baseosmgr: add Phase-1 unit tests + pathConfig seam#5922
eriknordmark wants to merge 3 commits into
lf-edge:masterfrom
eriknordmark:baseosmgr-unit-tests-phase1

Conversation

@eriknordmark
Copy link
Copy Markdown
Contributor

Description

Phase 1 of bringing pkg/pillar/cmd/baseosmgr/ from 0% to non-trivial
unit-test coverage. Companion to #5921 (the architecture doc).

The PR is split into two commits so the refactor can be reviewed
independently from the tests:

  1. baseosmgr: introduce pathConfig seam for persistent files
    refactor only; no behaviour change. Lifts the three on-disk paths
    baseosmgr persists across reboots
    (`current_retry_update_counter`, `config_retry_update_counter`,
    `forceFallbackCounter`) out of package-level constants and into
    a `pathConfig` struct hung off `baseOsMgrContext`.
    `defaultPathConfig()` returns the production paths under
    `/persist/`. The five callers (`saveCurrentRetryUpdateCounter`,
    `saveConfigRetryUpdateCounter`, `readSavedCurrentRetryUpdateCounter`,
    `readSavedConfigRetryUpdateCounter`, `readForceFallbackCounter`,
    `writeForceFallbackCounter`) now take `*baseOsMgrContext` and
    consult `ctxPtr.paths` instead of the constants.

  2. baseosmgr: add Phase-1 unit tests — 10 new test files. No
    production change.

Coverage delta

Source Before After
go test -count=1 -cover ./cmd/baseosmgr/... 0.0% 16.1%

Phase-1 covers the pure helpers, lookups, publishes, content-tree
status, the persistent counter files, and the trivial dispatcher /
delete wrappers. The remaining ~70% of statements is the
zboot-driven state machine (`doBaseOsStatusUpdate`,
`doBaseOsActivate`, `validatePartition`, `validateAndAssignPartition`,
`handleZbootTestComplete`, `handleForceFallback`,
`updateBaseOsStatusOnReboot`, `handleUpdateRetryCounter`, the
install worker, `shouldDeferForNodeDrain`) plus `Run()` — all
unblocked by Phase-2's Zboot / kubeapi / HVType seams.

Test files added (one per logical concern):

  • `helpers_test.go` — log init.
  • `mocks_test.go` — `mockPubSub` (Publication + Subscription) and
    the `newTestCtx` fixture (per-test temp paths + all pubs/subs
    wired up).
  • `baseosmgr_test.go` — `validateBaseOsConfig`, `appendError`,
    `publishBaseOSMgrStatus`.
  • `lookups_test.go` — `lookupBaseOsConfig / Status /
    StatusByPartLabel / StatusesByContentID / ConfigByVersion`,
    `lookupContentTreeStatus`.
  • `publish_test.go` — `publishBaseOsStatus`,
    `unpublishBaseOsStatus`, `publishZbootStatus`.
  • `counters_test.go` — save / readSaved current and config retry
    update counters; absent / present / overwrite / garbage.
  • `forcefallback_test.go` — read / write force-fallback counter;
    absent / present / overwrite / garbage.
  • `contenttree_test.go` — `checkContentTreeStatus` missing /
    loaded-no-change / loaded-from-initial / error;
    `handleContentTreeStatusImpl` no-fanout.
  • `globalconfig_test.go` — `handleGlobalConfigImpl` / `Delete`
    non-global key fast paths.
  • `wrappers_test.go` — trivial Delete dispatch wrappers,
    `maybeRetryInstall` non-TooEarly / no-config branches,
    `shouldDeferForNodeDrain` NOTSUPPORTED short-circuit,
    `handleNodeDrainStatusImpl` non-UPDATE ignore.

Why draft

Drafted because the Phase-2 refactor + tests (Zboot/kubeapi/HVType
seams) is the larger, more reviewable chunk and may inform feedback
on the layering used here. Happy to mark ready for review once
Phase 2 has been seen, or sooner if reviewers want to look now.

PR dependencies

None hard. Reads better after #5921.

How to test and validate this PR

```sh
cd pkg/pillar
go test -count=1 -race -cover ./cmd/baseosmgr/...
go test -count=1 -coverprofile=/tmp/b.cov ./cmd/baseosmgr/...
go tool cover -func=/tmp/b.cov
```

Expected: all tests pass, coverage ≈ 16.1%.

Changelog notes

No user-facing changes.

PR Backports

  • 16.0-stable: No, test-only addition.
  • 14.5-stable: No, test-only addition.
  • 13.4-stable: No, test-only addition.

Checklist

  • I've provided a proper description
  • I've added the proper documentation
  • I've tested my PR on amd64 device — N/A, host-side unit tests
  • I've tested my PR on arm64 device — N/A, host-side unit tests
  • I've written the test verification instructions
  • I've set the proper labels to this PR

eriknordmark and others added 3 commits May 8, 2026 02:07
Adds pkg/pillar/docs/baseosmgr.md describing the responsibilities,
pubsub inputs and outputs, internal logical components, control flow
through the five main paths (boot, BaseOsConfig install, content-tree
progress, test-complete commit, reboot-after-failed-update) plus the
force-fallback and retry-update side channels, and a debugging section.
Structured to mirror the existing nim.md and nodeagent.md so the pillar
docs remain consistent across microservices.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Lifts the three on-disk paths baseosmgr persists across reboots
(current_retry_update_counter, config_retry_update_counter,
forceFallbackCounter) out of package-level constants and into a
pathConfig struct hung off baseOsMgrContext. defaultPathConfig()
returns the production paths under /persist/, so behaviour is
unchanged. Run() initialises ctx.paths early.

The five callers — saveCurrentRetryUpdateCounter,
saveConfigRetryUpdateCounter, readSavedCurrentRetryUpdateCounter,
readSavedConfigRetryUpdateCounter, readForceFallbackCounter and
writeForceFallbackCounter — now take *baseOsMgrContext and consult
ctxPtr.paths instead of the constants. The corresponding callers in
Run() and handleForceFallback are updated.

This is the minimal seam needed for upcoming unit tests so the
counter persistence helpers can be driven against t.TempDir()
without touching /persist/.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds unit-test coverage for baseosmgr's pure helpers, lookup
functions, publication helpers and the persistent counter files.
No production behaviour change. The previous commit added the
pathConfig seam that lets the counter tests run against t.TempDir()
instead of /persist/.

Coverage of the package goes from 0% to 16.1% of statements
(go test -count=1 -cover). The remaining uncovered code is the
zboot-driven state machine (validatePartition,
validateAndAssignPartition, doBaseOsActivate, handleZbootTestComplete,
handleForceFallback, updateBaseOsStatusOnReboot, the install worker,
shouldDeferForNodeDrain) plus Run() and the agentlog-backed global
config branch — to be unlocked by a Zboot/kubeapi seam in Phase 2.

Files added (one per logical concern):

- helpers_test.go      log init helper
- mocks_test.go        mockPubSub (Publication+Subscription) and the
                       newTestCtx fixture (per-test temp paths +
                       all pubs/subs wired up)
- baseosmgr_test.go    validateBaseOsConfig, appendError,
                       publishBaseOSMgrStatus
- lookups_test.go      lookupBaseOsConfig / Status / StatusByPartLabel /
                       StatusesByContentID / ConfigByVersion,
                       lookupContentTreeStatus
- publish_test.go      publishBaseOsStatus, unpublishBaseOsStatus,
                       publishZbootStatus
- counters_test.go     save / readSaved current and config retry update
                       counters; absent / present / overwrite / garbage
- forcefallback_test.go read / write force-fallback counter; absent /
                        present / overwrite / garbage
- contenttree_test.go  checkContentTreeStatus missing / loaded-no-change /
                       loaded-from-initial / error;
                       handleContentTreeStatusImpl no-fanout
- globalconfig_test.go handleGlobalConfigImpl / Delete non-global key
                       fast paths
- wrappers_test.go     trivial Delete dispatch wrappers,
                       maybeRetryInstall non-TooEarly / no-config branches,
                       shouldDeferForNodeDrain NOTSUPPORTED short-circuit,
                       handleNodeDrainStatusImpl non-UPDATE ignore

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

❌ Patch coverage is 51.85185% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 22.28%. Comparing base (63fa63d) to head (f71bebd).
⚠️ Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
pkg/pillar/cmd/baseosmgr/paths.go 0.00% 5 Missing ⚠️
pkg/pillar/cmd/baseosmgr/baseosmgr.go 0.00% 3 Missing ⚠️
pkg/pillar/cmd/baseosmgr/forcefallback.go 57.14% 3 Missing ⚠️
pkg/pillar/cmd/baseosmgr/handlebaseos.go 83.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5922      +/-   ##
==========================================
+ Coverage   21.62%   22.28%   +0.66%     
==========================================
  Files         464      475      +11     
  Lines       83994    85700    +1706     
==========================================
+ Hits        18166    19101     +935     
- Misses      64300    64885     +585     
- Partials     1528     1714     +186     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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