Skip to content

Fix IntegrityError on multi-source copy with dependency solving - #4468

Open
guylain-lavoie wants to merge 1 commit into
pulp:mainfrom
guylain-lavoie:fix-copy-content-multi-source-depsolving
Open

Fix IntegrityError on multi-source copy with dependency solving#4468
guylain-lavoie wants to merge 1 commit into
pulp:mainfrom
guylain-lavoie:fix-copy-content-multi-source-depsolving

Conversation

@guylain-lavoie

Copy link
Copy Markdown

When copy_content ran with dependency_solving=True and multiple source repository versions in the config mapped to the same destination repository, the task failed with an IntegrityError on the unique (repository, number) constraint, because new_version() was called once per source entry on the same destination inside a single @transaction.atomic block.

Group units by destination repository before creating new versions, so that each destination receives exactly one new version regardless of how many source entries target it.

closes #4286

Assisted By: Claude (Anthropic)

📜 Checklist

  • Commits are cleanly separated with meaningful messages (simple features and bug fixes should be squashed to one commit)
  • A changelog entry or entries has been added for any significant changes
  • Follows the Pulp policy on AI Usage
  • (For new features) - User documentation and test coverage has been added

See: Pull Request Walkthrough

@guylain-lavoie

Copy link
Copy Markdown
Author

test_upload_signed_package is failing. It was modified 12 days ago, I do not think my code is in cause. Anyone knows if I am wrong?

@pedro-psb

pedro-psb commented Jun 3, 2026

Copy link
Copy Markdown
Member

Hey @guylain-lavoie, I've seen some sign on upload flaky errors lately, so I also believe it might not be your changes (and well, it doesn't really is related at all).
Can you rebase and force push?

@pedro-psb

Copy link
Copy Markdown
Member

This is probably something related to shared fixtures, parallel runs, cleanups and test expectations about the uniqueness of artifacts...

@guylain-lavoie
guylain-lavoie force-pushed the fix-copy-content-multi-source-depsolving branch from b871b76 to 1ee7b39 Compare June 3, 2026 19:22
@guylain-lavoie

Copy link
Copy Markdown
Author

Thank you @pedro-psb for your help. Rebased and pushed back. Unfortunately, same problem.

@dralley

dralley commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

@guylain-lavoie Could you rebase one more time please? We did some work with the tests to make them less flaky, if it fails again it probably is something with these specific tests

When copy_content ran with dependency_solving=True and multiple source
repository versions in the config mapped to the same destination
repository, the task failed with an IntegrityError on the unique
(repository, number) constraint because new_version() was called once
per source entry on the same destination inside a single
@transaction.atomic block.

Group units by destination repository before creating new versions, so
that each destination receives exactly one new version regardless of how
many source entries target it.

closes pulp#4286

Assisted By: Claude (Anthropic)
@guylain-lavoie
guylain-lavoie force-pushed the fix-copy-content-multi-source-depsolving branch from 1ee7b39 to c657820 Compare July 23, 2026 15:41
@dralley

dralley commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

I will take a look at this at some point in the next week

@guylain-lavoie

guylain-lavoie commented Jul 24, 2026

Copy link
Copy Markdown
Author

Thank you Daniel.
I asked claude to perform an analysis of the situation, hopefully it will help.


The test_upload_signed_package failure (assert package.signing_keys is not None, test_package_signing.py:529) is unrelated to this PR's change — it's a test-isolation race on main between package upload and any sync of the rpm-signed fixture repo.

Mechanism:

  1. The test uploads the signed kangaroo RPM (RPM_SIGNED_URL = rpm-signed/ + RPM_PACKAGE_FILENAME) and expects signing_keys to be populated by extraction on upload.
  2. The sync pipeline explicitly sets signing_keys = None — see the TODO at pulp_rpm/app/tasks/synchronizing.py:1470-1471 ("set signing_keys when we support package signing during sync"). This applies regardless of sync policy, since pkgId comes from the repodata primary.xml checksum, not the downloaded file.
  3. On upload of already-existing content, pulpcore's ContentUploadSerializer.create() calls retrieve() (which RpmPackageSerializer.retrieve() implements as a pkgId + domain lookup, serializers/package.py:304) and only touch()es the existing row — the freshly-extracted signing_keys are discarded. There's a second retrieve() in the IntegrityError fallback with the same behavior.
  4. test_upload_signed_package is @pytest.mark.parallel, and other parallel tests sync the rpm-signed fixture into the default domain: test_mirror_mode (test_sync.py:1129, parametrized ×2) and the RBAC tests (test_rbac_crud.py, e.g. the content-scoping test around line 434). Whenever one of those syncs the fixture before this test's upload lands, the upload resolves to the synced package with signing_keys=None and the assertion fails.

This PR only touches copy.py and adds two tests to test_copy.py; nothing in it writes signing_keys. The added tests plausibly shift pytest-xdist scheduling enough to change which side wins the race, which would explain why the failure appeared on this run — though I haven't been able to pull the other matrix jobs' tracebacks to confirm all four failed on the same assertion.

@ggainey

ggainey commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

All the test-scenarios fail on the same test, the failure being not reproducible locally. Concur that nothing in this PR appears to have anything to do with the failure.

@ggainey

ggainey commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

I am going to close and reopen this, to see if we can shake loose the test-runners. We appear to have been lost in a hole due to github's outage/issues from yesterday.

@ggainey ggainey closed this Aug 7, 2026
@ggainey ggainey reopened this Aug 7, 2026
@ggainey

ggainey commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

#4542 should fix the issue called out in #4468 (comment) , because it builds an rpm to sign instead of relying on an existing one from fixtures.

@dralley

dralley commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Agree, once that merges and this gets rebased again, it should work (this time)

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.

Multi-Repository Copy Fails with IntegrityError

4 participants