modules/fm-orchestrator
1
0
Fork 0
mirror of https://pagure.io/fm-orchestrator.git synced 2026-04-13 17:29:45 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Tweak behavior of get_compatible_base_module_modulemds

Browse Source 
Fix some oddities in the DBResolver implementation of
get_compatible_base_module_modulemds() and make the MBSResolver version -
which was previously just buggy - match that. (Tests for the MBSResolver
version are added in a subsequent commit.)

 * If an empty virtual_streams argument was passed in, *all* streams
   were considered compatible. Throw an exception in this case - it
   should be considered an error.
 * If stream_version_lte=True, but the stream from the base module
   wasn't in the form FOOx.y.z, then throw an exception. This was
   previously treated like stream_version_lte=False, which is just
   a recipe for confusion and mistakes.

test_get_reusable_module_use_latest_build() is rewritten to
comprehensively test all possibilities, including the case that changed
above.

test_add_default_modules_compatible_platforms() is changed to run
under allow_only_compatible_base_modules=False, since it expected
Fedora-style virtual streams (versions not in FOOx.y.z form, all
share the same stream), which doesn't make sense with
allow_only_compatible_base_modules=True.
This commit is contained in:
Owen W. Taylor
2020-11-20 15:49:14 -05:00
parent c4230a352d
commit 7761bbf5b4
6 changed files with 222 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
module_build_service/common/errors.py
View File

@@ -21,6 +21,10 @@ class UnprocessableEntity(ValueError):
pass
class StreamNotXyz(UnprocessableEntity):
pass
class Conflict(ValueError):
pass

25
module_build_service/resolver/DBResolver.py
View File

