Skip to content

Move configs-db integration tests to ./integration#7653

Open
friedrichg wants to merge 3 commits into
masterfrom
move-configs-db-tests-to-integration
Open

Move configs-db integration tests to ./integration#7653
friedrichg wants to merge 3 commits into
masterfrom
move-configs-db-tests-to-integration

Conversation

@friedrichg

@friedrichg friedrichg commented Jun 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What this PR does:

Moves the configs-db Postgres tests out of the bespoke integration-configs-db
CI job and into the standard ./integration/ e2e framework, fixing #7652.

The old integration-configs-db job ran go test -tags 'integration' ./pkg/configs/... ./pkg/ruler/...,
which re-ran the entire pkg/configs and pkg/ruler unit-test suites a
second time just so pkg/configs/api could swap its in-memory dbtest helper
for a Postgres-backed one via an integration build tag. Every other package in
those trees produced a bit-for-bit identical binary already built by the test
job and was run twice for no added coverage.

This consolidates onto the same e2e.Scenario pattern every other
Docker-dependent test in the repo uses:

  • New integration_configs_db-gated e2e suite in integration/configs_db_test.go
    spins up Postgres + a cortex -target=configs container and drives the configs
    HTTP API end-to-end. It reproduces every distinct Postgres-SQL path the old
    job exercised via pkg/configs/api/api_test.go:
    • GetConfig found / not-found (PostAndGet{Rules,Alertmanager}Config, GetMissingReturns404)
    • SetConfig + version bump (PostAndGet*, GetAllConfigsReturnsLatest)
    • tenant filtering (MultiTenantIsolation)
    • GetAllConfigs with data / empty (GetAllConfigsReturnsLatest, GetAllConfigsEmpty)
    • GetConfigs(since) ?since=<id> filter (GetConfigsSince)
    • read / write tenant-auth 401 (AnonymousReturns401, PostAnonymousReturns401)
    • migrations-on-startup (MigrationsRunOnStartup)
  • New e2e helpers: NewPostgres in integration/e2e/db, a Postgres image
    constant, a NewConfigs cortex service constructor in e2ecortex, and a
    dedicated ConfigsClient (kept separate from Client to avoid colliding with
    the existing multitenant-alertmanager helpers). ConfigsClient carries a 30s
    request timeout (matching Client) and its Post* helpers return the response
    body so an unexpected status surfaces the server's error in test output.
  • Drops 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 — this was the only place in
    the repo that conditionally compiled test code via a //go:build integration
    tag swap.
  • Deletes the configs-integration-test Makefile target and the
    integration-configs-db CI job, and removes 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.
  • Adds postgres:9.6.16 to the per-tag Preload Images step.

Coverage note: the backend-agnostic api_test.go tests (alertmanager-config
validation, request body-format parsing, template-file validation,
ParseConfigFormat) are intentionally not ported. They exercise handler
logic 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 coverage
and was part of the duplication this PR removes.

Which issue(s) this PR fixes:
Fixes #7652

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

Full detail on https://gist.github.com/friedrichg/9c9ae72a84257cebf19ae54b20475205

@friedrichg
Move configs-db integration tests to ./integration
3f63a60 
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 dosubot Bot added type/chore Something that needs to be done; not a bug or a feature type/tests labels Jun 28, 2026
@friedrichg
configs e2e: cover GetConfigs(since), empty GetAllConfigs, and POST 401
991b537 
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>
@friedrichg
configs e2e: add ConfigsClient request timeout and surface POST body
da6e085 
- 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

size/XL type/chore Something that needs to be done; not a bug or a feature type/tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CI: integration-configs-db duplicates pkg/configs and pkg/ruler unit-test runs

1 participant

@friedrichg