Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix depsolving bug where not all dependencies are copied #1730

Merged
merged 1 commit into from
Jun 1, 2020

Conversation

dralley
Copy link
Contributor

@dralley dralley commented May 31, 2020

@dralley dralley requested a review from ggainey May 31, 2020 17:29
@pulpbot
Copy link
Member

pulpbot commented May 31, 2020

Attached issue: https://pulp.plan.io/issues/6820

@dralley
Copy link
Contributor Author

dralley commented May 31, 2020

@dkliban Would appreciate it if this bugfix can go into the release tomorrow -- Katello needs it.

solvables_copied = run_solver_jobs(install_jobs)
result_solvables.update(solvables_copied)
# Depsolve using the list of unit install jobs, add them to the results
solvables_copied = run_solver_jobs(install_jobs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes mean that install_jobs[] is only ever going to be [] or, if we are-a module, [unit_install_job], and we will run_solver_jobs() every time throught the while-loop - am I reading that correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, no, I don't see why that would be the case? On either branch, the install job for the solvable that was popped is added to the list (lines 891 and 900), and on the module branch it includes all the artifacts as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, I wasn't reading the flow closely enough, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is true that it will run run_solver_jobs for each individual package being copied now, whereas before it was running it for all of them at once. That's what it did in Pulp 2 as well and what I was told by Igor Gnatenko it probably should do. I think by "installing" both of them at once it picks 5.21 which has no dependencies and neglects 0.71 entirely. There might be some hacks to get around that but I'd rather stick to what we know works until we have enough tests to start playing around with different ways to accomplish this with less overhead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concur, I recall that discussion in the Pulp2 context

@dralley dralley merged commit ace69e6 into pulp:master Jun 1, 2020
@dralley dralley deleted the fix-depsolv-bug branch June 1, 2020 14:43
@dralley
Copy link
Contributor Author

dralley commented Jun 1, 2020

Test script

from pulpcore.client.pulpcore import (
    ApiClient as CoreApiClient,
    Configuration,
    TasksApi
)
from pulpcore.client.pulp_rpm import (
    ApiClient as RpmApiClient,
    ContentPackagesApi,
    RepositoriesRpmApi,
    RemotesRpmApi,
    RpmCopyApi,
    RepositoriesRpmVersionsApi,
    RpmRepositorySyncURL,
    PublicationsRpmApi,
)
from pulp_2to3_migration.tests.functional.util import monitor_task


import socket

configuration = Configuration()
configuration.username = 'admin'
configuration.password = 'password'
configuration.host = 'http://{}:24817'.format(socket.gethostname())
configuration.safe_chars_for_path_param = '/'

core_client = CoreApiClient(configuration)
rpm_client = RpmApiClient(configuration)

# Create api clients for all resource types
rpm_repo_api = RepositoriesRpmApi(rpm_client)
rpm_copy_api = RpmCopyApi(rpm_client)
rpm_repo_versions_api = RepositoriesRpmVersionsApi(rpm_client)
rpm_content_api = ContentPackagesApi(rpm_client)
rpm_remote_api = RemotesRpmApi(rpm_client)
rpm_publication_api = PublicationsRpmApi(rpm_client)

tasks_api = TasksApi(core_client)


rpm_source_repo = rpm_repo_api.list(name='test_source').results[0]

try:
    rpm_dest_repo = rpm_repo_api.list(name='test_dest').results[0]
    delete_task = rpm_repo_api.delete(rpm_dest_repo.pulp_href)
    monitor_task(tasks_api, delete_task.task)
except IndexError:
    pass

rpm_dest_repo = rpm_repo_api.create({'name': 'test_dest'})

walrus_071 = rpm_content_api.list(name='walrus', version='0.71').results[0]  # -> whale -> shark, stork
walrus_521 = rpm_content_api.list(name='walrus', version='5.21').results[0]  # no deps

content = [walrus_071.pulp_href, walrus_521.pulp_href]

data = {
    "config": [
        {
            "source_repo_version": rpm_source_repo.latest_version_href,
            "dest_repo": rpm_dest_repo.pulp_href,
            "dest_base_version": 0,
            "content": content
        }
    ],
    "dependency_solving": True
}

copy_task = rpm_copy_api.copy_content(data)
monitor_task(tasks_api, copy_task.task)

dest_repo_version = rpm_repo_versions_api.list(rpm_dest_repo.pulp_href, number=1).results[0]
print(dest_repo_version)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants