PR#1401: buildonly support - fm-orchestrator

fm-orchestrator

#1401 buildonly support

Merged 4 years ago by vmaljulin. Opened 4 years ago by vmaljulin.

vmaljulin/fm-orchestrator FACTORY-4731 into master

buildonly support

Valerij Maljulin • 4 years ago

689d949

module_build_service/migrations/versions/4d1e2e13e514_buildonly.py

file added

+23

		`@@ -0,0 +1,23 @@`
		`+ """Add buildonly column`
		`+`
		`+ Revision ID: 4d1e2e13e514`
		`+ Revises: 1817e62719f9`
		`+ Create Date: 2019-10-01 13:49:20.446641`
		`+`
		`+ """`
		`+`
		`+ # revision identifiers, used by Alembic.`
		`+ revision = '4d1e2e13e514'`
		`+ down_revision = '1817e62719f9'`
		`+`
		`+ from alembic import op`
		`+ import sqlalchemy as sa`
		`+`
		`+`
		`+ def upgrade():`
		`+ op.add_column("component_builds", sa.Column("buildonly", sa.Boolean(), nullable=True))`
		`+`
		`+`
		`+ def downgrade():`
		`+ with op.batch_alter_table("component_builds", schema=None) as batch_op:`
		`+ batch_op.drop_column("buildonly")`

module_build_service/models.py

file modified

		`@@ -1219,6 +1219,8 @@`
		`# True when this component build is build-time only (should be tagged only`
		`# to -build tag)`
		`build_time_only = db.Column(db.Boolean, default=False)`
		`+ # True if buildonly is True in modulemd`
		`+ buildonly = db.Column(db.Boolean, default=False)`

		`# A monotonically increasing integer that represents which batch or`
		`# iteration this component is currently in. This relates to the owning`

module_build_service/scheduler/handlers/components.py

file modified

+16 -3

		`@@ -3,11 +3,11 @@`
		`""" Handlers for koji component build events on the message bus. """`

		`import logging`
		`-`
		`- import module_build_service.builder`
		`-`
		`import koji`
		`+ import module_build_service.builder`

		`+ from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder`
		`+ from module_build_service.utils.general import mmd_to_str`
		`from module_build_service import models, log, messaging`

		`logging.basicConfig(level=logging.DEBUG)`
		`@@ -56,6 +56,19 @@`
		`db_session.commit()`
		`return`

		`+ if (`
		`+ component_build.buildonly`
		`+ and config.system in ["koji", "test"]`
		`+ and state == koji.BUILD_STATES["COMPLETE"]`
		`+ ):`
		`+ koji_session = KojiModuleBuilder.get_session(config)`
		`+ rpms = koji_session.listBuildRPMs(component_build.nvr)`
		`+ mmd = parent.mmd()`
		`+ for artifact in rpms:`
		`+ mmd.add_rpm_filter(artifact["name"])`
		`+ parent.modulemd = mmd_to_str(mmd)`
		`+ db_session.commit()`
		`+`
		`further_work = []`

		`parent_current_batch = parent.current_batch()`

module_build_service/utils/submit.py

file modified

+1 -2

		`@@ -175,8 +175,6 @@`
		`"Custom component caches aren't allowed. "`
		`"%r bears cache %r" % (pkgname, pkg.get_cache())`
		`)`
		`- if pkg.get_buildonly() is True:`
		`- raise ValidationError('The usage of "buildonly" is not yet supported')`
		`if pkg.get_buildafter():`
		`raise ValidationError('The usage of "buildafter" is not yet supported')`
		`if not pkg.get_repository():`
		`@@ -508,6 +506,7 @@`
		`batch=batch,`
		`ref=component_ref,`
		`weight=rpm_weights[package],`
		`+ buildonly=component.get_buildonly()`
		`)`
		`db_session.add(build)`

tests/staged_data/testmodule_v2_buildonly.yaml

file added

+39

		`@@ -0,0 +1,39 @@`
		`+ document: modulemd`
		`+ version: 2`
		`+ data:`
		`+ summary: A test module in all its beautiful beauty`
		`+ description: >-`
		`+ This module demonstrates how to write simple modulemd files And can be used for`
		`+ testing the build and release pipeline. ’`
		`+ license:`
		`+ module:`
		`+ - MIT`
		`+ dependencies:`
		`+ - buildrequires:`
		`+ platform: [f28]`
		`+ requires:`
		`+ platform: [f28]`
		`+ references:`
		`+ community: https://docs.pagure.org/modularity/`
		`+ documentation: https://fedoraproject.org/wiki/Fedora_Packaging_Guidelines_for_Modules`
		`+ profiles:`
		`+ default:`
		`+ rpms:`
		`+ - tangerine`
		`+ api:`
		`+ rpms:`
		`+ - perl-Tangerine`
		`+ - tangerine`
		`+ components:`
		`+ rpms:`
		`+ perl-List-Compare:`
		`+ rationale: A dependency of tangerine.`
		`+ ref: master`
		`+ buildonly: true`
		`+ perl-Tangerine:`
		`+ rationale: Provides API for this module and is a dependency of tangerine.`
		`+ ref: master`
		`+ tangerine:`
		`+ rationale: Provides API for this module.`
		`+ ref: master`
		`+ buildorder: 10`

