From acda67d431c0d1deb3cda0a5027aeed508b58dd6 Mon Sep 17 00:00:00 2001 From: Tatiana Tereshchenko Date: Wed, 1 Sep 2021 21:25:18 +0200 Subject: [PATCH] Fix the case when pulp2 importer is not migrated but its LCE is used. closes #9372 https://pulp.plan.io/issues/9372 backports #8862 https://pulp.plan.io/issues/8862 --- CHANGES/9372.bugfix | 2 + pulp_2to3_migration/app/plugin/content.py | 85 +++++++++++-------- .../tests/functional/test_rpm_repo.py | 42 +++++++++ 3 files changed, 95 insertions(+), 34 deletions(-) create mode 100644 CHANGES/9372.bugfix diff --git a/CHANGES/9372.bugfix b/CHANGES/9372.bugfix new file mode 100644 index 00000000..ae0ee801 --- /dev/null +++ b/CHANGES/9372.bugfix @@ -0,0 +1,2 @@ +Fixed distibution tree migration issue "‘NoneType’ object has no attribute ‘url’". +(backported from #8862) diff --git a/pulp_2to3_migration/app/plugin/content.py b/pulp_2to3_migration/app/plugin/content.py index 06d37561..55430609 100644 --- a/pulp_2to3_migration/app/plugin/content.py +++ b/pulp_2to3_migration/app/plugin/content.py @@ -305,8 +305,10 @@ def get_remote_by_importer_id(importer_id): base_path = pulp2content.pulp2_storage_path remotes = set() missing_artifact = False + remote_declarative_artifacts = [] for image_relative_path in extra_info['download']['images']: + remote_url_tuples = [] image_path = os.path.join(base_path, image_relative_path) downloaded = os.path.exists(image_path) if downloaded: @@ -320,30 +322,34 @@ def get_remote_by_importer_id(importer_id): artifact = Artifact() lces = pulp2lazycatalog.filter(pulp2_storage_path=image_path) - if lces: - remote_declarative_artifacts = [] - for lce in lces: - remote = get_remote_by_importer_id(lce.pulp2_importer_id) - - if not remote and not downloaded: - continue + if not lces and not downloaded: + continue + # collect all urls and respective migrated remotes for the image + for lce in lces: + remote = get_remote_by_importer_id(lce.pulp2_importer_id) + if remote: remotes.add(remote) - da = DeclarativeArtifact( - artifact=artifact, - url=lce.pulp2_url, - relative_path=image_relative_path, - remote=remote, - deferred_download=not downloaded) - remote_declarative_artifacts.append(da) - - if not remote_declarative_artifacts: + remote_url_tuples.append((remote, lce.pulp2_url)) + + for remote, url in remote_url_tuples: + da = DeclarativeArtifact( + artifact=artifact, + url=lce.pulp2_url, + relative_path=image_relative_path, + remote=remote, + deferred_download=not downloaded) + remote_declarative_artifacts.append(da) + + if not remote_url_tuples: + # either no LCEs existed but it's a downloaded content (and we can + # proceed), or remotes for any of LCEs haven't been migrated (and + # nothing can be done at this point) + if not downloaded: missing_artifact = True break - d_artifacts.extend(remote_declarative_artifacts) - else: da = DeclarativeArtifact( artifact=artifact, url=NOT_USED, @@ -352,6 +358,8 @@ def get_remote_by_importer_id(importer_id): deferred_download=False) d_artifacts.append(da) + d_artifacts.extend(remote_declarative_artifacts) + # Only skip the rest of the steps if there are any images that are expected # to be downloaded. There are distribution trees without images in the wild, # e.g. CentOS 8 High Availability. @@ -416,23 +424,27 @@ def get_remote_by_importer_id(importer_id): pb.increment() continue + relative_path = ( + pulp_2to3_detail_content.relative_path_for_content_artifact + ) + remote_lce_tuples = [] + deferred_download = not pulp2content.downloaded + if is_lazy_type and pulp2lazycatalog: + for lce in pulp2lazycatalog: + remote = get_remote_by_importer_id(lce.pulp2_importer_id) + if remote: + remote_lce_tuples.append((remote, lce)) + # handle DA and RA creation for content that supports on_demand # Downloaded or on_demand content with LCEs. # # To create multiple remote artifacts, create multiple instances of # declarative content which will differ by url/remote in their # declarative artifacts - at_least_one_lce_migrated = False - for lce in pulp2lazycatalog: - remote = get_remote_by_importer_id(lce.pulp2_importer_id) - deferred_download = not pulp2content.downloaded - if not remote and deferred_download: - continue - relative_path = ( - pulp_2to3_detail_content.relative_path_for_content_artifact - ) + if remote_lce_tuples: + for remote, lce in remote_lce_tuples: da = DeclarativeArtifact( artifact=artifact, url=lce.pulp2_url, @@ -440,22 +452,27 @@ def get_remote_by_importer_id(importer_id): remote=remote, deferred_download=deferred_download) lce.is_migrated = True - at_least_one_lce_migrated = True dc = DeclarativeContent(content=pulp3content, d_artifacts=[da]) + + # yes, all LCEs are assigned for each dc to be resolved at a later + # stage. Some LCEs might be "bad" and not have a migrated importer + # but we still need to resolved such. It creates some duplicated LCEs + # to process later but ensures that all are resolved if at least one + # valid one is migrated. + future_relations.update({'lces': list(pulp2lazycatalog)}) dc.extra_data = future_relations await self.put(dc) - if not at_least_one_lce_migrated: + else: + # No migratable LCE available + if deferred_download: _logger.warn( _('On_demand content cannot be migrated without a remote ' 'pulp2 unit_id: {}'.format(pulp2content.pulp2_id) ) ) - future_relations.update({'lces': list(pulp2lazycatalog)}) - else: - relative_path = ( - pulp_2to3_detail_content.relative_path_for_content_artifact - ) + continue + da = DeclarativeArtifact( artifact=artifact, url=NOT_USED, diff --git a/pulp_2to3_migration/tests/functional/test_rpm_repo.py b/pulp_2to3_migration/tests/functional/test_rpm_repo.py index 40101cc7..d9266c1a 100644 --- a/pulp_2to3_migration/tests/functional/test_rpm_repo.py +++ b/pulp_2to3_migration/tests/functional/test_rpm_repo.py @@ -1,11 +1,30 @@ +import json import os import unittest + from pulp_2to3_migration.tests.functional.util import set_pulp2_snapshot from .common_plans import RPM_SIMPLE_PLAN, RPM_COMPLEX_PLAN from .constants import FIXTURES_BASE_URL from .rpm_base import BaseTestRpm, RepoInfo +RPM_KICKSTART_NO_IMPORTER_PLAN = json.dumps({ + "plugins": [{ + "type": "rpm", + "repositories": [ + { + "name": "c8base", + "repository_versions": [ + { + "pulp2_repository_id": "c8base", + "pulp2_distributor_repository_ids": ["c8base"] + }, + ] + } + ] + }] +}) + PULP_2_RPM_DATA = { 'remotes': 3, 'content_initial': { @@ -132,3 +151,26 @@ class TestRpmRepoMigrationComplexPlan(BaseTestRpmRepo, unittest.TestCase): """ plan_initial = RPM_COMPLEX_PLAN repo_info = RepoInfo(PULP_2_RPM_DATA) + + +@unittest.skip('The image files of a kickstart repo are too large for github, need to find ' + 'smaller ones.') +class TestRpmKickstartImmediateNoImporterPlan(unittest.TestCase): + """ + Test RPM repo migration when a kickstart tree is downloaded but its importer is not migrated. + """ + @classmethod + def setUpClass(cls): + """ + Create all the client instances needed to communicate with Pulp and run a migration. + """ + super().setUpClass() + + set_pulp2_snapshot(name='rpm_kickstart_immediate_no_rpm') + + def test_rpm_repo_migration(self): + """ + Make sure no-importer migration is successful for a downloaded kickstart tree. + """ + self.run_migration(RPM_KICKSTART_NO_IMPORTER_PLAN) + self.assertEqual(self.rpm_distribution_api.list().count, 1)