fedora-infra / toddlers

Clone
Source Code
GIT

#257 koji_block_retired: Get rawhide epel koji_tag
Merged 18 days ago by zlopez. Opened a month ago by lenkaseg.
fedora-infra/ lenkaseg/toddlers koji_fix_epel  into  main

file modified
+105 -4 
@@ -48,6 +48,7 @@ 
 

      def setup_method(self):
 

          """Initialize toddler."""
 

          self.toddler_cls = koji_block_retired.KojiBlockRetired()
 

+         self.toddler_cls.koji_session = MagicMock()
 

  

      def test_process(self):
 

          """
 
@@ -109,14 +110,16 @@ 
 

          assert caplog.records[-1].message == "dead.package file was not added, bailing"
 

  

      @patch("toddlers.plugins.koji_block_retired.KojiBlockRetired._create_session")
 

-     @patch("toddlers.plugins.koji_block_retired.KojiBlockRetired.get_rawhide_tag")
 

+     @patch(
 

+         "toddlers.plugins.koji_block_retired.KojiBlockRetired.get_fedora_rawhide_tag"
 

+     )
 

      def test_dead_package_added_to_main_branch(
 

-         self, mock_get_rawhide_tag, mock_session, caplog
 

+         self, mock_get_fedora_rawhide_tag, caplog
 

      ):
 

          """
 

          Assert that that main branch will be changed to rawhide tag
 

          """
 

-         mock_get_rawhide_tag.return_value = "f41"
 

+         mock_get_fedora_rawhide_tag.return_value = "f41"
 

          caplog.set_level(logging.INFO)
 

  

          message = MagicMock()
 
@@ -137,7 +140,7 @@ 
 

              "keytab": "keytab",
 

          }
 

          with patch.object(
 

-             self.toddler_cls, "get_rawhide_tag", MagicMock(return_value="f41")
 

+             self.toddler_cls, "get_fedora_rawhide_tag", MagicMock(return_value="f41")
 

          ):
 

              self.toddler_cls.process_block_retired(config, message)
 

          self.toddler_cls.koji_session.packageListBlock.assert_called_once_with(
 
@@ -263,3 +266,101 @@ 
 

              caplog.records[-1].message
 

              == "Commit is older than 1 day, probably new branch creation, bailing."
 

          )
 

+ 

+     def test_get_fedora_rawhide_tag(self):
 

+         self.toddler_cls.koji_session.getBuildTarget.return_value = {
 

+             "build_tag": 94099,
 

+             "build_tag_name": "f42-build",
 

+             "dest_tag": 94094,
 

+             "dest_tag_name": "f42-updates-candidate",
 

+             "id": 88,
 

+             "name": "rawhide",
 

+         }
 

+         assert self.toddler_cls.get_fedora_rawhide_tag(branch="rawhide") == "f42"
 

+ 

+     def test_get_fedora_rawhide_koji_error(self, caplog):
 

+         self.toddler_cls.koji_session.getBuildTarget.side_effect = koji.GenericError(
 

+             "Failed"
 

+         )
 

+         self.toddler_cls.get_fedora_rawhide_tag(branch="rawhide")
 

+         assert (
 

+             caplog.records[-1].message
 

+             == "Failed to get fedora rawhide tag in koji: Failed"
 

+         )
 

+ 

+     def test_get_epel_latest_tag(self):
 

+         self.toddler_cls.koji_session.getBuildTarget.return_value = {
 

+             "build_tag": 93222,
 

+             "build_tag_name": "epel10.0-build",
 

+             "dest_tag": 93225,
 

+             "dest_tag_name": "epel10.0-testing-candidate",
 

+             "id": 40886,
 

+             "name": "epel10",
 

+         }
 

+ 

+         assert self.toddler_cls.get_epel_latest_tag(branch="epel10") == "epel10.0"
 

+ 

+     def test_get_epel_latest_tag_koji_error(self, caplog):
 

+         self.toddler_cls.koji_session.getBuildTarget.side_effect = koji.GenericError(
 

+             "Failed"
 

+         )
 

+         self.toddler_cls.get_epel_latest_tag(branch="epel10")
 

+         assert (
 

+             caplog.records[-1].message
 

+             == "Failed to get latest epel tag in koji: Failed"
 

+         )
 

+ 

+     @patch("toddlers.plugins.koji_block_retired.KojiBlockRetired._create_session")
 

+     def test_branch_with_container(self, mock_session, caplog):
 

+         caplog.set_level(logging.INFO)
 

+         message = MagicMock()
 

+         message.body = {
 

+             "commit": {
 

+                 "stats": {"files": {"dead.package": {"additions": 1, "deletions": 0}}},
 

+                 "branch": "f38",
 

+                 "repo": "example-repo",
 

+                 "namespace": "containers",
 

+                 "date": "2024-09-16T12:12:46+01:00",
 

+             }
 

+         }
 

+         message._headers = {"sent-at": "2024-09-16T12:12:46+01:00"}
 

+         config = MagicMock()
 

+         config = {
 

+             "koji_url": "https://example.koji.org",
 

+             "principal": "principal",
 

+             "keytab": "keytab",
 

+         }
 

+         self.toddler_cls.process_block_retired(config, message)
 

+         self.toddler_cls.koji_session.packageListBlock.assert_called_once_with(
 

+             taginfo="f38-container",
 

+             pkginfo="example-repo",
 

+         )
 

+ 

+     @patch("toddlers.plugins.koji_block_retired.KojiBlockRetired._create_session")
 

+     @patch("toddlers.plugins.koji_block_retired.KojiBlockRetired.get_epel_latest_tag")
 

+     def test_branch_epel(self, mock_get_fedora_epel_tag, mock_session, caplog):
 

+         mock_get_fedora_epel_tag.return_value = "epel10.2"
 

+         caplog.set_level(logging.INFO)
 

+         message = MagicMock()
 

+         message.body = {
 

+             "commit": {
 

+                 "stats": {"files": {"dead.package": {"additions": 1, "deletions": 0}}},
 

+                 "branch": "epel10",
 

+                 "repo": "example-repo",
 

+                 "namespace": "example_ns",
 

+                 "date": "2024-09-16T12:12:46+01:00",
 

+             }
 

+         }
 

+         message._headers = {"sent-at": "2024-09-16T12:12:46+01:00"}
 

+         config = MagicMock()
 

+ 

+         config = {
 

+             "koji_url": "https://example.koji.org",
 

+             "principal": "principal",
 

+             "keytab": "keytab",
 

+         }
 

+         self.toddler_cls.process_block_retired(config, message)
 

+         self.toddler_cls.koji_session.packageListBlock.assert_called_once_with(
 

+             taginfo="epel10.2",
 

+             pkginfo="example-repo",
 

+         )

file modified
+39 -30 
@@ -13,7 +13,6 @@ 
 

  

  from fedora_messaging_git_hook_messages import CommitV1
 

  import koji
 

- import requests
 

  import toml
 

  

  
@@ -41,18 +40,25 @@ 
 

          self.koji_session = koji.ClientSession(koji_url)
 

          self.koji_session.gssapi_login(principal=principal, keytab=keytab)
 

  

-     def get_rawhide_tag(self):
 

-         releases = []
 

-         url = "https://bodhi.fedoraproject.org/releases?exclude_archived=true"
 

-         resp = requests.get(url)
 

-         if not resp.ok:
 

-             _log.info("Unable to retrieve active releases from Bodhi.")
 

-             return None
 

-         response = resp.json()
 

-         for b in response["releases"]:
 

-             if re.match(r"^(f)\d{1,2}$", b["branch"]):
 

-                 releases.append(b["branch"])
 

-         return max(set(releases))
 

+     def get_fedora_rawhide_tag(self, branch):
 

+         try:
 

+             _log.info("Getting koji tag for fedora rawhide")
 

+             target = self.koji_session.getBuildTarget(branch)
 

+             tag = target["build_tag_name"].removesuffix("-build")
 

+             return tag
 

+         except koji.GenericError as e:
 

+             _log.exception(f"Failed to get fedora rawhide tag in koji: {e}")
 

+         return None
 

+ 

+     def get_epel_latest_tag(self, branch):
 

+         try:
 

+             _log.info("Getting koji tag for latest epel release")
 

+             target = self.koji_session.getBuildTarget(branch)
 

+             tag = target["build_tag_name"].removesuffix("-build")
 

+             return tag
 

+         except koji.GenericError as e:
 

+             _log.exception(f"Failed to get latest epel tag in koji: {e}")
 

+         return None
 

  

      def accepts_topic(self, topic):
 

          """Returns a boolean whether this toddler is interested in messages
 
