Skip to content

fix(site-action): Pin cloned group to the site's bench versions (#7891) #7968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PrathmmeshJagdale wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix(site-action): Pin cloned group to the site's bench versions (#7891) #7968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6385dc
fix: pin cloned release group to site's current bench versions
PrathmmeshJagdale Nov 3, 2026
c637cb1
fix: resolve syntax indentation and formatting issues
PrathmmeshJagdale Nov 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions press/press/doctype/site_action/site_action.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"):
Expand All @@ -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"
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
94 changes: 92 additions & 2 deletions press/press/doctype/site_action/test_site_action.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -65,16 +66,23 @@ 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):
"""Test moving a site from shared bench to private bench"""
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)
Expand All @@ -85,16 +93,23 @@ 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):
"""Test the complete bench creation flow during site migration"""
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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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"""
Expand Down Expand Up @@ -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",
Expand Down
Loading