From 412f7e4ea9ed781c5cbc580336a35116a274ca75 Mon Sep 17 00:00:00 2001 From: phil Date: Wed, 12 Aug 2026 17:31:51 +0900 Subject: [PATCH] =?UTF-8?q?fix(nodecompat,chart):=20=EC=9E=90=EC=B2=B4=20?= =?UTF-8?q?=EC=88=98=EC=A7=91=EA=B8=B0=20=EC=A4=91=EB=B3=B5=20=EC=B0=A8?= =?UTF-8?q?=EB=8B=A8=20=EC=99=84=EC=84=B1=20+=20=EC=8A=A4=ED=81=AC?= =?UTF-8?q?=EB=A0=88=EC=9D=B4=ED=94=84=20=EB=B0=9C=EA=B2=AC=20opt-in=20?= =?UTF-8?q?=ED=99=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0.9.0 을 운영에 얹으면 조용히 메트릭을 잃는 결함 3건을 고친다. 셋 다 파드는 Ready, /metrics 는 200 이라 로그를 보지 않으면 드러나지 않는 종류다. ## 1. native collector 5그룹이 임베드 node_exporter 와 중복 등록 nativeCollectors 는 upstream collector 를 두 개(loadavg·uname)만 껐다. nodecompat 은 7그룹을 내므로 entropy·filefd·stat·vmstat·os 가 그대로 살아 같은 메트릭을 양쪽에서 등록했다. client_golang 은 충돌한 family 를 스크레이프에서 빼면서 200 을 계속 주므로(sink/metrics.go 가 ContinueOnError), 손실이 에러로 나타나지 않는다. Linux 실측으로 확인된 소실 대상: node_entropy_available_bits, node_filefd_allocated, node_procs_running, node_procs_blocked, node_os_version, node_vmstat_pgfault, node_vmstat_pgmajfault, node_vmstat_pswpin, node_vmstat_oom_kill. 차단 목록을 main.go 에 다시 적는 대신 nodecompat 이 자기 수집기 집합에서 파생시킨다(SupersededCollectors). subCollector 인터페이스에 Supersedes() 를 두어 새 수집기가 대체 대상을 밝히지 않으면 아예 추가되지 못하게 했다 — 두 목록이 어긋나는 것이 이 결함의 원인이었다. procs→"stat", osrelease→"os" 처럼 로컬 이름과 upstream 이름이 다른 두 건이 특히 빠지기 쉽다. 회귀 가드는 선언과 플래그의 정합성(단위)과 **실제 수집 결과의 교집합**(Linux 통합) 두 층이다. 후자는 결함을 되살려 9개 메트릭을 잡아내는 것으로 유효성을 확인했다. upstream 수집이 통째로 실패해도 교집합이 비어 통과하는 것을 막으려 "upstream family 20개 이상" 을 함께 단언한다. ## 2. nodevitals_build_info 가 소스에 박힌 버전을 신고 NewHeartbeat(cfg.Node, "0.8.5") 리터럴 탓에 0.9.0 이미지가 version="0.8.5" 를 냈다. 이 메트릭은 "이 노드가 어떤 빌드인가" 에 답하는 유일한 자기신고 수단이라, 그럴듯한 기본값은 확인 수단 자체를 없앤다. -ldflags -X main.version 주입으로 바꾸고 Chart.yaml 의 appVersion 을 단일 출처로 삼는다(Makefile·Dockerfile· release.yml 이 같은 값을 흘려보낸다). 미주입 시에는 "unknown" — 모르는 것을 모른다고 말하는 편이 아닐 수도 있는 릴리스를 자칭하는 것보다 낫다. ## 3. 스크레이프 발견 어노테이션이 업그레이드만으로 켜짐 0.9.0 이 파드 템플릿에 prometheus.io/scrape 를 무조건 렌더한다. 이미 Service / ServiceMonitor 로 수집하던 클러스터는 업그레이드만으로 role:pod 잡이 같은 파드를 한 벌 더 긁게 되어, 모든 시리즈가 job 라벨만 다른 2벌이 된다. 오류는 없고 카디널리티와 저장량만 두 배가 된다. serviceMonitor.enabled 와 같은 규칙을 적용해 scrapeAnnotations.enabled 로 분리하고 기본 off 로 둔다 — 발견 경로가 스스로 켜지는 것이 문제이지 기능 자체가 문제는 아니다. 어노테이션 정의를 configChecksums helper 에서 떼어냈다(이름과 내용이 어긋나 있었다). compatibility-check.sh 는 문서 전체에서 문자열만 grep 해 위치를 구분하지 못했고, 애초에 Makefile·CI 어디에도 연결돼 있지 않아 한 번도 실행된 적이 없었다. 파드 템플릿 안인지까지 검사하도록 고치고 chart-test 에 연결한다. Chart/appVersion 0.9.0 → 0.9.1. --- .github/workflows/release.yml | 6 ++ Dockerfile | 6 +- Makefile | 6 +- cmd/nodevitals/main.go | 37 +++++++-- cmd/nodevitals/main_test.go | 68 +++++++++++++++++ deploy/chart/Chart.yaml | 6 +- deploy/chart/templates/_helpers.tpl | 25 ++++++- deploy/chart/templates/daemonset-gpu.yaml | 1 + deploy/chart/templates/daemonset-single.yaml | 1 + deploy/chart/templates/daemonset-smart.yaml | 1 + deploy/chart/templates/daemonset.yaml | 1 + deploy/chart/tests/compatibility-check.sh | 21 +++++- deploy/chart/values.yaml | 11 +++ docs/COMPATIBILITY.md | 32 +++++--- internal/collector/heartbeat.go | 7 +- internal/collector/heartbeat_test.go | 24 ++++++ internal/nodecompat/collision_linux_test.go | 79 ++++++++++++++++++++ internal/nodecompat/entropy.go | 2 + internal/nodecompat/filefd.go | 2 + internal/nodecompat/loadavg.go | 2 + internal/nodecompat/nodecompat.go | 37 +++++++++ internal/nodecompat/osrelease.go | 4 + internal/nodecompat/procs.go | 4 + internal/nodecompat/superseded_test.go | 77 +++++++++++++++++++ internal/nodecompat/uname.go | 2 + internal/nodecompat/vmstat.go | 2 + 26 files changed, 435 insertions(+), 29 deletions(-) create mode 100644 cmd/nodevitals/main_test.go create mode 100644 internal/nodecompat/collision_linux_test.go create mode 100644 internal/nodecompat/superseded_test.go diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e3098b5..f873010 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -73,6 +73,10 @@ jobs: platforms: linux/amd64 load: true tags: ${{ env.IMG }}:scan + # 아래 push 스텝과 반드시 동일해야 한다 — build-arg 가 다르면 스캔한 + # 이미지와 발행하는 이미지가 다른 산출물이 된다. + build-args: | + VERSION=${{ steps.ver.outputs.app }} - name: Trivy scan (HIGH/CRITICAL block) if: steps.img.outputs.exists == 'false' @@ -93,6 +97,8 @@ jobs: provenance: true sbom: true tags: ${{ env.IMG }}:${{ steps.ver.outputs.app }} + build-args: | + VERSION=${{ steps.ver.outputs.app }} - uses: sigstore/cosign-installer@v3 if: steps.img.outputs.exists == 'false' diff --git a/Dockerfile b/Dockerfile index fb3b7a0..61fd25d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,8 +15,12 @@ COPY go.mod go.sum* ./ RUN go mod download COPY . . ARG TARGETARCH=amd64 +# VERSION becomes nodevitals_build_info{version=...}, the only way to ask a +# running node which build it is on. Left unset it stays "unknown" rather than +# naming a release the binary may not be. +ARG VERSION="" RUN CGO_ENABLED=1 GOOS=linux GOARCH=${TARGETARCH} \ - go build -trimpath -ldflags="-s -w" -tags gpu -o /out/nodevitals ./cmd/nodevitals + go build -trimpath -ldflags="-s -w -X main.version=${VERSION}" -tags gpu -o /out/nodevitals ./cmd/nodevitals FROM gcr.io/distroless/cc-debian12:nonroot # Links the ghcr package to this repository, so the image shows up under the diff --git a/Makefile b/Makefile index 6e1994b..c516a03 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,10 @@ fmt: build: # 이미지와 같은 CGO_ENABLED=1 — go-nvml 과 node_exporter 의 일부 collector 가 # cgo 를 요구한다. 0 으로 두면 로컬 게이트만 실패해 이미지와 어긋난다. - CGO_ENABLED=1 go build -trimpath -ldflags="-s -w" -tags gpu -o dist/nodevitals ./cmd/nodevitals + # + # -X main.version 은 Chart.yaml 의 appVersion 을 그대로 흘려보낸다. 릴리스 + # 파이프라인도 같은 값을 진실로 삼으므로, 버전이 사는 곳은 Chart.yaml 하나다. + CGO_ENABLED=1 go build -trimpath -ldflags="-s -w -X main.version=$(VERSION)" -tags gpu -o dist/nodevitals ./cmd/nodevitals docker: docker build --platform=linux/amd64 -t ghcr.io/keiailab/nodevitals:dev . @@ -35,6 +38,7 @@ chart-lint: chart-test: bash deploy/chart/tests/secret-isolation.sh bash deploy/chart/tests/tier-runtime.sh + bash deploy/chart/tests/compatibility-check.sh # Vuln-scan IMGREF, failing on HIGH/CRITICAL. Override IMGREF for the gpu image. scan: diff --git a/cmd/nodevitals/main.go b/cmd/nodevitals/main.go index 8310d44..9f1b862 100644 --- a/cmd/nodevitals/main.go +++ b/cmd/nodevitals/main.go @@ -27,6 +27,13 @@ import ( "github.com/KeiaiLab/nodevitals/internal/smartctlcompat" ) +// version 은 빌드 시 -ldflags "-X main.version=..." 로 주입된다. 소스에 릴리스 +// 번호를 적어 두면 bump 를 잊는 순간 이미지가 자기 버전을 틀리게 신고하고, +// 그것이 배포 검증의 유일한 자기신고 수단이라 확인할 방법 자체가 사라진다. +// 주입이 없으면 "unknown" 으로 남는다 — 모르는 것을 모른다고 말하는 편이, +// 아닐 수도 있는 릴리스를 자칭하는 것보다 낫다. +var version string + func main() { cfgPath := flag.String("config", "/etc/nodevitals/config.yaml", "config file path") flag.Parse() @@ -48,7 +55,7 @@ func main() { for _, tier := range tiers { switch tier { case "core": - reg.Add(collector.NewHeartbeat(cfg.Node, "0.8.5")) + reg.Add(collector.NewHeartbeat(cfg.Node, version)) reg.Add(collector.NewLoadAvg(cfg.Node, cfg.ProcRoot)) reg.Add(collector.NewCPU(cfg.Node, cfg.ProcRoot)) reg.Add(collector.NewMem(cfg.Node, cfg.ProcRoot)) @@ -118,7 +125,7 @@ func main() { // dashboards and alert rules built on node_* keep working untouched. neCount := 0 if cfg.NodeExporter.Enabled { - extraFlags := cfg.NodeExporter.ExtraFlags + extraFlags := nodeExporterFlags(cfg.NodeExporter) if cfg.NodeExporter.NativeCollectors { nc := nodecompat.New(cfg.ProcRoot, cfg.SysRoot, cfg.NodeExporter.RootFSPath, slog.Default()) if err := metrics.Register(nc); err != nil { @@ -126,10 +133,6 @@ func main() { os.Exit(1) } slog.Info("native nodecompat collectors registered") - extraFlags = append(extraFlags, - "--no-collector.loadavg", - "--no-collector.uname", - ) } c, err := nodeexporter.New(nodeexporter.Config{ ProcPath: cfg.ProcRoot, @@ -218,3 +221,25 @@ func main() { slog.Error("http shutdown", "err", err) } } + +// nodeExporterFlags 는 임베드 node_exporter 에 넘길 collector 플래그를 만든다. +// +// 자체 수집기가 켜지면 그것이 대체하는 upstream collector 를 **전부** 꺼야 한다. +// 하나라도 남으면 같은 메트릭 이름이 두 곳에서 등록되고, client_golang 은 충돌한 +// family 를 스크레이프 결과에서 빼면서도 200 을 계속 준다 — 파드는 Ready, /metrics +// 는 정상, 그 시리즈만 조용히 사라진다. +// +// 차단 목록은 nodecompat 이 자기 수집기 집합에서 파생시킨다. 여기에 이름을 다시 +// 적으면 nodecompat 에 수집기가 추가될 때마다 두 목록이 어긋난다 — 실제로 0.9.0 이 +// loadavg·uname 둘만 적어 나머지 다섯(entropy·filefd·stat·vmstat·os)이 중복됐다. +func nodeExporterFlags(cfg config.NodeExporterConfig) []string { + if !cfg.NativeCollectors { + return cfg.ExtraFlags + } + // cfg.ExtraFlags 에 그대로 append 하면 cap 여유가 있을 때 호출자의 배열에 + // 써 들어간다. config 는 한 번 읽어 계속 쓰이므로 복사해서 시작한다. + native := nodecompat.NoCollectorFlags() + flags := make([]string, 0, len(cfg.ExtraFlags)+len(native)) + flags = append(flags, cfg.ExtraFlags...) + return append(flags, native...) +} diff --git a/cmd/nodevitals/main_test.go b/cmd/nodevitals/main_test.go new file mode 100644 index 0000000..343ef87 --- /dev/null +++ b/cmd/nodevitals/main_test.go @@ -0,0 +1,68 @@ +package main + +import ( + "slices" + "testing" + + "github.com/KeiaiLab/nodevitals/internal/config" + "github.com/KeiaiLab/nodevitals/internal/nodecompat" +) + +// With the native collectors on, every upstream collector they replace has to +// be switched off. Leaving one enabled makes both register the same metric +// names, and client_golang drops the collided family from the scrape while +// still answering 200 — the loss never surfaces as an error. +func TestNodeExporterFlagsDisableEveryNativelyServedCollector(t *testing.T) { + flags := nodeExporterFlags(config.NodeExporterConfig{NativeCollectors: true}) + for _, name := range nodecompat.SupersededCollectors() { + want := "--no-collector." + name + if !slices.Contains(flags, want) { + t.Errorf("missing %q: upstream %q stays enabled alongside its native replacement (got %v)", + want, name, flags) + } + } +} + +// Without the native collectors the upstream ones are the only source, so +// disabling them would delete the metrics outright rather than deduplicate. +func TestNodeExporterFlagsLeaveUpstreamAloneWhenNativeIsOff(t *testing.T) { + flags := nodeExporterFlags(config.NodeExporterConfig{ + NativeCollectors: false, + ExtraFlags: []string{"--collector.systemd"}, + }) + for _, f := range flags { + if f != "--collector.systemd" { + t.Errorf("unexpected flag %q with nativeCollectors off; want only the operator's own flags", f) + } + } +} + +func TestNodeExporterFlagsKeepOperatorSuppliedFlags(t *testing.T) { + flags := nodeExporterFlags(config.NodeExporterConfig{ + NativeCollectors: true, + ExtraFlags: []string{"--collector.processes", "--collector.systemd"}, + }) + for _, want := range []string{"--collector.processes", "--collector.systemd"} { + if !slices.Contains(flags, want) { + t.Errorf("operator flag %q was dropped (got %v)", want, flags) + } + } +} + +// append onto a caller-owned slice can write through to its backing array when +// there is spare capacity. The config is read once and reused, so a mutation +// here would leak into anything else reading ExtraFlags. +func TestNodeExporterFlagsDoNotMutateConfig(t *testing.T) { + extra := make([]string, 1, 8) // 여유 cap — aliasing 이 드러나는 조건 + extra[0] = "--collector.systemd" + cfg := config.NodeExporterConfig{NativeCollectors: true, ExtraFlags: extra} + + nodeExporterFlags(cfg) + + if got := cfg.ExtraFlags; len(got) != 1 || got[0] != "--collector.systemd" { + t.Errorf("config.ExtraFlags was mutated: %v", got) + } + if got := extra[:cap(extra)]; got[1] != "" { + t.Errorf("wrote past the caller's slice into its backing array: %v", got) + } +} diff --git a/deploy/chart/Chart.yaml b/deploy/chart/Chart.yaml index 9393026..6c7e034 100644 --- a/deploy/chart/Chart.yaml +++ b/deploy/chart/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: nodevitals description: Unified hardware telemetry agent for Kubernetes nodes type: application -version: 0.9.0 -appVersion: "0.9.0" +version: 0.9.1 +appVersion: "0.9.1" kubeVersion: ">=1.26.0-0" home: https://github.com/keiailab/nodevitals @@ -47,4 +47,4 @@ annotations: url: https://raw.githubusercontent.com/KeiaiLab/nodevitals/main/docs/branding/symbol.png artifacthub.io/images: | - name: nodevitals - image: ghcr.io/keiailab/nodevitals:0.9.0 + image: ghcr.io/keiailab/nodevitals:0.9.1 diff --git a/deploy/chart/templates/_helpers.tpl b/deploy/chart/templates/_helpers.tpl index cad636d..c7d3d25 100644 --- a/deploy/chart/templates/_helpers.tpl +++ b/deploy/chart/templates/_helpers.tpl @@ -66,13 +66,32 @@ Call with (dict "ctx" . "tier" ""). {{- define "nodevitals.configChecksums" -}} {{- $ctx := .ctx -}} {{- $suffix := ternary "" (printf "-%s" .tier) (eq .tier "core") -}} -prometheus.io/scrape: "true" -prometheus.io/port: {{ $ctx.Values.metrics.port | default "9847" | quote }} -prometheus.io/path: "/metrics" checksum/config: {{ include (print $ctx.Template.BasePath "/configmap" $suffix ".yaml") $ctx | sha256sum }} checksum/webhook-secret: {{ include (print $ctx.Template.BasePath "/secret.yaml") $ctx | sha256sum }} {{- end -}} +{{/* +Prometheus pod-discovery annotations, for clusters whose scrape config picks +targets up by pod annotation (a `role: pod` job) rather than by Service or +ServiceMonitor. + +Off by default, like serviceMonitor.enabled, because a discovery mechanism that +turns itself on is the one that hurts: a cluster already scraping this chart +through a Service keeps doing so, and the pod job starts scraping the very same +pods as well. Every series then exists twice under two job labels — no error +anywhere, just doubled cardinality and storage. + +These belong in the *pod* template. An annotation on the DaemonSet object is +not propagated to its pods, so `role: pod` discovery would never see it. +*/}} +{{- define "nodevitals.scrapeAnnotations" -}} +{{- if .Values.scrapeAnnotations.enabled -}} +prometheus.io/scrape: "true" +prometheus.io/port: {{ .Values.metrics.port | default "9847" | quote }} +prometheus.io/path: "/metrics" +{{- end -}} +{{- end -}} + {{/* hostNetwork for a pod spec. /proc/net resolves against the *reading task's* network namespace, not the mounted path — so a pod-network container reading diff --git a/deploy/chart/templates/daemonset-gpu.yaml b/deploy/chart/templates/daemonset-gpu.yaml index 551e8b0..64706ed 100644 --- a/deploy/chart/templates/daemonset-gpu.yaml +++ b/deploy/chart/templates/daemonset-gpu.yaml @@ -21,6 +21,7 @@ spec: app.kubernetes.io/component: gpu annotations: {{- include "nodevitals.configChecksums" (dict "ctx" . "tier" "gpu") | nindent 8 }} + {{- include "nodevitals.scrapeAnnotations" . | nindent 8 }} spec: automountServiceAccountToken: false {{- with .Values.tiers.gpu.runtimeClassName }} diff --git a/deploy/chart/templates/daemonset-single.yaml b/deploy/chart/templates/daemonset-single.yaml index 290851d..8e6accc 100644 --- a/deploy/chart/templates/daemonset-single.yaml +++ b/deploy/chart/templates/daemonset-single.yaml @@ -23,6 +23,7 @@ spec: app.kubernetes.io/component: single annotations: {{- include "nodevitals.configChecksums" (dict "ctx" . "tier" "single") | nindent 8 }} + {{- include "nodevitals.scrapeAnnotations" . | nindent 8 }} spec: automountServiceAccountToken: false {{- include "nodevitals.hostNetwork" . | nindent 6 }} diff --git a/deploy/chart/templates/daemonset-smart.yaml b/deploy/chart/templates/daemonset-smart.yaml index 41290a6..cf19f09 100644 --- a/deploy/chart/templates/daemonset-smart.yaml +++ b/deploy/chart/templates/daemonset-smart.yaml @@ -21,6 +21,7 @@ spec: app.kubernetes.io/component: smart annotations: {{- include "nodevitals.configChecksums" (dict "ctx" . "tier" "smart") | nindent 8 }} + {{- include "nodevitals.scrapeAnnotations" . | nindent 8 }} spec: automountServiceAccountToken: false containers: diff --git a/deploy/chart/templates/daemonset.yaml b/deploy/chart/templates/daemonset.yaml index 792b18f..051fe3d 100644 --- a/deploy/chart/templates/daemonset.yaml +++ b/deploy/chart/templates/daemonset.yaml @@ -21,6 +21,7 @@ spec: app.kubernetes.io/component: core annotations: {{- include "nodevitals.configChecksums" (dict "ctx" . "tier" "core") | nindent 8 }} + {{- include "nodevitals.scrapeAnnotations" . | nindent 8 }} spec: automountServiceAccountToken: false {{- include "nodevitals.hostNetwork" . | nindent 6 }} diff --git a/deploy/chart/tests/compatibility-check.sh b/deploy/chart/tests/compatibility-check.sh index 7d55e82..62c3c15 100755 --- a/deploy/chart/tests/compatibility-check.sh +++ b/deploy/chart/tests/compatibility-check.sh @@ -9,10 +9,23 @@ CHART_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" echo "=== 1. Checking default template rendering ===" rendered="$(helm template nodevitals "$CHART_DIR")" -echo "=== 2. Checking vmagent auto-discovery annotations ===" -echo "$rendered" | grep -q 'prometheus.io/scrape: "true"' || { echo "FAIL: missing prometheus.io/scrape annotation"; exit 1; } -echo "$rendered" | grep -q 'prometheus.io/port: "9847"' || { echo "FAIL: missing prometheus.io/port annotation"; exit 1; } -echo "PASS: vmagent annotations present" +echo "=== 2. Checking vmagent auto-discovery annotations are opt-in ===" +# 기본 렌더에는 없어야 한다. 이 차트를 이미 Service/ServiceMonitor 로 수집하던 +# 클러스터에서는, 업그레이드만으로 role:pod 잡이 같은 파드를 한 벌 더 긁기 +# 시작해 모든 시리즈가 2벌이 된다 — 에러 없이, 청구서와 카디널리티로만 드러난다. +if echo "$rendered" | grep -q 'prometheus.io/scrape'; then + echo "FAIL: scrape annotations render by default; a chart user already scraping via Service would silently double-collect after an upgrade" + exit 1 +fi +echo "PASS: no scrape annotations unless asked for" + +# 켠 경우에는 **파드 템플릿 안**이어야 한다. DaemonSet 객체에 붙은 annotation 은 +# 파드로 전파되지 않으므로 role:pod 발견은 그것을 영영 보지 못한다. +disc_rendered="$(helm template nodevitals "$CHART_DIR" --set scrapeAnnotations.enabled=true)" +pod_meta="$(echo "$disc_rendered" | awk '/^ template:/,/^ spec:/')" +echo "$pod_meta" | grep -q 'prometheus.io/scrape: "true"' || { echo "FAIL: scrape annotation is not inside the pod template; role:pod discovery cannot see it"; exit 1; } +echo "$pod_meta" | grep -q 'prometheus.io/port: "9847"' || { echo "FAIL: port annotation is not inside the pod template"; exit 1; } +echo "PASS: scrape annotations land in the pod template when enabled" echo "=== 3. Checking gpu-operator & dcgmCompat rendering ===" gpu_rendered="$(helm template nodevitals "$CHART_DIR" --set tiers.gpu.enabled=true --set tiers.gpu.runtimeClassName=nvidia --set dcgmCompat.enabled=true)" diff --git a/deploy/chart/values.yaml b/deploy/chart/values.yaml index 1539e3e..fe4a378 100644 --- a/deploy/chart/values.yaml +++ b/deploy/chart/values.yaml @@ -222,6 +222,17 @@ webhooks: [] metrics: port: 9847 +# Prometheus 파드 어노테이션 발견(prometheus.io/scrape 등). ServiceMonitor CRD 가 +# 없고 스크레이프 설정이 `role: pod` 잡으로 대상을 잡는 클러스터(vmagent 의 +# kubernetes-pods 등)에서 켠다. 어노테이션은 파드 템플릿에 렌더된다 — DaemonSet +# 객체에 붙이면 파드로 전파되지 않아 role:pod 발견이 보지 못한다. +# +# serviceMonitor 와 마찬가지로 기본 off 다. 발견 경로가 스스로 켜지면, 이미 Service +# 로 수집하던 클러스터가 업그레이드만으로 같은 파드를 두 잡에서 긁게 되고, 모든 +# 시리즈가 job 라벨만 다른 2벌이 된다 — 에러는 어디에도 나지 않는다. +scrapeAnnotations: + enabled: false + # Prometheus Operator discovery. Off by default (requires the ServiceMonitor # CRD). When enabled, renders a headless Service + ServiceMonitor so the # /metrics endpoint of every nodevitals pod is scraped out of the box. diff --git a/docs/COMPATIBILITY.md b/docs/COMPATIBILITY.md index f1f59b3..181565c 100644 --- a/docs/COMPATIBILITY.md +++ b/docs/COMPATIBILITY.md @@ -1,7 +1,7 @@ # nodevitals — 서비스 전수 호환성 및 연동 명세서 (Compatibility Matrix) > 저장소: [`github.com/KeiaiLab/nodevitals`](https://github.com/KeiaiLab/nodevitals) -> 기준 버전: `v0.8.5` (Chart v0.8.6) +> 기준 버전: `v0.9.1` (Chart v0.9.1) > 최종 검증 일시: 2026년 8월 12일 본 문서는 `nodevitals`가 연동되는 주요 인프라 서비스, 관측 플랫폼, GPU 오퍼레이터, 가상머신(VM) 환경 간의 명시적 호환성 계약(Compatibility Contract)과 실측 검증 결과를 제공합니다. @@ -13,7 +13,7 @@ | 연동 대상 서비스 / 솔루션 | 호환성 상태 | 연동 메커니즘 & 수집 방식 | 비고 / 주요 구성 | |---|---|---|---| | **NVIDIA GPU-Operator** | **100% 호환 (DCGM 대체)** | `dcgmCompat.enabled: true` | `dcgmExporter.enabled: false` 설정 후 `DCGM_FI_*` 18개 메트릭 승계 | -| **VictoriaMetrics (vmagent)** | **100% 호환 (자동 탐지)** | Pod Annotation (`prometheus.io/scrape: "true"`) | `vmagent` kubernetes-pods 잡 자동 수집 (`port: 9847`) | +| **VictoriaMetrics (vmagent)** | **호환 (opt-in 자동 탐지)** | Pod Annotation — `scrapeAnnotations.enabled: true` 필요 (기본 `false`) | `vmagent` kubernetes-pods 잡 자동 수집 (`port: 9847`). **Service/ServiceMonitor 로 이미 수집 중이면 켜지 말 것 — 이중 수집** (§2.2) | | **VictoriaMetrics (vmsingle/cluster)** | **100% 호환** | TSDB Scrape & Remote Write | Prometheus TSDB 1.0 표준 데이터 100% 수용 | | **Prometheus Operator / Alertmanager** | **100% 호환** | `/metrics` + Service/PodMonitor | `node_*`, `DCGM_FI_*`, `smartctl_*` 기존 알림 룰 그대로 동작 | | **Grafana Dashboard Stack** | **100% 호환** | PromQL 드롭인 쿼리 | 기존 `node_exporter`, `dcgm`, `smartctl` 전용 대시보드 변경 0 | @@ -48,16 +48,28 @@ ### 2.2 VictoriaMetrics (`vmagent`) 연동 `keiailab-platform`과 같이 Prometheus Operator CRD 대신 `vmagent` 정적 수집 스택을 사용하는 환경의 호환성입니다. -- **자동 발견 어노테이션 (Auto-Discovery Pod Annotations)**: - `nodevitals` 파드 템플릿에 아래 어노테이션이 기본 렌더링됩니다: +- **자동 발견 어노테이션 (Auto-Discovery Pod Annotations)** — **opt-in 입니다**: ```yaml - metadata: - annotations: - prometheus.io/scrape: "true" - prometheus.io/port: "9847" - prometheus.io/path: "/metrics" + scrapeAnnotations: + enabled: true # 기본값 false ``` -- **`vmagent` 수집 동기화**: `vmagent`의 `kubernetes-pods` 메트릭 수집 작업이 해당 어노테이션을 감지하여 별도의 CRD 등록 없이 즉시 `/metrics` 수집을 시작합니다. + 켜면 파드 템플릿(`spec.template.metadata.annotations`)에 아래가 렌더됩니다: + ```yaml + prometheus.io/scrape: "true" + prometheus.io/port: "9847" + prometheus.io/path: "/metrics" + ``` + DaemonSet 객체가 아니라 **파드 템플릿**이어야 합니다 — 객체의 어노테이션은 파드로 전파되지 않아 + `role: pod` 발견이 영영 보지 못합니다. + +- **`vmagent` 수집 동기화**: 켜면 `vmagent`의 `kubernetes-pods`(`role: pod`) 작업이 어노테이션을 감지해 + CRD 등록 없이 즉시 `/metrics` 를 수집합니다. + +> [!WARNING] +> **이미 Service / ServiceMonitor 로 수집 중이라면 켜지 마십시오.** `kubernetes-service-endpoints` +> 계열 작업이 같은 파드를 이미 긁고 있는 상태에서 이것을 켜면, 동일한 시리즈가 `job` 라벨만 다른 +> **2벌**로 저장됩니다. 오류는 어디에도 나지 않고 카디널리티와 저장량만 두 배가 되므로, +> 수집 경로는 **하나만** 켜 두십시오. 기본값이 `false` 인 이유가 이것입니다. ### 2.3 Standalone Linux VM / 베어메탈 호스트 연동 Kubernetes 클러스터 외부의 독립 Linux 가상머신(VM) 또는 베어메탈 전용 장비에서의 기동 가이드입니다. diff --git a/internal/collector/heartbeat.go b/internal/collector/heartbeat.go index bc84325..cda911e 100644 --- a/internal/collector/heartbeat.go +++ b/internal/collector/heartbeat.go @@ -14,9 +14,14 @@ type heartbeatCollector struct { } // NewHeartbeat returns a collector that emits nodevitals_up and nodevitals_build_info. +// +// An un-injected version becomes "unknown", never a release number. This metric +// is the only thing that can answer "which build is actually running on this +// node", so a plausible-looking default would take that answer away: the 0.9.0 +// image reported version="0.8.5" for exactly this reason. func NewHeartbeat(node, version string) Collector { if version == "" { - version = "0.8.5" + version = "unknown" } return &heartbeatCollector{node: node, version: version} } diff --git a/internal/collector/heartbeat_test.go b/internal/collector/heartbeat_test.go index 0500674..8c86efb 100644 --- a/internal/collector/heartbeat_test.go +++ b/internal/collector/heartbeat_test.go @@ -5,6 +5,30 @@ import ( "testing" ) +// nodevitals_build_info 는 배포 검증의 유일한 자기신고 수단이다. 버전이 주입되지 +// 않았을 때 그럴듯한 릴리스 번호를 채우면, 실제로 도는 이미지가 무엇인지 물어볼 +// 곳이 없어진다 — 0.9.0 이미지가 version="0.8.5" 를 내던 것이 정확히 그 상태였다. +func TestHeartbeatReportsUnknownRatherThanInventingAVersion(t *testing.T) { + c := NewHeartbeat("node-1", "") + + samples, err := c.Collect(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + for _, s := range samples { + if s.Metric != "nodevitals_build_info" { + continue + } + if got := s.Labels["version"]; got != "unknown" { + t.Errorf("version=%q with nothing injected; a build that does not know its "+ + "own version must say so rather than name a release it may not be", got) + } + return + } + t.Fatal("nodevitals_build_info not emitted") +} + func TestHeartbeatCollector(t *testing.T) { c := NewHeartbeat("node-1", "0.8.5") if c.Name() != "heartbeat" { diff --git a/internal/nodecompat/collision_linux_test.go b/internal/nodecompat/collision_linux_test.go new file mode 100644 index 0000000..0621182 --- /dev/null +++ b/internal/nodecompat/collision_linux_test.go @@ -0,0 +1,79 @@ +//go:build linux + +// 이 검사는 Linux 에서만 의미가 있다. entropy·filefd·stat·vmstat 은 upstream 에 +// Linux 전용 구현뿐이라, darwin 에서 돌리면 upstream 쪽 집합이 애초에 비어 +// 교집합도 비고 "충돌 없음"으로 통과해버린다 — 실제로는 아무것도 검사하지 않은 채. +package nodecompat_test + +import ( + "io" + "log/slog" + "testing" + + "github.com/prometheus/client_golang/prometheus" + + "github.com/KeiaiLab/nodevitals/internal/nodecompat" + "github.com/KeiaiLab/nodevitals/internal/nodeexporter" +) + +// 자체 수집기와 임베드 node_exporter 가 같은 메트릭 이름을 내면 client_golang 이 +// 충돌한 family 를 스크레이프에서 빼면서도 200 을 준다. 앞선 단위 테스트는 +// "선언과 플래그가 서로 일관적"인지만 보므로, 선언 자체가 틀렸을 때 — +// 예를 들어 procs 가 upstream "stat" 이 아니라 존재하지도 않는 "procs" 를 +// 대체한다고 선언했을 때 — 는 잡지 못한다. 실제로 양쪽을 수집해 대조한다. +func TestNativeCollectorsDoNotCollideWithEmbeddedNodeExporter(t *testing.T) { + quiet := slog.New(slog.NewTextHandler(io.Discard, nil)) + + native := names(t, nodecompat.New("/proc", "/sys", "/", quiet)) + + // node_exporter 의 collector 들은 init() 에서 전역 kingpin 에 플래그를 등록하고 + // nodeexporter.New 가 그것을 딱 한 번 파싱한다. 한 프로세스에서 구성을 바꿔 + // 두 번 만들 수 없으므로 이 테스트가 유일한 호출자여야 한다. + ne, err := nodeexporter.New(nodeexporter.Config{ + ProcPath: "/proc", + SysPath: "/sys", + ExtraFlags: nodecompat.NoCollectorFlags(), + }, quiet) + if err != nil { + t.Fatalf("build embedded node_exporter: %v", err) + } + upstream := names(t, ne) + + // 이 단언이 없으면, upstream 수집이 통째로 실패했을 때도 교집합이 비어 + // 통과한다 — 검사한 게 없는데 초록불이 켜지는 바로 그 상태. + if len(upstream) < 20 { + t.Fatalf("embedded node_exporter yielded only %d metric families; "+ + "too few to prove anything about collisions", len(upstream)) + } + if len(native) == 0 { + t.Fatal("native collectors yielded no metric families") + } + + for name := range native { + if upstream[name] { + t.Errorf("%q is emitted by both the native collector and the embedded "+ + "node_exporter; the collided family gets dropped from every scrape "+ + "while /metrics still answers 200", name) + } + } +} + +// names 는 collector 를 실제로 수집해 방출된 메트릭 이름을 모은다. 수집 오류는 +// 무시한다 — 컨테이너에 없는 하드웨어를 읽는 collector 는 정상적으로 실패하고, +// 여기서 필요한 건 "어떤 이름을 쓰는가"뿐이다. +func names(t *testing.T, c prometheus.Collector) map[string]bool { + t.Helper() + reg := prometheus.NewRegistry() + if err := reg.Register(c); err != nil { + t.Fatalf("register collector: %v", err) + } + families, err := reg.Gather() + if err != nil { + t.Logf("gather reported errors (expected for absent hardware): %v", err) + } + out := make(map[string]bool, len(families)) + for _, f := range families { + out[f.GetName()] = true + } + return out +} diff --git a/internal/nodecompat/entropy.go b/internal/nodecompat/entropy.go index ec0601f..1173a33 100644 --- a/internal/nodecompat/entropy.go +++ b/internal/nodecompat/entropy.go @@ -32,6 +32,8 @@ func newEntropy(procRoot string) subCollector { func (c *entropyCollector) Name() string { return "entropy" } +func (c *entropyCollector) Supersedes() string { return "entropy" } + func (c *entropyCollector) Collect(ch chan<- prometheus.Metric) error { availPath := filepath.Join(c.procRoot, "sys/kernel/random/entropy_avail") if data, err := os.ReadFile(availPath); err == nil { diff --git a/internal/nodecompat/filefd.go b/internal/nodecompat/filefd.go index c189364..6957520 100644 --- a/internal/nodecompat/filefd.go +++ b/internal/nodecompat/filefd.go @@ -33,6 +33,8 @@ func newFileFD(procRoot string) subCollector { func (c *fileFDCollector) Name() string { return "filefd" } +func (c *fileFDCollector) Supersedes() string { return "filefd" } + func (c *fileFDCollector) Collect(ch chan<- prometheus.Metric) error { path := filepath.Join(c.procRoot, "sys/fs/file-nr") data, err := os.ReadFile(path) diff --git a/internal/nodecompat/loadavg.go b/internal/nodecompat/loadavg.go index 987f1f1..eb8482e 100644 --- a/internal/nodecompat/loadavg.go +++ b/internal/nodecompat/loadavg.go @@ -38,6 +38,8 @@ func newLoadAvg(procRoot string) subCollector { func (c *loadAvgCollector) Name() string { return "loadavg" } +func (c *loadAvgCollector) Supersedes() string { return "loadavg" } + func (c *loadAvgCollector) Collect(ch chan<- prometheus.Metric) error { path := filepath.Join(c.procRoot, "loadavg") data, err := os.ReadFile(path) diff --git a/internal/nodecompat/nodecompat.go b/internal/nodecompat/nodecompat.go index 508eeff..066ae57 100644 --- a/internal/nodecompat/nodecompat.go +++ b/internal/nodecompat/nodecompat.go @@ -12,6 +12,12 @@ import ( // subCollector is the internal interface for individual metric group collectors. type subCollector interface { Name() string + // Supersedes is the upstream node_exporter collector name whose metrics + // this one takes over. It is a compile-time obligation on purpose: a new + // sub-collector that does not answer it cannot be added to the set, and an + // upstream collector left enabled alongside its native replacement makes + // both register the same metric names. + Supersedes() string Collect(ch chan<- prometheus.Metric) error } @@ -44,6 +50,37 @@ func New(procRoot, sysRoot, rootFS string, log *slog.Logger) *Exporter { } } +// SupersededCollectors returns the upstream node_exporter collector names that +// this package's native collectors replace. It is derived from the set itself, +// not from a second hand-kept list: the two drifting apart is precisely how a +// native collector ends up running alongside the upstream one it replaced. +func SupersededCollectors() []string { + e := New("", "", "", nil) + seen := make(map[string]bool, len(e.subs)) + names := make([]string, 0, len(e.subs)) + for _, sub := range e.subs { + n := sub.Supersedes() + if n == "" || seen[n] { + continue + } + seen[n] = true + names = append(names, n) + } + return names +} + +// NoCollectorFlags returns the node_exporter flags that disable every upstream +// collector superseded by this package. Pass them to nodeexporter.Config's +// ExtraFlags whenever the native collectors are enabled. +func NoCollectorFlags() []string { + superseded := SupersededCollectors() + flags := make([]string, 0, len(superseded)) + for _, n := range superseded { + flags = append(flags, "--no-collector."+n) + } + return flags +} + // Describe satisfies prometheus.Collector. func (e *Exporter) Describe(ch chan<- *prometheus.Desc) { // Unchecked collector: Describe emits nothing, allowing dynamically created metrics. diff --git a/internal/nodecompat/osrelease.go b/internal/nodecompat/osrelease.go index 73a1ef8..0b87139 100644 --- a/internal/nodecompat/osrelease.go +++ b/internal/nodecompat/osrelease.go @@ -35,6 +35,10 @@ func newOSRelease(rootFS string) subCollector { func (c *osReleaseCollector) Name() string { return "osrelease" } +// node_os_info / node_os_version come from upstream's "os" collector +// (os_release.go), which is not named after the file it reads. +func (c *osReleaseCollector) Supersedes() string { return "os" } + func (c *osReleaseCollector) Collect(ch chan<- prometheus.Metric) error { m, err := parseOSRelease(c.rootFS) if err != nil { diff --git a/internal/nodecompat/procs.go b/internal/nodecompat/procs.go index 6a71b5e..3a54274 100644 --- a/internal/nodecompat/procs.go +++ b/internal/nodecompat/procs.go @@ -34,6 +34,10 @@ func newProcs(procRoot string) subCollector { func (c *procsCollector) Name() string { return "procs" } +// node_procs_running / node_procs_blocked belong to upstream's "stat" +// collector, not to a "procs" one — that collector does not exist. +func (c *procsCollector) Supersedes() string { return "stat" } + func (c *procsCollector) Collect(ch chan<- prometheus.Metric) error { path := filepath.Join(c.procRoot, "stat") file, err := os.Open(path) diff --git a/internal/nodecompat/superseded_test.go b/internal/nodecompat/superseded_test.go new file mode 100644 index 0000000..2496d4a --- /dev/null +++ b/internal/nodecompat/superseded_test.go @@ -0,0 +1,77 @@ +package nodecompat + +import ( + "strings" + "testing" +) + +// Every native sub-collector takes over a metric group that an upstream +// node_exporter collector also owns. If it does not say which one, main.go +// cannot disable that upstream collector, and both register the same metric +// names — client_golang then drops the collided family from the scrape while +// still returning 200, so the loss is silent. +func TestEverySubCollectorDeclaresSupersededUpstreamCollector(t *testing.T) { + e := New("/proc", "/sys", "/", nil) + if len(e.subs) == 0 { + t.Fatal("no sub-collectors registered; this guard would pass vacuously") + } + for _, sub := range e.subs { + if sub.Supersedes() == "" { + t.Errorf("sub-collector %q declares no superseded upstream collector: "+ + "the embedded node_exporter will keep emitting the same metrics and duplicate it", + sub.Name()) + } + } +} + +// The flags are what actually reach node_exporter's kingpin parser, which +// rejects anything outside the --collector.* / --no-collector.* namespace. +func TestNoCollectorFlagsCoverEverySupersededCollector(t *testing.T) { + flags := NoCollectorFlags() + superseded := SupersededCollectors() + // Without this both lists can be empty and every assertion below passes + // vacuously — the exact shape of the bug this guards against. + if len(superseded) == 0 { + t.Fatal("SupersededCollectors() is empty; no upstream collector would be disabled") + } + if len(flags) != len(superseded) { + t.Fatalf("got %d flags for %d superseded collectors: %v", len(flags), len(superseded), flags) + } + for _, name := range superseded { + want := "--no-collector." + name + if !contains(flags, want) { + t.Errorf("missing %q; upstream %q stays enabled and duplicates the native collector", want, name) + } + } + for _, f := range flags { + if !strings.HasPrefix(f, "--no-collector.") { + t.Errorf("flag %q is outside the --no-collector.* namespace and node_exporter will refuse to start", f) + } + } +} + +// procs supersedes upstream "stat" and osrelease supersedes upstream "os" — +// neither name matches the sub-collector's own Name(). Deriving the flags from +// Name() would leave both upstream collectors enabled. +func TestSupersededNamesAreUpstreamNamesNotLocalNames(t *testing.T) { + e := New("/proc", "/sys", "/", nil) + want := map[string]string{ + "procs": "stat", + "osrelease": "os", + } + for _, sub := range e.subs { + if w, ok := want[sub.Name()]; ok && sub.Supersedes() != w { + t.Errorf("sub-collector %q supersedes %q, want upstream name %q", + sub.Name(), sub.Supersedes(), w) + } + } +} + +func contains(haystack []string, needle string) bool { + for _, s := range haystack { + if s == needle { + return true + } + } + return false +} diff --git a/internal/nodecompat/uname.go b/internal/nodecompat/uname.go index 03f3d9a..0ddf898 100644 --- a/internal/nodecompat/uname.go +++ b/internal/nodecompat/uname.go @@ -20,6 +20,8 @@ func newUname() subCollector { func (c *unameCollector) Name() string { return "uname" } +func (c *unameCollector) Supersedes() string { return "uname" } + func (c *unameCollector) Collect(ch chan<- prometheus.Metric) error { var uts unix.Utsname if err := unix.Uname(&uts); err != nil { diff --git a/internal/nodecompat/vmstat.go b/internal/nodecompat/vmstat.go index 5df9025..89e6f86 100644 --- a/internal/nodecompat/vmstat.go +++ b/internal/nodecompat/vmstat.go @@ -31,6 +31,8 @@ func newVMStat(procRoot string) subCollector { func (c *vmstatCollector) Name() string { return "vmstat" } +func (c *vmstatCollector) Supersedes() string { return "vmstat" } + func (c *vmstatCollector) Collect(ch chan<- prometheus.Metric) error { path := filepath.Join(c.procRoot, "vmstat") file, err := os.Open(path)