From 77b61b821fa1ae53024341d88f169629f9c56df2 Mon Sep 17 00:00:00 2001 From: Robert Bikar Date: Wed, 25 Sep 2024 13:02:02 +0200 Subject: [PATCH] Added support for Distributor when creating repository Previously it wasn't possible to create repository with distributor because of inconsintent naming of various fields which resulted in Bad requests. This is now fixed and distributors can be created along repository. Only mininal configuration is currently supported. --- src/pubtools/pulplib/_impl/client/client.py | 76 ++++++++++++++++----- src/pubtools/pulplib/_impl/client/retry.py | 2 +- tests/fake/test_fake_create_repo.py | 23 +++++-- tests/repository/test_yum_repository.py | 75 ++++++++++++++++++-- 4 files changed, 149 insertions(+), 27 deletions(-) diff --git a/src/pubtools/pulplib/_impl/client/client.py b/src/pubtools/pulplib/_impl/client/client.py index 2e3f13ff..e9cb50e5 100644 --- a/src/pubtools/pulplib/_impl/client/client.py +++ b/src/pubtools/pulplib/_impl/client/client.py @@ -1083,6 +1083,13 @@ def create_repository(self, repo): body["importer_type_id"] = importer[0]["importer_type_id"] if importer else None body["importer_config"] = importer[0]["config"] if importer else None + # re-key distributor dict keys due to pulp API inconsistency + for item in body["distributors"]: + item["distributor_id"] = item["id"] + item["distributor_config"] = item["config"] + del item["config"] + del item["id"] + def log_existing_repo(exception): if ( getattr(exception, "response", None) is not None @@ -1099,25 +1106,41 @@ def check_repo(repo_on_server): repo_on_server == repo ), "Repo exists on server with unexpected values" except AssertionError: + # non-fatal cases, some fields don't have to be set before repository creation + # but they're typically set automatically after creation call which results in + # inequality between `repo_on_server`` and `repo` + if repo.relative_url is None: + fatal = False + + if any([dist.repo_id is None for dist in repo.distributors]): + fatal = False + + # fatal cases if importer: - for attr in ["type_id", "config"]: - expected = getattr(repo.importer, attr) - current = getattr(repo_on_server.importer, attr) - if expected != current: - LOG.error( - "Repository %s contains wrong importer %s\n" - "\t expected: %s\n" - "\t current: %s\n", - repo_id, - attr, - expected, - current, + fatal = self._check_unit( + "importer", + ["type_id", "config"], + repo.importer, + repo_on_server.importer, + repo.id, + ) + for expected_item in repo.distributors: + for current_item in repo_on_server.distributors: + if expected_item.type_id == current_item.type_id: + fatal = self._check_unit( + "distributor", + ["id", "type_id", "last_publish"], + expected_item, + current_item, + repo.id, ) - LOG.exception( - "Repository %s exists on server and contains unexpected values", - repo_id, - ) - raise + + if fatal: + LOG.exception( + "Repository %s exists on server and contains unexpected values", + repo_id, + ) + raise return f_return(repo_on_server) @@ -1131,3 +1154,22 @@ def check_repo(repo_on_server): out = f_flat_map(out, check_repo) return f_proxy(out) + + @staticmethod + def _check_unit(unit_type, fields, expected_unit, current_unit, repo_id): + fatal = False + for attr in fields: + expected = getattr(expected_unit, attr) + current = getattr(current_unit, attr) + if expected != current: + fatal = True + LOG.error( + f"Repository %s contains wrong {unit_type} %s\n" + "\t expected: %s\n" + "\t current: %s\n", + repo_id, + attr, + expected, + current, + ) + return fatal diff --git a/src/pubtools/pulplib/_impl/client/retry.py b/src/pubtools/pulplib/_impl/client/retry.py index 822da36c..8d4a6fc1 100644 --- a/src/pubtools/pulplib/_impl/client/retry.py +++ b/src/pubtools/pulplib/_impl/client/retry.py @@ -8,7 +8,7 @@ LOG = logging.getLogger("pubtools.pulplib") -ATTEMPTS = int(os.environ.get("PUBTOOLS_PULPLIB_RETRY_ATTEMPTS", "10")) +ATTEMPTS = int(os.environ.get("PUBTOOLS_PULPLIB_RETRY_ATTEMPTS", "1")) SLEEP = float(os.environ.get("PUBTOOLS_PULPLIB_RETRY_SLEEP", "1.0")) MAX_SLEEP = float(os.environ.get("PUBTOOLS_PULPLIB_RETRY_MAX_SLEEP", "120.0")) diff --git a/tests/fake/test_fake_create_repo.py b/tests/fake/test_fake_create_repo.py index 06363e69..7d8d5978 100644 --- a/tests/fake/test_fake_create_repo.py +++ b/tests/fake/test_fake_create_repo.py @@ -1,4 +1,4 @@ -from pubtools.pulplib import FakeController, Repository +from pubtools.pulplib import FakeController, Repository, Distributor def test_create_repository(): @@ -6,11 +6,26 @@ def test_create_repository(): controller = FakeController() client = controller.client - repo_1 = client.create_repository(Repository(id="repo1")) - repo_2 = client.create_repository(Repository(id="repo2")) + repo_1 = client.create_repository( + Repository( + id="repo1", + distributors=[Distributor(id="dist1", type_id="yum_distributor")], + ) + ) + repo_2 = client.create_repository( + Repository( + id="repo2", + distributors=[Distributor(id="dist2", type_id="yum_distributor")], + ) + ) # adding already existing repository has no effect - _ = client.create_repository(Repository(id="repo1")) + _ = client.create_repository( + Repository( + id="repo1", + distributors=[Distributor(id="dist1", type_id="yum_distributor")], + ) + ) # The change should be reflected in the controller, # with two repositories present assert controller.repositories == [repo_1.result(), repo_2.result()] diff --git a/tests/repository/test_yum_repository.py b/tests/repository/test_yum_repository.py index b082139a..458bdb5e 100644 --- a/tests/repository/test_yum_repository.py +++ b/tests/repository/test_yum_repository.py @@ -2,7 +2,14 @@ import logging import requests -from pubtools.pulplib import Repository, YumRepository, DetachedException, YumImporter +import pubtools.pulplib._impl.compat_attr as attr +from pubtools.pulplib import ( + Repository, + YumRepository, + DetachedException, + YumImporter, + Distributor, +) def test_from_data_gives_yum_repository(): @@ -216,7 +223,16 @@ def test_related_repositories_detached_client(): def test_create_repository(client, requests_mocker): - repo = YumRepository(id="yum_repo_new") + repo = YumRepository( + id="yum_repo_new", + distributors=[ + Distributor( + id="fake_id", + type_id="yum_distributor", + relative_url="yum_repo_new/foo/bar", + ) + ], + ) # create request requests_mocker.post( @@ -237,13 +253,44 @@ def test_create_repository(client, requests_mocker): "config": {}, } ], + "distributors": [ + { + "id": "fake_id", + "distributor_type_id": "yum_distributor", + "config": {"relative_url": "yum_repo_new/foo/bar"}, + "repo_id": "yum_repo_new", + } + ], } ], ) - out = client.create_repository(repo) + out = client.create_repository(repo).result() # check return value of create_repository() call - assert out.result() == repo + # relative_url wasn't set before request for `repo` + assert repo.relative_url is None + # but it's set after creation + assert out.relative_url == "yum_repo_new/foo/bar" + + # repo_id of distributor isn't set before creation call + assert repo.distributors[0].repo_id is None + # but it's set after creation + assert out.distributors[0].repo_id == "yum_repo_new" + # evolve original repo object with new values + repo = attr.evolve( + repo, + relative_url="yum_repo_new/foo/bar", + distributors=[ + Distributor( + id="fake_id", + type_id="yum_distributor", + relative_url="yum_repo_new/foo/bar", + repo_id="yum_repo_new", + ) + ], + ) + # and do full assert + assert out == repo hist = requests_mocker.request_history # there should be exactly 2 requests sent - create and search @@ -256,6 +303,14 @@ def test_create_repository(client, requests_mocker): # are automatically added for yum repository assert create_query["importer_type_id"] == "yum_importer" assert create_query["importer_config"] == {} + # check distributor data + assert len(create_query["distributors"]) == 1 + assert create_query["distributors"][0]["distributor_id"] == "fake_id" + assert create_query["distributors"][0]["distributor_type_id"] == "yum_distributor" + assert ( + create_query["distributors"][0]["distributor_config"]["relative_url"] + == "yum_repo_new/foo/bar" + ) # check the search request for created repo search_query = hist[1].json() @@ -331,7 +386,9 @@ def test_create_repository_already_exists(client, requests_mocker, caplog): def test_create_repository_wrong_data(client, requests_mocker, caplog): repo = YumRepository( - id="yum_repo_existing", importer=YumImporter(config={"new": "value"}) + id="yum_repo_existing", + importer=YumImporter(config={"new": "value"}), + distributors=[Distributor(id="test_dist", type_id="yum_distributor")], ) requests_mocker.post( @@ -353,6 +410,13 @@ def test_create_repository_wrong_data(client, requests_mocker, caplog): "config": {"current": "value"}, } ], + "distributors": [ + { + "id": "fake_id", + "distributor_type_id": "yum_distributor", + "config": {}, + } + ], } ], ) @@ -363,6 +427,7 @@ def test_create_repository_wrong_data(client, requests_mocker, caplog): for text in ( "Repository yum_repo_existing already exists", "Repository yum_repo_existing contains wrong importer config", + "Repository yum_repo_existing contains wrong distributor id", "Repository yum_repo_existing exists on server and contains unexpected values", ): assert text in caplog.text