From e5ab9b3d5dae6935e18919d278569d122b70818a Mon Sep 17 00:00:00 2001 From: Jiri Kyjovsky Date: Mon, 10 Jul 2023 13:22:43 +0200 Subject: [PATCH] frontend: don't show new action task if namespace is deleting itself wait for the end of the deletion of project to avoid race condition when other action like fork or create is called immidiatelly after delete --- frontend/coprs_frontend/coprs/models.py | 1 + .../coprs/views/backend_ns/backend_general.py | 18 +++++++++-- .../coprs_frontend/tests/coprs_test_case.py | 26 ++++++++++++++-- .../test_backend_ns/test_backend_general.py | 30 ++++++++++++++++--- 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/frontend/coprs_frontend/coprs/models.py b/frontend/coprs_frontend/coprs/models.py index 2502783ae..73e0781ae 100644 --- a/frontend/coprs_frontend/coprs/models.py +++ b/frontend/coprs_frontend/coprs/models.py @@ -2129,6 +2129,7 @@ class Action(db.Model, helpers.Serializer): # see ActionTypeEnum action_type = db.Column(db.Integer, nullable=False) # copr, ...; downcase name of class of modified object + # TODO: replace this in the future with enhanced ActionTypeEnum() object_type = db.Column(db.String(20)) # id of the modified object object_id = db.Column(db.Integer) diff --git a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py index 59a11c877..8d180459b 100755 --- a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py +++ b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py @@ -1,5 +1,5 @@ import flask -from copr_common.enums import StatusEnum +from copr_common.enums import StatusEnum, ActionTypeEnum from coprs import db, app from coprs import models from coprs.logic import actions_logic @@ -287,14 +287,28 @@ def build_task_canceled(task_id): @backend_ns.route("/pending-actions/") def pending_actions(): 'get the list of actions backand should take care of' + busy_namespaces = set() data = [] + # waiting repos are ordered for action in actions_logic.ActionsLogic.get_waiting(): + if ( + action.object_type == "copr" + and action.action_type == ActionTypeEnum("delete") + ): + busy_namespaces.add(action.copr.full_name) + elif action.copr and action.copr.full_name in busy_namespaces: + # e.g. copr delete _ && copr fork _; will cause race condition + # so don't process new action with the same namespace until delete + # action is processed + # https://github.com/fedora-copr/copr/issues/2698 + continue + data.append({ 'id': action.id, 'priority': action.priority or action.default_priority, }) - return flask.json.dumps(data) + return flask.json.dumps(data) @backend_ns.route("/action//") diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py b/frontend/coprs_frontend/tests/coprs_test_case.py index 36b10cd36..7eb93d345 100644 --- a/frontend/coprs_frontend/tests/coprs_test_case.py +++ b/frontend/coprs_frontend/tests/coprs_test_case.py @@ -684,13 +684,35 @@ def f_actions(self, f_db): old_value="asd/qwe", new_value=None, result=BackendResultEnum("waiting"), - created_on=int(time.time())) + created_on=int(time.time()), + copr_id=self.c1.id) self.cancel_build_action = models.Action(action_type=ActionTypeEnum("cancel_build"), data=json.dumps({'task_id': 123}), result=BackendResultEnum("waiting"), - created_on=int(time.time())) + created_on=int(time.time()), + copr_id=self.c1.id) self.db.session.add_all([self.delete_action, self.cancel_build_action]) + @pytest.fixture + def f_actions_delete_and_create(self, f_db, f_actions): # pylint: disable=unused-argument + data_dict = { + "ownername": "fooownername", + "projectname": "foocoprname", + "project_dirnames": ["foodirname"], + "chroots": "foochroot", + "appstream": False, + "devel": False, + } + + self.create_action = models.Action(action_type=ActionTypeEnum("createrepo"), + object_type="repository", + object_id=0, + data=json.dumps(data_dict), + result=BackendResultEnum("waiting"), + created_on=int(time.time()), + copr_id=self.c1.id) + self.db.session.add(self.create_action) + @pytest.fixture def f_modules(self): self.m1 = models.Module(name="first-module", stream="foo", version=1, copr_id=self.c1.id, copr=self.c1, diff --git a/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py b/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py index 4c8d9097d..e59376617 100644 --- a/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py +++ b/frontend/coprs_frontend/tests/test_views/test_backend_ns/test_backend_general.py @@ -11,6 +11,7 @@ from coprs import app +# pylint: disable=unused-argument class TestGetBuildTask(CoprsTestCase): def test_module_name_empty(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db): @@ -403,10 +404,7 @@ def test_waiting_actions_only_lists_not_started_or_ended( def test_pending_actions_list(self, f_users, f_coprs, f_actions, f_db): r = self.tc.get("/backend/pending-actions/", headers=self.auth_header) actions = json.loads(r.data.decode("utf-8")) - assert actions == [ - {'id': 1, 'priority': DefaultActionPriorityEnum("delete")}, - {'id': 2, 'priority': DefaultActionPriorityEnum("cancel_build")} - ] + assert actions == [{'id': 1, 'priority': DefaultActionPriorityEnum("delete")}] self.delete_action.result = BackendResultEnum("success") self.db.session.add(self.delete_action) @@ -415,6 +413,27 @@ def test_pending_actions_list(self, f_users, f_coprs, f_actions, f_db): r = self.tc.get("/backend/pending-actions/", headers=self.auth_header) actions = json.loads(r.data.decode("utf-8")) assert len(actions) == 1 + assert actions == [{'id': 2, 'priority': DefaultActionPriorityEnum("cancel_build")}] + + def test_dont_send_pending_actions_whe_delete( + self, f_users, f_coprs, f_actions_delete_and_create, f_db + ): + r = self.tc.get("/backend/pending-actions/", headers=self.auth_header) + actions = json.loads(r.data.decode("utf-8")) + assert actions == [{'id': 1, 'priority': DefaultActionPriorityEnum("delete")}] + + self.delete_action.result = BackendResultEnum("success") + self.db.session.add(self.delete_action) + self.db.session.commit() + + # now other pending tasks are unlocked since delete ended + r = self.tc.get("/backend/pending-actions/", headers=self.auth_header) + actions = json.loads(r.data.decode("utf-8")) + assert len(actions) == 2 + assert sorted(actions, key=lambda d: d["id"]) == [ + {'id': 2, 'priority': DefaultActionPriorityEnum("cancel_build")}, + {'id': 3, 'priority': DefaultActionPriorityEnum("createrepo")}, + ] def test_get_action_succeeded(self, f_users, f_coprs, f_actions, f_db): r = self.tc.get("/backend/action/1/", @@ -576,3 +595,6 @@ def test_importing_queue_multiple_bg(self, f_users, f_coprs, f_mock_chroots, f_d data = json.loads(r.data.decode("utf-8")) assert data[0]["srpm_url"] == "http://foo" assert data[1]["srpm_url"] == "http://bar" + + +# pylint: enable=unused-argument