diff --git a/pubtools/_pulp/services/fakepulp.py b/pubtools/_pulp/services/fakepulp.py index 6c30544e..caef88be 100644 --- a/pubtools/_pulp/services/fakepulp.py +++ b/pubtools/_pulp/services/fakepulp.py @@ -136,6 +136,11 @@ def load_initial(self): self.ctrl.insert_repository(FileRepository(id="redhat-maintenance")) self.ctrl.insert_repository(FileRepository(id="all-iso-content")) self.ctrl.insert_repository(YumRepository(id="all-rpm-content")) + # Repos required for common test data (RHELDST-23264) + self.ctrl.insert_repository(YumRepository(id="all-rpm-content-e8")) + self.ctrl.insert_repository(YumRepository(id="all-rpm-content-54")) + self.ctrl.insert_repository(YumRepository(id="all-erratum-content-2019")) + self.ctrl.insert_repository(YumRepository(id="all-erratum-content-2020")) def load(self): """Load data into the fake from previously serialized state (if any). diff --git a/pubtools/_pulp/tasks/push/items/__init__.py b/pubtools/_pulp/tasks/push/items/__init__.py index 269d071e..a2ebb930 100644 --- a/pubtools/_pulp/tasks/push/items/__init__.py +++ b/pubtools/_pulp/tasks/push/items/__init__.py @@ -1,12 +1,13 @@ from .base import PulpPushItem, State from .rpm import PulpRpmPushItem from .file import PulpFilePushItem -from .erratum import PulpErratumPushItem +from .erratum import PulpErratumPushItem, ErratumPushItemException from .modulemd import PulpModuleMdPushItem from .comps import PulpCompsXmlPushItem from .productid import PulpProductIdPushItem __all__ = [ + "ErratumPushItemException", "PulpPushItem", "PulpRpmPushItem", "PulpFilePushItem", diff --git a/pubtools/_pulp/tasks/push/items/base.py b/pubtools/_pulp/tasks/push/items/base.py index 96a2ecbc..dcb1a4e8 100644 --- a/pubtools/_pulp/tasks/push/items/base.py +++ b/pubtools/_pulp/tasks/push/items/base.py @@ -95,6 +95,24 @@ class PulpPushItem(object): to a single unit (e.g. modulemd YAML files; comps.xml files). """ + MULTI_UPLOAD_CONTEXT = False + """ + Can the push item class create a different upload context based on the data + it holds. e.g PulpRpmPushItem has checksum separated upload repos/contexts. + + If true, subclass should override upload_repo. + """ + + @property + def upload_repo(self): + """Given a PulpPushItem, returns the repo its pushsource.PushItem + should be uploaded to. + + Subclasses MAY override this to return a variable default destination. + Subclasses must set MULTI_UPLOAD_CONTEXT to True for this to be used. + """ + return "" # pragma: no cover + @classmethod def for_item(cls, pushsource_item, **kwargs): """Given a pushsource.PushItem, returns an instance of a PulpPushItem wrapper @@ -136,12 +154,11 @@ def match_items_units(cls, items, units): return klass.match_items_units(items, units) - @classmethod - def upload_context(cls, pulp_client): + def upload_context(self, pulp_client): """Return a context object used during uploads. - The context object will be shared across all uploads for a specific content - type. + The context object should be shared across all uploads for a specific + content type. Subclasses classes may introduce multiple contexts. Subclasses MAY override this to provide their own context (e.g. to cache a value rather than recalculating per upload). diff --git a/pubtools/_pulp/tasks/push/items/erratum.py b/pubtools/_pulp/tasks/push/items/erratum.py index bb9dbfa2..1002639c 100644 --- a/pubtools/_pulp/tasks/push/items/erratum.py +++ b/pubtools/_pulp/tasks/push/items/erratum.py @@ -1,5 +1,5 @@ import logging - +import re import attr from pushsource import ErratumPushItem from pubtools.pulplib import ( @@ -18,24 +18,44 @@ class ErratumUploadContext(UploadContext): """A custom context for Erratum uploads. - This context object avoids having to query the UPLOAD_REPO repeatedly. + This context object avoids having to query the PushItem's repo repeatedly. """ upload_repo = attr.ib(default=None) +class ErratumPushItemException(BaseException): + """ + Custom exception for PulpErratumPushItem specific issues. + + Mainly used for when a year value can't be parsed from the advisory name. + """ + + @supports_type(ErratumPushItem) @attr.s(frozen=True, slots=True) class PulpErratumPushItem(PulpPushItem): """Handler for errata.""" - # Erratums are always uploaded to this repo first. - # Pulp stores the erratum pkglist for (erratum_id, repo) pair. If an erratum is - # uploaded for an already existing ID, the pkglist is overwritten if the repo is - # same, else is appended. To keep the pkglist consistent, all the erratums are - # uploaded to the same repo instead of a random repo from the list of destinations. - # For more details, refer RHELDST-12782. - UPLOAD_REPO = "all-rpm-content" + MULTI_UPLOAD_CONTEXT = True + ADVISORY_PATTERN = re.compile(r"RH.A-(\d{4})", re.IGNORECASE) + CONTENT_SPLIT_RANGES = [(2014, 2040), (8014, 8040)] + + @property + def upload_repo(self): + # Split errata into different repos by year, assuming the advisory name + # is formatted like RHXA-YYYY + name_match = self.ADVISORY_PATTERN.match(self.pushsource_item.name) + if not name_match: + LOG.error("Bad Advisory name: '%s' does not contain a reasonable year value.", + self.pushsource_item.name) + raise ErratumPushItemException + year = int(name_match.group(1)) + if not any([r[0] <= year <= r[1] for r in self.CONTENT_SPLIT_RANGES]): + LOG.warning("%s was not in a valid date range for repo content splitting, using the default.", + self.pushsource_item.name ) + year = "0000" + return "all-erratum-content-%s" % year @property def unit_type(self): @@ -89,11 +109,10 @@ def with_unit(self, unit): return out - @classmethod - def upload_context(cls, pulp_client): + def upload_context(self, pulp_client): return ErratumUploadContext( client=pulp_client, - upload_repo=pulp_client.get_repository(cls.UPLOAD_REPO), + upload_repo=pulp_client.get_repository(self.upload_repo), ) @classmethod diff --git a/pubtools/_pulp/tasks/push/items/rpm.py b/pubtools/_pulp/tasks/push/items/rpm.py index b6cd8dda..3e8d606c 100644 --- a/pubtools/_pulp/tasks/push/items/rpm.py +++ b/pubtools/_pulp/tasks/push/items/rpm.py @@ -9,9 +9,11 @@ @attr.s(frozen=True, slots=True) class RpmUploadContext(UploadContext): - """A custom context for RPM uploads. + """ + A custom context for RPM uploads. - This context object avoids having to query the all-rpm-content repo repeatedly. + This context object lets us avoid querying the all-rpm-content-XX repos + repeatedly. """ upload_repo = attr.ib(default=None) @@ -22,8 +24,12 @@ class RpmUploadContext(UploadContext): class PulpRpmPushItem(PulpPushItem): """Handler for RPMs.""" - # RPMs are always uploaded to this repo first. - UPLOAD_REPO = "all-rpm-content" + MULTI_UPLOAD_CONTEXT = True + + @property + def upload_repo(self): + # Split RPMs into different repos by checksum + return "all-rpm-content-%s" % self.pushsource_item.sha256sum[0:2] @property def unit_type(self): @@ -89,16 +95,15 @@ def match_items_units(cls, items, units): for item in items: yield item.with_unit(units_by_sum.get(item.pushsource_item.sha256sum)) - @classmethod - def upload_context(cls, pulp_client): + def upload_context(self, pulp_client): return RpmUploadContext( client=pulp_client, - upload_repo=pulp_client.get_repository(cls.UPLOAD_REPO), + upload_repo=pulp_client.get_repository(self.upload_repo), ) @property def can_pre_push(self): - # We support pre-push by uploading to all-rpm-content first. + # We support pre-push by uploading to all-rpm-content-XX first. return True @property diff --git a/pubtools/_pulp/tasks/push/phase/upload.py b/pubtools/_pulp/tasks/push/phase/upload.py index b8f65a3f..3b3cf445 100644 --- a/pubtools/_pulp/tasks/push/phase/upload.py +++ b/pubtools/_pulp/tasks/push/phase/upload.py @@ -60,13 +60,17 @@ def run(self): else: # This item is not in Pulp, or otherwise needs a reupload. item_type = type(item) - if item_type not in upload_context: - upload_context[item_type] = item_type.upload_context( - self.pulp_client - ) - - ctx = upload_context[item_type] - + if item.MULTI_UPLOAD_CONTEXT: + if item_type not in upload_context: + upload_context[item_type] = {} + if item.upload_repo not in upload_context[item_type]: + upload_context[item_type][item.upload_repo] = \ + item.upload_context(self.pulp_client) + ctx = upload_context[item_type][item.upload_repo] + else: + if item_type not in upload_context: + upload_context[item_type] = item.upload_context(self.pulp_client) + ctx = upload_context[item_type] uploading += 1 self.put_future_output(item.ensure_uploaded(ctx)) diff --git a/tests/fake/test_fake_persistence.py b/tests/fake/test_fake_persistence.py index bb7f248a..e5ed28b2 100644 --- a/tests/fake/test_fake_persistence.py +++ b/tests/fake/test_fake_persistence.py @@ -24,7 +24,10 @@ def test_state_persisted(tmpdir, data_path): # It should already have a few repos since there is some default # state. repo_ids = sorted([repo.id for repo in client.search_repository()]) - assert repo_ids == ["all-iso-content", "all-rpm-content", "redhat-maintenance"] + assert repo_ids == ["all-erratum-content-2019", "all-erratum-content-2020", + "all-iso-content", "all-rpm-content", + "all-rpm-content-54", "all-rpm-content-e8", + "redhat-maintenance"] # Now add a bit more state. # We use modules here because that's one of the more complex types diff --git a/tests/logs/push/test_association_race/test_association_race.pulp.yaml b/tests/logs/push/test_association_race/test_association_race.pulp.yaml index d731f31a..343d0d6a 100644 --- a/tests/logs/push/test_association_race/test_association_race.pulp.yaml +++ b/tests/logs/push/test_association_race/test_association_race.pulp.yaml @@ -1,8 +1,16 @@ repos: +- _class: YumRepository + id: all-erratum-content-2019 +- _class: YumRepository + id: all-erratum-content-2020 - _class: FileRepository id: all-iso-content - _class: YumRepository id: all-rpm-content +- _class: YumRepository + id: all-rpm-content-54 +- _class: YumRepository + id: all-rpm-content-e8 - _class: YumRepository arch: x86_64 eng_product_id: 123 @@ -131,7 +139,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2020 - dest1 rights: Copyright 2020 Red Hat Inc severity: None @@ -2757,7 +2765,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2019 - dest1 rights: Copyright 2019 Red Hat Inc severity: None @@ -2903,7 +2911,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2020 - dest1 rights: Copyright 2020 Red Hat Inc severity: Important @@ -7876,7 +7884,7 @@ units: version: '5.21' release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-e8 - dest2 requires: - _class: RpmDependency @@ -7906,7 +7914,7 @@ units: provides: [] release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-54 - dest1 requires: - _class: RpmDependency diff --git a/tests/logs/push/test_push/test_empty_push.pulp.yaml b/tests/logs/push/test_push/test_empty_push.pulp.yaml index 8a22d3ac..fb52b268 100644 --- a/tests/logs/push/test_push/test_empty_push.pulp.yaml +++ b/tests/logs/push/test_push/test_empty_push.pulp.yaml @@ -1,8 +1,16 @@ repos: +- _class: YumRepository + id: all-erratum-content-2019 +- _class: YumRepository + id: all-erratum-content-2020 - _class: FileRepository id: all-iso-content - _class: YumRepository id: all-rpm-content +- _class: YumRepository + id: all-rpm-content-54 +- _class: YumRepository + id: all-rpm-content-e8 - _class: YumRepository arch: x86_64 eng_product_id: 123 diff --git a/tests/logs/push/test_push/test_nopublish_push.pulp.yaml b/tests/logs/push/test_push/test_nopublish_push.pulp.yaml index 9b274ddd..bb097060 100644 --- a/tests/logs/push/test_push/test_nopublish_push.pulp.yaml +++ b/tests/logs/push/test_push/test_nopublish_push.pulp.yaml @@ -1,8 +1,16 @@ repos: +- _class: YumRepository + id: all-erratum-content-2019 +- _class: YumRepository + id: all-erratum-content-2020 - _class: FileRepository id: all-iso-content - _class: YumRepository id: all-rpm-content +- _class: YumRepository + id: all-rpm-content-54 +- _class: YumRepository + id: all-rpm-content-e8 - _class: YumRepository arch: x86_64 eng_product_id: 123 @@ -131,7 +139,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2020 - dest1 rights: Copyright 2020 Red Hat Inc severity: None @@ -2757,7 +2765,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2019 - dest1 rights: Copyright 2019 Red Hat Inc severity: None @@ -2903,7 +2911,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2020 - dest1 rights: Copyright 2020 Red Hat Inc severity: Important @@ -7876,7 +7884,7 @@ units: version: '5.21' release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-e8 - dest1 - dest2 requires: @@ -7907,7 +7915,7 @@ units: provides: [] release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-54 - dest1 requires: - _class: RpmDependency diff --git a/tests/logs/push/test_push/test_typical_push.pulp.yaml b/tests/logs/push/test_push/test_typical_push.pulp.yaml index 65dc67b8..99f8d59f 100644 --- a/tests/logs/push/test_push/test_typical_push.pulp.yaml +++ b/tests/logs/push/test_push/test_typical_push.pulp.yaml @@ -1,8 +1,16 @@ repos: +- _class: YumRepository + id: all-erratum-content-2019 +- _class: YumRepository + id: all-erratum-content-2020 - _class: FileRepository id: all-iso-content - _class: YumRepository id: all-rpm-content +- _class: YumRepository + id: all-rpm-content-54 +- _class: YumRepository + id: all-rpm-content-e8 - _class: YumRepository arch: x86_64 eng_product_id: 123 @@ -131,7 +139,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2020 - dest1 rights: Copyright 2020 Red Hat Inc severity: None @@ -2757,7 +2765,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2019 - dest1 rights: Copyright 2019 Red Hat Inc severity: None @@ -2903,7 +2911,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2020 - dest1 rights: Copyright 2020 Red Hat Inc severity: Important @@ -7880,7 +7888,7 @@ units: version: '5.21' release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-e8 - dest1 - dest2 requires: @@ -7912,7 +7920,7 @@ units: provides: [] release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-54 - dest1 requires: - _class: RpmDependency diff --git a/tests/logs/push/test_push/test_update_push.pulp.yaml b/tests/logs/push/test_push/test_update_push.pulp.yaml index 4e83d71c..c42421f5 100644 --- a/tests/logs/push/test_push/test_update_push.pulp.yaml +++ b/tests/logs/push/test_push/test_update_push.pulp.yaml @@ -1,8 +1,16 @@ repos: +- _class: YumRepository + id: all-erratum-content-2019 +- _class: YumRepository + id: all-erratum-content-2020 - _class: FileRepository id: all-iso-content - _class: YumRepository id: all-rpm-content +- _class: YumRepository + id: all-rpm-content-54 +- _class: YumRepository + id: all-rpm-content-e8 - _class: YumRepository arch: x86_64 eng_product_id: 123 @@ -131,7 +139,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2020 - dest1 rights: Copyright 2020 Red Hat Inc severity: None @@ -2757,7 +2765,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2019 - dest1 rights: Copyright 2019 Red Hat Inc severity: None @@ -2903,7 +2911,7 @@ units: type: other release: '0' repository_memberships: - - all-rpm-content + - all-erratum-content-2020 - dest1 rights: Copyright 2020 Red Hat Inc severity: Important @@ -7881,7 +7889,7 @@ units: version: '5.21' release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-e8 - dest1 - dest2 requires: diff --git a/tests/logs/push/test_push_prepush/test_pre_push.pulp.yaml b/tests/logs/push/test_push_prepush/test_pre_push.pulp.yaml index 8a5af83b..65701d74 100644 --- a/tests/logs/push/test_push_prepush/test_pre_push.pulp.yaml +++ b/tests/logs/push/test_push_prepush/test_pre_push.pulp.yaml @@ -1,8 +1,16 @@ repos: +- _class: YumRepository + id: all-erratum-content-2019 +- _class: YumRepository + id: all-erratum-content-2020 - _class: FileRepository id: all-iso-content - _class: YumRepository id: all-rpm-content +- _class: YumRepository + id: all-rpm-content-54 +- _class: YumRepository + id: all-rpm-content-e8 - _class: YumRepository arch: x86_64 eng_product_id: 123 @@ -33,7 +41,7 @@ units: version: '5.21' release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-e8 requires: - _class: RpmDependency epoch: '0' @@ -62,7 +70,7 @@ units: provides: [] release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-54 requires: - _class: RpmDependency epoch: '0' diff --git a/tests/logs/push/test_push_prepush/test_pre_push_no_dest.pulp.yaml b/tests/logs/push/test_push_prepush/test_pre_push_no_dest.pulp.yaml index 758689be..6ecbacdf 100644 --- a/tests/logs/push/test_push_prepush/test_pre_push_no_dest.pulp.yaml +++ b/tests/logs/push/test_push_prepush/test_pre_push_no_dest.pulp.yaml @@ -1,8 +1,16 @@ repos: +- _class: YumRepository + id: all-erratum-content-2019 +- _class: YumRepository + id: all-erratum-content-2020 - _class: FileRepository id: all-iso-content - _class: YumRepository id: all-rpm-content +- _class: YumRepository + id: all-rpm-content-54 +- _class: YumRepository + id: all-rpm-content-e8 - _class: YumRepository arch: x86_64 eng_product_id: 123 @@ -33,7 +41,7 @@ units: version: '5.21' release: '1' repository_memberships: - - all-rpm-content + - all-rpm-content-e8 requires: - _class: RpmDependency epoch: '0' diff --git a/tests/push/test_pulperratumpushitem.py b/tests/push/test_pulperratumpushitem.py index c99c3784..318cd832 100644 --- a/tests/push/test_pulperratumpushitem.py +++ b/tests/push/test_pulperratumpushitem.py @@ -1,8 +1,8 @@ from pubtools.pulplib import ErratumUnit from pushsource import ErratumPushItem -from pubtools._pulp.tasks.push.items import PulpErratumPushItem, State - +from pubtools._pulp.tasks.push.items import PulpErratumPushItem, State, ErratumPushItemException +import pytest def test_erratum_publishes_all_repos(): item = PulpErratumPushItem( @@ -30,3 +30,35 @@ def test_erratum_publishes_all_repos(): # republished for all of them. # all-rpm-content is an exception given that those repos don't get published. assert item.publish_pulp_repos == ["existing1", "existing2", "new1", "new2"] + +def test_erratum_upload_repo_normal(): + # Test upload_repo maps name to expected repo + + item = PulpErratumPushItem( + pushsource_item=ErratumPushItem(name="RHSA-2019:1234") + ) + + assert item.upload_repo == "all-erratum-content-2019" + + +def test_erratum_upload_repo_default(): + # Test upload_repo maps name to default repo when advisory year is outside + # the expected range + + item = PulpErratumPushItem( + pushsource_item=ErratumPushItem(name="RHSA-1999:1234") + ) + + assert item.upload_repo == "all-erratum-content-0000" + + + +def test_erratum_upload_repo_bad_format(): + # Test upload_repo maps name to default repo when advisory year is outside + # the expected range + + with pytest.raises(ErratumPushItemException): + item = PulpErratumPushItem( + pushsource_item=ErratumPushItem(name="RHSA-fail:1234") + ) + item.upload_repo \ No newline at end of file diff --git a/tests/push/test_push.py b/tests/push/test_push.py index dc6f35d7..de919929 100644 --- a/tests/push/test_push.py +++ b/tests/push/test_push.py @@ -131,7 +131,7 @@ def test_typical_push( ) # It should have invoked hook(s). - assert len(hookspy) == 25 + assert len(hookspy) == 29 (hook_name, hook_kwargs) = hookspy[0] assert hook_name == "task_start" (hook_name, hook_kwargs) = hookspy[1] diff --git a/tests/push/test_push_prepush.py b/tests/push/test_push_prepush.py index 4b7e1b9f..8c1f4ce9 100644 --- a/tests/push/test_push_prepush.py +++ b/tests/push/test_push_prepush.py @@ -57,14 +57,16 @@ def test_pre_push( # It should have uploaded some stuff assert units + # RPMs are separated into different arc repos by checksum + expected_repo_mapping = {"test-srpm01": ["all-rpm-content-54"], + "walrus": ["all-rpm-content-e8"]} + for unit in units: # The only type of content is RPMs, because that's all we support for # pre-push right now assert isinstance(unit, RpmUnit) - # And the only repo containing those RPMs should be all-rpm-content, - # because that's how pre-push works - assert unit.repository_memberships == ["all-rpm-content"] + assert unit.repository_memberships == expected_repo_mapping[unit.name] def test_pre_push_no_dest( @@ -124,4 +126,4 @@ def test_pre_push_no_dest( assert isinstance(units[0], RpmUnit) # Only to this repo - assert units[0].repository_memberships == ["all-rpm-content"] + assert units[0].repository_memberships == ["all-rpm-content-e8"] diff --git a/tests/push/test_upload_sharing.py b/tests/push/test_upload_sharing.py index f3b52c30..0d27b9e3 100644 --- a/tests/push/test_upload_sharing.py +++ b/tests/push/test_upload_sharing.py @@ -39,8 +39,9 @@ def test_uploads_shared(data_path): """Upload phase allows for uploads of identical content to be reused.""" pulp_ctrl = FakeController() + pulp_ctrl.insert_repository(YumRepository(id="all-rpm-content-54")) + pulp_ctrl.insert_repository(YumRepository(id="all-rpm-content-e8")) - pulp_ctrl.insert_repository(YumRepository(id="all-rpm-content")) pulp_ctrl.insert_repository(YumRepository(id="repo1")) pulp_ctrl.insert_repository(YumRepository(id="repo2")) pulp_ctrl.insert_repository(YumRepository(id="repo3"))