Repository navigation
fix(config): honor documented keys, parse strictly, validate monitors - #51
Open
mattmattox wants to merge 11 commits into
Open
mattmattox wants to merge 11 commits into
mattmattox wants to merge 11 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>
…alidators Top-level config now fails to load on unknown keys (yaml KnownFields / json DisallowUnknownFields) and the error names the key. Monitor config maps stay free-form, but every monitor validator now logs a [WARN] line listing keys its parser does not read, including nested mount points, log patterns, endpoints, kubelet auth and the DNS sections. ValidateWithRegistry runs each enabled monitor's registered validator so -validate-config catches per-type errors instead of reporting success on configs that fail at startup. Memory, disk and CPU thresholds accept YAML integers, and the disk and DNS built-in defaults use the keys their parsers read. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…hart The default config, every example, and the chart ConfigMap used keys no parser reads (paths, loadAverageThresholds, memoryThresholds, domains, count, checkPods, pattern, exitCodeMapping, and others), so those monitors silently ran on built-in defaults and the logpattern examples failed to start. All of them now use the parser keys; the chart values are renamed to match (cpu.warningLoadFactor/criticalLoadFactor, memory.*Threshold/checkOOMKills, disk.mountPoints/checkReadonly/checkIOHealth) and the dead exporters.http bind/port/tls/endpoints and settings.enableRemediation values are removed. A new test loads every YAML under config/ plus the rendered chart ConfigMap, runs registry validation, constructs each monitor, and fails on any unknown-key warning. 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.
Nothing unmarshalled strictly, and the Helm ConfigMap, default config, every example, and two built-in monitor defaults used keys the parsers never read, so operators' CPU, memory, and disk thresholds were silently discarded. The logpattern example could not start, yet
-validate-configpassed because per-type validators never ran.Top-level config now fails on unknown keys with the key named. Monitor config maps stay free-form, but every monitor validator warns on keys it ignores, nested sections included.
ValidateWithRegistryruns the registered validator for each enabled monitor. Threshold parsers accept YAML integers.All shipped YAML and the chart use the real keys; chart values for cpu, memory, disk are renamed to match, and the dead
exporters.httpbind/port/tls/endpoints andsettings.enableRemediationvalues are removed. A new test loads every config underconfig/plus the rendered chart, validates it, constructs each monitor, and fails on any unknown-key warning.Chart values renamed:
monitors.cpu.loadAverageThresholds→warningLoadFactor/criticalLoadFactor;monitors.memory.memoryThresholds/swapThresholds/oomDetection→warningThreshold/criticalThreshold/swapWarningThreshold/swapCriticalThreshold/checkOOMKills;monitors.disk.paths→mountPoints.🤖 Generated with Claude Code