Skip to content

fix(migration): dedup SQL migration versions - #4428

Merged
jayzhudev merged 1 commit into
NVIDIA:mainfrom
jayzhudev:fix/db-migrations
Jul 31, 2026
Merged

fix(migration): dedup SQL migration versions#4428
jayzhudev merged 1 commit into
NVIDIA:mainfrom
jayzhudev:fix/db-migrations

Conversation

@jayzhudev

Copy link
Copy Markdown
Contributor

Two database migrations on main use SQLx version 20260722120000. Because SQLx records migration versions as primary keys, fresh database migration fails when the second migration is recorded, blocking Core CI and dependent PRs.

This change assigns the newer BMC suppression migration version 20260722120001 without changing its SQL content. It also adds a regression test covering all legacy, squash, and post-squash migrations so duplicate versions fail with a focused diagnostic.

Related issues

Resolves #4427

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

Databases that recorded preserve_machine_ipv6_loopback as version 20260722120000 will apply the BMC suppression migration as 20260722120001 normally. Any database that already recorded bmc_suppressions as 20260722120000 requires migration-history repair before upgrading.

@jayzhudev jayzhudev self-assigned this Jul 31, 2026
@jayzhudev
jayzhudev requested a review from a team as a code owner July 31, 2026 01:47
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 561130de-f436-420b-8abe-52a588a3122b

📥 Commits

Reviewing files that changed from the base of the PR and between 654ce83 and bb771ad.

📒 Files selected for processing (2)
  • crates/api-db/migrations/20260722120001_bmc_suppressions.sql
  • crates/api-db/src/migrations/mod.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/api-db/migrations/20260722120001_bmc_suppressions.sql
  • crates/api-db/src/migrations/mod.rs

Summary by CodeRabbit

  • New Features

    • Added support for recording subsystem-specific suppression requests for BMC MAC addresses, including reasons, request times, and optional acknowledgements.
  • Tests

    • Added validation to ensure database migration versions remain unique.

Walkthrough

Adds persistent storage for BMC suppression requests and a migration test that detects duplicate versions across configured migrators.

Changes

Migration integrity

Layer / File(s) Summary
BMC suppression table
crates/api-db/migrations/20260722120001_bmc_suppressions.sql
Adds subsystem-constrained suppression records with request and optional acknowledgement timestamps, keyed by BMC MAC address and subsystem.
Migration version validation
crates/api-db/src/migrations/mod.rs
Adds a test that ensures migration versions are unique across legacy and epoch migrators.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the migration version deduplication, which is the primary change.
Description check ✅ Passed The description accurately explains the duplicate version, the corrective migration rename, and the regression test.
Linked Issues check ✅ Passed The changes assign a unique migration version and add duplicate-version validation, addressing the coding objectives in [#4427].
Out of Scope Changes check ✅ Passed The migration version update and regression test are directly related to the linked issue and stated pull request objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@spydaNVIDIA spydaNVIDIA left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beat me to it :)

@jayzhudev
jayzhudev enabled auto-merge (squash) July 31, 2026 02:02
Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
@jayzhudev
jayzhudev force-pushed the fix/db-migrations branch from 654ce83 to bb771ad Compare July 31, 2026 02:13
@jayzhudev
jayzhudev merged commit 160bb20 into NVIDIA:main Jul 31, 2026
62 checks passed
chet added a commit that referenced this pull request Jul 31, 2026
…4432)

PR #4389 and PR #4428 independently fixed the same duplicate migration
version from opposite sides. The loopback preservation migration first
reached `main` in #3913 as `20260722120000`, while the BMC suppression
migration came later. #4428 correctly moved the newer BMC migration to
`20260722120001`, but #4389 had already renamed the older loopback
migration to that same version on an earlier base. Since #4428 merged
first, #4389 left `main` with two `20260722120001` migrations and
removed the identity existing databases may already have in
`_sqlx_migrations`.

So, restore `20260722120000_preserve_machine_ipv6_loopback.sql` and
leave `20260722120001_bmc_suppressions.sql` where it is. The loopback
migration SQL is byte-for-byte identical to #3913, existing database
histories keep matching the published version and checksum, and SQLx
sees each migration version exactly once again.

## Related issues

None -- urgent post-merge repair for #4389 and #4428.

## Type of Change

- [ ] **Add** - New feature or capability
- [ ] **Change** - Changes in existing functionality
- [x] **Fix** - Bug fixes
- [ ] **Remove** - Removed features or deprecated functionality
- [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.)

## Breaking Changes

- [ ] **This PR contains breaking changes**

