diff --git a/mchub/models/magic_castle/magic_castle.py b/mchub/models/magic_castle/magic_castle.py index a731a019..0299b47a 100644 --- a/mchub/models/magic_castle/magic_castle.py +++ b/mchub/models/magic_castle/magic_castle.py @@ -584,7 +584,16 @@ def plan_destruction(self): def create_plan(self, github_sha=None, run_id=None): logger.debug(f"Call <{self.__class__.__name__}:create_plan>") - self.tfcloud_run = TerraformCloudRunORM() + # A castle has exactly one Terraform Cloud run record. Reuse it when + # planning again instead of replacing it with another row carrying the + # same foreign key. + if self.tfcloud_run is None: + self.tfcloud_run = TerraformCloudRunORM() + else: + self.tfcloud_run.run_id = None + self.tfcloud_run.plan = None + self.tfcloud_run.apply_log_url = None + self.tfcloud_run.tf_state = None try: if github_sha is None and run_id is None: @@ -636,4 +645,4 @@ def apply(self): raise RunIDNotSet tf = get_terraform_cloud() - tf.apply_run(self.tfcloud_run.run_id) \ No newline at end of file + tf.apply_run(self.tfcloud_run.run_id) diff --git a/mchub/models/terraform_cloud.py b/mchub/models/terraform_cloud.py index e14862b7..0a0bf22e 100644 --- a/mchub/models/terraform_cloud.py +++ b/mchub/models/terraform_cloud.py @@ -9,4 +9,8 @@ class TerraformCloudRunORM(db.Model): apply_log_url = db.Column(db.String) tf_state = db.Column(db.PickleType()) magic_castle = db.relationship("MagicCastleORM", back_populates="tfcloud_run") - magic_castle_id = db.Column(db.Integer, db.ForeignKey("magiccastle.id")) \ No newline at end of file + magic_castle_id = db.Column( + db.Integer, + db.ForeignKey("magiccastle.id"), + unique=True, + ) diff --git a/migrations/versions/0005_enforce_one_tfcloud_run_per_cluster.py b/migrations/versions/0005_enforce_one_tfcloud_run_per_cluster.py new file mode 100644 index 00000000..d21b6a2f --- /dev/null +++ b/migrations/versions/0005_enforce_one_tfcloud_run_per_cluster.py @@ -0,0 +1,51 @@ +"""Enforce one Terraform Cloud run per Magic Castle + +Revision ID: 0005 +Revises: 0004 +Create Date: 2026-09-03 00:00:00.000000 + +""" +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = "0005" +down_revision = "0004" +branch_labels = None +depends_on = None + + +def upgrade(): + bind = op.get_bind() + + # The ORM relationship has always been scalar, but the database previously + # allowed several runs to reference the same castle. Keep the most recently + # inserted run before adding the missing database invariant. + bind.execute( + sa.text( + """ + DELETE FROM terraformcloudrun + WHERE magic_castle_id IS NOT NULL + AND id NOT IN ( + SELECT MAX(id) + FROM terraformcloudrun + WHERE magic_castle_id IS NOT NULL + GROUP BY magic_castle_id + ) + """ + ) + ) + + with op.batch_alter_table("terraformcloudrun") as batch_op: + batch_op.create_unique_constraint( + "uq_terraformcloudrun_magic_castle_id", + ["magic_castle_id"], + ) + + +def downgrade(): + with op.batch_alter_table("terraformcloudrun") as batch_op: + batch_op.drop_constraint( + "uq_terraformcloudrun_magic_castle_id", + type_="unique", + ) diff --git a/tests/test_migrations.py b/tests/test_migrations.py new file mode 100644 index 00000000..2c3a8d47 --- /dev/null +++ b/tests/test_migrations.py @@ -0,0 +1,61 @@ +from pathlib import Path + +import sqlalchemy as sa +from flask_migrate import upgrade + +from .mocks.configuration.config_mock import config_auth_none_mock # noqa: F401 + + +def test_tfcloud_run_migration_deduplicates_existing_rows(tmp_path): + from mchub import create_app + from mchub.database import db + + database_path = tmp_path / "migration.db" + migrations_path = Path(__file__).parents[1] / "migrations" + app = create_app(db_path=f"sqlite:///{database_path}") + + with app.app_context(): + upgrade(directory=str(migrations_path), revision="0004") + db.session.execute( + sa.text( + "INSERT INTO magiccastle (hostname) VALUES ('duplicate.example.com')" + ) + ) + magic_castle_id = db.session.execute( + sa.text( + "SELECT id FROM magiccastle WHERE hostname = 'duplicate.example.com'" + ) + ).scalar_one() + db.session.execute( + sa.text( + """ + INSERT INTO terraformcloudrun (run_id, magic_castle_id) + VALUES ('OLD_RUN', :magic_castle_id), + ('CURRENT_RUN', :magic_castle_id) + """ + ), + {"magic_castle_id": magic_castle_id}, + ) + db.session.commit() + + upgrade(directory=str(migrations_path), revision="head") + + runs = db.session.execute( + sa.text( + """ + SELECT run_id + FROM terraformcloudrun + WHERE magic_castle_id = :magic_castle_id + """ + ), + {"magic_castle_id": magic_castle_id}, + ).scalars().all() + assert runs == ["CURRENT_RUN"] + + constraints = sa.inspect(db.engine).get_unique_constraints( + "terraformcloudrun" + ) + assert any( + constraint["column_names"] == ["magic_castle_id"] + for constraint in constraints + ) diff --git a/tests/unit/magic_castle/test_magic_castle.py b/tests/unit/magic_castle/test_magic_castle.py index 4dc2f587..e6219a57 100644 --- a/tests/unit/magic_castle/test_magic_castle.py +++ b/tests/unit/magic_castle/test_magic_castle.py @@ -27,6 +27,32 @@ def test_create_magic_castle_plan_valid(app): cluster.plan_creation(deepcopy(VALID_CLUSTER_CONFIGURATION)) +def test_create_plan_reuses_tfcloud_run(app): + from mchub.database import db + from mchub.models.magic_castle.magic_castle import MagicCastle, MagicCastleORM + from mchub.models.terraform_cloud import TerraformCloudRunORM + + orm = db.session.scalar( + db.select(MagicCastleORM).filter_by( + hostname="valid1.magic-castle.cloud" + ) + ) + cluster = MagicCastle(orm) + original_run_id = cluster.tfcloud_run.id + + cluster.create_plan(run_id="REPLACEMENT_RUN_ID") + + runs = db.session.scalars( + db.select(TerraformCloudRunORM).filter_by(magic_castle_id=orm.id) + ).all() + assert len(runs) == 1 + assert runs[0].id == original_run_id + assert runs[0].run_id == "REPLACEMENT_RUN_ID" + assert runs[0].plan == {"MOCK": "PLAN_LOG"} + assert runs[0].apply_log_url is None + assert runs[0].tf_state is None + + def test_create_magic_castle_twice(app): from mchub.models.magic_castle.magic_castle import MagicCastle from mchub.exceptions.invalid_usage_exception import ( @@ -223,4 +249,4 @@ def test_allocated_resources_not_found(app): "pre_allocated_cores": 0, "pre_allocated_volume_count": 0, "pre_allocated_volume_size": 0, - } \ No newline at end of file + }