From e11b925cee5f23205ca6b59a7238fb33c3047fad Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Aug 31 2020 08:50:31 +0000 Subject: Do not remove compose if latest-* symlink points to it. There are two reasons for this: - The compose can still be reused by future composes to significantly reduce their generation time. If we remove it after 24 hours, next compose will have to generate everything from scratch again. - For periodic composes, we always want to have the latest compose available so other service can point to this location. This is needed for Fedora ELN or internal Integration composes. --- diff --git a/server/odcs/server/backend.py b/server/odcs/server/backend.py index 45f62c9..7298f59 100644 --- a/server/odcs/server/backend.py +++ b/server/odcs/server/backend.py @@ -218,6 +218,11 @@ class RemoveExpiredComposesThread(BackendThread): if not os.path.exists(compose.target_dir): continue + # Do not remove the latest successfull compose. + if get_latest_symlink(compose): + log.info("%r: Not removing compose, the latest-* symlink points to it.") + continue + log.info("%r: Removing compose", compose) if compose.removed_by: state_reason = "Removed by {}.".format(compose.removed_by) @@ -837,6 +842,27 @@ def generate_compose_symlink(compose): os.symlink(compose_dir, symlink) +def get_latest_symlink(compose): + """ + Returns the latest-* symlink associated with this compose or None + if it does not exists. + """ + # Ignore old composes which do not have the compose_type and pungi_compose_id set. + if not compose.compose_type or not compose.pungi_compose_id: + return None + + symlink_dir = os.path.join(compose.target_dir, compose.compose_type) + symlink = os.path.join(symlink_dir, compose.pungi_compose_id) + latest_name = "latest-%s" % "-".join(compose.pungi_compose_id.split("-")[:2]) + latest_symlink = os.path.join(symlink_dir, latest_name) + + # Return Non if `latest_symlink` points to the different dir than `symlink`. + if os.path.realpath(symlink) != os.path.realpath(latest_symlink): + return None + + return latest_symlink + + def remove_compose_symlink(compose): """ Removes non-latest symlink generated by the `generate_compose_symlink`. @@ -848,14 +874,7 @@ def remove_compose_symlink(compose): symlink_dir = os.path.join(compose.target_dir, compose.compose_type) symlink = os.path.join(symlink_dir, compose.pungi_compose_id) - - # Check if latest_symlink points to the same directory as the non-latest - # symlink. In this case, we will remove the latest-symlink later too. - latest_name = "latest-%s" % "-".join(compose.pungi_compose_id.split("-")[:2]) - latest_symlink = os.path.join(symlink_dir, latest_name) - remove_latest_symlink = os.path.realpath(symlink) == os.path.realpath( - latest_symlink - ) + latest_symlink = get_latest_symlink(compose) # Remove non-latest symlink. log.info("%r: Removing %s symlink.", compose, symlink) @@ -866,7 +885,7 @@ def remove_compose_symlink(compose): raise # Remove latest symlink. - if remove_latest_symlink: + if latest_symlink: log.info("%r: Removing %s symlink.", compose, latest_symlink) try: os.unlink(latest_symlink) diff --git a/server/tests/test_remove_expired_composes_thread.py b/server/tests/test_remove_expired_composes_thread.py index be7e3b9..2c6777d 100644 --- a/server/tests/test_remove_expired_composes_thread.py +++ b/server/tests/test_remove_expired_composes_thread.py @@ -76,10 +76,14 @@ class TestRemoveExpiredComposesThread(ModelsBaseTest): @patch("os.unlink") @patch("os.path.realpath") @patch("os.path.exists") - def test_a_compose_which_state_is_done_is_removed(self, exists, realpath, unlink): + @patch("odcs.server.backend.get_latest_symlink") + def test_a_compose_which_state_is_done_is_removed( + self, latest_symlink, exists, realpath, unlink + ): """ Test that we do remove a compose in done state. """ + latest_symlink.return_value = None realpath.return_value = "/odcs-real" c = db.session.query(Compose).filter(Compose.id == 1).one() c.time_to_expire = datetime.utcnow() - timedelta(seconds=120) @@ -98,11 +102,30 @@ class TestRemoveExpiredComposesThread(ModelsBaseTest): mock.call( AnyStringWith("test_composes/nightly/compose-1-10-2020110.n.0") ), - mock.call(AnyStringWith("test_composes/nightly/latest-compose-1")), mock.call(AnyStringWith("test_composes/odcs-1")), ] ) + @patch("os.unlink") + @patch("os.path.realpath") + @patch("os.path.exists") + def test_latest_compose_not_removed(self, exists, realpath, unlink): + """ + Test that we do remove a compose in done state. + """ + realpath.return_value = "/odcs-real" + c = db.session.query(Compose).filter(Compose.id == 1).one() + c.time_to_expire = datetime.utcnow() - timedelta(seconds=120) + c.state = COMPOSE_STATES["done"] + c.compose_type = "nightly" + c.pungi_compose_id = "compose-1-10-2020110.n.0" + db.session.add(c) + db.session.commit() + self.thread.do_work() + db.session.expunge_all() + c = db.session.query(Compose).filter(Compose.id == 1).one() + self.assertEqual(c.state, COMPOSE_STATES["done"]) + def test_a_compose_which_state_is_done_is_removed_keep_state_reason(self): """ Test that we do remove a compose in done state.