#1263 Query the Red Hat Product Pages to see if this is a Z stream build when configured
Merged 4 years ago by mprahl. Opened 4 years ago by mprahl.

@@ -30,6 +30,8 @@ 

  import re

  import sys

  

+ from six import string_types

+ 

  from module_build_service import logger

  

  
@@ -623,6 +625,25 @@ 

              "type": bool,

              "default": False,

              "desc": "Allow module scratch builds",

+         },

+         "product_pages_url": {

+             "type": str,

+             "default": "",

+             "desc": "The URL to the Product Pages. This is queried to determine if a base module "

+                     "stream has been released. If it has, the stream may be modified automatically "

+                     "to use a different support stream.",

+         },

+         "product_pages_module_streams": {

+             "type": dict,

+             "default": {},

+             "desc": "The keys are regexes of base module streams that should be checked in the Red "

+                     "Hat Product Pages. The values are tuples. The first value is a string that "

+                     "should be appended to the stream if there is a match and the release the "

+                     "stream represents has been released. The second value is a template string "

+                     "that represents the release in Product Pages and can accept format kwargs of "

+                     "x, y, and z (represents the version). The third value is an optional template "

+                     "string that represent the Product Pages release for major releases "

+                     "(e.g. 8.0.0). After the first match, the rest will be ignored."

          }

      }

  
@@ -838,3 +859,42 @@ 

                      s, ", ".join(SUPPORTED_RESOLVERS.keys()))

              )

          self._resolver = s

+ 

+     def _setifok_product_pages_module_streams(self, d):

+         if not isinstance(d, dict):

+             raise ValueError("PRODUCT_PAGES_MODULE_STREAMS must be a dict")

+ 

+         for regex, values in d.items():

+             try:

+                 re.compile(regex)

+             except (TypeError, re.error):

+                 raise ValueError(

+                     'The regex `%r` in the configuration "PRODUCT_PAGES_MODULE_STREAMS" is invalid'

+                     % regex

+                 )

+ 

+             if not isinstance(values, list) and not isinstance(values, tuple):

+                 raise ValueError(

+                     'The values in the configured dictionary for "PRODUCT_PAGES_MODULE_STREAMS" '

+                     "must be a list or tuple"

+                 )

+ 

+             if len(values) != 3:

+                 raise ValueError(

+                     "There must be three entries in each value in the dictionary configured for "

+                     '"PRODUCT_PAGES_MODULE_STREAMS"'

+                 )

+ 

+             for i, value in enumerate(values):

+                 if not isinstance(value, string_types):

+                     # The last value is optional

+                     if value is None and i == 2:

+                         continue

+ 

+                     raise ValueError(

+                         'The value in the %i index of the values in "PRODUCT_PAGES_MODULE_STREAMS" '

+                         "must be a string"

+                         % i

+                     )

+ 

+         self._product_pages_module_streams = d

@@ -658,14 +658,14 @@ 

              )

  

  

- def _handle_base_module_virtual_stream_br(mmd):

+ def _modify_buildtime_streams(mmd, new_streams_func):

      """

-     Translate a base module virtual stream buildrequire to an actual stream on the input modulemd.

+     Modify buildtime streams using the input new_streams_func.

  

      :param Modulemd.ModuleStream mmd: the modulemd to apply the overrides on

+     :param function new_streams: a function that takes the parameters (module_name, module_streams),

+         and returns the streams that should be set on the buildtime dependency.

      """

-     from module_build_service.resolver import system_resolver

- 

      deps = mmd.get_dependencies()

      for dep in deps:

          overridden = False
@@ -675,50 +675,8 @@ 

          new_dep = Modulemd.Dependencies()

  

          for name, streams in brs.items():

-             if name not in conf.base_module_names:

-                 if streams == []:

-                     new_dep.set_empty_buildtime_dependencies_for_module(name)

-                 else:

-                     for stream in streams:

-                         new_dep.add_buildtime_stream(name, stream)

-                 continue

- 

-             new_streams = copy.deepcopy(streams)

-             for i, stream in enumerate(streams):

-                 # Ignore streams that start with a minus sign, since those are handled in the

-                 # MSE code

-                 if stream.startswith("-"):

-                     continue

- 

-                 # Check if the base module stream is available

-                 log.debug('Checking to see if the base module "%s:%s" is available', name, stream)

-                 if system_resolver.get_module_count(name=name, stream=stream) > 0:

-                     continue

- 

