Skip to content

Pulp import export support #455

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.

Sign up for GitHub

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

Open
aidenfine wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Pulp import export support #455

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8c7ddb7
Add pulp import/export defaults and custom paths
aidenfine Apr 17, 2026
090e0ad
Use pulp ENV names
aidenfine Apr 17, 2026
52092e3
add pulp role docs
aidenfine Apr 17, 2026
27c6311
Merge user defined variables and default variables into one
aidenfine Apr 17, 2026
a1733c5
Change user facing args to content-*
aidenfine Apr 17, 2026
59a42fb
add import/export args to deploy-dev
aidenfine Apr 17, 2026
4ffabc2
mount file paths and improve tests to verify paths are volume mounts
aidenfine Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/playbooks/deploy/metadata.obsah.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ variables:
action: append_unique
type: FQDN
parameter: --certificate-cname
pulp_extra_import_paths:
Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

The user doesn't care that it's Pulp right? How about:

Suggested change
pulp_extra_import_paths:
content_extra_import_paths:

What makes them "extra" ?

Copy link
Copy Markdown
Author

@aidenfine aidenfine Apr 17, 2026

Choose a reason for hiding this comment

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

Good point. I used extra because users can add extra or additional import/export paths. Would you prefer something other than extra?

Copy link
Copy Markdown

@ianballou ianballou Apr 17, 2026

Choose a reason for hiding this comment

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

I'm trying to remember, I think the installer uses this "extra" terminology.

Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

Ahh, it uses additional. So there are the "you can use these out of the box paths" and then there are "user has some setup, they want to use these paths and accept the requirements to make this work".

help: Extra file path that Pulp can use for content imports. Argument may be used more than once.
action: append_unique
type: AbsolutePath
parameter: --pulp-extra-import-path
pulp_extra_export_paths:
help: Extra file path that Pulp can use for conent exports. Argument may be used more than once.
action: append_unique
type: AbsolutePath
parameter: --pulp-extra-export-path


include:
- _certificate_source
Expand Down
34 changes: 34 additions & 0 deletions src/roles/pulp/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
Pulp Role
=========