@@ -104,14 +110,19 @@ 
 

              )
 

              return
 

  

-         if msg["commit"]["branch"] in ["main", "rawhide"]:
 

-             branch_name = self.get_rawhide_tag()
 

+         repo = msg["commit"]["repo"]
 

+         branch = msg["commit"]["branch"]
 

+ 

+         if branch in ["main", "rawhide"]:
 

+             tag_name = self.get_fedora_rawhide_tag("rawhide")
 

          elif "container" in msg["commit"]["namespace"]:
 

-             branch_name = msg["commit"]["branch"] + "-container"
 

+             tag_name = (
 

+                 "".join([branch, "-container"]) if "container" not in branch else branch
 

+             )
 

+         elif re.match(r"^epel\d{2,}$", branch):
 

+             tag_name = self.get_epel_latest_tag(branch)
 

          else:
 

-             branch_name = msg["commit"]["branch"]
 

- 

-         repo = msg["commit"]["repo"]
 

+             tag_name = branch
 

  

          _log.info("Processing Koji block retired for %s", repo)
 

  
@@ -122,17 +133,17 @@ 
 

          # FIXME: This introduces a theoretical race condition when a package is
 

          # built after all builds were untagged and before the package is blocked
 

          try:
 

-             _log.info(f"Untagging package {repo}, tag: {branch_name}")
 