-                 # If the base module stream is not available, check if there's a virtual stream

-                 log.debug(

-                     'Checking to see if there is a base module "%s" with the virtual stream "%s"',

-                     name, stream,

-                 )

-                 base_module_mmd = system_resolver.get_latest_with_virtual_stream(

-                     name=name, virtual_stream=stream

-                 )

-                 if not base_module_mmd:

-                     # If there isn't this base module stream or virtual stream available, skip it,

-                     # and let the dep solving code deal with it like it normally would

-                     log.warning(

-                         'There is no base module "%s" with stream/virtual stream "%s"',

-                         name, stream,

-                     )

-                     continue

- 

-                 latest_stream = base_module_mmd.get_stream_name()

-                 log.info(

-                     'Replacing the buildrequire "%s:%s" with "%s:%s", since "%s" is a virtual '

-                     "stream",

-                     name, stream, name, latest_stream, stream

-                 )

-                 new_streams[i] = latest_stream

+             new_streams = new_streams_func(name, streams)

+             if streams != new_streams:

                  overridden = True

  

              if new_streams == []:
@@ -741,6 +699,179 @@ 

              mmd.add_dependencies(new_dep)

  

  

+ def resolve_base_module_virtual_streams(name, streams):

+     """

+     Resolve any base module virtual streams and return a copy of `streams` with the resolved values.

+ 

+     :param str name: the module name

+     :param str streams: the streams to resolve

+     :return: the resolved streams

+     :rtype: list

+     """

+     from module_build_service.resolver import system_resolver

+ 

+     if name not in conf.base_module_names:

+         return streams

+ 

+     new_streams = copy.deepcopy(streams)

+     for i, stream in enumerate(streams):

+         # Ignore streams that start with a minus sign, since those are handled in the

+         # MSE code

+         if stream.startswith("-"):

+             continue

+ 

+         # Check if the base module stream is available

+         log.debug('Checking to see if the base module "%s:%s" is available', name, stream)

+         if system_resolver.get_module_count(name=name, stream=stream) > 0:

+             continue

+ 

+         # If the base module stream is not available, check if there's a virtual stream

+         log.debug(

+             'Checking to see if there is a base module "%s" with the virtual stream "%s"',

+             name, stream,

+         )

+         base_module_mmd = system_resolver.get_latest_with_virtual_stream(

+             name=name, virtual_stream=stream

+         )

+         if not base_module_mmd:

+             # If there isn't this base module stream or virtual stream available, skip it,

+             # and let the dep solving code deal with it like it normally would

+             log.warning(

+                 'There is no base module "%s" with stream/virtual stream "%s"',

+                 name, stream,

+             )

+             continue

+ 

+         latest_stream = base_module_mmd.get_stream_name()

+         log.info(

+             'Replacing the buildrequire "%s:%s" with "%s:%s", since "%s" is a virtual '

+             "stream",

+             name, stream, name, latest_stream, stream

+         )

+         new_streams[i] = latest_stream

+ 

+     return new_streams

+ 

+ 

+ def _process_support_streams(mmd, params):

+     """

+     Check if any buildrequired base modules require a support stream suffix.

+ 

+     This checks the Red Hat Product Pages to see if the buildrequired base module stream has been

+     released, if yes, then add the appropriate stream suffix.

+ 

+     :param Modulemd.ModuleStream mmd: the modulemd to apply the overrides on

+     :param dict params: the API parameters passed in by the user

+     """

+     config_msg = (

+         'Skipping the release date checks for adding a stream suffix since "%s" '

+         "is not configured"

+     )

+     if not conf.product_pages_url:

+         log.debug(config_msg, "product_pages_url")

+         return

+     elif not conf.product_pages_module_streams:

+         log.debug(config_msg, "product_pages_module_streams")

+         return

+ 

+     buildrequire_overrides = params.get("buildrequire_overrides", {})

+ 

+     def new_streams_func(name, streams):

+         if name not in conf.base_module_names:

+             log.debug("The module %s is not a base module. Skipping the release date check.", name)

+             return streams

+         elif name in buildrequire_overrides:

+             log.debug(

+                 "The module %s is a buildrequire override. Skipping the release date check.", name)

+             return streams

+ 

+         new_streams = copy.deepcopy(streams)

+         for i, stream in enumerate(streams):

+             for regex, values in conf.product_pages_module_streams.items():

+                 if re.match(regex, stream):

