Repository navigation
fix(monitors): emit monitor_up and DNSLatencyHigh, enable kubernetes monitors by default - #52
Open
mattmattox wants to merge 12 commits into
Open
mattmattox wants to merge 12 commits into
mattmattox wants to merge 12 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>
…monitors by default monitor_up was registered but never set, so the IPv6 dashboard panels reading it were permanently 0. ExportStatus now sets it to 1 per exported status and to 0 when the status is the BaseMonitor check-failure status (error, panic or timeout). The unused monitor_type label is dropped so the label set matches the other per-monitor metrics. The DNS monitor now emits a DNSLatencyHigh condition from the p95 of the cycle's successful queries against the existing latencyThreshold key, so the NodeDoctorDNSLatencyHigh alert can fire. The four kubernetes-* monitors gain DefaultConfig so ApplyDefaultMonitors adds them, and the chart enables kubelet, apiserver, runtime, capacity, gateway and connectivity monitors by default so the kubernetes-health dashboard and the kubelet/apiserver alerts have data in a default install. Dashboards now filter on conditions the code actually emits (KubeletDown, KubeletUnhealthy, APIServerUnreachable, CPUThermalHealthy, DNSResolutionDown); a dashboards test enforces this for every dashboard and PrometheusRule. deployment/prometheusrule.yaml and deployment/servicemonitor.yaml are now rendered from the chart via make manifests-generate and checked in CI with make manifests-verify-generated. 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.
node_doctor_monitor_monitor_upwas registered but never set, so four IPv6 dashboard panels read 0 forever. It is now 1 per exported status and 0 on a check failure; the never-populatedmonitor_typelabel is dropped.The DNS monitor emits
DNSLatencyHighfrom the p95 of successful queries against the existinglatencyThreshold(default 1s), so theNodeDoctorDNSLatencyHighalert can fire.The four
kubernetes-*monitors gainDefaultConfig, and the chart enables kubelet, apiserver, runtime, capacity, gateway, and connectivity monitors by default, so the kubernetes-health dashboard and kubelet/apiserver alerts have data. Existing RBAC covers the default settings.Dashboards now filter on conditions the code emits; the "Swap Pressure" stat had no backing condition and shows memory warning events instead. A new test checks every
condition_typein dashboards and PrometheusRules against literals inpkg/, and ahelmtest renders the chart and runs it through the real validators.deployment/prometheusrule.yamlandservicemonitor.yamlare now generated from the chart (make manifests-generate) and drift-checked in CI.🤖 Generated with Claude Code