Repository navigation
fix(lifecycle): stop detector before exporters, honor shutdown timeout, fix health server details - #50
Open
mattmattox wants to merge 7 commits into
Open
fix(lifecycle): stop detector before exporters, honor shutdown timeout, fix health server details#50mattmattox wants to merge 7 commits into
mattmattox wants to merge 7 commits into
Conversation
Cast statfs Bsize to int64 so pkg/monitors/system builds on macOS, where the field is uint32. Make the unit tests portable: short unix socket paths under the 104-byte limit, squat the wildcard address in the Prometheus bind-failure test, discover the loopback interface name, and skip tests that need 127.0.0.0/8 aliases or inotify rename semantics when the host lacks them. Remove the unused command-call tracking and testKernelPanicLog from the logpattern mocks, and revive the memory estimation assertions that were commented out. Closes #20 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Delete process docs copied from another project's template (docs/development/*, docs/deployment.md, .githooks/README.md) and replace docs/README.md with an index of the docs that exist, grouped for operators and developers. Repoint the links that targeted the deleted files and add scripts/check-doc-links.sh to catch broken relative links. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ftovers scripts/validate-pipeline-local.sh only had case branches for another project's components, so it ran zero checks and reported success. It now runs gofmt, GOOS=linux go vet, golangci-lint (SKIP_LINT=1 to allow it missing), gosec when installed, go test -short -race, helm lint and helm-verify-generated, and exits non-zero on any failure. Makefile: validate-component passes --component=; test-e2e uses -tags=e2e and checks for kind; COVERAGE_THRESHOLD is 70 and ci.yml calls make coverage-threshold so there is one source; docker-push and push-*-image require an explicit VERSION and never tag or push :latest. Removed the template header, helm-publish stub, deploy-*, bump*, qa-check, devils-advocate, check-kubeconfig-* and their variables. Deleted dead template files: init-from-template.sh, substitute-variables.sh, validate-api.sh, the GPG scripts, .githooks pre-push and setup.sh, hooks/, .validation.json, .version-rc. validate-repo-cleanliness.sh no longer rejects the repo's own Dockerfile.overlay-test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The chart and deploy manifests referenced supporttools/node-doctor-controller, but nothing built or published it, so enabling the controller always ended in ImagePullBackOff. Add Dockerfile.controller (static Go binary on scratch, non-root, writable /data for SQLite), wire it into the Makefile image targets, build it in ci.yml, and publish, sign and guard it in release.yml alongside the other two images. The chart's controller tag now follows image.tag instead of appVersion, which carries a v prefix that no published image tag has. deploy/controller pins the current release instead of :latest, and the e2e harness builds and loads the controller image kind expects. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…load strategy validation The registry only received the circuit breaker config on hot reload, so a fresh process ran with the package default (5 / 5m / 2) instead of the configured values, and circuitBreaker.enabled was read nowhere outside defaults and validation. NewRegistryFromConfig now wires the registry at startup through the same conversion ApplyConfig uses, and a disabled breaker bypasses trip/open checks while reporting Closed. Circuit breaker defaults apply regardless of enabled so a disabled block no longer leaves zeros the reload guard rejects. Remediate read dryRun without the lock and recordRateLimitEntry read maxPerHour before taking it; both raced with the setters reachable from hot reload. The reload validator accepted only restart/recreate/script/webhook, none of which exist, so every real strategy failed reload validation. It now uses the exported strategy list from pkg/types, and the config error message builds from the same list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A plain bool could not tell an absent circuitBreaker block from an explicit enabled: false, so omitting the block disabled the breaker. CircuitBreakerConfig.Enabled is now a *bool like ReloadConfig.Enabled: nil becomes true in ApplyDefaults and only an explicit false disables. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…t, fix health server details main never called det.Stop(): it cancelled its own context, which the detector does not use, then stopped exporters while the status loop was still exporting to them. The 30s shutdown select had a default branch, so it never waited and always logged success. Shutdown now runs detector -> exporters (reverse start order, health server last) in a goroutine and waits on it against the timeout. Detector: Stop no longer closes statusChan, which a fan-in goroutine from a monitor that timed out on Stop could still send on. The monitor Stop call moved inside the timed goroutine so a hung Stop cannot wedge shutdown. stopMonitorByName removes the handle only after a successful stop. Exporter iterations copy the slice under RLock. Health server: Content-Type is set before WriteHeader on /healthz and /ready, /status reports the version passed via SetVersion, Start honors Enabled and stops when its context is cancelled, and SetReady(false) holds against incoming statuses so main can withdraw readiness before monitors stop. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
On SIGTERM, main cancelled a context the detector never used, then stopped exporters while the detector was still exporting to them, and the 30s wait had a
defaultbranch so it returned at once and always logged a clean stop. Shutdown now withdraws readiness, callsdet.Stop(), stops exporters in reverse start order with the health server last, and waits for that against the timeout.Detector:
Stop()no longer closes the status channel while a timed-out monitor's fan-in goroutine may still send (panic); the monitorStop()runs inside the timed goroutine so a hung monitor cannot block shutdown;stopMonitorByNamekeeps the handle until stop succeeds; the exporter slice is copied under the read lock.Health server: Content-Type set before WriteHeader on
/healthzand/ready;/statusreports the build version instead ofv0.1.0;StarthonorsEnabledand its context;SetReady(false)holds against incoming statuses.🤖 Generated with Claude Code