Move configs-db integration tests to ./integration#7653
Open
friedrichg wants to merge 3 commits into
Open
Conversation
The integration-configs-db CI job was the only test suite that built a parallel "swap dbtest variant via build tag" pipeline outside the ./integration/ e2e framework, and as a side effect re-ran the entire pkg/configs and pkg/ruler unit-test suites against a Postgres-backed dbtest just so pkg/configs/api could exercise the real DB. See #7652. This consolidates onto the ./integration/ pattern every other Docker-dependent test in the repo uses: - New integration_configs_db-gated e2e test in integration/configs_db_test.go spins up Postgres + a cortex -target=configs container, drives the configs HTTP API end-to-end (round-trip, 401/404, multi-tenant isolation, "newest version" admin endpoint, migrations-on-startup). - New e2e helpers: NewPostgres in integration/e2e/db, Postgres image constant, NewConfigs cortex service constructor in e2ecortex, and a dedicated ConfigsClient (kept separate from Client to avoid colliding with the existing alertmanager helpers). - Drop the integration build tag from pkg/configs/db/dbtest by deleting dbtest/integration.go and renaming dbtest/unit.go to dbtest.go. The package becomes a single-variant in-memory helper. - Delete the configs-integration-test Makefile target and the integration-configs-db CI job; remove it from deploy.needs. The existing discover-tags auto-discovery picks up the new build tag and runs it in the standard integration matrix on amd64 and arm64. - Add postgres:9.6.16 to the per-tag Preload Images step. Signed-off-by: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com>
dosubot
Bot
added
type/chore
Restore the Postgres-specific SQL paths the deleted integration-configs-db job exercised via pkg/configs/api/api_test.go that the initial e2e port missed: - TestConfigsDB_GetConfigsSince exercises the ?since=<id> filter (GetConfigs(since) SQL), the one query path not hit by the other tests. - TestConfigsDB_GetAllConfigsEmpty asserts GetAllConfigs returns an empty set (not an error) on a freshly-migrated DB. - TestConfigsDB_PostAnonymousReturns401 covers the write-path tenant-auth check, mirroring the existing read-path 401 test. Add GetAllRulesConfigsSince to ConfigsClient to drive the since filter. The backend-agnostic api_test.go tests (validation, body-format, template parsing) are intentionally not ported: they exercise handler logic that is identical across DB backends and still run against the in-memory DB in `make test`, so running them against Postgres only re-introduces the duplication this change set removed. Signed-off-by: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com>
- Add a 30s request timeout, matching the sibling Client: a `timeout` field set in NewConfigsClient and applied via context.WithTimeout in do(), so a hung configs service fails fast instead of blocking until the global test timeout. - Return the response body from PostRulesConfig/PostAlertmanagerConfig (and postConfig), matching the convention of Client's *Raw helpers. Success-path POST assertions now use require.Equalf to print the body on an unexpected status, so a future 400/500 is debuggable instead of a bare "expected 204, got N". Signed-off-by: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.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
2 participants
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.
What this PR does:
Moves the configs-db Postgres tests out of the bespoke
integration-configs-dbCI job and into the standard
./integration/e2e framework, fixing #7652.The old
integration-configs-dbjob rango test -tags 'integration' ./pkg/configs/... ./pkg/ruler/...,which re-ran the entire
pkg/configsandpkg/rulerunit-test suites asecond time just so
pkg/configs/apicould swap its in-memorydbtesthelperfor a Postgres-backed one via an
integrationbuild tag. Every other package inthose trees produced a bit-for-bit identical binary already built by the
testjob and was run twice for no added coverage.
This consolidates onto the same
e2e.Scenariopattern every otherDocker-dependent test in the repo uses:
integration_configs_db-gated e2e suite inintegration/configs_db_test.gospins up Postgres + a
cortex -target=configscontainer and drives the configsHTTP API end-to-end. It reproduces every distinct Postgres-SQL path the old
job exercised via
pkg/configs/api/api_test.go:GetConfigfound / not-found (PostAndGet{Rules,Alertmanager}Config,GetMissingReturns404)SetConfig+ version bump (PostAndGet*,GetAllConfigsReturnsLatest)MultiTenantIsolation)GetAllConfigswith data / empty (GetAllConfigsReturnsLatest,GetAllConfigsEmpty)GetConfigs(since)?since=<id>filter (GetConfigsSince)AnonymousReturns401,PostAnonymousReturns401)MigrationsRunOnStartup)NewPostgresinintegration/e2e/db, aPostgresimageconstant, a
NewConfigscortex service constructor ine2ecortex, and adedicated
ConfigsClient(kept separate fromClientto avoid colliding withthe existing multitenant-alertmanager helpers).
ConfigsClientcarries a 30srequest timeout (matching
Client) and itsPost*helpers return the responsebody so an unexpected status surfaces the server's error in test output.
integrationbuild tag frompkg/configs/db/dbtestby deletingdbtest/integration.goand renamingdbtest/unit.gotodbtest.go. Thepackage becomes a single-variant in-memory helper — this was the only place in
the repo that conditionally compiled test code via a
//go:build integrationtag swap.
configs-integration-testMakefile target and theintegration-configs-dbCI job, and removes it fromdeploy.needs. Theexisting
discover-tagsauto-discovery picks up the new build tag and runs itin the standard integration matrix on amd64 and arm64.
postgres:9.6.16to the per-tag Preload Images step.Coverage note: the backend-agnostic
api_test.gotests (alertmanager-configvalidation, request body-format parsing, template-file validation,
ParseConfigFormat) are intentionally not ported. They exercise handlerlogic that behaves identically on any DB backend and still run against the
in-memory DB in
make test; running them against Postgres added no SQL coverageand was part of the duplication this PR removes.
Which issue(s) this PR fixes:
Fixes #7652
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]docs/configuration/v1-guarantees.mdupdated if this PR introduces experimental flagsFull detail on https://gist.github.com/friedrichg/9c9ae72a84257cebf19ae54b20475205