nodeagent: Phase 2 — Zboot/RebootStore/path seams + tests#5915
Draft
eriknordmark wants to merge 4 commits intolf-edge:masterfrom
Draft
nodeagent: Phase 2 — Zboot/RebootStore/path seams + tests#5915eriknordmark wants to merge 4 commits intolf-edge:masterfrom
eriknordmark wants to merge 4 commits intolf-edge:masterfrom
Conversation
Lifts four pieces of logic out of nodeagent.go and handletimers.go without changing any behaviour, so the upcoming unit tests can drive them directly: - incrementRestartCounterIn(path) — file-path-parameterised core of incrementRestartCounter. - parseSMARTDataFiles(currPath, prevPath, curr, prev) — likewise for parseSMARTData. - synthesizeRebootReason(...) and truncateRebootStack(...) — pure functions extracted from handleLastRebootReason. - nodeagentContext.startNodeOperation — function-pointer seam defaulting to "go handleNodeOperation"; scheduleNodeOperation now calls it instead of inlining the goroutine spawn, so tests can observe the scheduled op without actually launching the reboot path that would call zboot.Reset/Poweroff and os.Exit. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds unit-test coverage for nodeagent's pure logic and per-handler
state transitions. No production behaviour change. The previous
commit added the small refactors that make these tests possible.
Coverage of the package goes from 0% to 48.7% of statements
(go test -count=1 -cover). The remaining uncovered code is the
Run() wiring, the goroutine-driven handleNodeOperation, the
installer-log replay, and the thin Create/Modify/Delete pubsub
wrappers — all expected at this phase.
Files added (one per logical concern):
- helpers_test.go test logger init
- mocks_test.go mockPubSub (Publication+Subscription)
and newTestCtx fixture
- restartcounter_test.go §3.1 absent / existing / garbage
- smart_test.go §3.2 both / one missing / malformed
- maintmode_test.go §3.3 add+remove, idempotent, multi
- rebootreason_test.go §3.4 first boot, power-fail, kernel,
unknown, stored-wins, stack truncation
- handletimers_test.go §3.5 fallback / reset / vault-locked /
upgrade-test-validation timers,
scheduleNodeOperation idempotency
- zedagentstatus_test.go §3.6 ConfigGetStatus transitions,
handleDeviceCmd reboot/shutdown/poweroff,
handleZedAgentStatusImpl, certs-refused
- zbootstatus_test.go §3.7 other-updating reboot, current→active
commit, doZbootBaseOsTestValidationComplete,
initiateBaseOsControllerTestComplete
- handlers_test.go §3.8 volume / TPM / vault / kube
node-drain / allDomainsHalted
Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three small interfaces let the goroutine-spawned reboot path, the boot-reason reconstruction, and the file-touching helpers be driven from unit tests without actually invoking /sbin/zboot, persisting files under /persist, or os.Exit-ing the test binary. - Zboot wraps the subset of pkg/pillar/zboot used by nodeagent (EveCurrentPartition, IsCurrentPartitionStateInProgress, IsValidPartitionLabel, GetValidPartitionLabels, GetOtherPartition, IsOtherPartitionStateUpdating, Reset, Poweroff). realZboot adapts the package; nodeagentContext.zboot is defaulted to it. - RebootStore wraps the subset of pkg/pillar/agentlog used by nodeagent for reboot-reason persistence. realRebootStore adapts the package. - pathConfig holds the on-disk paths nodeagent reads or writes (firstboot file, installer log + send-require, restart counter, fault injection, SMART current/previous). defaultPathConfig reuses the existing package constants where they exist. The production code is updated to dispatch through ctxPtr.zboot, ctxPtr.rebootStore and ctxPtr.paths instead of calling the zboot / agentlog packages directly. Two adjacent fixes pulled in: - handleNodeOperation now uses ctxPtr.minRebootDelay rather than the package constant, finally honouring the comment "Some constants.. Declared here as variables to enable unit tests". - newNodeagentContext receives logArg (was an unused param) and threads it into realRebootStore. Test fixture (mocks_test.go) is extended with mockZboot and mockRebootStore so existing Phase-1 tests keep building. No production behaviour change. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Builds on the seams added in the previous commit to bring nodeagent's package coverage from 48.7% to 70.3%. No production change. New test files: - nodeoperation_test.go: handleNodeOperation reboot / poweroff / shutdown paths, plus waitForAllDomainsHalted's empty/bounded-wait branches. - handlelastreboot_test.go: handleLastRebootReason wrapper for the stored-reason, first-boot, stack-tagging, and restart-counter branches (uses the RebootStore + path seams). - installlog_test.go: handleInstallationLog with and without the send-require marker (uses agentbase.Init for ctx.Logger()). - wrappers_test.go: smoke tests for the trivial Create / Modify / Delete dispatch wrappers across ZedAgentStatus, ZbootStatus, VaultStatus, VolumeMgrStatus, TpmSanityStatus, NodeDrainStatus, and the GlobalConfig non-global key fast path. New tests folded into existing files: - restartcounter_test.go: incrementRestartCounter wrapper using ctx.paths.restartCounterFile. - smart_test.go: parseSMARTData wrapper using ctx.paths.smartCurrent / smartPrevious. - zbootstatus_test.go: publishZbootConfigAll, publishZbootConfig rejecting an invalid label, getZbootConfigAll empty + populated, handleDeviceTimers as a no-op baseline. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #5915 +/- ##
==========================================
+ Coverage 21.62% 22.74% +1.11%
==========================================
Files 464 475 +11
Lines 83994 85741 +1747
==========================================
+ Hits 18166 19502 +1336
- Misses 64300 64507 +207
- Partials 1528 1732 +204 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Phase 2 of the nodeagent unit-test work. Stacks on #5914 (Phase 1) and is best reviewed after it. Brings package coverage from 48.7% (end of Phase 1) to 70.3% of statements.
The PR is split into two commits so the refactor can be reviewed independently from the tests:
nodeagent: add Zboot/RebootStore/path seams— refactor only, no behaviour change. Three small interfaces let the goroutine-spawned reboot path, the boot-reason reconstruction, and the file-touching helpers be driven from unit tests without invoking/sbin/zboot, persisting under/persist, or callingos.Exitfrom the test binary.Zboot— wraps the subset ofpkg/pillar/zboot(current/other partition, valid labels, Reset, Poweroff).realZbootadapts the package;nodeagentContext.zbootdefaults to it.RebootStore— wrapspkg/pillar/agentlogreboot/boot-reason persistence.pathConfig— centralises the on-disk paths nodeagent reads or writes (firstboot file, installer log + send-require, restart counter, fault-injection file, SMART current/previous). Reuses existing package constants where they exist.handleNodeOperationnow usesctxPtr.minRebootDelayrather than the package constant — finally honours the existing comment "Some constants.. Declared here as variables to enable unit tests".newNodeagentContextnow uses itslogArgparameter (was_).mocks_test.gois extended withmockZbootandmockRebootStoreso existing Phase-1 tests keep building.nodeagent: add Phase-2 unit tests— uses the seams. No production change.nodeoperation_test.go—handleNodeOperationreboot/poweroff/shutdown pluswaitForAllDomainsHalted's empty and bounded-wait branches.handlelastreboot_test.go—handleLastRebootReasonwrapper (stored-reason, first-boot, stack-tagging, restart-counter).installlog_test.go—handleInstallationLogwith/without the send-require marker.wrappers_test.go— smoke tests for the trivial Create/Modify/Delete dispatch wrappers across all six pubsub topics + the GlobalConfig non-global-key fast path.incrementRestartCounterandparseSMARTDatawrapper tests,publishZbootConfigAll,publishZbootConfiginvalid-label rejection,getZbootConfigAllempty/populated,handleDeviceTimersno-op baseline.Coverage delta
go test -count=1 -cover ./cmd/nodeagent/...Remaining 0% targets are now exclusively things best covered end-to-end by Eden (Phase 3 of the plan):
Run()lifecycle / pubsub wiring,newNodeagentContext,initNodeDrainPubSub— agent boot.realZboot.X,realRebootStore.X) — already covered bypkg/pillar/zbootandpkg/pillar/agentlogtests, plus Eden e2e.Why draft
Stacks on #5914. Will mark ready for review once that PR has had a pass.
How to test and validate this PR
Expected: all tests pass, coverage ≈ 70.3%.
Changelog notes
No user-facing changes.
PR Backports
Checklist