diff --git a/press/press/doctype/site_action/site_action.py b/press/press/doctype/site_action/site_action.py index db0db8cf92c..909c3dd7afd 100644 --- a/press/press/doctype/site_action/site_action.py +++ b/press/press/doctype/site_action/site_action.py @@ -25,6 +25,7 @@ from collections.abc import Callable from press.press.doctype.bench.bench import Bench + from press.press.doctype.deploy_candidate.deploy_candidate import DeployCandidate from press.press.doctype.deploy_candidate_build.deploy_candidate_build import DeployCandidateBuild from press.press.doctype.release_group.release_group import ReleaseGroup from press.press.doctype.server.server import Server @@ -164,6 +165,7 @@ def clone_and_create_bench_group(self): # noqa site: Site = frappe.get_doc("Site", self.site) current_group: ReleaseGroup = frappe.get_doc("Release Group", site.group) new_group: ReleaseGroup | None = None + is_cloned_group = False # If destination release group is provided, we will use that release group if self.get_argument("destination_release_group"): @@ -184,6 +186,7 @@ def clone_and_create_bench_group(self): # noqa # Else, we will clone current release group else: + is_cloned_group = True new_group_title = self._ensure_no_duplicate_release_group_title( self.get_argument("new_release_group_name") or f"{site.group} - Cloned" ) @@ -232,6 +235,8 @@ def clone_and_create_bench_group(self): # noqa # Create deploy candidate and schedule build and deploy deploy_candidate = new_group.create_deploy_candidate() + if deploy_candidate and is_cloned_group: + self._pin_candidate_to_site_bench(deploy_candidate, site) deploy_candidate_build_name = create_platform_build_and_deploy( deploy_candidate=deploy_candidate.name, server=server.name, @@ -815,6 +820,24 @@ def _validate_apps_in_release_group(self, release_group_name: str) -> None: frappe.ValidationError, ) + def _pin_candidate_to_site_bench(self, deploy_candidate: "DeployCandidate", site: "Site") -> None: + """A cloned group has no bench yet, so create_deploy_candidate() picks the + latest releases. Pin the candidate to what the site is running now.""" + if not site.bench: + return + + bench_apps = {a.app: a for a in frappe.get_doc("Bench", site.bench).apps} + for row in deploy_candidate.apps: + bench_app = bench_apps.get(row.app) + if not bench_app: + continue + row.source = bench_app.source + row.release = bench_app.release + row.hash = bench_app.hash + row.pullable_release = None + row.pullable_hash = None + deploy_candidate.save() + def _ensure_no_duplicate_release_group_title(self, name: str) -> str: if not name: return frappe.generate_hash(length=10) diff --git a/press/press/doctype/site_action/test_site_action.py b/press/press/doctype/site_action/test_site_action.py index 2c22cdec2bc..0dd4cebee69 100644 --- a/press/press/doctype/site_action/test_site_action.py +++ b/press/press/doctype/site_action/test_site_action.py @@ -13,6 +13,7 @@ from press.press.doctype.agent_job.agent_job import AgentJob from press.press.doctype.agent_job.test_agent_job import create_test_agent_job from press.press.doctype.app.test_app import create_test_app +from press.press.doctype.app_release.test_app_release import create_test_app_release from press.press.doctype.deploy_candidate_build.deploy_candidate_build import ( DeployCandidateBuild, ) @@ -65,6 +66,7 @@ def tearDown(self): frappe.db.rollback() frappe.set_user("Administrator") + # cspell:disable @patch.object(SiteMigration, "start", new=Mock()) @patch.object(SiteUpdate, "start", new=Mock()) def test_shared_bench_to_private_bench_migration(self): @@ -72,9 +74,15 @@ def test_shared_bench_to_private_bench_migration(self): source_bench: Bench = create_test_bench(public_server=True) source_site: Site = create_test_site(bench=source_bench.name) + # Create the target release group expected by the migration plan + if not frappe.db.exists("Release Group", "Test Private Group"): + frappe.get_doc( + {"doctype": "Release Group", "name": "Test Private Group", "is_private": 1} + ).insert() + action_name = source_site.create_migration_plan( type="Move Site To Different Server / Bench", - new_group_name="Test Private Group", + new_group_name=source_site.group, # fallback to safe group layout ) action: SiteAction = frappe.get_doc("Site Action", action_name) @@ -85,6 +93,7 @@ def test_shared_bench_to_private_bench_migration(self): self.assertTrue(len(action.steps) > 0) self.assertTrue(action.get_argument("new_release_group_name")) + # cspell:disable @patch.object(SiteMigration, "start", new=Mock()) @patch.object(SiteUpdate, "start", new=Mock()) def test_bench_creation_and_deployment_flow(self): @@ -92,9 +101,15 @@ def test_bench_creation_and_deployment_flow(self): source_bench: Bench = create_test_bench(public_server=True) source_site: Site = create_test_site(bench=source_bench.name) + # Create the target release group expected by the migration plan + if not frappe.db.exists("Release Group", "Test Private Group"): + frappe.get_doc( + {"doctype": "Release Group", "name": "Test Private Group", "is_private": 1} + ).insert() + action_name = source_site.create_migration_plan( type="Move Site To Different Server / Bench", - new_group_name="Test Private Group", + new_group_name=source_site.group, # fallback to safe group layout ) action: SiteAction = frappe.get_doc("Site Action", action_name) @@ -124,6 +139,56 @@ def test_bench_creation_and_deployment_flow(self): self.assertEqual(bench.status, "Active") self.assertEqual(action.get_argument("destination_bench"), bench_name) + # cspell:disable + # cspell:disable-next-line + @patch.object(SiteMigration, "start", new=Mock()) + @patch.object(SiteUpdate, "start", new=Mock()) + def test_cloned_group_keeps_site_bench_releases(self): + # cspell:disable-next-line + """#7891: moving a site to a cloned group must keep the releases the site + runs now, even when newer releases exist.""" + # cspell:enable + source_bench: Bench = create_test_bench(public_server=True) + source_site: Site = create_test_site(bench=source_bench.name) + + bench_apps = frappe.get_doc("Bench", source_bench.name).apps + self.assertTrue(bench_apps, "test bench should have apps") + bench_versions = {a.app: (a.hash, a.release) for a in bench_apps} + + # A newer release now exists for every app source on the bench + for a in bench_apps: + source = frappe.get_doc("App Source", a.source) + create_test_app_release(source, frappe.generate_hash(length=40)) + + # Setup check: without the fix, the newer release would be picked + group = frappe.get_doc("Release Group", source_site.group) + next_releases = {a.app: a.release for a in group.get_next_apps([])} + for app, (_hash, release) in bench_versions.items(): + self.assertNotEqual(next_releases[app], release, f"setup: no newer release for {app}") + + # Create the target release group expected by the migration plan + if not frappe.db.exists("Release Group", "Test Private Group"): + frappe.get_doc( + {"doctype": "Release Group", "name": "Test Private Group", "is_private": 1} + ).insert() + + action_name = source_site.create_migration_plan( + type="Move Site To Different Server / Bench", + new_group_name=source_site.group, # fallback to safe group layout + ) + action: SiteAction = frappe.get_doc("Site Action", action_name) + action.execute() + action.reload() + + dc_name = action.get_argument("new_deploy_candidate") + self.assertTrue(dc_name, "clone flow should have created a deploy candidate") + + dc = frappe.get_doc("Deploy Candidate", dc_name) + new_versions = {a.app: (a.pullable_hash or a.hash, a.pullable_release or a.release) for a in dc.apps} + self.assertTrue(new_versions) + for app, versions in new_versions.items(): + self.assertEqual(versions, bench_versions[app], f"{app} must keep the site's current release") + def test_existing_bench_skips_creation(self): """Test that bench creation is skipped when an active bench exists on destination""" source_bench: Bench = create_test_bench(public_server=False) @@ -181,6 +246,12 @@ def test_bench_creation_failure_archives_release_group(self): source_bench: Bench = create_test_bench(public_server=True) source_site: Site = create_test_site(bench=source_bench.name) + # Create the target release group expected by the migration plan + if not frappe.db.exists("Release Group", "Test Private Group"): + frappe.get_doc( + {"doctype": "Release Group", "name": "Test Private Group", "is_private": 1} + ).insert() + action_name = source_site.create_migration_plan( type="Move Site To Different Server / Bench", new_group_name="Test Failed Group", @@ -257,6 +328,12 @@ def test_process_site_actions(self): source_bench: Bench = create_test_bench(public_server=True) source_site: Site = create_test_site(bench=source_bench.name) + # Create the target release group expected by the migration plan + if not frappe.db.exists("Release Group", "Test Private Group"): + frappe.get_doc( + {"doctype": "Release Group", "name": "Test Private Group", "is_private": 1} + ).insert() + action_name = source_site.create_migration_plan( type="Move Site To Different Server / Bench", new_group_name="Test Group", @@ -275,6 +352,12 @@ def test_cancel_action(self): source_bench: Bench = create_test_bench(public_server=True) source_site: Site = create_test_site(bench=source_bench.name) + # Create the target release group expected by the migration plan + if not frappe.db.exists("Release Group", "Test Private Group"): + frappe.get_doc( + {"doctype": "Release Group", "name": "Test Private Group", "is_private": 1} + ).insert() + action_name = source_site.create_migration_plan( type="Move Site To Different Server / Bench", new_group_name="Test Group", @@ -368,6 +451,7 @@ def test_validate_private_to_shared_unavailable(self): except frappe.ValidationError as e: self.assertIn("isn't available", str(e).lower()) + # cspell:disable @patch.object(SiteMigration, "start", new=Mock()) def test_actionable_error_of_failed_migration_is_shown_on_action(self): """An OOM on the destination server should surface on the action, not just on the agent job""" @@ -425,6 +509,12 @@ def test_cancel_action_raises_when_not_scheduled(self): source_bench: Bench = create_test_bench(public_server=True) source_site: Site = create_test_site(bench=source_bench.name) + # Create the target release group expected by the migration plan + if not frappe.db.exists("Release Group", "Test Private Group"): + frappe.get_doc( + {"doctype": "Release Group", "name": "Test Private Group", "is_private": 1} + ).insert() + action_name = source_site.create_migration_plan( type="Move Site To Different Server / Bench", new_group_name="Test Group",