From 68220e487def957ff1aa3ff1f6bb2359371a18f2 Mon Sep 17 00:00:00 2001 From: Joep van Delft Date: Mon, 8 May 2023 20:46:59 +0200 Subject: [PATCH] rhcos buildfinder: Find valid releases (#768) Currently for 4.14, the rhcos jobs for the arches have resulted in good builds, yet, the `release` job failed. This leads to the circumstance that the builds are listed in `builds.json`, but are not viable for use. In particular, the `oscontainer` key is missing from the metadata. This commit ensures that the newest rhcos is selected where also this label is set. --- Makefile | 2 +- doozerlib/rhcos.py | 31 +++++++++++++++++-------------- tests/test_rhcos.py | 25 +++++++++++++++++++++++-- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index e3d2f3cd..7a38780e 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ venv: # source venv/bin/activate lint: - ./venv/bin/python -m flake8 + ./venv/bin/python -m flake8 doozerlib/ tests/ test: lint ./venv/bin/python -m pytest --verbose --color=yes tests/ diff --git a/doozerlib/rhcos.py b/doozerlib/rhcos.py index 99ee8f20..e4ae44be 100644 --- a/doozerlib/rhcos.py +++ b/doozerlib/rhcos.py @@ -170,17 +170,23 @@ def _latest_rhcos_build_id(self) -> Optional[str]: return None multi_url = self.runtime.group_config.urls.rhcos_release_base["multi"] - build_id = None + arches_building = [] if multi_url: - # Make sure all rhcos arch builds are complete arches_building = self.runtime.group_config.arches - for b in data["builds"]: - if len(b["arches"]) == len(arches_building): - build_id = b["id"] - break - else: - build_id = data["builds"][0]["id"] - return build_id + for b in data["builds"]: + # Make sure all rhcos arch builds are complete + if multi_url and len(b["arches"]) != len(arches_building): + continue + if not self.meta_has_required_attributes(self.rhcos_build_meta(b["id"])): + continue + return b["id"] + + def meta_has_required_attributes(self, meta): + required_keys = [payload_tag.build_metadata_key for payload_tag in self.runtime.group_config.rhcos.payload_tags] + for metadata_key in required_keys: + if metadata_key not in meta: + return False + return True @retry(reraise=True, stop=stop_after_attempt(10), wait=wait_fixed(3)) def rhcos_build_meta(self, build_id: str, meta_type: str = "meta") -> Dict: @@ -191,7 +197,7 @@ def rhcos_build_meta(self, build_id: str, meta_type: str = "meta") -> Dict: :return: Returns a Dict containing the parsed requested metadata. See the RHCOS release browser for examples: https://releases-rhcos-art.apps.ocp-virt.prod.psi.redhat.com/ Example 'meta.json': - https://releases-rhcos-art.apps.ocp-virt.prod.psi.redhat.com/storage/releases/rhcos-4.1/410.81.20200520.0/meta.json + https://releases-rhcos-art.apps.ocp-virt.prod.psi.redhat.com/storage/prod/streams/4.14-9.2/builds/414.92.202305050010-0/x86_64/meta.json { "buildid": "410.81.20200520.0", ... @@ -209,10 +215,7 @@ def _rhcos_build_meta(self, build_id: str, meta_type: str = "meta") -> Dict: """ See public API rhcos_build_meta for details. """ - url = f"{self.rhcos_release_url()}/{build_id}/" - # before 4.3 the arch was not included in the path - vtuple = tuple(int(f) for f in self.version.split(".")) - url += f"{meta_type}.json" if vtuple < (4, 3) else f"{self.brew_arch}/{meta_type}.json" + url = f"{self.rhcos_release_url()}/{build_id}/{self.brew_arch}/{meta_type}.json" with request.urlopen(url) as req: return json.loads(req.read().decode()) diff --git a/tests/test_rhcos.py b/tests/test_rhcos.py index 598afe66..8c97e015 100755 --- a/tests/test_rhcos.py +++ b/tests/test_rhcos.py @@ -62,7 +62,9 @@ def test_release_url(self): self.assertIn("4.9-s390x", rhcos.RHCOSBuildFinder(self.runtime, "4.9", "s390x").rhcos_release_url()) @patch('urllib.request.urlopen') - def test_build_id(self, mock_urlopen): + @patch("doozerlib.rhcos.RHCOSBuildFinder.meta_has_required_attributes") + def test_build_id(self, meta_has_required_attributes, mock_urlopen): + meta_has_required_attributes.return_value = True builds = [{'id': 'id-1'}, {'id': 'id-2'}] _urlopen_json_cm(mock_urlopen, dict(builds=builds)) self.assertEqual('id-1', rhcos.RHCOSBuildFinder(self.runtime, "4.4")._latest_rhcos_build_id()) @@ -70,7 +72,7 @@ def test_build_id(self, mock_urlopen): _urlopen_json_cm(mock_urlopen, dict(builds=[])) self.assertIsNone(rhcos.RHCOSBuildFinder(self.runtime, "4.2", "ppc64le")._latest_rhcos_build_id()) - self.assertIn('/rhcos-4.2-ppc64le/', mock_urlopen.call_args_list[1][0][0]) + self.assertIn('/rhcos-4.2-ppc64le/', mock_urlopen.call_args_list[2][0][0]) @patch('urllib.request.urlopen') def test_build_id_multi(self, mock_urlopen): @@ -80,6 +82,25 @@ def test_build_id_multi(self, mock_urlopen): self.runtime.group_config.arches = ['arch1', 'arch2', 'arch3'] self.assertEqual('id-2', rhcos.RHCOSBuildFinder(self.runtime, "4.4")._latest_rhcos_build_id()) + @patch('urllib.request.urlopen') + @patch('doozerlib.rhcos.RHCOSBuildFinder.rhcos_build_meta') + def test_build_id_build_release_job_completes(self, rhcos_build_meta, mock_urlopen): # XXX: Change name + # If not all required attributes exist, which can happen if the rhcos release job did not succesfully complete, take the previous + self.runtime.group_config.rhcos = Model(dict(payload_tags=[dict(name="spam", build_metadata_key="spam"), dict(name="eggs", primary=True, build_metadata_key="eggs")])) + + def mock_rhcos_build_meta(build_id): + if build_id == 'id-1': + return {'spam': 'sha:123'} + if build_id == 'id-2': + return {'spam': 'sha:345', 'eggs': 'sha:789'} + + rhcos_build_meta.side_effect = mock_rhcos_build_meta + builds = [{'id': 'id-1', 'arches': ['arch1']}, {'id': 'id-2', 'arches': ['arch1']}] + _urlopen_json_cm(mock_urlopen, dict(builds=builds)) + self.runtime.group_config.urls = Model(dict(rhcos_release_base=dict(multi='some_url'))) + self.runtime.group_config.arches = ['arch1'] + self.assertEqual('id-2', rhcos.RHCOSBuildFinder(self.runtime, "4.14")._latest_rhcos_build_id()) + @patch('urllib.request.urlopen') def test_build_find_failure(self, mock_urlopen): mock_urlopen.side_effect = URLError("test")