From ab70329c354ebc74b699e92f1928573e3fea1de6 Mon Sep 17 00:00:00 2001 From: Jakub Kadlčík Date: Apr 23 2019 09:04:45 +0000 Subject: [PATCH 1/3] [frontend] build is not finished when not even SRPM is finished When build doesn't have any build_chroots, it is in the phase of building SRPM package. That obviously means, that the build itselfis not finished yet. --- diff --git a/frontend/coprs_frontend/coprs/models.py b/frontend/coprs_frontend/coprs/models.py index 391aa14..de82c34 100644 --- a/frontend/coprs_frontend/coprs/models.py +++ b/frontend/coprs_frontend/coprs/models.py @@ -1021,7 +1021,12 @@ class Build(db.Model, helpers.Serializer): Build is finished only if all its build_chroots are in finished state or the build was canceled. """ - return self.canceled or all([chroot.finished for chroot in self.build_chroots]) + if self.canceled: + return True + if not self.build_chroots: + # Not even SRPM is finished + return False + return all([chroot.finished for chroot in self.build_chroots]) @property def persistent(self): diff --git a/frontend/coprs_frontend/tests/test_models.py b/frontend/coprs_frontend/tests/test_models.py index dc105bd..f1ff059 100644 --- a/frontend/coprs_frontend/tests/test_models.py +++ b/frontend/coprs_frontend/tests/test_models.py @@ -1,3 +1,4 @@ +from copr_common.enums import StatusEnum from tests.coprs_test_case import CoprsTestCase @@ -42,3 +43,19 @@ class TestBuildModel(CoprsTestCase): # even though we blacklised (by mistake) all chroots, package builds # against all chroots (fallback) assert len(list(self.p1.chroots)) == 15 + + def test_finished(self, f_users, f_coprs, f_mock_chroots, f_builds, f_db): + self.b1.build_chroots[0].status = StatusEnum("pending") + assert not self.b1.finished + + self.b1.build_chroots[0].status = StatusEnum("succeeded") + assert self.b1.finished + + self.b1.build_chroots[0].status = StatusEnum("running") + assert not self.b1.finished + + self.b1.build_chroots[0].status = StatusEnum("failed") + assert self.b1.finished + + self.b1.source_status = StatusEnum("canceled") + assert self.b1.finished From ee4e16188101755bf05845ddd0017bde00469799 Mon Sep 17 00:00:00 2001 From: Jakub Kadlčík Date: Apr 23 2019 09:04:45 +0000 Subject: [PATCH 2/3] [frontend] add mechanism to block build batch until other one finishes --- diff --git a/frontend/coprs_frontend/alembic/schema/versions/8ae65946df53_add_blocked_by_column_for_batch.py b/frontend/coprs_frontend/alembic/schema/versions/8ae65946df53_add_blocked_by_column_for_batch.py new file mode 100644 index 0000000..4e7f967 --- /dev/null +++ b/frontend/coprs_frontend/alembic/schema/versions/8ae65946df53_add_blocked_by_column_for_batch.py @@ -0,0 +1,24 @@ +"""Add blocked_by column for batch + +Revision ID: 8ae65946df53 +Revises: b64659389c54 +Create Date: 2019-03-27 18:38:03.758974 + +""" + +# revision identifiers, used by Alembic. +revision = '8ae65946df53' +down_revision = '9bc8681ed275' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('batch', sa.Column('blocked_by_id', sa.Integer(), nullable=True)) + op.create_foreign_key('batch_blocked_by_id_fkey', 'batch', 'batch', ['blocked_by_id'], ['id']) + + +def downgrade(): + op.drop_constraint('batch_blocked_by_id_fkey', 'batch', type_='foreignkey') + op.drop_column('batch', 'blocked_by_id') diff --git a/frontend/coprs_frontend/coprs/models.py b/frontend/coprs_frontend/coprs/models.py index de82c34..a01e9e1 100644 --- a/frontend/coprs_frontend/coprs/models.py +++ b/frontend/coprs_frontend/coprs/models.py @@ -1029,6 +1029,10 @@ class Build(db.Model, helpers.Serializer): return all([chroot.finished for chroot in self.build_chroots]) @property + def blocked(self): + return bool(self.batch and self.batch.blocked_by and not self.batch.blocked_by.finished) + + @property def persistent(self): """ Find out if this build is persistent. @@ -1445,6 +1449,12 @@ class Group(db.Model, helpers.Serializer): class Batch(db.Model): id = db.Column(db.Integer, primary_key=True) + blocked_by_id = db.Column(db.Integer, db.ForeignKey("batch.id"), nullable=True) + blocked_by = db.relationship("Batch", remote_side=[id]) + + @property + def finished(self): + return all([b.finished for b in self.builds]) class Module(db.Model, helpers.Serializer): 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 bed6289..fd75f14 100644 --- a/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py +++ b/frontend/coprs_frontend/coprs/views/backend_ns/backend_general.py @@ -183,8 +183,9 @@ def pending_jobs(): """ Return the job queue. """ + srpm_tasks = [build for build in BuildsLogic.get_pending_srpm_build_tasks() if not build.blocked] build_records = ([get_build_record(task) for task in BuildsLogic.get_pending_build_tasks()] + - [get_srpm_build_record(task) for task in BuildsLogic.get_pending_srpm_build_tasks()]) + [get_srpm_build_record(task) for task in srpm_tasks]) log.info('Selected build records: {}'.format(build_records)) return flask.jsonify(build_records) diff --git a/frontend/coprs_frontend/tests/coprs_test_case.py b/frontend/coprs_frontend/tests/coprs_test_case.py index 3c003c3..31fdfaf 100644 --- a/frontend/coprs_frontend/tests/coprs_test_case.py +++ b/frontend/coprs_frontend/tests/coprs_test_case.py @@ -443,6 +443,13 @@ class CoprsTestCase(object): copr=self.c1, copr_dir=self.c4_dir, name="hello-world", source_type=0) + @pytest.fixture + def f_batches(self): + self.batch1 = models.Batch() + self.batch2 = models.Batch() + self.batch3 = models.Batch() + self.batch4 = models.Batch() + def request_rest_api_with_auth(self, url, login=None, token=None, content=None, method="GET", 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 a2323fc..0df50d4 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 @@ -1,6 +1,6 @@ import json -from copr_common.enums import BackendResultEnum +from copr_common.enums import BackendResultEnum, StatusEnum from tests.coprs_test_case import CoprsTestCase from coprs.logic.builds_logic import BuildsLogic @@ -47,6 +47,22 @@ class TestWaitingBuilds(CoprsTestCase): data = json.loads(r.data.decode("utf-8")) assert data[0]["build_id"] == 3 + def test_pending_blocked_builds(self, f_users, f_coprs, f_mock_chroots, f_builds, f_batches, f_db): + for build in [self.b2, self.b3, self.b4]: + build.source_status = StatusEnum("pending") + + self.b2.batch = self.batch2 + self.b3.batch = self.batch3 + self.batch3.blocked_by = self.batch2 + self.db.session.commit() + + r = self.tc.get("/backend/pending-jobs/") + data = json.loads(r.data.decode("utf-8")) + + ids = [job["build_id"] for job in data] + assert self.b3.id not in ids + assert {self.b2.id, self.b4.id}.issubset(ids) + # status = 0 # failure # status = 1 # succeeded From 5f9b011aa70409e5340baf24476a0d43480dcc01 Mon Sep 17 00:00:00 2001 From: Jakub Kadlčík Date: Apr 23 2019 09:04:45 +0000 Subject: [PATCH 3/3] [frontend] respect module buildorder by setting dependencies among batches --- diff --git a/frontend/coprs_frontend/coprs/logic/modules_logic.py b/frontend/coprs_frontend/coprs/logic/modules_logic.py index 4ca5e99..e930f4d 100644 --- a/frontend/coprs_frontend/coprs/logic/modules_logic.py +++ b/frontend/coprs_frontend/coprs/logic/modules_logic.py @@ -111,8 +111,10 @@ class ModuleBuildFacade(object): return [batches[number] for number in sorted(batches.keys())] def add_builds(self, rpms, module): + blocked_by_id = None for group in self.get_build_batches(rpms): batch = models.Batch() + batch.blocked_by_id = blocked_by_id db.session.add(batch) for pkgname, rpm in group.items(): clone_url = self.get_clone_url(pkgname, rpm) @@ -123,6 +125,9 @@ class ModuleBuildFacade(object): build.module_id = module.id db.session.add(build) + # Every batch needs to by blocked by the previous one + blocked_by_id = batch.id + def get_clone_url(self, pkgname, rpm): if rpm.get_repository(): return rpm.get_repository() diff --git a/frontend/coprs_frontend/tests/test_logic/test_modules_logic.py b/frontend/coprs_frontend/tests/test_logic/test_modules_logic.py index d31536e..f6e2a2b 100644 --- a/frontend/coprs_frontend/tests/test_logic/test_modules_logic.py +++ b/frontend/coprs_frontend/tests/test_logic/test_modules_logic.py @@ -56,6 +56,19 @@ class TestModuleBuildFacade(CoprsTestCase): assert batches[1] == {"tomcatjss": pkg2, "ldapjdk": pkg3} assert batches[2] == {"pki-core": pkg4, "dogtag-pki": pkg5} + def test_add_builds_batches(self, f_users, f_coprs, f_mock_chroots, f_builds, f_modules, f_db): + pkg1 = Modulemd.ComponentRpm(name="foo", rationale="foo package") + pkg2 = Modulemd.ComponentRpm(name="bar", rationale="bar package", buildorder=10) + pkg3 = Modulemd.ComponentRpm(name="baz", rationale="baz package", buildorder=10) + + generator = ModulemdGenerator(name="testmodule", stream="master", version=123, summary="some summary") + facade = ModuleBuildFacade(self.u1, self.c1, generator.generate(), "testmodule.yaml") + facade.add_builds({"foo": pkg1, "bar": pkg2, "baz": pkg3}, self.m1) + + b1, b2, b3 = self.m1.builds + assert b1.batch != b2.batch == b3.batch + assert b2.batch.blocked_by == b1.batch + class TestModulemdGenerator(CoprsTestCase): config = {"DIST_GIT_URL": "http://distgiturl.org"}