-             self.koji_session.untagBuild(tag=branch_name, build=repo)
 

+             _log.info(f"Untagging package {repo}, tag: {tag_name}")
 

+             self.koji_session.untagBuild(tag=tag_name, build=repo)
 

          except koji.GenericError as e:
 

              _log.exception(f"Failed to untag build in koji: {e}")
 

  

          try:
 

-             _log.info(f"Blocking package {repo}, tag: {branch_name}")
 

-             self.koji_session.packageListBlock(taginfo=branch_name, pkginfo=repo)
 

+             _log.info(f"Blocking package {repo}, tag: {tag_name}")
 

+             self.koji_session.packageListBlock(taginfo=tag_name, pkginfo=repo)
 

          except (koji.GenericError, koji.ActionNotAllowed) as e:
 

              _log.exception(
 

-                 f"Failed to block retired package {repo} on branch {branch_name}: {e}"
 

+                 f"Failed to block retired package {repo} on branch {tag_name}: {e}"
 

              )
 

  

          # TODO: In case of exception while blocking a package in koji,
 
@@ -142,15 +153,13 @@ 
 

          # in the epel build tag. Therefore unblock all retired EPEL packages in the
 

          # built tag since it does not hurt if the package does not move to RHEL.
 

          if "stg" not in koji_url:
 

-             if "epel" in branch_name:
 

-                 branch_name = branch_name + "-build"
 

+             if "epel" in tag_name:
 

+                 tag_name = "".join([tag_name, "-build"])
 

                  try:
 

                      _log.info(
 

-                         f"Unblocking EPEL tag for package: {repo}, tag: {branch_name}"
 

-                     )
 

-                     self.koji_session.packageListUnblock(
 

-                         taginfo=branch_name, pkginfo=repo
 

+                         f"Unblocking EPEL tag for package: {repo}, tag: {tag_name}"
 

                      )
 

+                     self.koji_session.packageListUnblock(taginfo=tag_name, pkginfo=repo)
 

                  except koji.GenericError as e:
 

                      _log.exception(f"Failed unblocking epel build tag: {e}")

Signed-off-by: Lenka Segura lsegura@redhat.com

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/d3c9e37a8e804fdea40fc5ffcc4ca7a9

rebased onto a932eaa
a month ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/f993b327bafa40039310adb484964f3e

rebased onto a932eaa
a month ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/21219e209dbb407b86f10350b500c65f

rebased onto a932eaa
a month ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/a942d26723944cae81bebb09a1198311

Mmm, coverage is complaining about line 120 in koji_block_retired.py... which is weird because this test should cover it: https://pagure.io/fedora-infra/toddlers/pull-request/257#_1__82
What am I not seeing?

Ah, I see it, it's namespace, not branch.

rebased onto a932eaa
a month ago

Here I have to check whether the message already contains the -container in branch or not. Or add -container only in case branch does not contain it already.

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/ae66b53571324064bc81ad31cb4d0b2d

Ready for review!

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/af5e1ee93c724c71adc90055ec6da383

It looks good to me.

It would be probably good if @carlwgeorge look at it as well.

The leading EPEL 10 branch will be just epel10, with no dot. If I'm reading the rest of the code correctly, when we're retiring non-leading branches such as epel10.0, it should reach the else condition because then the koji tag will match the branch name.

I think we should avoid calling this rawhide, as rawhide is a specific Fedora thing. How about latest or leading instead?

I think there is a better way to do this part. In order for fedpkg to know how to build from the epel10 branch, we have an epel10 koji target that will always point to the latest epel10.X-build tag. Using getBuildTarget would be more efficient and avoid the packaging.version import.

target = self.koji_session.getBuildTarget(branch)
tag = target['build_tag_name'].removesuffix('-build')

fedpkg does something similar to get build tag macro definitions for EPEL 10 and ELN.

https://pagure.io/fedpkg/blob/master/f/fedpkg/__init__.py#_214-237