## Testing

- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [x] Manual testing performed
- [ ] No testing required (docs, internal refactor, etc.)

## Additional Notes

- `cargo test -p carbide-api-db migrations::tests --lib` (4 passed)
- `cargo make format-nightly`
- `cargo make clippy`
- `cargo make carbide-lints`
- `git diff --check`
- Restored migration blob matches the original #3913 blob exactly
(`eaff208e4ea6d3956bdb158a84c45ee6c1362ecf`)
- This restores the migration identities intended by #4428. A database
first initialized during either brief duplicate-version window may have
recorded the wrong checksum at `20260722120000` or `20260722120001`;
inspect its schema and migration history and perform site-specific
repair before retrying rather than blindly replaying these
non-idempotent migrations

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
polarweasel pushed a commit to polarweasel/infra-controller that referenced this pull request Jul 31, 2026
Two database migrations on `main` use SQLx version `20260722120000`.
Because SQLx records migration versions as primary keys, fresh database
migration fails when the second migration is recorded, blocking Core CI
and dependent PRs.

This change assigns the newer BMC suppression migration version
`20260722120001` without changing its SQL content. It also adds a
regression test covering all legacy, squash, and post-squash migrations
so duplicate versions fail with a focused diagnostic.

## Related issues

Resolves NVIDIA#4427

## Type of Change

- [ ] **Add** - New feature or capability
- [ ] **Change** - Changes in existing functionality
- [x] **Fix** - Bug fixes
- [ ] **Remove** - Removed features or deprecated functionality
- [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.)

## Breaking Changes

- [ ] **This PR contains breaking changes**

## Testing

- [x] Unit tests added/updated
- [ ] Integration tests added/updated
- [x] Manual testing performed
- [ ] No testing required (docs, internal refactor, etc.)

## Additional Notes

Databases that recorded `preserve_machine_ipv6_loopback` as version
`20260722120000` will apply the BMC suppression migration as
`20260722120001` normally. Any database that already recorded
`bmc_suppressions` as `20260722120000` requires migration-history repair
before upgrading.

Signed-off-by: Jay Zhu <jayzhu@nvidia.com>
Signed-off-by: Alex Ball <aball@nvidia.com>
polarweasel pushed a commit to polarweasel/infra-controller that referenced this pull request Jul 31, 2026
…VIDIA#4432)

PR NVIDIA#4389 and PR NVIDIA#4428 independently fixed the same duplicate migration
version from opposite sides. The loopback preservation migration first
reached `main` in NVIDIA#3913 as `20260722120000`, while the BMC suppression
migration came later. NVIDIA#4428 correctly moved the newer BMC migration to
`20260722120001`, but NVIDIA#4389 had already renamed the older loopback
migration to that same version on an earlier base. Since NVIDIA#4428 merged
first, NVIDIA#4389 left `main` with two `20260722120001` migrations and
removed the identity existing databases may already have in
`_sqlx_migrations`.

So, restore `20260722120000_preserve_machine_ipv6_loopback.sql` and
leave `20260722120001_bmc_suppressions.sql` where it is. The loopback
migration SQL is byte-for-byte identical to NVIDIA#3913, existing database
histories keep matching the published version and checksum, and SQLx
sees each migration version exactly once again.

## Related issues

None -- urgent post-merge repair for NVIDIA#4389 and NVIDIA#4428.

## Type of Change

- [ ] **Add** - New feature or capability
- [ ] **Change** - Changes in existing functionality
- [x] **Fix** - Bug fixes
- [ ] **Remove** - Removed features or deprecated functionality
- [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.)

## Breaking Changes

- [ ] **This PR contains breaking changes**

## Testing

- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [x] Manual testing performed
- [ ] No testing required (docs, internal refactor, etc.)

## Additional Notes

- `cargo test -p carbide-api-db migrations::tests --lib` (4 passed)
- `cargo make format-nightly`
- `cargo make clippy`
- `cargo make carbide-lints`
- `git diff --check`
- Restored migration blob matches the original NVIDIA#3913 blob exactly
(`eaff208e4ea6d3956bdb158a84c45ee6c1362ecf`)
- This restores the migration identities intended by NVIDIA#4428. A database
first initialized during either brief duplicate-version window may have
recorded the wrong checksum at `20260722120000` or `20260722120001`;
inspect its schema and migration history and perform site-specific
repair before retrying rather than blindly replaying these
non-idempotent migrations

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
Signed-off-by: Alex Ball <aball@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: duplicate SQL migration version blocks Core CI and database migration

2 participants