[PULP-1749] Add migrate endpoint for push repositories by gerrod3 · Pull Request #2406 · pulp/pulp_container

gerrod3 · 2026-07-08T19:52:28Z

Summary

Add a migrate action on push repositories that dispatches a task to convert ContainerPushRepository into ContainerRepository
Support optional copy_versions to preserve full repository version history (defaults to copying only the latest version)
Reassociate distributions, return the created repository in the task result, and add functional test coverage

Test plan

Run pytest pulp_container/tests/functional/api/test_migrate_push_repository.py
Verify POST /pulp/api/v3/repositories/container/container-push/{pk}/migrate/ returns 202 and completes successfully
Confirm migrated repository appears under container repositories with preserved content and distribution link
Test copy_versions: true when multiple repository versions exist

Made with Cursor

mdellweg · 2026-07-13T10:02:41Z

Do you plan to add a management command to migrate all remaining repositories?

Do you think we need to keep the old href's around as an alias? (Or by returning a clever redirect?)

gerrod3 · 2026-07-15T18:15:43Z

Do you plan to add a management command to migrate all remaining repositories?

Yeah. I think I'll do it in a separate PR once I get this right.

Do you think we need to keep the old href's around as an alias? (Or by returning a clever redirect?)

No, this is one downside of the migration is that it's a completely new object, old hrefs won't work. I think it won't be a big deal since these repos are auto-created on push and auto-deleted when the distro is removed, so I don't expect many users are keeping track of these hrefs. Adding a compatibility layer is a lot of effort so my plan is to make the docs explicit on how the migration works so users understand what is happening.

Provide an async API to convert legacy ContainerPushRepository instances into ContainerRepository while preserving content and distribution links. Co-authored-by: Cursor <cursoragent@cursor.com>

mdellweg · 2026-08-05T08:11:21Z

+    original_name = push_repository.name
+    temp_name = f"{original_name}__migrating__{push_repository.pk}"
+
+    with transaction.atomic():


Is there any structural difference between the ContainerRepository and the ContainerPushRepository table?
(I am wondering if we could do something clever in SQL to just change the DNA of the existing object.)

As far as I know the inheritance here is managed with a one-to-one relation so that the pk of the parent model is also the pk of the child model. And since the content models in those repositories as well as the repository versions are compatible (as far as I know), this could be a single create and delete in a transaction.

For example, we used this code in an old pulp_file migration:

def migrate_data_from_old_model_to_new_model_up(apps, schema_editor): """ Move objects from FileDistribution to NewFileDistribution.""" FileDistribution = apps.get_model('file', 'FileDistribution') NewFileDistribution = apps.get_model('file', 'NewFileDistribution') for file_distribution in FileDistribution.objects.all(): with transaction.atomic(): NewFileDistribution( pulp_id=file_distribution.pulp_id, pulp_created=file_distribution.pulp_created, pulp_last_updated=file_distribution.pulp_last_updated, pulp_type=file_distribution.pulp_type, name=file_distribution.name, base_path=file_distribution.base_path, content_guard=file_distribution.content_guard, remote=file_distribution.remote, publication=file_distribution.publication ).save() file_distribution.delete()

Maybe the "trick" here is that the raw Django migration level objects don't "know" about the pulp specific MasterDetailModel and can carefully work around that.

e.g. to turn a file_repository into a pgp keyring (yes, we should not do that becuase they are not as compatible...) i needed to call these three commands:

insert into core_openpgpkeyring (repository_ptr_id) VALUES ('019fd0fc-103f-775a-bb16-f92440ba53dc'); update core_repository set pulp_type = 'core.openpgp' where pulp_id = '019fd0fc-103f-775a-bb16-f92440ba53dc'; delete from file_filerepository where repository_ptr_id = '019fd0fc-103f-775a-bb16-f92440ba53dc';

Ok, I've tested this out and yeah the ORM code in the migration behaves differently from normal ORM code and runs into issues with the multi-table inheritance. But we can still do the SQL migrate manually and we can preserve mostly everything. See my latest commit.