+                     log.debug(

+                         'The regex `%s` from the configuration "product_pages_module_streams" '

+                         "matched the stream %s",

+                         regex, stream,

+                     )

+                     stream_suffix, pp_release_template, pp_major_release_template = values

+                     break

+             else:

+                 log.debug(

+                     'No regexes in the configuration "product_pages_module_streams" matched the '

+                     "stream %s. Skipping the release date check for this stream.",

+                     stream,

+                 )

+                 continue

+ 

+             if stream.endswith(stream_suffix):

+                 log.debug(

+                     'The stream %s already contains the stream suffix of "%s". Skipping the '

+                     "release date check.",

+                     stream, stream_suffix

+                 )

+                 continue

+ 

+             stream_version = models.ModuleBuild.get_stream_version(stream)

+             if not stream_version:

+                 log.debug("A stream version couldn't be parsed from %s", stream)

+                 continue

+ 

+             # Convert the stream_version float to an int to make the math below deal with only

+             # integers

+             stream_version_int = int(stream_version)

+             # For example 80000 => 8

+             x = stream_version_int // 10000

+             # For example 80100 => 1

+             y = (stream_version_int - x * 10000) // 100

+             # For example 80104 => 4

+             z = stream_version_int - x * 10000 - y * 100

+             # Check if the stream version is x.0.0

+             if stream_version_int % 10000 == 0 and pp_major_release_template:

+                 # For example, el8.0.0 => rhel-8-0

+                 pp_release = pp_major_release_template.format(x=x, y=y, z=z)

+             else:

+                 # For example el8.0.1 => rhel-8-0.1

+                 pp_release = pp_release_template.format(x=x, y=y, z=z)

+ 

+             url = "{}/api/v7/releases/{}/?fields=ga_date".format(

+                 conf.product_pages_url.rstrip("/"), pp_release)

+ 

+             try:

+                 pp_rv = requests.get(url, timeout=15)

+                 pp_json = pp_rv.json()

+             # Catch requests failures and JSON parsing errors

+             except (requests.exceptions.RequestException, ValueError):

+                 log.exception(

+                     "The query to the Product Pages at %s failed. Assuming it is not yet released.",

+                     url,

+                 )

+                 continue

+ 

+             ga_date = pp_json.get("ga_date")

+             if not ga_date:

+                 log.debug("A release date for the release %s could not be determined", pp_release)

+                 continue

+ 

+             if datetime.strptime(ga_date, '%Y-%m-%d') > datetime.utcnow():

+                 log.debug(

+                     "The release %s hasn't been released yet. Not adding a stream suffix.",

+                     ga_date

+                 )

+                 continue

+ 

+             new_stream = stream + stream_suffix

+             log.info(

+                 'Replacing the buildrequire "%s:%s" with "%s:%s", since the stream is released',

+                 name, stream, name, new_stream

+             )

+             new_streams[i] = new_stream

+ 

+         return new_streams

+ 

+     _modify_buildtime_streams(mmd, new_streams_func)

+ 