Deploys and manages [Pulp](https://pulpproject.org/) as Podman containers.
Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

Suggested change
Deploys and manages [Pulp](https://pulpproject.org/) as Podman containers.
Deploys and manages [Pulp](https://pulpproject.org/) as Podman quadlets.


Variables
---------
- `pulp_container_image`: Container image to use for Pulp (default: `quay.io/foreman/pulp`)
Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

I am wary of storing defaults in the documentation here. The code can show those and reduce our overhead.

Copy link
Copy Markdown
Author

@aidenfine aidenfine Apr 17, 2026

Choose a reason for hiding this comment

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

I was hoping someone would comment on the docs. I tried to follow similar patterns from the roles/hammer/README.md. I also feel this is kinda an info dump but wanted to get opinions on it. I don't mind removing this doc from the PR if it makes more sense not to have this

Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

I like the having of a README. I can see now that this is following roughly the outline from the hammer role. I think as long as we are consistent in the structure then keep it.

Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

And we should define that roughly in https://github.com/theforeman/foremanctl/blob/master/docs/developer/playbooks-and-roles.md#shared-metadata-fragments

You do not have to do that here in this PR. I am noting this as a reminder.

- `pulp_container_tag`: Container image tag (default: `"3.73"`)
- `pulp_registry_auth_file`: Path to the registry authentication file (default: `/etc/foreman/registry-auth.json`)
- `pulp_worker_count`: Number of Pulp workers (default: min of 8 or CPU count)
- `pulp_content_origin`: URL for the Pulp content service (default: `http://{{ fqdn }}:24816`)
- `pulp_pulp_url`: URL for the Pulp API service (default: `http://{{ fqdn }}:24817`)
- `pulp_volumes`: Volume mounts for Pulp containers (default: `/var/lib/pulp:/var/lib/pulp`)
- `pulp_enable_analytics`: Enable Pulp analytics (default: `false`)
- `pulp_default_import_path`: Default paths Pulp can use for content imports (default: `[/var/lib/pulp/imports]`)
- `pulp_default_export_path`: Default paths Pulp can use for content exports (default: `[/var/lib/pulp/exports]`)
- `pulp_extra_import_paths`: Additional paths Pulp can use for content imports (default: `[]`)
- `pulp_extra_export_paths`: Additional paths Pulp can use for content exports (default: `[]`)
- `pulp_plugins`: Additional Pulp plugins to enable (default: `[pulp_container, pulp_rpm]`)
- `pulp_database_name`: Name of the Pulp database (default: `pulp`)
- `pulp_database_user`: Database user (default: `pulp`)
- `pulp_database_host`: Database host (default: `localhost`)
- `pulp_database_port`: Database port (default: `5432`)
- `pulp_database_password`: Database password (required, no default)
- `pulp_database_ssl_mode`: Database SSL mode (default: `disabled`)
- `pulp_database_ssl_ca`: Path to the database SSL CA certificate on the control node (default: empty)

Usage Inside foremanctl
Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

I am not sure this detail makes sense at this level. I'd rather see a section in the deployment document around content or add a content document.

-----------------------
When used as part of `foremanctl`, the variables are setup as the following
- `pulp_worker_count`: `--pulp-worker-count`
- `pulp_extra_import_paths`: `--pulp-extra-import-path` (may be used specified multiple times)
- `pulp_extra_export_paths`: `--pulp-extra-export-path` (may be used specified multiple times)
11 changes: 11 additions & 0 deletions src/roles/pulp/defaults/main.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ pulp_pulp_url: "http://{{ ansible_facts['fqdn'] }}:24817"

pulp_enable_analytics: false

pulp_default_import_path:
Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

This says import_path but it's an array.
I wonder can we have a single variable for import and one for export, and they are just lists that appended to?

Copy link
Copy Markdown
Author

@aidenfine aidenfine Apr 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is a good idea. So instead of 4 variables (2 default, 2 lists to hold user passed values). We could just have 2 lists both by default containing the default path and just append user passed values to these lists.

Am I understanding this correctly?

- /var/lib/pulp/imports
pulp_default_export_path:
- /var/lib/pulp/exports
pulp_extra_import_paths: []
pulp_extra_export_paths: []

pulp_default_plugins:
- pulp_certguard
- pulp_file
Expand Down Expand Up @@ -66,6 +73,10 @@ pulp_settings_other_env:
PULP_CONTENT_WORKERS: "{{ pulp_content_service_worker_count }}"
PULP_TOKEN_AUTH_DISABLED: "true"
PULP_FLATPAK_INDEX: "true"
PULP_ALLOWED_IMPORT_PATHS: >-
{{ pulp_default_import_path + pulp_extra_import_paths }}
PULP_ALLOWED_EXPORT_PATHS: >-
{{ pulp_default_export_path + pulp_extra_export_paths }}
Copy link
Copy Markdown
Contributor

@sjha4 sjha4 Apr 17, 2026

Choose a reason for hiding this comment

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

Locally I have this:
PULP_ALLOWED_IMPORT_PATHS: >-
['/var/lib/pulp/sync_imports', '/var/lib/pulp/imports']
PULP_ALLOWED_EXPORT_PATHS: >-
['/var/lib/pulp/exports']

These would be the default paths..


pulp_settings_env: "{{ pulp_settings_database_env | ansible.builtin.combine(pulp_settings_other_env) }}"

Expand Down
16 changes: 16 additions & 0 deletions src/roles/pulp/tasks/main.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,22 @@
- tmp
- assets
- media
- imports
- exports

- name: Create extra import directories
Copy link
Copy Markdown
Member

@ehelms ehelms Apr 17, 2026

Choose a reason for hiding this comment

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

I assume these would have to be mounted into the container?

Copy link
Copy Markdown
Author

@aidenfine aidenfine Apr 17, 2026

Choose a reason for hiding this comment

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

Yeah I didn't think about that, I will add this

ansible.builtin.file:
path: "{{ item }}"
state: directory
mode: "0755"
loop: "{{ pulp_extra_import_paths }}"

- name: Create extra export directories
ansible.builtin.file:
path: "{{ item }}"
state: directory
mode: "0755"
loop: "{{ pulp_extra_export_paths }}"

- name: Create DB password secret
containers.podman.podman_secret:
Expand Down
6 changes: 6 additions & 0 deletions tests/pulp_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,9 @@ def test_pulp_worker_target(server):
def test_pulp_manager_check(server):
result = server.run("podman exec -ti pulp-api pulpcore-manager check --deploy")
assert result.succeeded

def test_pulp_default_import_directory(server):
assert server.file("/var/lib/pulp/imports").is_directory

def test_pulp_default_export_directory(server):
assert server.file("/var/lib/pulp/exports").is_directory