Instead of conflating branch and tag, can we rename the branch_name variable to tag_name? I know historically those have matched, but it's no longer the case with EPEL 10. Some of the other code in this file would be less confusing with that change as well, such as tag=branch_name and tag: {branch_name}.

rebased onto b4d420f
24 days ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/91f11e8f36cd4a8e998aee05df5da23e

Hopefully all points addressed, thanks a lot for the review @carlwgeorge !

This if statement to set the tag name for rawhide is being run twice, on lines 117-118 and 122-123 (line numbers in your fork, not what's displayed here in the PR).

This regex matches too many things. On EPEL 8 and 9 branches, the tag name always matches the branch name, so they can be handled by the else condition and avoid the koji API call. The same thing is true for EPEL 10+ minor version branches (epel10.0, epel11.5, etc). This should only match major version branches who build to a tag with a minor version (epel10, epel11, etc). It would probably also be better to not limit it to two digits. I suggest:

elif re.match(r"^epel\d{2,}$", branch):

In get_fedora_rawhide_tag it returns None if it can't fetch the active releases from bodhi. Does it make sense to do the same here, instead of returning the branch as-is?

I noticed something else while looking over these changes. The logic in get_fedora_rawhide_tag works, but is more complicated than what we're doing now in get_epel_latest_tag. The rawhide koji target changes its build tag over time, similar to epel10, so I think we can use the same process to map the rawhide branch to a tag. This would be more efficient than querying bodhi and needing to process all those results, and would also avoid the need for importing requests.

target = session.getBuildTarget('rawhide')
tag = target["build_tag_name"].removesuffix("-build")
Edited 23 days ago by carlwgeorge

The branch in this message should be just epel10, no trailing dot.

rebased onto a120f81
21 days ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/2a86693e3d214a92bec23eeecfbe1337

rebased onto a120f81
21 days ago

This regex matches too many things. On EPEL 8 and 9 branches, the tag name always matches the branch name, so they can be handled by the else condition and avoid the koji API call. The same thing is true for EPEL 10+ minor version branches (epel10.0, epel11.5, etc). This should only match major version branches who build to a tag with a minor version (epel10, epel11, etc). It would probably also be better to not limit it to two digits. I suggest:

elif re.match(r"^epel\d{2,}$", branch):

Thanks for spotting that, much better now:

In [434]: epels = ["epel8", "epel9", "epel10", "epel11", "epel10.12", "epel11.3", "epel111", "epel111.1"]

Before:

In [437]: for e in epels:
     ...:     if re.match(r"^(epel)\d{1,2}$", e):
     ...:         print(f"passes: {e}")
     ...:     else:
     ...:         print(f"doesn't pass: {e}")
     ...: 
passes: epel8
passes: epel9
passes: epel10
passes: epel11
doesn't pass: epel10.12
doesn't pass: epel11.3
doesn't pass: epel111
doesn't pass: epel111.1

After:

In [436]: for e in epels:
     ...:     if re.match(r"^epel\d{2,}$", e):
     ...:         print(f"passes: {e}")
     ...:     else:
     ...:         print(f"doesn't pass: {e}")
     ...: 
doesn't pass: epel8
doesn't pass: epel9
passes: epel10
passes: epel11
doesn't pass: epel10.12
doesn't pass: epel11.3
passes: epel111
doesn't pass: epel111.1

I noticed something else while looking over these changes. The logic in get_fedora_rawhide_tag works, but is more complicated than what we're doing now in get_epel_latest_tag. The rawhide koji target changes its build tag over time, similar to epel10, so I think we can use the same process to map the rawhide branch to a tag. This would be more efficient than querying bodhi and needing to process all those results, and would also avoid the need for importing requests.

target = session.getBuildTarget('rawhide') tag = target["build_tag_name"].removesuffix("-build")

Cool, thanks! getBuildTarget does not like the main option, so in case of main, it sends rawhide:

        if branch in ["main", "rawhide"]:
            tag_name = self.get_fedora_rawhide_tag("rawhide")
Edited 21 days ago by lenkaseg

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/bf8cd39fb42c4fc8acdb99d4ea738d9f

rebased onto a120f81
21 days ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/77f718c4295d41bfb54b741ab9d31230

LGTM :thumbsup:

rebased onto c7b7479
19 days ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/71c21d5c967c49039ac00d0f4cde4428

rebased onto c7b7479
19 days ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/f546e617c6324567bea0cbf2aa30090a

Pull-Request has been merged by zlopez
18 days ago
Metadata
None
No Tags
Changes Summary 2
+105 -4
file changed
tests/plugins/test_koji_block_retired.py
+39 -30
file changed
toddlers/plugins/koji_block_retired.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.