+ 

  def submit_module_build(username, mmd, params):

      """

      Submits new module build.
@@ -773,7 +904,8 @@ 

      if "default_streams" in params:

          default_streams = params["default_streams"]

      _apply_dep_overrides(mmd, params)

-     _handle_base_module_virtual_stream_br(mmd)

+     _modify_buildtime_streams(mmd, resolve_base_module_virtual_streams)

+     _process_support_streams(mmd, params)

  

      mmds = generate_expanded_mmds(db.session, mmd, raise_if_stream_ambigous, default_streams)

      if not mmds:

@@ -0,0 +1,37 @@ 

+ document: modulemd

+ version: 1

+ 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: el8.2.1

+         requires:

+             platform: el8.0.0

+     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

+             perl-Tangerine:

+                 rationale: Provides API for this module and is a dependency of tangerine.

+                 ref: master

+             tangerine:

+                 rationale: Provides API for this module.

+                 buildorder: 10

+                 ref: master

@@ -2436,3 +2436,151 @@ 

          dep = mmd.get_dependencies()[0]

          assert dep.get_buildtime_streams("platform") == ["el8.25.0"]

          assert dep.get_runtime_streams("platform") == ["el8"]

+ 

+     @pytest.mark.parametrize(

+         "pp_url, pp_streams, get_rv, br_stream, br_override, expected_stream",

+         (

+             # Test a stream of a major release

+             (

+                 "https://pp.domain.local/pp/",

+                 {r"el.+": ("z", "rhel-{x}-{y}.{z}", "rhel-{x}-{y}")},

+                 {"ga_date": "2019-05-07"},

+                 "el8.0.0",

+                 {},

+                 "el8.0.0z",

+             ),

+             # Test when the releases GA date is far in the future

+             (

+                 "https://pp.domain.local/pp/",

+                 {r"el.+": ("z", "rhel-{x}-{y}.{z}", "rhel-{x}-{y}")},

+                 {"ga_date": "2099-10-30"},

+                 "el8.0.0",

+                 {},

+                 "el8.0.0",

+             ),

+             # Test when product_pages_url isn't set

+             (

+                 "",

+                 {r"el.+": ("z", "rhel-{x}-{y}.{z}", "rhel-{x}-{y}")},

+                 {"ga_date": "2019-05-07"},

+                 "el8.0.0",

+                 {},

+                 "el8.0.0",

+             ),

+             # Test when the release isn't found in Product Pages

+             (

+                 "https://pp.domain.local/pp/",

+                 {r"el.+": ("z", "rhel-{x}-{y}.{z}", "rhel-{x}-{y}")},

+                 {"detail": "Not found."},

+                 "el8.0.0",

+                 {},

+                 "el8.0.0",

+             ),

+             # Test when a non-major release stream

+             (

+                 "https://pp.domain.local/pp/",

+                 {r"el.+": ("z", "rhel-{x}-{y}.{z}", "rhel-{x}-{y}")},

+                 {"ga_date": "2019-05-07"},

+                 "el8.2.1",

+                 {},

+                 "el8.2.1z",

+             ),

+             # Test that when buildrequire overrides is set for platform, nothing changes

+             (

+                 "https://pp.domain.local/pp/",

+                 {r"el.+": ("z", "rhel-{x}-{y}.{z}", "rhel-{x}-{y}")},

+                 {"ga_date": "2019-05-07"},

+                 "el8.0.0",

+                 {"platform": ["el8.0.0"]},

+                 "el8.0.0",

+             ),

+             # Test when product_pages_module_streams is not set

+             (

+                 "https://pp.domain.local/pp/",

+                 {},

+                 {"ga_date": "2019-05-07"},

+                 "el8.0.0",

+                 {},

+                 "el8.0.0",

+             ),

+             # Test when there is no stream that matches the configured regexes

+             (

+                 "https://pp.domain.local/pp/",

+                 {r"js.+": ("z", "js-{x}-{y}", "js-{x}-{y}")},

+                 {"ga_date": "2019-05-07"},

+                 "el8.0.0",

+                 {},

+                 "el8.0.0",

+             ),

+             # Test when there is no configured special Product Pages template for major releases

+             (

+                 "https://pp.domain.local/pp/",

+                 {r"el.+": ("z", "rhel-{x}-{y}", None)},

+                 {"ga_date": "2019-05-07"},

+                 "el8.0.0",

+                 {},

+                 "el8.0.0z",

+             ),

+         ),

+     )

+     @patch(

+         "module_build_service.config.Config.product_pages_url",

+         new_callable=PropertyMock,

+     )

+     @patch(

+         "module_build_service.config.Config.product_pages_module_streams",

+         new_callable=PropertyMock,

+     )

+     @patch("requests.get")

+     @patch("module_build_service.auth.get_user", return_value=user)

+     @patch("module_build_service.scm.SCM")

+     def test_submit_build_automatic_z_stream_detection(

+         self, mocked_scm, mocked_get_user, mock_get, mock_pp_streams, mock_pp_url, pp_url,

+         pp_streams, get_rv, br_stream, br_override, expected_stream,

+     ):

+         # Configure the Product Pages URL

+         mock_pp_url.return_value = pp_url

+         mock_pp_streams.return_value = pp_streams

+         # Mock the Product Pages query

+         mock_get.return_value.json.return_value = get_rv

+         mmd = load_mmd_file(path.join(base_dir, "staged_data", "platform.yaml"))

+         # Create the required platforms

+         for stream in ("el8.0.0", "el8.0.0z", "el8.2.1", "el8.2.1z"):

+             mmd = mmd.copy(mmd.get_module_name(), stream)

+             import_mmd(db.session, mmd)

+ 

+         # Use a testmodule that buildrequires platform:el8.0.0 or platform:el8.2.1

+         FakeSCM(

+             mocked_scm,

+             "testmodule",

+             "testmodule_{}.yaml".format(br_stream.replace(".", "")),

+             "620ec77321b2ea7b0d67d82992dda3e1d67055b4",

+         )

+ 

+         post_url = "/module-build-service/2/module-builds/"

+         scm_url = (

+             "https://src.stg.fedoraproject.org/modules/testmodule.git?#"

+             "68931c90de214d9d13feefbd35246a81b6cb8d49"

+         )

+         payload = {"branch": "master", "scmurl": scm_url}

+         if br_override:

+             payload["buildrequire_overrides"] = br_override

+         rv = self.client.post(post_url, json=payload)

+         data = json.loads(rv.data)

+ 

+         mmd = load_mmd(data[0]["modulemd"])

+         assert len(mmd.get_dependencies()) == 1

+         dep = mmd.get_dependencies()[0]

+         assert dep.get_buildtime_streams("platform") == [expected_stream]

+         # The runtime stream suffix should remain unchanged

+         assert dep.get_runtime_streams("platform") == ["el8.0.0"]

+ 

+         if pp_url and not br_override and pp_streams.get(r"el.+"):

+             if br_stream == "el8.0.0":

+                 pp_release = "rhel-8-0"

+             else:

+                 pp_release = "rhel-8-2.1"

+             expected_url = "{}api/v7/releases/{}/?fields=ga_date".format(pp_url, pp_release)

+             mock_get.assert_called_once_with(expected_url, timeout=15)

+         else:

+             mock_get.assert_not_called()

In certain use-cases, a module's buildrequires may remain the in the modulemd, but a different support stream of the buildrequired base module should be used. For example, since RHEL 8.0.0 is GA, any modules that buildrequire platform:el8.0.0 should buildrequire platform:el8.0.0z instead.

The first commit refactors _handle_base_module_virtual_stream_br so that some of its functionality can be reused in the second commit.

rebased onto e6e2071ac4ec0568dafce26bbe9657264aa4ace8

4 years ago

2 new commits added

  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

2 new commits added

  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

2 new commits added

  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

2 new commits added

  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

2 new commits added

  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

2 new commits added

  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

2 new commits added

  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

Hm, is it OK to hardcode "el" and "rhel-{}-{}" in the code? It would be nice to have it in a config, but I also understand that if this is the single product for which we plan to support something like that, it might be hard to come up with general way how to include that in config. Just sharing that opinion/idea. Not a blocker.

Hm, is it OK to hardcode "el" and "rhel-{}-{}" in the code? It would be nice to have it in a config, but I also understand that if this is the single product for which we plan to support something like that, it might be hard to come up with general way how to include that in config. Just sharing that opinion/idea. Not a blocker.

As discussed in IRC, I'll add a config with keys as regex to match the stream and the values as the string to append to the stream. For example {r"el.+": "z"}.

Hm, is it OK to hardcode "el" and "rhel-{}-{}" in the code? It would be nice to have it in a config, but I also understand that if this is the single product for which we plan to support something like that, it might be hard to come up with general way how to include that in config. Just sharing that opinion/idea. Not a blocker.

As discussed in IRC, I'll add a config with keys as regex to match the stream and the values as the string to append to the stream. For example {r"el.+": "z"}.

I started to look at the configuration, but even what I suggested is not enough because you need a way to convert the module stream to the release name in Product Pages. For example. el8.0.0 to rhel-8-0 and el8.0.1 to rhel-8-0.1. This starts becoming a very complex configuration. I'll keep thinking about it.

1 new commit added

  • Make the Product Pages code configurable
4 years ago

Okay, I added another commit that makes the code configurable. I'll merge the last two commits if we agree with this approach.

@jkaluza could you please review?

3 new commits added

  • Make the Product Pages code configurable
  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

+1. I don't see a reason why this should not work. I did not check the PP API, but the functions to change the streams look correct to me.

2 new commits added

  • Query the Red Hat Product Pages to see if this is a Z stream build when configured
  • Breakout some of the functionality in _handle_base_module_virtual_stream_br so it can be reused for a future feature
4 years ago

+1. I don't see a reason why this should not work. I did not check the PP API, but the functions to change the streams look correct to me.

Thanks! I just squashed the last two commits. After C3I runs, I'll merge this.

rebased onto 13932e8fb2a41aa4aec0576a40e3f488a896de69

4 years ago

rebased onto 00ea4e3

4 years ago

Pull-Request has been merged by mprahl

4 years ago