diff --git a/internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.down.sql b/internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.down.sql new file mode 100644 index 00000000..7c4b13e2 --- /dev/null +++ b/internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.down.sql @@ -0,0 +1,30 @@ +-- Rollback: recreate the deprecated `(aws, sagemaker)` ServiceConfig +-- row from `(aws, savings-plans-sagemaker)` for emergency rollback. +-- +-- Lossy by design: the original PR #71 row's term/payment were copied +-- forward into `(aws, savings-plans-sagemaker)` by migration 000040, +-- but any divergence between that row and a hypothetical edited +-- `(aws, sagemaker)` row at the time of 000045 is not recoverable — +-- only the post-040 term/payment survive in the split row. +-- +-- Idempotent in two directions: +-- * If `(aws, savings-plans-sagemaker)` does not exist (e.g. someone +-- rolled back past migration 000040 before invoking this down), +-- the SELECT returns zero rows and the INSERT is a no-op. +-- * If `(aws, sagemaker)` already exists (manual restore, or this +-- down was already run), `ON CONFLICT (provider, service) DO +-- NOTHING` keeps the existing row untouched. + +INSERT INTO service_configs ( + provider, service, enabled, term, payment, coverage, ramp_schedule, + include_engines, exclude_engines, include_regions, exclude_regions, + include_types, exclude_types, created_at, updated_at +) +SELECT 'aws', 'sagemaker', + enabled, term, payment, coverage, ramp_schedule, + include_engines, exclude_engines, include_regions, exclude_regions, + include_types, exclude_types, + NOW(), NOW() +FROM service_configs +WHERE provider = 'aws' AND service = 'savings-plans-sagemaker' +ON CONFLICT (provider, service) DO NOTHING; diff --git a/internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.up.sql b/internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.up.sql new file mode 100644 index 00000000..c9565c12 --- /dev/null +++ b/internal/database/postgres/migrations/000045_drop_aws_sagemaker_service_config.up.sql @@ -0,0 +1,23 @@ +-- Drop the deprecated `(aws, sagemaker)` ServiceConfig row left behind +-- by migration 000040. +-- +-- Background: PR #71 introduced an interim `(aws, sagemaker)` row with +-- its own term/payment selects. Migration 000040 (PR #123) split the +-- legacy `(aws, savings-plans)` umbrella into four per-plan-type rows +-- and copied the sagemaker row's term/payment forward into the new +-- `(aws, savings-plans-sagemaker)` row, but intentionally KEPT the +-- `(aws, sagemaker)` row for one stable release as a backward-compat +-- safety net for users mid-rollout. See the `Deprecation follow-up` +-- comment at the bottom of 000040_split_savingsplans.up.sql. +-- +-- This migration removes that deprecated row now that the per-plan-type +-- cards are the canonical source of SageMaker SP defaults. +-- +-- Idempotent: the WHERE clause matches zero rows on installations that +-- never had PR #71's row (or that already ran this migration), so the +-- DELETE is a safe no-op. +-- +-- Tracked in issue #133. + +DELETE FROM service_configs + WHERE provider = 'aws' AND service = 'sagemaker'; diff --git a/internal/database/postgres/migrations/split_savingsplans_test.go b/internal/database/postgres/migrations/split_savingsplans_test.go index 746494d6..2c684362 100644 --- a/internal/database/postgres/migrations/split_savingsplans_test.go +++ b/internal/database/postgres/migrations/split_savingsplans_test.go @@ -137,14 +137,20 @@ func TestMigration_SplitSavingsPlans(t *testing.T) { require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) got := queryAWSSPRows(t, pool) - // Expect: 4 split rows + sagemaker (kept). Umbrella deleted. - require.Len(t, got, 5) + // Expect: 4 split rows. Umbrella deleted by 000040; PR #71's + // `(aws, sagemaker)` row is also deleted by 000045 (issue #133) + // after 000040 has copied its term/payment forward into the + // new `savings-plans-sagemaker` slot. + require.Len(t, got, 4) _, hasUmbrella := got["savings-plans"] assert.False(t, hasUmbrella, "umbrella should be deleted") _, hasSagemaker := got["sagemaker"] - assert.True(t, hasSagemaker, "PR #71 sagemaker row must be kept for one release") + assert.False(t, hasSagemaker, "PR #71 sagemaker row should be deleted by 000045 (issue #133)") // sagemaker slot inherits from PR #71's row (1, partial-upfront). + // The inheritance happens during 000040, which runs before + // 000045 deletes the source row, so the values still flow + // through into the persisted split row. smRow := got["savings-plans-sagemaker"] assert.Equal(t, 1, smRow.term, "sagemaker slot should inherit term from (aws, sagemaker)") assert.Equal(t, "partial-upfront", smRow.payment, "sagemaker slot should inherit payment from (aws, sagemaker)")