tests/test_build/test_build.py

file modified

+69 -1

		`@@ -18,7 +18,7 @@`
		`from module_build_service import models, conf, build_logs`
		`from module_build_service.scheduler import make_simple_stop_condition`

		`- from mock import patch, PropertyMock, Mock`
		`+ from mock import patch, PropertyMock, Mock, MagicMock`
		`from werkzeug.datastructures import FileStorage`
		`import kobo`
		`import pytest`
		`@@ -497,6 +497,74 @@`
		`assert module_build.module_builds_trace[4].state == models.BUILD_STATES["ready"]`
		`assert len(module_build.module_builds_trace) == 5`

		`+ @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session")`
		`+ @patch("module_build_service.auth.get_user", return_value=user)`
		`+ @patch("module_build_service.scm.SCM")`
		`+ def test_submit_build_buildonly(`
		`+ self, mocked_scm, mocked_get_user, mocked_get_session, conf_system, dbg, hmsc, db_session`
		`+ ):`
		`+ def list_build_rpms(nvr):`
		`+ assert nvr == "perl-List-Compare-1-1"`
		`+ return [{"name": "perl-List-Compare"}]`
		`+ koji_session = MagicMock()`
		`+ koji_session.listBuildRPMs = list_build_rpms`
		`+ mocked_get_session.return_value = koji_session`
		`+`
		`+ yaml_file = "testmodule_v2_buildonly.yaml"`
		`+ FakeSCM(mocked_scm, "testmodule", yaml_file, "620ec77321b2ea7b0d67d82992dda3e1d67055b4")`
		`+`
		`+ rv = self.client.post(`
		`+ "/module-build-service/1/module-builds/",`
		`+ data=json.dumps({`
		`+ "branch": "master",`
		`+ "scmurl": "https://src.stg.fedoraproject.org/modules/"`
		`+ "testmodule.git?#620ec77321b2ea7b0d67d82992dda3e1d67055b4",`
		`+ }),`
		`+ )`
		`+`
		`+ data = json.loads(rv.data)`
		`+ module_build_id = data["id"]`
		`+`
		`+ # Check that components are tagged after the batch is built.`
		`+ tag_groups = [`
		`+ {"perl-Tangerine-1-1", "perl-List-Compare-1-1"},`
		`+ {"tangerine-1-1"},`
		`+ ]`
		`+`
		`+ def on_finalize_cb(cls, succeeded):`
		`+ assert succeeded is True`
		`+`
		`+ def on_tag_artifacts_cb(cls, artifacts, dest_tag=True):`
		`+ assert tag_groups.pop(0) == set(artifacts)`
		`+`
		`+ FakeModuleBuilder.on_finalize_cb = on_finalize_cb`
		`+ FakeModuleBuilder.on_tag_artifacts_cb = on_tag_artifacts_cb`
		`+`
		`+ # Check that the components are added to buildroot after the batch`
		`+ # is built.`
		`+ buildroot_groups = [`
		`+ {"module-build-macros-1-1"},`
		`+ {"perl-Tangerine-1-1", "perl-List-Compare-1-1"},`
		`+ {"tangerine-1-1"},`
		`+ ]`
		`+`
		`+ def on_buildroot_add_artifacts_cb(cls, artifacts, install):`
		`+ assert buildroot_groups.pop(0) == set(artifacts)`
		`+`
		`+ FakeModuleBuilder.on_buildroot_add_artifacts_cb = on_buildroot_add_artifacts_cb`
		`+`
		`+ self.run_scheduler(db_session)`
		`+`
		`+ # All components should be built and module itself should be in "done"`
		`+ # or "ready" state.`
		`+ for build in db_session.query(models.ComponentBuild).filter_by(`
		`+ module_id=module_build_id).all():`
		`+ assert build.state == koji.BUILD_STATES["COMPLETE"]`
		`+ assert build.module_build.state in [`
		`+ models.BUILD_STATES["done"],`
		`+ models.BUILD_STATES["ready"],`
		`+ ]`
		`+`
		`@pytest.mark.parametrize("gating_result", (True, False))`
		`@patch("module_build_service.auth.get_user", return_value=user)`
		`@patch("module_build_service.scm.SCM")`

tests/test_models/test_models.py

file modified

		`@@ -24,6 +24,7 @@`
		`nvr="before_models_committed-0.0.0-0.module_before_models_committed_0_0",`
		`batch=1,`
		`module_id=1,`
		`+ buildonly=True,`
		`)`

		`db_session.add(component_build)`

vmaljulin commented 4 years ago

This fixes #1307

Signed-off-by: Valerij Maljulin vmaljuli@redhat.com

rebased onto c176a2cbd74b88435b56d360e875bb64f144bd58

4 years ago

jkaluza commented on line 5 of module_build_service/models.py 4 years ago

You need to generate database migration scripts if you change the DB schema.

@mprahl, how do you normally generate this? I have some script to do that no matter what project I'm in, but I'm looking for some more standard way of doing so.

