From a7d4f0c1f9066c197abbb1e192e552533322f18c Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 10 Nov 2025 13:00:44 +0100 Subject: [PATCH 1/8] Validate generated reference docs in CI The `doc` target now generates reference documentation files via `reference-help` and `check-doc` validates all generated files are committed. This replaces scattered unit tests that only validated 3 out of 4 files and had reliability issues. Previously, `mimir-flags-defaults.json` wasn't validated, leading to incorrect values being committed (e.g., 9223372036854776000 instead of 9223372036854775807 for max int64). --- .github/workflows/flaky-tests.yml | 2 +- Makefile | 8 +++- cmd/mimir/main_test.go | 58 ----------------------------- tools/config-inspector/main_test.go | 22 ----------- 4 files changed, 8 insertions(+), 82 deletions(-) diff --git a/.github/workflows/flaky-tests.yml b/.github/workflows/flaky-tests.yml index e305c795596..067130054bb 100644 --- a/.github/workflows/flaky-tests.yml +++ b/.github/workflows/flaky-tests.yml @@ -54,6 +54,6 @@ jobs: time-range: "7d" top-k: "3" skip-posting-issues: ${{ github.event_name == 'workflow_dispatch' && inputs.skip-posting-issues || 'false' }} - ignored-tests: "TestOurUpstreamTestCasesAreInSyncWithUpstream,TestConfigDescriptorIsUpToDate" # TestOurUpstreamTestCasesAreInSyncWithUpstream is supposed to block upstream mimir-prometheus updates, so it's also expected to fail. TestConfigDescriptorIsUpToDate is flaky. + ignored-tests: "TestOurUpstreamTestCasesAreInSyncWithUpstream" # TestOurUpstreamTestCasesAreInSyncWithUpstream is supposed to block upstream mimir-prometheus updates, so it's also expected to fail. env: GITHUB_TOKEN: ${{ steps.github-token.outputs.token }} diff --git a/Makefile b/Makefile index 799548d4ee1..3123b5e5a96 100644 --- a/Makefile +++ b/Makefile @@ -508,7 +508,7 @@ check-protobuf-format: embedmd -w $< doc: ## Generates the config file documentation. -doc: clean-doc $(DOC_TEMPLATES:.template=.md) $(DOC_EMBED:.md=.md.embedmd) +doc: reference-help clean-doc $(DOC_TEMPLATES:.template=.md) $(DOC_EMBED:.md=.md.embedmd) # Make up markdown files prettier. When running with check-doc target, it will fail if this produces any change. prettier --write "**/*.md" # Make operations/helm/charts/*/README.md @@ -644,6 +644,12 @@ clean-doc: ## Clean the documentation files generated from templates. check-doc: ## Check the documentation files are up to date. check-doc: doc + @git diff --exit-code -- \ + cmd/mimir/help.txt.tmpl \ + cmd/mimir/help-all.txt.tmpl \ + cmd/mimir/config-descriptor.json \ + operations/mimir/mimir-flags-defaults.json \ + || (echo "Please update generated reference documentation by running 'make doc' and committing the changes" && false) @find . -name "*.md" | xargs git diff --exit-code -- \ || (echo "Please update generated documentation by running 'make doc' and committing the changes" && false) diff --git a/cmd/mimir/main_test.go b/cmd/mimir/main_test.go index 94f6f77cd1c..3881cc42990 100644 --- a/cmd/mimir/main_test.go +++ b/cmd/mimir/main_test.go @@ -248,64 +248,6 @@ ruler_storage: } } -func TestHelp(t *testing.T) { - for _, tc := range []struct { - name string - arg string - filename string - }{ - { - name: "basic", - arg: "-h", - filename: "help.txt.tmpl", - }, - { - name: "all", - arg: "-help-all", - filename: "help-all.txt.tmpl", - }, - } { - t.Run(tc.name, func(t *testing.T) { - oldArgs, oldStdout, oldStderr, oldTestMode, oldCmdLine := os.Args, os.Stdout, os.Stderr, testMode, flag.CommandLine - restored := false - restoreIfNeeded := func() { - if restored { - return - } - - os.Stdout = oldStdout - os.Stderr = oldStderr - os.Args = oldArgs - testMode = oldTestMode - flag.CommandLine = oldCmdLine - restored = true - } - t.Cleanup(restoreIfNeeded) - - testMode = true - co := test.CaptureOutput(t) - - const cmd = "./cmd/mimir/mimir" - os.Args = []string{cmd, tc.arg} - - // reset default flags - flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError) - - main() - - stdout, stderr := co.Done() - - // Restore stdout and stderr before reporting errors to make them visible. - restoreIfNeeded() - - expected, err := os.ReadFile(tc.filename) - require.NoError(t, err) - assert.Equalf(t, string(expected), string(stdout), "%s %s output changed; try `make reference-help`", cmd, tc.arg) - assert.Empty(t, stderr) - }) - } -} - func testSingle(t *testing.T, arguments []string, configYAML string, stdoutMessage, stderrMessage, stdoutExcluded, stderrExcluded string, assertConfig func(*testing.T, *mimir.Config)) { t.Helper() oldArgs, oldStdout, oldStderr, oldTestMode := os.Args, os.Stdout, os.Stderr, testMode diff --git a/tools/config-inspector/main_test.go b/tools/config-inspector/main_test.go index a3049f50af7..d1a40a4d92d 100644 --- a/tools/config-inspector/main_test.go +++ b/tools/config-inspector/main_test.go @@ -1,25 +1,3 @@ // SPDX-License-Identifier: AGPL-3.0-only package main - -import ( - "os" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/grafana/mimir/pkg/mimir" - "github.com/grafana/mimir/pkg/mimirtool/config" -) - -func TestConfigDescriptorIsUpToDate(t *testing.T) { - const descriptorLocation = "../../cmd/mimir/config-descriptor.json" - upToDateBytes, err := config.Describe(&mimir.Config{}) - require.NoError(t, err) - - committedBytes, err := os.ReadFile(descriptorLocation) - require.NoError(t, err) - - assert.JSONEq(t, string(upToDateBytes), string(committedBytes), "config descriptor is not up to date; run `make reference-help`") -} From a5c22e2fc78772177dbeed7b416afa3e03924aff Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 10 Nov 2025 13:35:50 +0100 Subject: [PATCH 2/8] Add reference-help to container targets The doc target depends on reference-help, so reference-help must also run in the container when BUILD_IN_CONTAINER=true. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3123b5e5a96..ea7cf556870 100644 --- a/Makefile +++ b/Makefile @@ -254,7 +254,7 @@ GOVOLUMES= -v mimir-go-cache:/go/cache \ # Mount local ssh credentials to be able to clone private repos when doing `mod-check` SSHVOLUME= -v ~/.ssh/:/root/.ssh:$(CONTAINER_MOUNT_OPTIONS) -exes $(EXES) $(EXES_RACE) protos $(PROTO_GOS) lint lint-gh-action lint-packaging-scripts test test-with-race cover shell mod-check check-protos doc format dist build-mixin format-mixin check-mixin-tests license check-license conftest-fmt check-conftest-fmt helm-conftest-test helm-conftest-quick-test conftest-verify check-helm-tests build-helm-tests print-go-version format-promql-tests check-promql-tests format-protobuf check-protobuf-format: fetch-build-image +exes $(EXES) $(EXES_RACE) protos $(PROTO_GOS) lint lint-gh-action lint-packaging-scripts test test-with-race cover shell mod-check check-protos reference-help doc format dist build-mixin format-mixin check-mixin-tests license check-license conftest-fmt check-conftest-fmt helm-conftest-test helm-conftest-quick-test conftest-verify check-helm-tests build-helm-tests print-go-version format-promql-tests check-promql-tests format-protobuf check-protobuf-format: fetch-build-image @echo ">>>> Entering build container: $@" $(SUDO) time docker run --rm $(TTY) -i $(SSHVOLUME) $(GOVOLUMES) $(BUILD_IMAGE) GOOS=$(GOOS) GOARCH=$(GOARCH) BINARY_SUFFIX=$(BINARY_SUFFIX) $@; From 51643cc6cbdf7d1d3c415869c0a5aaf0fbd17637 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 10 Nov 2025 13:38:25 +0100 Subject: [PATCH 3/8] Remove reference-help from container targets reference-help should not run in the container target list because it would try to resolve its binary dependencies on the host. Instead, when doc runs in the container, it will call reference-help which will then build Linux binaries inside the container. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ea7cf556870..3123b5e5a96 100644 --- a/Makefile +++ b/Makefile @@ -254,7 +254,7 @@ GOVOLUMES= -v mimir-go-cache:/go/cache \ # Mount local ssh credentials to be able to clone private repos when doing `mod-check` SSHVOLUME= -v ~/.ssh/:/root/.ssh:$(CONTAINER_MOUNT_OPTIONS) -exes $(EXES) $(EXES_RACE) protos $(PROTO_GOS) lint lint-gh-action lint-packaging-scripts test test-with-race cover shell mod-check check-protos reference-help doc format dist build-mixin format-mixin check-mixin-tests license check-license conftest-fmt check-conftest-fmt helm-conftest-test helm-conftest-quick-test conftest-verify check-helm-tests build-helm-tests print-go-version format-promql-tests check-promql-tests format-protobuf check-protobuf-format: fetch-build-image +exes $(EXES) $(EXES_RACE) protos $(PROTO_GOS) lint lint-gh-action lint-packaging-scripts test test-with-race cover shell mod-check check-protos doc format dist build-mixin format-mixin check-mixin-tests license check-license conftest-fmt check-conftest-fmt helm-conftest-test helm-conftest-quick-test conftest-verify check-helm-tests build-helm-tests print-go-version format-promql-tests check-promql-tests format-protobuf check-protobuf-format: fetch-build-image @echo ">>>> Entering build container: $@" $(SUDO) time docker run --rm $(TTY) -i $(SSHVOLUME) $(GOVOLUMES) $(BUILD_IMAGE) GOOS=$(GOOS) GOARCH=$(GOARCH) BINARY_SUFFIX=$(BINARY_SUFFIX) $@; From 715ac2969525b92dc3126f42dcaf985f3aaf5c1a Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 10 Nov 2025 13:41:45 +0100 Subject: [PATCH 4/8] Fix doc target to work in container mode Split doc target into container and non-container versions. In container mode, doc enters the container with BUILD_IN_CONTAINER=false to avoid resolving reference-help dependencies on the host, which would build macOS binaries that can't run in the Linux container. --- Makefile | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 3123b5e5a96..55bee6083a1 100644 --- a/Makefile +++ b/Makefile @@ -254,10 +254,14 @@ GOVOLUMES= -v mimir-go-cache:/go/cache \ # Mount local ssh credentials to be able to clone private repos when doing `mod-check` SSHVOLUME= -v ~/.ssh/:/root/.ssh:$(CONTAINER_MOUNT_OPTIONS) -exes $(EXES) $(EXES_RACE) protos $(PROTO_GOS) lint lint-gh-action lint-packaging-scripts test test-with-race cover shell mod-check check-protos doc format dist build-mixin format-mixin check-mixin-tests license check-license conftest-fmt check-conftest-fmt helm-conftest-test helm-conftest-quick-test conftest-verify check-helm-tests build-helm-tests print-go-version format-promql-tests check-promql-tests format-protobuf check-protobuf-format: fetch-build-image +exes $(EXES) $(EXES_RACE) protos $(PROTO_GOS) lint lint-gh-action lint-packaging-scripts test test-with-race cover shell mod-check check-protos format dist build-mixin format-mixin check-mixin-tests license check-license conftest-fmt check-conftest-fmt helm-conftest-test helm-conftest-quick-test conftest-verify check-helm-tests build-helm-tests print-go-version format-promql-tests check-promql-tests format-protobuf check-protobuf-format: fetch-build-image @echo ">>>> Entering build container: $@" $(SUDO) time docker run --rm $(TTY) -i $(SSHVOLUME) $(GOVOLUMES) $(BUILD_IMAGE) GOOS=$(GOOS) GOARCH=$(GOARCH) BINARY_SUFFIX=$(BINARY_SUFFIX) $@; +doc: fetch-build-image + @echo ">>>> Entering build container: $@" + $(SUDO) time docker run --rm $(TTY) -i $(SSHVOLUME) $(GOVOLUMES) $(BUILD_IMAGE) GOOS=$(GOOS) GOARCH=$(GOARCH) BINARY_SUFFIX=$(BINARY_SUFFIX) BUILD_IN_CONTAINER=false $@; + else exes: $(EXES) @@ -290,6 +294,13 @@ lint-packaging-scripts: packaging/nfpm/mimir/postinstall.sh packaging/nfpm/mimir lint-gh-action: operations/mimir-rules-action/entrypoint.sh shellcheck $? +doc: ## Generates the config file documentation. +doc: reference-help clean-doc $(DOC_TEMPLATES:.template=.md) $(DOC_EMBED:.md=.md.embedmd) + # Make up markdown files prettier. When running with check-doc target, it will fail if this produces any change. + prettier --write "**/*.md" + # Make operations/helm/charts/*/README.md + helm-docs + lint: ## Run lints to check for style issues. lint: check-makefiles check-merge-conflicts misspell -error $(DOC_SOURCES_PATH) @@ -507,13 +518,6 @@ check-protobuf-format: %.md.embedmd : %.md embedmd -w $< -doc: ## Generates the config file documentation. -doc: reference-help clean-doc $(DOC_TEMPLATES:.template=.md) $(DOC_EMBED:.md=.md.embedmd) - # Make up markdown files prettier. When running with check-doc target, it will fail if this produces any change. - prettier --write "**/*.md" - # Make operations/helm/charts/*/README.md - helm-docs - license: ## Add license header to files. go run ./tools/add-license ./cmd ./integration ./pkg ./tools ./packaging ./development ./mimir-build-image ./operations ./.github From fe1df0beb20916b671332d6b68079c98b6f9508f Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 10 Nov 2025 13:47:10 +0100 Subject: [PATCH 5/8] Add check-reference-help target Split reference help validation from check-doc into its own target that always runs outside the container. This allows CI to validate reference documentation without needing the build container. --- .github/workflows/test-build-deploy.yml | 2 ++ Makefile | 15 +++++++-------- operations/mimir/mimir-flags-defaults.json | 6 +++--- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.github/workflows/test-build-deploy.yml b/.github/workflows/test-build-deploy.yml index cce32e17a3f..c8a59d10d0c 100644 --- a/.github/workflows/test-build-deploy.yml +++ b/.github/workflows/test-build-deploy.yml @@ -79,6 +79,8 @@ jobs: run: make BUILD_IN_CONTAINER=false check-protos - name: Check Generated Documentation run: make BUILD_IN_CONTAINER=false check-doc + - name: Check Reference Help Documentation + run: make BUILD_IN_CONTAINER=false check-reference-help - name: Check White Noise run: make BUILD_IN_CONTAINER=false check-white-noise - name: Check License Header diff --git a/Makefile b/Makefile index 55bee6083a1..7936845a100 100644 --- a/Makefile +++ b/Makefile @@ -254,14 +254,10 @@ GOVOLUMES= -v mimir-go-cache:/go/cache \ # Mount local ssh credentials to be able to clone private repos when doing `mod-check` SSHVOLUME= -v ~/.ssh/:/root/.ssh:$(CONTAINER_MOUNT_OPTIONS) -exes $(EXES) $(EXES_RACE) protos $(PROTO_GOS) lint lint-gh-action lint-packaging-scripts test test-with-race cover shell mod-check check-protos format dist build-mixin format-mixin check-mixin-tests license check-license conftest-fmt check-conftest-fmt helm-conftest-test helm-conftest-quick-test conftest-verify check-helm-tests build-helm-tests print-go-version format-promql-tests check-promql-tests format-protobuf check-protobuf-format: fetch-build-image +exes $(EXES) $(EXES_RACE) protos $(PROTO_GOS) lint lint-gh-action lint-packaging-scripts test test-with-race cover shell mod-check check-protos doc format dist build-mixin format-mixin check-mixin-tests license check-license conftest-fmt check-conftest-fmt helm-conftest-test helm-conftest-quick-test conftest-verify check-helm-tests build-helm-tests print-go-version format-promql-tests check-promql-tests format-protobuf check-protobuf-format: fetch-build-image @echo ">>>> Entering build container: $@" $(SUDO) time docker run --rm $(TTY) -i $(SSHVOLUME) $(GOVOLUMES) $(BUILD_IMAGE) GOOS=$(GOOS) GOARCH=$(GOARCH) BINARY_SUFFIX=$(BINARY_SUFFIX) $@; -doc: fetch-build-image - @echo ">>>> Entering build container: $@" - $(SUDO) time docker run --rm $(TTY) -i $(SSHVOLUME) $(GOVOLUMES) $(BUILD_IMAGE) GOOS=$(GOOS) GOARCH=$(GOARCH) BINARY_SUFFIX=$(BINARY_SUFFIX) BUILD_IN_CONTAINER=false $@; - else exes: $(EXES) @@ -648,14 +644,17 @@ clean-doc: ## Clean the documentation files generated from templates. check-doc: ## Check the documentation files are up to date. check-doc: doc + @find . -name "*.md" | xargs git diff --exit-code -- \ + || (echo "Please update generated documentation by running 'make doc' and committing the changes" && false) + +check-reference-help: ## Check the reference help documentation is up to date. +check-reference-help: reference-help @git diff --exit-code -- \ cmd/mimir/help.txt.tmpl \ cmd/mimir/help-all.txt.tmpl \ cmd/mimir/config-descriptor.json \ operations/mimir/mimir-flags-defaults.json \ - || (echo "Please update generated reference documentation by running 'make doc' and committing the changes" && false) - @find . -name "*.md" | xargs git diff --exit-code -- \ - || (echo "Please update generated documentation by running 'make doc' and committing the changes" && false) + || (echo "Please update generated reference documentation by running 'make reference-help' and committing the changes" && false) # Tool is developed in the grafana/technical-documentation repository: # https://github.com/grafana/technical-documentation/tree/main/tools/doc-validator diff --git a/operations/mimir/mimir-flags-defaults.json b/operations/mimir/mimir-flags-defaults.json index 2e8f0ba988e..220aaaf11ce 100644 --- a/operations/mimir/mimir-flags-defaults.json +++ b/operations/mimir/mimir-flags-defaults.json @@ -35,9 +35,9 @@ "server.grpc-max-recv-msg-size-bytes": 104857600, "server.grpc-max-send-msg-size-bytes": 104857600, "server.grpc-max-concurrent-streams": 100, - "server.grpc.keepalive.max-connection-idle": 9223372036854776000, - "server.grpc.keepalive.max-connection-age": 9223372036854776000, - "server.grpc.keepalive.max-connection-age-grace": 9223372036854776000, + "server.grpc.keepalive.max-connection-idle": 9223372036854775807, + "server.grpc.keepalive.max-connection-age": 9223372036854775807, + "server.grpc.keepalive.max-connection-age-grace": 9223372036854775807, "server.grpc.keepalive.time": 7200000000000, "server.grpc.keepalive.timeout": 20000000000, "server.grpc.keepalive.min-time-between-pings": 10000000000, From 8c6011d15ad067aceef68c1eb0327565f8248bcb Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 10 Nov 2025 14:08:23 +0100 Subject: [PATCH 6/8] Remove reference-help from doc target dependencies --- Makefile | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 7936845a100..88cb8e57b08 100644 --- a/Makefile +++ b/Makefile @@ -290,13 +290,6 @@ lint-packaging-scripts: packaging/nfpm/mimir/postinstall.sh packaging/nfpm/mimir lint-gh-action: operations/mimir-rules-action/entrypoint.sh shellcheck $? -doc: ## Generates the config file documentation. -doc: reference-help clean-doc $(DOC_TEMPLATES:.template=.md) $(DOC_EMBED:.md=.md.embedmd) - # Make up markdown files prettier. When running with check-doc target, it will fail if this produces any change. - prettier --write "**/*.md" - # Make operations/helm/charts/*/README.md - helm-docs - lint: ## Run lints to check for style issues. lint: check-makefiles check-merge-conflicts misspell -error $(DOC_SOURCES_PATH) @@ -514,6 +507,13 @@ check-protobuf-format: %.md.embedmd : %.md embedmd -w $< +doc: ## Generates the config file documentation. +doc: clean-doc $(DOC_TEMPLATES:.template=.md) $(DOC_EMBED:.md=.md.embedmd) + # Make up markdown files prettier. When running with check-doc target, it will fail if this produces any change. + prettier --write "**/*.md" + # Make operations/helm/charts/*/README.md + helm-docs + license: ## Add license header to files. go run ./tools/add-license ./cmd ./integration ./pkg ./tools ./packaging ./development ./mimir-build-image ./operations ./.github From 25fd965f7fa735130e5d9506477fa569c13bd740 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 10 Nov 2025 16:35:09 +0100 Subject: [PATCH 7/8] Add phony target Signed-off-by: Dimitar Dimitrov --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 88cb8e57b08..b1c7f18d288 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ help: # WARNING: do not commit to a repository! -include Makefile.local -.PHONY: all test test-with-race integration-tests cover clean images protos exes dist doc clean-doc check-doc push-multiarch-build-image license check-license format check-mixin check-mixin-jb check-mixin-mixtool check-mixin-runbooks check-mixin-mimirtool-rules build-mixin format-mixin check-jsonnet-manifests format-jsonnet-manifests push-multiarch-mimir list-image-targets check-jsonnet-getting-started mixin-screenshots +.PHONY: all test test-with-race integration-tests cover clean images protos exes dist doc clean-doc check-doc check-reference-help push-multiarch-build-image license check-license format check-mixin check-mixin-jb check-mixin-mixtool check-mixin-runbooks check-mixin-mimirtool-rules build-mixin format-mixin check-jsonnet-manifests format-jsonnet-manifests push-multiarch-mimir list-image-targets check-jsonnet-getting-started mixin-screenshots .DEFAULT_GOAL := all # Version number From 32d5f930296e724b1406579bf20261f992b3b3b3 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 10 Nov 2025 16:53:30 +0100 Subject: [PATCH 8/8] Delete empty test file Signed-off-by: Dimitar Dimitrov --- tools/config-inspector/main_test.go | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 tools/config-inspector/main_test.go diff --git a/tools/config-inspector/main_test.go b/tools/config-inspector/main_test.go deleted file mode 100644 index d1a40a4d92d..00000000000 --- a/tools/config-inspector/main_test.go +++ /dev/null @@ -1,3 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-only - -package main