From 63f57ff0b2270e8d29f908d5ff045acf74b7e16d Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Jan 10 2018 09:30:51 +0000 Subject: [PATCH 1/2] Add compose state reason to track state change reason --- diff --git a/server/odcs/server/backend.py b/server/odcs/server/backend.py index 4655e94..808f23e 100644 --- a/server/odcs/server/backend.py +++ b/server/odcs/server/backend.py @@ -167,6 +167,7 @@ class RemoveExpiredComposesThread(BackendThread): for compose in composes: log.info("%r: Removing compose", compose) compose.state = COMPOSE_STATES["removed"] + compose.state_reason = "Compose is expired" compose.time_removed = datetime.utcnow() db.session.commit() if not compose.reused_id: @@ -462,6 +463,7 @@ gpgcheck=0 _write_repo_file(compose, repofile) compose.state = COMPOSE_STATES["done"] + compose.state_reason = "Compose is generated successfully" log.info("%r: Compose done", compose) compose.time_done = datetime.utcnow() db.session.add(compose) @@ -514,6 +516,7 @@ def generate_pungi_compose(compose): # If there is no exception generated by the pungi.run(), we know # the compose has been successfully generated. compose.state = COMPOSE_STATES["done"] + compose.state_reason = "Compose is generated successfully" log.info("%r: Compose done", compose) compose.time_done = datetime.utcnow() db.session.add(compose) @@ -569,7 +572,7 @@ def generate_compose(compose_id, lost_compose=False): else: generate_pungi_compose(compose) validate_pungi_compose(compose) - except Exception: + except Exception as e: # Something went wrong, log the exception and update the compose # state in database. if compose: @@ -577,10 +580,12 @@ def generate_compose(compose_id, lost_compose=False): else: log.exception("Error while generating compose %d", compose_id) compose.state = COMPOSE_STATES["failed"] + compose.state_reason = "Error while generating compose: %s" % str(e) compose.time_done = datetime.utcnow() db.session.add(compose) db.session.commit() + compose = Compose.query.filter(Compose.id == compose_id).one() # consolidate duplicate files in compose target dir if compose and compose.reused_id is None and compose.source_type != PungiSourceType.PULP: try: @@ -613,6 +618,7 @@ class ComposerThread(BackendThread): the ThreadPoolExecutor can start working on it. """ compose.state = COMPOSE_STATES["generating"] + compose.state_reason = "Compose thread started" db.session.add(compose) db.session.commit() self.currently_generating.append(compose.id) diff --git a/server/odcs/server/migrations/versions/a8e259e0208c_add_compose_state_reason.py b/server/odcs/server/migrations/versions/a8e259e0208c_add_compose_state_reason.py new file mode 100644 index 0000000..cb733f3 --- /dev/null +++ b/server/odcs/server/migrations/versions/a8e259e0208c_add_compose_state_reason.py @@ -0,0 +1,22 @@ +"""Add Compose.state_reason + +Revision ID: a8e259e0208c +Revises: e2163db7b15d +Create Date: 2018-01-10 15:27:18.492001 + +""" + +# revision identifiers, used by Alembic. +revision = 'a8e259e0208c' +down_revision = 'e2163db7b15d' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('composes', sa.Column('state_reason', sa.String(), nullable=True)) + + +def downgrade(): + op.drop_column('composes', 'state_reason') diff --git a/server/odcs/server/models.py b/server/odcs/server/models.py index 34a11a0..fa0d41e 100644 --- a/server/odcs/server/models.py +++ b/server/odcs/server/models.py @@ -109,6 +109,8 @@ class Compose(ODCSBase): sigkeys = db.Column(db.String) # COMPOSES_STATES state = db.Column(db.Integer, nullable=False, index=True) + # Reason of state change + state_reason = db.Column(db.String, nullable=True) # COMPOSE_RESULTS results = db.Column(db.Integer, nullable=False) # White-space separated list of packages @@ -256,6 +258,7 @@ class Compose(ODCSBase): 'source': self.source, 'state': self.state, 'state_name': INVERSE_COMPOSE_STATES[self.state], + 'state_reason': self.state_reason, 'time_to_expire': self._utc_datetime_to_iso(self.time_to_expire), 'time_submitted': self._utc_datetime_to_iso(self.time_submitted), 'time_done': self._utc_datetime_to_iso(self.time_done), diff --git a/server/tests/test_backend.py b/server/tests/test_backend.py index da46d1d..88c3121 100644 --- a/server/tests/test_backend.py +++ b/server/tests/test_backend.py @@ -32,8 +32,8 @@ from odcs.common.types import COMPOSE_FLAGS, COMPOSE_RESULTS, COMPOSE_STATES from odcs.server.pdc import ModuleLookupError from odcs.server.pungi import PungiSourceType from odcs.server.backend import (resolve_compose, get_reusable_compose, - generate_pulp_compose, validate_pungi_compose, - generate_pungi_compose) + generate_compose, generate_pulp_compose, + generate_pungi_compose, validate_pungi_compose) from odcs.server.utils import makedirs import odcs.server.backend from .utils import ModelsBaseTest @@ -236,6 +236,9 @@ gpgcheck=0 """ _write_repo_file.assert_called_once_with(c, expected_repofile) + self.assertEqual(c.state, COMPOSE_STATES["done"]) + self.assertEqual(c.state_reason, 'Compose is generated successfully') + @patch("odcs.server.pulp.Pulp._rest_post") @patch("odcs.server.backend._write_repo_file") def test_generate_pulp_compose_content_set_not_found( @@ -252,7 +255,12 @@ gpgcheck=0 c = Compose.create( db.session, "me", PungiSourceType.PULP, "foo-1 foo-2", COMPOSE_RESULTS["repository"], 3600) - self.assertRaises(ValueError, generate_pulp_compose, c) + db.session.add(c) + db.session.commit() + + with patch.object(odcs.server.backend.conf, 'pulp_server_url', + "https://localhost/"): + generate_compose(1) expected_query = { "criteria": { @@ -268,6 +276,10 @@ gpgcheck=0 expected_query) _write_repo_file.assert_not_called() + c1 = Compose.query.filter(Compose.id == 1).one() + self.assertEqual(c1.state, COMPOSE_STATES["failed"]) + self.assertRegexpMatches(c1.state_reason, r'Error while generating compose: Failed to find all the content_sets.*') + class TestGeneratePungiCompose(ModelsBaseTest): diff --git a/server/tests/test_models.py b/server/tests/test_models.py index 811db73..b45d3eb 100644 --- a/server/tests/test_models.py +++ b/server/tests/test_models.py @@ -50,7 +50,9 @@ class TestModels(ModelsBaseTest): self.assertTrue(c.time_to_expire) expected_json = {'source_type': 2, 'state': 0, 'time_done': None, - 'state_name': 'wait', 'source': u'testmodule-master', + 'state_name': 'wait', + 'state_reason': None, + 'source': u'testmodule-master', 'owner': u'me', 'result_repo': 'http://localhost/odcs/latest-odcs-1-1/compose/Temporary', 'result_repofile': 'http://localhost/odcs/latest-odcs-1-1/compose/Temporary/odcs-1.repo', diff --git a/server/tests/test_remove_expired_composes_thread.py b/server/tests/test_remove_expired_composes_thread.py index fb387f7..ded5ee3 100644 --- a/server/tests/test_remove_expired_composes_thread.py +++ b/server/tests/test_remove_expired_composes_thread.py @@ -80,6 +80,7 @@ class TestRemoveExpiredComposesThread(ModelsBaseTest): db.session.expunge_all() c = db.session.query(Compose).filter(Compose.id == 1).one() self.assertEqual(c.state, COMPOSE_STATES["removed"]) + self.assertEqual(c.state_reason, 'Compose is expired') def test_does_not_remove_a_compose_which_is_not_expired(self): """ diff --git a/server/tests/test_views.py b/server/tests/test_views.py index 8563f6e..6b0eade 100644 --- a/server/tests/test_views.py +++ b/server/tests/test_views.py @@ -260,7 +260,9 @@ class TestViews(ViewBaseTest): data = json.loads(rv.data.decode('utf8')) expected_json = {'source_type': 2, 'state': 0, 'time_done': None, - 'state_name': 'wait', 'source': u'testmodule-master', + 'state_name': 'wait', + 'state_reason': None, + 'source': u'testmodule-master', 'owner': u'dev', 'result_repo': 'http://localhost/odcs/latest-odcs-%d-1/compose/Temporary' % data['id'], 'result_repofile': 'http://localhost/odcs/latest-odcs-%d-1/compose/Temporary/odcs-%d.repo' % (data['id'], data['id']), @@ -765,7 +767,9 @@ class TestViews(ViewBaseTest): data = json.loads(rv.data.decode('utf8')) expected_json = {'source_type': 2, 'state': 0, 'time_done': None, - 'state_name': 'wait', 'source': u'testmodule-master', + 'state_name': 'wait', + 'state_reason': None, + 'source': u'testmodule-master', 'owner': u'unknown', 'result_repo': 'http://localhost/odcs/latest-odcs-%d-1/compose/Temporary' % data['id'], 'result_repofile': 'http://localhost/odcs/latest-odcs-%d-1/compose/Temporary/odcs-%d.repo' % (data['id'], data['id']), From 96f7be93170087f05318959bd2e9b58e2146faa1 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Jan 10 2018 12:27:10 +0000 Subject: [PATCH 2/2] Do not use expire_all() in _wait_for_compose_state --- diff --git a/server/odcs/server/backend.py b/server/odcs/server/backend.py index 808f23e..531b53c 100644 --- a/server/odcs/server/backend.py +++ b/server/odcs/server/backend.py @@ -585,15 +585,15 @@ def generate_compose(compose_id, lost_compose=False): db.session.add(compose) db.session.commit() - compose = Compose.query.filter(Compose.id == compose_id).one() - # consolidate duplicate files in compose target dir - if compose and compose.reused_id is None and compose.source_type != PungiSourceType.PULP: - try: - log.info("Running hardlink to consolidate duplicate files in compose target dir") - odcs.server.utils.hardlink(conf.target_dir) - except Exception as ex: - # not fail, just show warning message - log.warn("Error while running hardlink on system: %s" % ex.message, exc_info=True) + compose = Compose.query.filter(Compose.id == compose_id).one() + # consolidate duplicate files in compose target dir + if compose and compose.reused_id is None and compose.source_type != PungiSourceType.PULP: + try: + log.info("Running hardlink to consolidate duplicate files in compose target dir") + odcs.server.utils.hardlink(conf.target_dir) + except Exception as ex: + # not fail, just show warning message + log.warn("Error while running hardlink on system: %s" % ex.message, exc_info=True) class ComposerThread(BackendThread):