@@ -6,7 +6,7 @@ import sqlalchemy
from sqlalchemy.orm import aliased
from module_build_service.common import log, models
from module_build_service.common.errors import UnprocessableEntity
from module_build_service.common.errors import StreamNotXyz, UnprocessableEntity
from module_build_service.common.utils import load_mmd
from module_build_service.resolver.base import GenericResolver
@@ -111,26 +111,31 @@ class DBResolver(GenericResolver):
:param stream_version_lte: If True, the compatible streams are limited
by the stream version computed from `stream`. If False, even the
modules with higher stream version are returned.
:param virtual_streams: List of virtual streams. If set, also modules
with incompatible stream version are returned in case they share
one of the virtual streams.
:param virtual_streams: List of virtual streams.
Modules are returned if they share one of the virtual streams.
:param states: List of states the returned compatible modules should
be in.
:return list: List of Modulemd objects.
"""
if not virtual_streams:
raise RuntimeError("Virtual stream list must not be empty")
name = base_module_mmd.get_module_name()
stream = base_module_mmd.get_stream_name()
builds = []
stream_version = None
if stream_version_lte:
stream_in_xyz_format = len(str(models.ModuleBuild.get_stream_version(
stream, right_pad=False))) >= 5
if stream_in_xyz_format:
stream_version = models.ModuleBuild.get_stream_version(stream)
else:
log.warning(
"Cannot get compatible base modules, because stream_version_lte is used, "
"but stream %r is not in x.y.z format." % stream)
if not stream_in_xyz_format:
raise StreamNotXyz(
"Cannot get compatible base modules, because stream of resolved "
"base module %s:%s is not in x.y.z format." % (
base_module_mmd.get_module_name(), stream
))
stream_version = models.ModuleBuild.get_stream_version(stream)
builds = models.ModuleBuild.get_last_builds_in_stream_version_lte(
self.db_session, name, stream_version, virtual_streams, states)

114
module_build_service/resolver/MBSResolver.py
View File

@@ -9,7 +9,7 @@ import kobo.rpmlib
from module_build_service.common import models
from module_build_service.common.config import conf
from module_build_service.common.errors import UnprocessableEntity
from module_build_service.common.errors import StreamNotXyz, UnprocessableEntity
from module_build_service.common.request_utils import requests_session
from module_build_service.common.utils import load_mmd, import_mmd
from module_build_service.resolver.KojiResolver import KojiResolver
@@ -38,11 +38,12 @@ class MBSResolver(KojiResolver):
states = states or ["ready"]
query = {
"name": name,
"stream": stream,
"state": states,
"verbose": True,
"order_desc_by": "version",
}
if stream is not None:
query["stream"] = stream
if version is not None:
query["version"] = str(version)
if context is not None:
@@ -145,55 +146,11 @@ class MBSResolver(KojiResolver):
if data["items"]:
return load_mmd(data["items"][0]["modulemd"])
def get_module_modulemds(
self,
name,
stream,
version=None,
context=None,
strict=False,
stream_version_lte=False,
virtual_streams=None,
states=None,
):
"""
Gets the module modulemds from the resolver.
:param name: a string of the module's name
:param stream: a string of the module's stream
:param version: a string or int of the module's version. When None, latest version will
be returned.
:param context: a string of the module's context. When None, all contexts will
be returned.
:kwarg strict: Normally this function returns [] if no module can be
found. If strict=True, then a UnprocessableEntity is raised.
:kwarg stream_version_lte: If True and if the `stream` can be transformed to
"stream version", the returned list will include all the modules with stream version
less than or equal the stream version computed from `stream`.
:kwarg virtual_streams: a list of the virtual streams to filter on. The filtering uses "or"
logic. When falsy, no filtering occurs.
:return: List of Modulemd metadata instances matching the query
"""
yaml = None
local_modules = models.ModuleBuild.local_modules(self.db_session, name, stream)
if local_modules:
return [m.mmd() for m in local_modules]
extra_args = {}
if stream_version_lte and (
len(str(models.ModuleBuild.get_stream_version(stream, right_pad=False))) >= 5
):
stream_version = models.ModuleBuild.get_stream_version(stream)
extra_args["stream_version_lte"] = stream_version
if virtual_streams:
extra_args["virtual_stream"] = virtual_streams
modules = self._get_modules(name, stream, version, context, strict=strict, states=states,
**extra_args)
def _modules_to_modulemds(self, modules, strict):
if not modules:
return []
yaml = None
mmds = []
for module in modules:
if module:
@@ -209,6 +166,34 @@ class MBSResolver(KojiResolver):
mmds.append(load_mmd(yaml))
return mmds
def get_module_modulemds(
self,
name,
stream,
version=None,
context=None,
strict=False
):
"""
Gets the module modulemds from the resolver.
:param name: a string of the module's name
:param stream: a string of the module's stream
:param version: a string or int of the module's version. When None, latest version will
be returned.
:param context: a string of the module's context. When None, all contexts will
be returned.
:kwarg strict: Normally this function returns [] if no module can be
found. If strict=True, then a UnprocessableEntity is raised.
:return: List of Modulemd metadata instances matching the query
"""
local_modules = models.ModuleBuild.local_modules(self.db_session, name, stream)
if local_modules:
return [m.mmd() for m in local_modules]
modules = self._get_modules(name, stream, version, context, strict=strict)
return self._modules_to_modulemds(modules, strict)
def get_compatible_base_module_modulemds(
self, base_module_mmd, stream_version_lte, virtual_streams, states
):
@@ -225,18 +210,37 @@ class MBSResolver(KojiResolver):
:param stream_version_lte: If True, the compatible streams are limited
by the stream version computed from `stream`. If False, even the
modules with higher stream version are returned.
:param virtual_streams: List of virtual streams. If set, also modules
with incompatible stream version are returned in case they share
one of the virtual streams.
:param virtual_streams: List of virtual streams.
Modules are returned if they share one of the virtual streams.
:param states: List of states the returned compatible modules should
be in.
:return list: List of Modulemd objects.
"""
if not virtual_streams:
raise RuntimeError("Virtual stream list must not be empty")
name = base_module_mmd.get_module_name()
stream = base_module_mmd.get_stream_name()
return self.get_module_modulemds(
name, stream, stream_version_lte=stream_version_lte, virtual_streams=virtual_streams,
states=states)
extra_args = {}
if stream_version_lte:
stream = base_module_mmd.get_stream_name()
stream_in_xyz_format = len(str(models.ModuleBuild.get_stream_version(
stream, right_pad=False))) >= 5
if not stream_in_xyz_format:
raise StreamNotXyz(
"Cannot get compatible base modules, because stream of resolved "
"base module %s:%s is not in x.y.z format." % (
base_module_mmd.get_module_name(), stream
))
stream_version = models.ModuleBuild.get_stream_version(stream)
extra_args["stream_version_lte"] = stream_version
extra_args["virtual_stream"] = virtual_streams
modules = self._get_modules(name, None, states=states, **extra_args)
return self._modules_to_modulemds(modules, False)
def get_buildrequired_modulemds(self, name, stream, base_module_mmd):
"""

33
tests/test_resolver/test_db.py
View File

@@ -11,7 +11,7 @@ import pytest
from module_build_service.builder.MockModuleBuilder import load_local_builds
from module_build_service.common import models
from module_build_service.common.config import conf
from module_build_service.common.errors import UnprocessableEntity
from module_build_service.common.errors import StreamNotXyz, UnprocessableEntity
from module_build_service.common.models import ModuleBuild
from module_build_service.common.modulemd import Modulemd
from module_build_service.common.utils import import_mmd, load_mmd, mmd_to_str
@@ -80,6 +80,37 @@ class TestDBModule:
"platform:f29.2.0:3:00000000"
}
@pytest.mark.parametrize("provide_test_data",
[{"multiple_stream_versions": True}], indirect=True)
def test_get_compatible_base_module_modulemds_no_virtual(self, provide_test_data):
resolver = mbs_resolver.GenericResolver.create(db_session, conf, backend="db")
platform = db_session.query(ModuleBuild).filter_by(name="platform", stream="f29.1.0").one()
platform_mmd = platform.mmd()
with pytest.raises(RuntimeError, match=r"Virtual stream list must not be empty"):
resolver.get_compatible_base_module_modulemds(
platform_mmd, stream_version_lte=True, virtual_streams=[],
states=[models.BUILD_STATES["ready"]]
)
def test_get_compatible_base_module_modulemds_stream_not_xyz(
self, require_platform_and_default_arch, caplog
):
resolver = mbs_resolver.GenericResolver.create(db_session, conf, backend="db")
platform = db_session.query(ModuleBuild).filter_by(name="platform", stream="f28").one()
platform_mmd = platform.mmd()
with pytest.raises(
StreamNotXyz,
match=(r"Cannot get compatible base modules, because stream "
r"of resolved base module platform:f28 is not in x\.y\.z format")
):
resolver.get_compatible_base_module_modulemds(
platform_mmd, stream_version_lte=True, virtual_streams=["f29"],
states=[models.BUILD_STATES["ready"]])
@pytest.mark.parametrize("empty_buildrequires", [False, True])
def test_get_module_build_dependencies(self, empty_buildrequires, reuse_component_init_data):
"""

