Merge pull request #1090 from frappe/site-migration-restore-fail-auto-drop

fix(SiteMigration): Remove site in destination (idempotently) before restoring

Author: balamurali27

press/api/tests/test_site.py

@@ -305,9 +305,11 @@ def test_restore_job_updates_apps_table_with_output_from_job(self):
 		with fake_agent_job(
 			"Restore Site",
 			"Success",
-			output="""frappe	15.0.0-dev HEAD
+			data=frappe._dict(
+				output="""frappe	15.0.0-dev HEAD
 insights 0.8.3	    HEAD
-""",
+"""
+			),
 		):
 			restore(
 				site.name,
@@ -347,9 +349,7 @@ def test_restore_job_updates_apps_table_when_only_frappe_is_installed(self):
 		self.assertEqual(site.status, "Active")
 
 		with fake_agent_job(
-			"Restore Site",
-			"Success",
-			output="""frappe 15.0.0-dev HEAD""",
+			"Restore Site", "Success", data=frappe._dict(output="""frappe 15.0.0-dev HEAD""")
 		):
 			restore(
 				site.name,
@@ -384,9 +384,11 @@ def test_new_site_from_backup_job_updates_apps_table_with_output_from_job(self):
 		with fake_agent_job(
 			"New Site from Backup",
 			"Success",
-			output="""frappe	15.0.0-dev HEAD
+			data=frappe._dict(
+				output="""frappe	15.0.0-dev HEAD
 erpnext 0.8.3	    HEAD
-""",
+"""
+			),
 		):
 			new(
 				{

press/press/doctype/agent_job/test_agent_job.py

@@ -22,12 +22,24 @@
 from press.utils.test import foreground_enqueue_doc
 
 
+def fn_appender(before_insert: Callable, prepare_agent_responses: Callable):
+	def new_before_insert(self):
+		before_insert(self)
+		prepare_agent_responses(self)
+
+	return new_before_insert
+
+
+before_insert: Callable = lambda self: None
+
+
 def fake_agent_job_req(
 	job_name: str,
 	status: Literal["Success", "Pending", "Running", "Failure"],
-	output: str = "",
-	steps: list[dict] = None,
+	data: dict,
+	steps: list[dict],
 ) -> Callable:
+	data = data or {}
 	steps = steps or []
 
 	def prepare_agent_responses(self):
@@ -38,8 +50,10 @@ def prepare_agent_responses(self):
 		steps: list of {"name": "Step name", "status": "status"} dictionaries
 		"""
 		nonlocal status
+		nonlocal job_name
+		if self.job_type != job_name:  # only fake the job we want to fake
+			return
 		job_id = int(make_autoname(".#"))
-
 		if steps:
 			needed_steps = frappe.get_all(
 				"Agent Job Type Step", {"parent": job_name}, pluck="step_name"
@@ -79,9 +93,7 @@ def prepare_agent_responses(self):
 			f"https://{self.server}:443/agent/jobs/{str(job_id)}",
 			# TODO:  populate steps with data from agent job type #
 			json={
-				"data": {
-					"output": output,
-				},
+				"data": data,
 				# TODO: uncomment lines as needed and make new parameters #
 				"duration": "00:00:13.496281",
 				"end": "2023-08-20 18:24:41.506067",
@@ -113,14 +125,16 @@ def prepare_agent_responses(self):
 			status=200,
 		)
 
-	return prepare_agent_responses
+	global before_insert
+	before_insert = fn_appender(before_insert, prepare_agent_responses)
+	return before_insert
 
 
 @contextmanager
 def fake_agent_job(
 	job_name: str,
-	status: Literal["Success", "Pending", "Running", "Failure"],
-	output: str = "",
+	status: Literal["Success", "Pending", "Running", "Failure"] = "Success",
+	data: dict = None,
 	steps: list[dict] = None,
 ):
 	"""Fakes agent job request and response. Also polls the job.
@@ -130,7 +144,7 @@ def fake_agent_job(
 	with responses.mock, patch.object(
 		AgentJob,
 		"before_insert",
-		fake_agent_job_req(job_name, status, output, steps),
+		fake_agent_job_req(job_name, status, data, steps),
 		create=True,
 	), patch(
 		"press.press.doctype.agent_job.agent_job.frappe.enqueue_doc",
@@ -144,6 +158,8 @@ def fake_agent_job(
 			{}
 		)  # due to bug in FF related to only_if_creator docperm
 		yield
+		global before_insert
+		before_insert = lambda self: None  # noqa
 
 
 @patch.object(AgentJob, "enqueue_http_request", new=Mock())

press/press/doctype/proxy_server/test_proxy_server.py

@@ -17,7 +17,7 @@
 
 
 @patch.object(BaseServer, "after_insert", new=Mock())
-@patch.object(ProxyServer, "validate", new=Mock())
+@patch.object(ProxyServer, "validate_domains", new=Mock())
 def create_test_proxy_server(
 	hostname: str = "n",
 	domain: str = "fc.dev",

press/press/doctype/registry_server/registry_server.js

@@ -8,6 +8,12 @@ frappe.ui.form.on('Registry Server', {
 			[__('Ping Ansible Unprepared'), 'ping_ansible_unprepared', true],
 			[__('Prepare Server'), 'prepare_server', true, !frm.doc.is_server_setup],
 			[__('Setup Server'), 'setup_server', true, !frm.doc.is_server_setup],
+			[
+				__('Update TLS Certificate'),
+				'update_tls_certificate',
+				true,
+				frm.doc.is_server_setup,
+			],
 			[
 				__('Fetch Keys'),
 				'fetch_keys',

press/press/doctype/site_migration/site_migration.js

@@ -3,6 +3,13 @@
 
 frappe.ui.form.on('Site Migration', {
 	refresh: function (frm) {
+		frm.set_query('site', () => {
+			return {
+				filters: {
+					status: 'Active',
+				},
+			};
+		});
 		frm.set_query('source_bench', () => {
 			return {
 				filters: {

press/press/doctype/site_migration/site_migration.py

@@ -199,6 +199,11 @@ def add_steps_for_cluster_migration(self):
 				"method_name": self.backup_source_site.__name__,
 				"status": "Pending",
 			},
+			{
+				"step_title": self.archive_site_on_destination_server.__doc__,
+				"method_name": self.archive_site_on_destination_server.__name__,
+				"status": "Pending",
+			},
 			{
 				"step_title": self.restore_site_on_destination_server.__doc__,
 				"method_name": self.restore_site_on_destination_server.__name__,
@@ -250,6 +255,11 @@ def add_steps_for_server_migration(self):
 				"method_name": self.backup_source_site.__name__,
 				"status": "Pending",
 			},
+			{
+				"step_title": self.archive_site_on_destination_server.__doc__,
+				"method_name": self.archive_site_on_destination_server.__name__,
+				"status": "Pending",
+			},
 			{
 				"step_title": self.restore_site_on_destination_server.__doc__,
 				"method_name": self.restore_site_on_destination_server.__name__,
@@ -312,6 +322,13 @@ def backup_source_site(self):
 
 		return frappe.get_doc("Agent Job", backup.job)
 
+	def archive_site_on_destination_server(self):
+		"""Archive site on destination (case of retry)"""
+		agent = Agent(self.destination_server)
+		site = frappe.get_doc("Site", self.site)
+		site.bench = self.destination_bench
+		return agent.archive_site(site, force=True)
+
 	def restore_site_on_destination_server(self):
 		"""Restore site on destination"""
 		agent = Agent(self.destination_server)
@@ -369,7 +386,9 @@ def reset_site_status_on_destination(self):
 			self.run_next_step()
 			job = None
 		else:
-			job = site.update_site_config({"maintenance_mode": 0})
+			job = site.update_site_config(
+				{"maintenance_mode": 0}
+			)  # will do run_next_step in callback
 		site.reload()
 		site.status = site.status_before_update
 		site.status_before_update = None
@@ -387,11 +406,11 @@ def adjust_plan_if_required(self):
 		destination_server_team = frappe.db.get_value(
 			"Server", self.destination_server, "team"
 		)
-		if site.team != destination_server_team:
+		if site.team == destination_server_team:
+			site.change_plan("Unlimited")
+			self.update_next_step_status("Success")
+		else:
 			self.update_next_step_status("Skipped")
-			return
-		site.change_plan("Unlimited")
-		self.update_next_step_status("Success")
 		self.run_next_step()
 
 

press/press/doctype/site_migration/test_site_migration.py

@@ -2,10 +2,90 @@
 # Copyright (c) 2021, Frappe and Contributors
 # See license.txt
 
+import frappe
+from unittest.mock import patch
 
-# import frappe
-import unittest
+from press.press.doctype.remote_file.remote_file import RemoteFile
 
+from frappe.tests.utils import FrappeTestCase
+from press.press.doctype.agent_job.agent_job import poll_pending_jobs
+from press.press.doctype.agent_job.test_agent_job import fake_agent_job
 
-class TestSiteMigration(unittest.TestCase):
-	pass
+from press.press.doctype.site.test_site import create_test_bench, create_test_site
+
+
+class TestSiteMigration(FrappeTestCase):
+	@patch.object(RemoteFile, "download_link", new="http://test.com")
+	def test_in_cluster_site_migration_goes_through_all_steps_and_updates_site(self):
+		site = create_test_site()
+		bench = create_test_bench()
+		site_migration = frappe.get_doc(
+			{
+				"doctype": "Site Migration",
+				"site": site.name,
+				"destination_bench": bench.name,
+			}
+		).insert()
+
+		with fake_agent_job("Update Site Configuration"), fake_agent_job(
+			"Backup Site",
+			data={
+				"backups": {
+					"database": {
+						"file": "a.sql.gz",
+						"path": "/home/frappe/a.sql.gz",
+						"size": 1674818,
+						"url": "https://a.com/a.sql.gz",
+					},
+					"public": {
+						"file": "b.tar",
+						"path": "/home/frappe/b.tar",
+						"size": 1674818,
+						"url": "https://a.com/b.tar",
+					},
+					"private": {
+						"file": "a.tar",
+						"path": "/home/frappe/a.tar",
+						"size": 1674818,
+						"url": "https://a.com/a.tar",
+					},
+					"site_config": {
+						"file": "a.json",
+						"path": "/home/frappe/a.json",
+						"size": 595,
+						"url": "https://a.com/json",
+					},
+				},
+				"offsite": {
+					"a.sql.gz": "bucket.frappe.cloud/2023-10-10/a.sql.gz",
+					"a.tar": "bucket.frappe.cloud/2023-10-10/a.tar",
+					"b.tar": "bucket.frappe.cloud/2023-10-10/b.tar",
+					"a.json": "bucket.frappe.cloud/2023-10-10/a.json",
+				},
+			},
+		), fake_agent_job("New Site from Backup"), fake_agent_job(
+			"Archive Site"
+		), fake_agent_job(
+			"Remove Site from Upstream"
+		), fake_agent_job(
+			"Add Site to Upstream"
+		), fake_agent_job(
+			"Update Site Configuration"
+		):
+			site_migration.start()
+			poll_pending_jobs()
+			poll_pending_jobs()
+			poll_pending_jobs()
+			poll_pending_jobs()
+			poll_pending_jobs()
+			poll_pending_jobs()
+			poll_pending_jobs()
+			site_migration.reload()
+			self.assertEqual(site_migration.status, "Running")
+			poll_pending_jobs()
+		site_migration.reload()
+		self.assertEqual(site_migration.status, "Success")
+		site.reload()
+		self.assertEqual(site.status, "Active")
+		self.assertEqual(site.bench, bench.name)
+		self.assertEqual(site.server, bench.server)

press/press/doctype/tls_certificate/tls_certificate.py

@@ -163,8 +163,8 @@ def update_server_tls_certifcate(server, certificate):
 			proxysql_admin_password = server.get_password("proxysql_admin_password")
 		ansible = Ansible(
 			playbook="tls.yml",
-			user=server.ssh_user or "root",
-			port=server.ssh_port or 22,
+			user=server.get("ssh_user") or "root",
+			port=server.get("ssh_port") or 22,
 			server=server,
 			variables={
 				"certificate_private_key": certificate.private_key,