jkaluza commented on line 21 of module_build_service/scheduler/handlers/components.py 4 years ago

It looks good to me, can you write test for this bit?

factory2jenkins commented 4 years ago

Build #386 failed (commit: c176a2cbd74b88435b56d360e875bb64f144bd58).
Rebase or make new commits to rebuild.

mprahl commented 4 years ago

You need to generate database migration scripts if you change the DB schema.
@mprahl, how do you normally generate this? I have some script to do that no matter what project I'm in, but I'm looking for some more standard way of doing so.

I usually delete any local databases, run mbs-manager upgradedb on the master branch (no changes made), and then run mbs-manager db migrate.

Edited 4 years ago by mprahl

cqi commented on line 21 of module_build_service/scheduler/handlers/components.py 4 years ago

libmodulemd spec v2 tells that "... after the build is complete"[1]. state check should be added in this if branch because it does not make sense to run following lines of code for a FAILED or CANCELED component.

buildonly should probably be handled as earlier as possible in this event handler method and just after:

if component_build.package == "module-build-macros" and state != koji.BUILD_STATES["COMPLETE"]:

See line 118[2], if there is any failed component in current branch, _finalize returns immediately even if current component is built successfully. Hence, no chance to add built RPMs to data.filter.rpms. @jkaluza, I'm not sure if this could affect module build resubmit and the component reuse.

Can add_rpm_filter deduplicate or just ignore the name which was added already?

[1] https://github.com/fedora-modularity/libmodulemd/blob/master/spec.v2.yaml#L285
[2] https://pagure.io/fm-orchestrator/blob/master/f/module_build_service/scheduler/handlers/components.py#_118

cqi commented on line 21 of module_build_service/scheduler/handlers/components.py 4 years ago

Hmm, component reuse should also handle buildonly as well. If component A will be reused and component.buildonly is True and component A will not be rebuilt, the component's built RPMs should be added to new module build's data.filter.rpms. @jkaluza, please correct me if I'm wrong.

1 new commit added

fixup! buildonly support This fixes #1307

4 years ago

rebased onto db14675ba6e4d25f73c98acaf8617f3335e0b01c

4 years ago

factory2jenkins commented 4 years ago

Build #430 failed (commit: 15b7293c3e0f3d213866e1972b7ddc2b4d5c8258).
Rebase or make new commits to rebuild.

rebased onto f71a847a81451aafae5e276a1392edf2c6623ab7

4 years ago

factory2jenkins commented 4 years ago

Build #445 failed (commit: 6d515ed0526f029d1267959b9b0f376df0c08833).
Rebase or make new commits to rebuild.

rebased onto 103ae5a64a81ea6a4d2f250be0143e6ea291e4f9

4 years ago

vmaljulin commented 4 years ago

db migration scripts were added

jkaluza commented on line 2 of module_build_service/migrations/versions/4d1e2e13e514_.py 4 years ago

Add message here, for example Add buildonly column.

jkaluza commented on line 19 of module_build_service/migrations/versions/4d1e2e13e514_.py 4 years ago

Remove these comments please.

jkaluza commented on line 24 of module_build_service/migrations/versions/4d1e2e13e514_.py 4 years ago

SQLite cannot remove column like this, you need to use:

    with op.batch_alter_table("component_builds", schema=None) as batch_op:
        batch_op.drop_column("buildonly")

jkaluza commented on line 20 of module_build_service/scheduler/handlers/components.py 4 years ago

This code should run only when "koji" builder is used:

if conf.system in ["koji", "test"]:

rebased onto 75a9c406c903417aad6c98b272daeed9f0845506

4 years ago

rebased onto 2de95d03c8f78a9dcf849c69b602de75119a6a70

4 years ago

vmaljulin commented 4 years ago

Comments were addressed

factory2jenkins commented 4 years ago

Build #461 failed (commit: 2de95d03c8f78a9dcf849c69b602de75119a6a70).
Rebase or make new commits to rebuild.

rebased onto 1c4b9f082150b9c133675e4da5854c6cca85be5d

4 years ago

jkaluza commented on line 20 of module_build_service/scheduler/handlers/components.py 4 years ago

There are too many spaces here I think. Can you reformat it like this?

    if (
        component_build.buildonly
        and config.system in ["koji", "test"]
        and state == koji.BUILD_STATES["COMPLETE"]
    ):

jkaluza commented 4 years ago

@vmaljulin, just one more formatting issue and we are done I think :).

rebased onto 689d949

4 years ago

vmaljulin commented 4 years ago

@vmaljulin, just one more formatting issue and we are done I think :).

Done

Pull-Request has been merged by vmaljulin

4 years ago

Metadata

Assignee

None

Tags

No Tags

fm-orchestrator

Source Code

#1401 buildonly support Merged 4 years ago by vmaljulin. Opened 4 years ago by vmaljulin. vmaljulin/fm-orchestrator FACTORY-4731 into master

Metadata

Flags

Changes Summary 7

#1401 buildonly support

Merged 4 years ago by vmaljulin. Opened 4 years ago by vmaljulin.

vmaljulin/fm-orchestrator FACTORY-4731 into master