7
tests/test_scheduler/test_default_modules.py
View File

@@ -90,6 +90,13 @@ def test_add_default_modules_platform_not_available(require_empty_database):
default_modules.add_default_modules(mmd)
# The fedora-style base module versioning here, with f27/f28 sharing the same
# virtual_stream doesn't work with allow_only_compatible_base_modules, since
# that expects FOOx.y.z versioning.
@patch(
"module_build_service.common.config.Config.allow_only_compatible_base_modules",
new=False
)
@patch("module_build_service.scheduler.default_modules._get_default_modules")
def test_add_default_modules_compatible_platforms(mock_get_dm, require_empty_database):
"""

158
tests/test_scheduler/test_reuse.py
View File

@@ -2,11 +2,14 @@
# SPDX-License-Identifier: MIT
from __future__ import absolute_import
from datetime import timedelta
import mock
import pytest
from sqlalchemy.orm.session import make_transient
from module_build_service.common import models
from module_build_service.common.errors import StreamNotXyz
from module_build_service.common.modulemd import Modulemd
from module_build_service.common.utils import import_mmd, load_mmd, mmd_to_str
from module_build_service.scheduler.db_session import db_session
@@ -293,80 +296,129 @@ class TestUtilsModuleReuse:
assert reusable_module.id == build_module.reused_module_id
assert reusable_module.id == reused_module.id
@pytest.mark.parametrize("allow_ocbm", (True, False))
_EXCEPTION = object()
@pytest.mark.parametrize("allow_ocbm,build_req,available_req,expected", [
# When allow_only_compatible_base_modules is false, only modules
# built against the exact same base module stream can be used
(False, "f29.2.0", ("f29.2.0",), "f29.2.0"),
(False, "f29.2.0", ("f29.0.0",), None),
# When it is true, any base module x.y2.z2 where y2.z2 <= y1.z1
# is a candidate
(True, "f29.2.0", ("f29.2.0", "f29.0.0", "f29.3.0"), "f29.2.0"),
(True, "f29.2.0", ("f29.1.0", "f29.0.0", "f29.3.0"), "f29.1.0"),
# But if the major is different, they don't (the code below adds "f28.0.0"
# with the "f29" virtual stream, so it would be a candidate otherwise)
(True, "f29.2.0", ("f28.0.0",), None),
# the virtual stream must also match (+ is used to skip virtual stream addition)
(True, "f29.2.0", ("+f29.1.0",), None),
# If the buildrequired base module isn't in x.y.z form, we raise an
# exception
(True, "f29", ("f29",), _EXCEPTION),
(True, "f29", ("f29.0.0",), _EXCEPTION),
])
@mock.patch(
"module_build_service.common.config.Config.allow_only_compatible_base_modules",
new_callable=mock.PropertyMock,
)
def test_get_reusable_module_use_latest_build(self, cfg, allow_ocbm):
def test_get_reusable_module_use_latest_build(
self, cfg, allow_ocbm, build_req, available_req, expected):
"""
Test that the `get_reusable_module` tries to reuse the latest module in case when
multiple modules can be reused allow_only_compatible_base_modules is True.
"""
cfg.return_value = allow_ocbm
# Set "fedora" virtual stream to platform:f28.
platform_f28 = db_session.query(models.ModuleBuild).filter_by(name="platform").one()
mmd = platform_f28.mmd()
xmd = mmd.get_xmd()
xmd["mbs"]["virtual_streams"] = ["fedora"]
mmd.set_xmd(xmd)
platform_f28.modulemd = mmd_to_str(mmd)
platform_f28.update_virtual_streams(db_session, ["fedora"])
# Create platform:f29 with "fedora" virtual stream.
mmd = load_mmd(read_staged_data("platform"))
mmd = mmd.copy("platform", "f29")
xmd = mmd.get_xmd()
xmd["mbs"]["virtual_streams"] = ["fedora"]
mmd.set_xmd(xmd)
platform_f29 = import_mmd(db_session, mmd)[0]
# Create all the platform streams we need
needed_platform_streams = set([build_req])
needed_platform_streams.update(available_req)
platform_modules = {}
for stream in needed_platform_streams:
skip_virtual = False
if stream.startswith('+'):
skip_virtual = True
stream = stream[1:]
# Create another copy of `testmodule:master` which should be reused, because its
# stream version will be higher than the previous one. Also set its buildrequires
# to platform:f29.
latest_module = db_session.query(models.ModuleBuild).filter_by(
name="testmodule", state=models.BUILD_STATES["ready"]).one()
# This is used to clone the ModuleBuild SQLAlchemy object without recreating it from
# scratch.
db_session.expunge(latest_module)
make_transient(latest_module)
# Create platform:f29.0.0 with "f29" virtual stream.
mmd = load_mmd(read_staged_data("platform"))
mmd = mmd.copy("platform", stream)
xmd = mmd.get_xmd()
xmd["mbs"]["virtual_streams"] = [] if skip_virtual else ["f29"]
mmd.set_xmd(xmd)
platform_modules[stream] = import_mmd(db_session, mmd)[0]
# Change the platform:f28 buildrequirement to platform:f29 and recompute the build_context.
mmd = latest_module.mmd()
xmd = mmd.get_xmd()
xmd["mbs"]["buildrequires"]["platform"]["stream"] = "f29"
mmd.set_xmd(xmd)
latest_module.modulemd = mmd_to_str(mmd)
contexts = models.ModuleBuild.contexts_from_mmd(
latest_module.modulemd
)
latest_module.build_context = contexts.build_context
latest_module.context = contexts.context
latest_module.buildrequires = [platform_f29]
# Modify a module to buildrequire a particular platform stream
def modify_buildrequires(module, platform_module):
mmd = module.mmd()
deps = mmd.get_dependencies()[0]
deps.clear_buildtime_dependencies()
deps.add_buildtime_stream('platform', platform_module.stream)
xmd = mmd.get_xmd()
xmd["mbs"]["buildrequires"]["platform"]["stream"] = platform_module.stream
mmd.set_xmd(xmd)
module.modulemd = mmd_to_str(mmd)
contexts = models.ModuleBuild.contexts_from_mmd(
module.modulemd
)
module.build_context = contexts.build_context
module.context = contexts.context
module.buildrequires = [platform_module]
# Set the `id` to None, so new one is generated by SQLAlchemy.
latest_module.id = None
db_session.add(latest_module)
db_session.commit()
stream_to_testmodule_id = {}
first = True
for stream in available_req:
if stream.startswith('+'):
stream = stream[1:]
if first:
# Reuse the testmodule:master already in the database
module = db_session.query(models.ModuleBuild).filter_by(
name="testmodule", state=models.BUILD_STATES["ready"]).one()
modify_buildrequires(module, platform_modules[stream])
time_completed = module.time_completed
first = False
else:
# Create another copy of `testmodule:master`, and modify it
module = db_session.query(models.ModuleBuild).filter_by(
name="testmodule", state=models.BUILD_STATES["ready"]).first()
# This is used to clone the ModuleBuild SQLAlchemy object without recreating it from
# scratch.
db_session.expunge(module)
make_transient(module)
modify_buildrequires(module, platform_modules[stream])
# Add modules with ascending time_completed
time_completed += timedelta(days=1)
module.time_completed = time_completed
# Set the `id` to None, so new one is generated by SQLAlchemy.
module.id = None
db_session.add(module)
db_session.commit()
stream_to_testmodule_id[stream] = module.id
module = db_session.query(models.ModuleBuild)\
.filter_by(name="testmodule")\
.filter_by(state=models.BUILD_STATES["build"])\
.one()
modify_buildrequires(module, platform_modules[build_req])
db_session.commit()
reusable_module = get_reusable_module(module)
if allow_ocbm:
assert reusable_module.id == latest_module.id
if expected is TestUtilsModuleReuse._EXCEPTION:
with pytest.raises(StreamNotXyz):
get_reusable_module(module)
else:
# There are two testmodules in ready state, the first one with
# lower id is what we want.
first_module = db_session.query(models.ModuleBuild).filter_by(
name="testmodule", state=models.BUILD_STATES["ready"]
).order_by(models.ModuleBuild.id).first()
assert reusable_module.id == first_module.id
reusable_module = get_reusable_module(module)
if expected:
assert reusable_module.id == stream_to_testmodule_id[expected]
else:
assert reusable_module is None
@pytest.mark.parametrize("allow_ocbm", (True, False))
@mock.patch(