mdellweg

I very much like this approach!

mdellweg · 2026-08-06T08:04:52Z

+            pk=push_repository_pk
+        )
+        if push_repository.pulp_type != ContainerPushRepository.get_pulp_type():
+            raise RuntimeError(


This is a sanity check, since we expect the transition down below to happen in a transaction.
Maybe with a comment we can prevent future readers to panic.

mdellweg · 2026-08-06T08:08:06Z

+        push_repository_pk (str): The primary key for the push repository to migrate.
+    """
+    with transaction.atomic():
+        push_repository = ContainerPushRepository.objects.select_for_update().get(


I'm not sure what the select_for_update is supposed to protect here, but why not play it extra safe...
OTOH, does this "select for update" the base Repository row too?

How about adding cursor.execute("set transaction isolation level serializable") as the very first statement in this transaction?

It could be the single one measure to protect all the reads happening here, including the GenericForeignKeys.

It may be a hit on performance, and could lead to SerializationErrors but for this kind of once in a lifetime migration operation it's probably a good tradeoff.

Yes, select_for_update locks both parents and childs row. Added the SET transaction level.

mdellweg · 2026-08-06T08:17:54Z

+            # Deleting the push child cascades its pending M2M rows; parent repo is kept.
+            cursor.execute(
+                """
+                DELETE FROM container_containerpushrepository
+                WHERE repository_ptr_id = %s
+                """,
+                [push_repository_pk],
+            )


Is Django really setting up cascade delete in the database?
I guess this is just something to keep looking out for.

Apparently not, good call out, fixed.

mdellweg · 2026-08-06T08:18:05Z

+        UserRole.objects.filter(content_type=push_ct, object_id=str(push_repository_pk)).update(
+            content_type=container_ct
+        )
+        GroupRole.objects.filter(content_type=push_ct, object_id=str(push_repository_pk)).update(
+            content_type=container_ct
+        )


mdellweg · 2026-08-06T08:19:13Z

+                [push_repository_pk, signing_service_id],
+            )
+
+            cursor.execute(


Hah, I guess it wouldn't be dramatic to loose the pending items accidentally, but yes, good call too.

Reminds me that "pending content" should probably be a pulpcore thing by now.

mdellweg · 2026-08-06T08:33:43Z

+                f"Container repository child row already exists for {push_repository_pk}."
+            )
+
+        signing_service_id = push_repository.manifest_signing_service_id


This is the one "normal" field we have to transfer from the child table, right?

github-actions Bot added the no-issue label Jul 8, 2026

gerrod3 force-pushed the migrate-push branch from 5bda07b to 510aebd Compare July 8, 2026 21:01

github-actions Bot added the multi-commit label Jul 9, 2026

mdellweg changed the title ~~Add migrate endpoint for push repositories~~ [PULP-1749] Add migrate endpoint for push repositories Jul 15, 2026

gerrod3 marked this pull request as ready for review July 27, 2026 14:30

Add migrate endpoint for push repositories.

21863e9

Provide an async API to convert legacy ContainerPushRepository instances into ContainerRepository while preserving content and distribution links. Co-authored-by: Cursor <cursoragent@cursor.com>

gerrod3 force-pushed the migrate-push branch from 16a52e8 to 21863e9 Compare August 4, 2026 18:39

github-actions Bot removed the multi-commit label Aug 4, 2026

mdellweg reviewed Aug 5, 2026

View reviewed changes

github-actions Bot added the multi-commit label Aug 5, 2026

mdellweg reviewed Aug 6, 2026

View reviewed changes

Switch to in-place SQL swap

faf176f

gerrod3 force-pushed the migrate-push branch from dd0f42c to faf176f Compare August 6, 2026 13:44

mdellweg approved these changes Aug 6, 2026

View reviewed changes

Uh oh!

Conversation

gerrod3 commented Jul 8, 2026

Summary

Test plan

Uh oh!

mdellweg commented Jul 13, 2026

Uh oh!

gerrod3 commented Jul 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdellweg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants