Enables FilesystemExporter to incrementally export rpms #3511

parthaa · 2023-01-13T23:13:32Z

It accepts a start repository version and calculates what content artifacts need to get copied over.

parthaa · 2023-01-16T15:37:08Z

Also check out Katello/katello#10417 (comment) on how this PR works with katello

ggainey · 2023-01-16T21:21:53Z

This PR currently supports start_repository_version only if you have specified repository_version. publication= is a major workflow for users of this functionality, but if you specify start_repository_version= and publication=, you get a 500. Rude. :)

I think we should always accept publication=. Actually, more strongly - I contend that we should support a start_publication= param as well. You can only specify one of publication/repository_version, and only one of start_publication/start_repository_version, and the combinatorics should all work.

(Also - we prob need some error-checking at the view-level, you should not be able to specify a delta that "goes back in time" - once we have start and end repo-version, we should insure that start < end...)

The other thing I'm noting is, if I start with a repo, remove some RPMs to create version-N, add them back in to create version-M, and export start_repository_version=N/repository_version=M, I get an empty export. I expect somewhere in the "find the content for a version" is making An Assumption that may not be valid.

Testing/investigation continues.

parthaa · 2023-01-16T22:10:09Z

This PR currently supports start_repository_version only if you have specified repository_version. publication= is a major workflow for users of this functionality, but if you specify start_repository_version= and publication=, you get a 500. Rude. :)

Interesting - Katello only uses publication. Wonder why start_repository_version= and publication= is failing for you. I used that combination for all my tests :).

I think we should always accept publication=. Actually, more strongly - I contend that we should support a start_publication= param as well. You can only specify one of publication/repository_version, and only one of start_publication/start_repository_version, and the combinatorics should all work.

I wonder if we should limit the scope of this PR to only publication and start_publication and leave the version compare to a different PR? Katello really only needs this combination to work. That would potentially mean small set of changes.

@sjha4 @ianballou - Do you guys know if we create publications every time with publish a Content View. @ggainey is proposing using start-publication for incremental

The other thing I'm noting is, if I start with a repo, remove some RPMs to create version-N, add them back in to create version-M, and export start_repository_version=N/repository_version=M, I get an empty export. I expect somewhere in the "find the content for a version" is making An Assumption that may not be valid

I will look into this case

sjha4 · 2023-01-16T22:19:58Z

We may not create "new" publications if our contents_changed dynflow middleware detects nothing has changed in which case we reuse the previous version's publication. Every repo version however will have a publication URL which could be used here I think with the catch that the publication may not be unique between versions.

ianballou · 2023-01-16T22:27:28Z

Here's the publication logic: https://github.com/Katello/katello/blob/69d4da4767ea71810a4323c02ff0ce3969418289/app/lib/actions/pulp3/orchestration/repository/generate_metadata.rb#L15

parthaa · 2023-01-20T20:14:45Z

This PR currently supports start_repository_version only if you have specified repository_version. publication= is a major workflow for users of this functionality, but if you specify start_repository_version= and publication=, you get a 500. Rude. :)

I created a repo with feed and synced (so I should have version 1)
I uploaded a rpm to the repo
Then created a publication for the latest version
I created an exporter with the following data

{
  "pulp_href": "/pulp/api/v3/exporters/core/filesystem/8f7a1139-5153-4e54-9e9d-994152379d01/",
  "pulp_created": "2023-01-20T18:27:54.693496Z",
  "name": "open_Export-Library-SYNCABLE_2.0-67",
  "path": "/var/lib/pulp/exports/open/Export-Library-SYNCABLE/2.0/2023-01-20T18-27-51-00-00/custom/goals/auto",
  "method": "hardlink"
}

I then did the export on it

$ curl --cert /home/vagrant/foreman-certs/client_cert.pem --key /home/vagrant/foreman-certs/client_key.pem \
   -X POST -H 'Content-Type: application/json' \
   -d '{"publication":"/pulp/api/v3/publications/rpm/rpm/62b3af46-d059-4d2b-a898-1dd7955ec531/", "start_repository_version":"/pulp/api/v3/repositories/rpm/rpm/6c98c027-53ff-440e-a5ce-ba68cc0833cc/versions/4/"}' \
    https://centos8-katello-devel-stable.example.com/pulp/api/v3/exporters/core/filesystem/8f7a1139-5153-4e54-9e9d-994152379d01/exports/

Works well - journalctl output

Jan 20 20:06:28 centos8-katello-devel-stable.example.com pulpcore-api[15405]: pulp [0ed95cc8205048eca7cf18c134bf3b94]:  - - [20/Jan/2023:20:06:28 +0000] "POST /pulp/api/v3/exporters/core/filesystem/8f7a1139-5153-4e54-9e9d-994152379d01/exports/ HTTP/1.1" 202 67 "-" "curl/7.61.1"
Jan 20 20:06:28 centos8-katello-devel-stable.example.com pulpcore-worker-4[23566]: pulp [0ed95cc8205048eca7cf18c134bf3b94]: pulpcore.tasking.pulpcore_worker:INFO: Starting task 7b7d754c-771b-4c51-a443-dd5402be20da
Jan 20 20:06:28 centos8-katello-devel-stable.example.com pulpcore-worker-4[23566]: pulp [0ed95cc8205048eca7cf18c134bf3b94]: pulpcore.app.tasks.export:INFO: Exporting: file_system_exporter=open_Export-Library-SYNCABLE_2.0-67, publication=62b3af46-d059-4d2b-a898-1dd7955ec531, start_repo_version=e5e32d67-0e8c-455a-9c65-eb9cb5242ac7, path=/var/lib/pulp/exports/open/Export-Library-SYNCABLE/2.0/2023-01-20T18-27-51-00-00/custom/goals/auto
Jan 20 20:06:28 centos8-katello-devel-stable.example.com pulpcore-worker-4[23566]: pulp [0ed95cc8205048eca7cf18c134bf3b94]: pulpcore.tasking.pulpcore_worker:INFO: Task completed 7b7d754c-771b-4c51-a443-dd5402be20da
Jan 20 20:07:10 centos8-katello-devel-stable.example.com pulpcore-api[15405]: pulp [6d519aaaad01486bbfdda21d9685c6b5]:  - - [20/Jan/2023:20:07:10 +0000] "GET /pulp/api/v3/tasks/7b7d754c-771b-4c51-a443-dd5402be20da/ HTTP/1.1" 200 744 "-" "Pulp-CLI/0.14.0"

ianballou · 2023-01-20T22:19:59Z

I wonder if we should limit the scope of this PR to only publication and start_publication and leave the version compare to a different PR? Katello really only needs this combination to work. That would potentially mean small set of changes.

I'm still digging into this myself, but it sounds like Katello only really needs to give the start repo version and the end publication to get what we want -- if that's the case then I'm with Partha in that perhaps the start publication could be added later. I'd be curious to hear though if Pulp folks think we might be missing something and do need a start publication.

mdellweg · 2023-01-23T10:30:36Z

Can we phrase this feature around "content" instead of "rpms"?

parthaa · 2023-01-23T13:35:38Z

Can we phrase this feature around "content" instead of "rpms"?

I think only Yum/File repos are supported for FS Exporter at this point. That being said where do you see this PR being specific to rpms?

mdellweg · 2023-01-24T08:20:24Z

Can we phrase this feature around "content" instead of "rpms"?

I think only Yum/File repos are supported for FS Exporter at this point. That being said where do you see this PR being specific to rpms?

I meant the docs and changelog. Sorry for not being specific enough.

Thank you for making the feature agnostic.

ggainey

Some minor wordsmithing, and one bugfix :)

pulpcore/app/viewsets/exporter.py

CHANGES/3413.feature

pulpcore/app/serializers/exporter.py

mdellweg

Some formatting and what i meant by agnostic in the changelog...

CHANGES/3413.feature

dralley · 2023-01-27T05:42:06Z

@parthaa Can you rebase and squash commits?

dralley · 2023-01-27T05:45:20Z

pulpcore/app/tasks/export.py

@@ -138,8 +144,11 @@ def _export_publication_to_file_system(
        "content_artifact", "content_artifact__artifact"
    ).iterator():
        # Artifact isn't guaranteed to be present
-        if pa.content_artifact.artifact:
+        if pa.content_artifact.artifact and (
+            start_repo_version is None or pa.content_artifact in difference_content_artifacts


difference_content_artifacts appears to be a queryset, do we know that this isn't going to be re-evaluated every time in a loop?

Thats a good point I am curious about that :). How would I be able to check if its getting re evaluated ?

You could explicitly cast to a set

ggainey

Looks like all current comments are responded to. Tested with an RPM repo in my test env, worked like a champ. Looks good to me!

chris1984 · 2023-01-31T20:56:42Z

Tested it here locally with Katello and was able to export incrementally and import without any issues

Export:

[root@centos8-stream-katello-nightly ~]# hammer content-export incremental version --format=syncable --organization-id 1 --id 3
[.................................................................................................................................................................] [100%]
Generated /var/lib/pulp/exports/Default_Organization/view/2.0/2023-01-31T20-48-04-00-00/metadata.json

Import:

[root@centos8-stream-katello-nightly ~]# ll /var/lib/pulp/imports/2023-01-31T20-48-04-00-00/^C
[root@centos8-stream-katello-nightly ~]# hammer content-import version --organization-id 4 --path /var/lib/pulp/imports/2023-01-31T20-48-04-00-00/
[.................................................................................................................................................................] [100%]

pulpcore/app/tasks/export.py

ggainey · 2023-02-01T19:18:33Z

@parthaa OK, please rebase this on top of current-main to address the lint-issues, and resubmit.

It accepts a start repository version and calculates what content artifacts need to get copied over. fixes pulp#3413 lint-fixes Addressed PR Comments

patchback · 2023-02-02T12:22:55Z

Backport to 3.21: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.21/70b1eb04a4349bedfa6781c8598b53a99f9a63a1/pr-3511

Backported as #3546

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-02-02T12:23:09Z

Backport to 3.22: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.22/70b1eb04a4349bedfa6781c8598b53a99f9a63a1/pr-3511

Backported as #3547

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

dralley · 2023-02-02T15:27:11Z

@ggainey How far should this go back, is 3.18 possible?

parthaa · 2023-02-02T16:42:35Z

@ggainey How far should this go back, is 3.18 possible?

@dralley - I'll request it if needed and if customers are desperate for it.

patchback · 2023-03-02T17:04:41Z

Backport to 3.18: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 70b1eb0 on top of patchback/backports/3.18/70b1eb04a4349bedfa6781c8598b53a99f9a63a1/pr-3511

Backporting merged PR #3511 into main

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/pulp/pulpcore.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.18/70b1eb04a4349bedfa6781c8598b53a99f9a63a1/pr-3511 upstream/3.18

Now, cherry-pick PR Enables FilesystemExporter to incrementally export rpms #3511 contents into that branch:
```
$ git cherry-pick -x 70b1eb04a4349bedfa6781c8598b53a99f9a63a1
```
If it'll yell at you with something like fatal: Commit 70b1eb04a4349bedfa6781c8598b53a99f9a63a1 is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 70b1eb04a4349bedfa6781c8598b53a99f9a63a1
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Enables FilesystemExporter to incrementally export rpms #3511 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.18/70b1eb04a4349bedfa6781c8598b53a99f9a63a1/pr-3511

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

dralley · 2023-03-02T17:06:53Z

@parthaa It looks like there is a request for this to go into 3.18 (6.12.z), could you investigate which other patches are necessary to backport it?

parthaa force-pushed the incremental-fs-export branch 9 times, most recently from 503d304 to f037333 Compare January 14, 2023 06:48

parthaa mentioned this pull request Jan 15, 2023

Fixes #35948 - Support for incremental syncable exports Katello/katello#10417

Merged

ggainey changed the title ~~Enables one to incrementally export rpms~~ Enables FilesystemExporter to incrementally export rpms Jan 16, 2023

ipanova marked this pull request as draft January 17, 2023 14:26

ggainey requested changes Jan 24, 2023

View reviewed changes

pulpcore/app/viewsets/exporter.py Outdated Show resolved Hide resolved

CHANGES/3413.feature Outdated Show resolved Hide resolved

pulpcore/app/serializers/exporter.py Outdated Show resolved Hide resolved

mdellweg reviewed Jan 25, 2023

View reviewed changes

CHANGES/3413.feature Outdated Show resolved Hide resolved

dralley reviewed Jan 27, 2023

View reviewed changes

parthaa force-pushed the incremental-fs-export branch from c18c8a8 to 207237e Compare January 27, 2023 16:24

ggainey marked this pull request as ready for review January 27, 2023 21:12

parthaa force-pushed the incremental-fs-export branch 2 times, most recently from d5373bc to 81873ca Compare January 27, 2023 22:07

parthaa requested review from dralley and mdellweg and removed request for mdellweg and dralley January 31, 2023 18:06

ggainey requested review from ggainey and mdellweg January 31, 2023 18:12

ggainey approved these changes Jan 31, 2023

View reviewed changes

dralley reviewed Jan 31, 2023

View reviewed changes

pulpcore/app/tasks/export.py Show resolved Hide resolved

dralley approved these changes Jan 31, 2023

View reviewed changes

ggainey added backport-3.21 backport-3.22 labels Feb 1, 2023

