Skip to content

fix(migration): put the published loopback migration timestamp back - #4432

Merged
chet merged 1 commit into
NVIDIA:mainfrom
chet:fix/preserve-loopback-migration-version
Jul 31, 2026
Merged

fix(migration): put the published loopback migration timestamp back#4432
chet merged 1 commit into
NVIDIA:mainfrom
chet:fix/preserve-loopback-migration-version

Conversation

@chet

@chet chet commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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
  • 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

  • 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 feat(api): persist DPU IPv6 loopback reservations #3913 blob exactly (eaff208e4ea6d3956bdb158a84c45ee6c1362ecf)
  • This restores the migration identities intended by fix(migration): dedup SQL migration versions #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

The loopback preservation migration has been on `main` as `20260722120000`, so existing databases can already have that version and checksum in `_sqlx_migrations`. A merge race renamed it to the same `20260722120001` version now used by the newer BMC suppression migration.

So, restore the loopback migration's original filename and leave the BMC migration at `20260722120001`. The SQL stays byte-for-byte identical, existing migration histories keep matching, and SQLx sees each version exactly once.

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
@chet
chet requested a review from a team as a code owner July 31, 2026 04:30
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

💤 Files selected but had no reviewable changes (1)
  • crates/api-db/migrations/20260722120000_preserve_machine_ipv6_loopback.sql
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 87dfba18-6ddb-4836-94ab-81b44159cc1a

📥 Commits

Reviewing files that changed from the base of the PR and between 5b7e5d0 and eff78a8.

📒 Files selected for processing (1)
  • crates/api-db/migrations/20260722120000_preserve_machine_ipv6_loopback.sql

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

chet commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full_review, thanks!

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

@chet I will perform a full review of #4432.

✅ Action performed

Full review finished.

@chet chet changed the title fix(migration): restore the published loopback version fix(migration): put the published loopback migration timestamp back Jul 31, 2026
@chet
chet enabled auto-merge (squash) July 31, 2026 04:54
chet added a commit to chet/bare-metal-manager-core that referenced this pull request Jul 31, 2026
Core migrations have historically used `...120000` placeholder timestamps, which means two independently authored PRs can both look fine until their versions collide on `main` -- exactly what NVIDIA#4432 is repairing now. The duplicate-version test can tell us once both migrations are together, but it cannot prevent that collision while the PRs are still independent.

So, this adds a separate `migration-police` job that looks only at new files under `crates/api-db/migrations/` and rejects timestamps whose minute and second fields are both `00`. Existing migrations stay accepted, and failures name the migration plus the required `YYYYMMDDhhmmss_description.sql` format in the job log.

`STYLE_GUIDE.md` and `AGENTS.md` now point contributors at the same fully populated timestamp rule.

This supports NVIDIA#4431

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
@chet
chet merged commit 587165f into NVIDIA:main Jul 31, 2026
111 of 113 checks passed
@chet
chet deleted the fix/preserve-loopback-migration-version branch July 31, 2026 06:11
chet added a commit to chet/bare-metal-manager-core that referenced this pull request Jul 31, 2026
Core migrations have historically used `...120000` placeholder timestamps, which means two independently authored PRs can both look fine until their versions collide on `main` -- exactly what NVIDIA#4432 is repairing now. The duplicate-version test can tell us once both migrations are together, but it cannot prevent that collision while the PRs are still independent.

So, this adds a separate `migration-police` job that looks only at new files under `crates/api-db/migrations/` and rejects timestamps whose minute and second fields are both `00`. Existing migrations stay accepted, and failures name the migration plus the required `YYYYMMDDhhmmss_description.sql` format in the job log.

`STYLE_GUIDE.md` and `AGENTS.md` now point contributors at the same fully populated timestamp rule.

This supports NVIDIA#4431

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
chet added a commit that referenced this pull request Jul 31, 2026
This enforces that migration files have their timestamp fully populated,
and not end in `0000`.

Core migrations have more recently been using `...120000` placeholder
timestamps, which means two independently authored PRs can both look
fine until their versions collide on `main` -- exactly what #4432 is
repairing now. The duplicate-version test can tell us once both
migrations are together, but it cannot prevent that collision while the
PRs are still independent.

So, this adds a separate `migration-police` job that looks only at new
files under `crates/api-db/migrations/` and rejects timestamps whose
minute and second fields are both `00`. Existing migrations stay
accepted, and failures name the migration plus the required
`YYYYMMDDhhmmss_description.sql` format in the job log.

`STYLE_GUIDE.md` and `AGENTS.md` now point contributors at the same
fully populated timestamp rule.

## Related issues

This supports #4431

## Type of Change

- [x] **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
- [x] Manual testing performed
- [ ] No testing required (docs, internal refactor, etc.)

Focused checks covered a valid full timestamp, a rejected `0000`
placeholder, an accepted zero-seconds timestamp with a populated minute,
historical migrations against `origin/main`, an invalid base revision,
shell syntax, workflow YAML parsing, and `git diff --check`.

The required local gates also passed:

- `cargo make format-nightly`
- `cargo make clippy`
- `cargo make carbide-lints`

## Additional Notes

`cargo test -p carbide-api-db migration_versions_are_unique --lib`
currently fails on the pre-existing duplicate `20260722120001` migration
version. #4432 repairs that collision and reports the migration test
passing.

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
…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>
polarweasel pushed a commit to polarweasel/infra-controller that referenced this pull request Jul 31, 2026
This enforces that migration files have their timestamp fully populated,
and not end in `0000`.

Core migrations have more recently been using `...120000` placeholder
timestamps, which means two independently authored PRs can both look
fine until their versions collide on `main` -- exactly what NVIDIA#4432 is
repairing now. The duplicate-version test can tell us once both
migrations are together, but it cannot prevent that collision while the
PRs are still independent.

So, this adds a separate `migration-police` job that looks only at new
files under `crates/api-db/migrations/` and rejects timestamps whose
minute and second fields are both `00`. Existing migrations stay
accepted, and failures name the migration plus the required
`YYYYMMDDhhmmss_description.sql` format in the job log.

`STYLE_GUIDE.md` and `AGENTS.md` now point contributors at the same
fully populated timestamp rule.

## Related issues

This supports NVIDIA#4431

## Type of Change

- [x] **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
- [x] Manual testing performed
- [ ] No testing required (docs, internal refactor, etc.)

Focused checks covered a valid full timestamp, a rejected `0000`
placeholder, an accepted zero-seconds timestamp with a populated minute,
historical migrations against `origin/main`, an invalid base revision,
shell syntax, workflow YAML parsing, and `git diff --check`.

The required local gates also passed:

- `cargo make format-nightly`
- `cargo make clippy`
- `cargo make carbide-lints`

## Additional Notes

`cargo test -p carbide-api-db migration_versions_are_unique --lib`
currently fails on the pre-existing duplicate `20260722120001` migration
version. NVIDIA#4432 repairs that collision and reports the migration test
passing.

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.

2 participants