parthaa force-pushed the incremental-fs-export branch from 52278ff to 82b921c Compare February 1, 2023 16:38

Enables one to incrementally export rpms

ed537a3

It accepts a start repository version and calculates what content artifacts need to get copied over. fixes pulp#3413 lint-fixes Addressed PR Comments

parthaa force-pushed the incremental-fs-export branch from 82b921c to ed537a3 Compare February 1, 2023 19:22

mdellweg merged commit 70b1eb0 into pulp:main Feb 2, 2023

patchback bot mentioned this pull request Feb 2, 2023

[PR #3511/70b1eb04 backport][3.21] Enables FilesystemExporter to incrementally export rpms #3546

Merged

patchback bot mentioned this pull request Feb 2, 2023

[PR #3511/70b1eb04 backport][3.22] Enables FilesystemExporter to incrementally export rpms #3547

Merged

parthaa deleted the incremental-fs-export branch February 2, 2023 16:54

dralley added the backport-3.18 label Mar 2, 2023

dralley removed the backport-3.18 label Mar 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enables FilesystemExporter to incrementally export rpms #3511

Enables FilesystemExporter to incrementally export rpms #3511

parthaa commented Jan 13, 2023

parthaa commented Jan 16, 2023

ggainey commented Jan 16, 2023

parthaa commented Jan 16, 2023 •

edited

sjha4 commented Jan 16, 2023

ianballou commented Jan 16, 2023

parthaa commented Jan 20, 2023 •

edited

ianballou commented Jan 20, 2023

mdellweg commented Jan 23, 2023

parthaa commented Jan 23, 2023 •

edited

mdellweg commented Jan 24, 2023 •

edited

ggainey left a comment

mdellweg left a comment

dralley commented Jan 27, 2023

dralley Jan 27, 2023

parthaa Jan 27, 2023

dralley Jan 27, 2023

parthaa Jan 27, 2023

ggainey left a comment

chris1984 commented Jan 31, 2023 •

edited

ggainey commented Feb 1, 2023

patchback bot commented Feb 2, 2023 •

edited

patchback bot commented Feb 2, 2023 •

edited

dralley commented Feb 2, 2023

parthaa commented Feb 2, 2023 •

edited

patchback bot commented Mar 2, 2023 •

edited

dralley commented Mar 2, 2023

Enables FilesystemExporter to incrementally export rpms #3511

Enables FilesystemExporter to incrementally export rpms #3511

Conversation

parthaa commented Jan 13, 2023

parthaa commented Jan 16, 2023

ggainey commented Jan 16, 2023

parthaa commented Jan 16, 2023 • edited

sjha4 commented Jan 16, 2023

ianballou commented Jan 16, 2023

parthaa commented Jan 20, 2023 • edited

ianballou commented Jan 20, 2023

mdellweg commented Jan 23, 2023

parthaa commented Jan 23, 2023 • edited

mdellweg commented Jan 24, 2023 • edited

ggainey left a comment

Choose a reason for hiding this comment

mdellweg left a comment

Choose a reason for hiding this comment

dralley commented Jan 27, 2023

dralley Jan 27, 2023

Choose a reason for hiding this comment

parthaa Jan 27, 2023

Choose a reason for hiding this comment

dralley Jan 27, 2023

Choose a reason for hiding this comment

parthaa Jan 27, 2023

Choose a reason for hiding this comment

ggainey left a comment

Choose a reason for hiding this comment

chris1984 commented Jan 31, 2023 • edited

ggainey commented Feb 1, 2023

patchback bot commented Feb 2, 2023 • edited

Backport to 3.21: 💚 backport PR created

patchback bot commented Feb 2, 2023 • edited

Backport to 3.22: 💚 backport PR created

dralley commented Feb 2, 2023

parthaa commented Feb 2, 2023 • edited

patchback bot commented Mar 2, 2023 • edited

Backport to 3.18: 💔 cherry-picking failed — conflicts found

Backporting merged PR #3511 into main

dralley commented Mar 2, 2023

parthaa commented Jan 16, 2023 •

edited

parthaa commented Jan 20, 2023 •

edited

parthaa commented Jan 23, 2023 •

edited

mdellweg commented Jan 24, 2023 •

edited

chris1984 commented Jan 31, 2023 •

edited

patchback bot commented Feb 2, 2023 •

edited

patchback bot commented Feb 2, 2023 •

edited

parthaa commented Feb 2, 2023 •

edited

patchback bot commented Mar 2, 2023 •

edited