Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions development/playbooks/deploy-dev/metadata.obsah.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ variables:
foreman_development_github_username:
help: GitHub username to add as additional remote for git checkouts
action: store
pulp_import_paths:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could define these in a file like https://github.com/theforeman/foremanctl/blob/master/src/playbooks/_database_connection/metadata.obsah.yaml and then include that file in both deploy and deploy-dev.

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

include:
- _flavor_features
11 changes: 11 additions & 0 deletions src/playbooks/deploy/metadata.obsah.yaml
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_import_paths:
help: Extra file path that Pulp can use for content imports. Argument may be used more than once.
action: append_unique
type: AbsolutePath
parameter: --content-import-path
pulp_export_paths:
help: Extra file path that Pulp can use for content exports. Argument may be used more than once.
action: append_unique
type: AbsolutePath
parameter: --content-export-path


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

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

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

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

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

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_import_paths`: Paths Pulp can use for content imports (default: `[/var/lib/pulp/sync_imports, /var/lib/pulp/imports]`)
- `pulp_export_paths`: Paths Pulp can use for content exports (default: `[/var/lib/pulp/exports]`)
- `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

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_import_paths`: `--content-import-path` (may be specified multiple times)
- `pulp_export_paths`: `--content-export-path` (may be specified multiple times)
16 changes: 14 additions & 2 deletions src/roles/pulp/defaults/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ pulp_worker_count: "{{ [8, ansible_facts['processor_nproc']] | min }}"
pulp_content_service_worker_count: "{{ (2 * ([8, ansible_facts['processor_nproc']] | min)) + 1 }}"
pulp_api_service_worker_count: "{{ ([4, ansible_facts['processor_nproc']] | min) + 1 }}"

pulp_volumes:
- /var/lib/pulp:/var/lib/pulp
pulp_volumes: >-
{{
['/var/lib/pulp:/var/lib/pulp'] +
(pulp_import_paths | map('regex_replace', '^(.+)$', '\1:\1') | list) +
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What are these regexs doing?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

lets take one item from pulp_import_paths and call it string1, so string1="/foo/bar"
this regex turns string1 into the same format as the line above. ('/var/lib/pulp:/var/lib/pulp').

So after string1 runs through regex, we get "/foo/bar:/foo:bar"

(pulp_export_paths | map('regex_replace', '^(.+)$', '\1:\1') | list)
}}

pulp_api_container_name: pulp-api
pulp_content_container_name: pulp-content
Expand All @@ -22,6 +26,10 @@ pulp_pulp_url: "http://{{ ansible_facts['fqdn'] }}:24817"

pulp_enable_analytics: false

# pulp import/export paths
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is redundant.

pulp_import_paths: ["/var/lib/pulp/sync_imports", "/var/lib/pulp/imports"]
pulp_export_paths: ["/var/lib/pulp/exports"]
Comment thread
ehelms marked this conversation as resolved.

pulp_default_plugins:
- pulp_certguard
- pulp_file
Expand Down Expand Up @@ -66,6 +74,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_import_paths }}
PULP_ALLOWED_EXPORT_PATHS: >-
{{ pulp_export_paths }}

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

Expand Down
14 changes: 14 additions & 0 deletions src/roles/pulp/tasks/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@
- assets
- media

- name: Create import directories
ansible.builtin.file:
path: "{{ item }}"
state: directory
mode: "0755"
loop: "{{ pulp_import_paths }}"

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

- name: Create DB password secret
containers.podman.podman_secret:
state: present
Expand Down
31 changes: 31 additions & 0 deletions tests/pulp_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ def pulp_status_curl(server):
def pulp_status(pulp_status_curl):
return json.loads(pulp_status_curl.stdout)

@pytest.fixture(scope="module")
def pulp_import_export_paths(server):
result = server.run("podman inspect pulp-api --format '{{json .Config.Env}}'")
assert result.succeeded
env = {v.split('=', 1)[0]: v.split('=', 1)[1] for v in json.loads(result.stdout)}
import_paths = json.loads(env['PULP_ALLOWED_IMPORT_PATHS'].replace("'", '"'))
export_paths = json.loads(env['PULP_ALLOWED_EXPORT_PATHS'].replace("'", '"'))
return import_paths, export_paths


def test_pulp_api_service(server):
pulp_api = server.service("pulp-api")
assert pulp_api.is_running
Expand Down Expand Up @@ -73,3 +83,24 @@ 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_import_directories(server, pulp_import_export_paths):
import_paths, _ = pulp_import_export_paths
for path in import_paths:
assert server.file(path).is_directory

def test_pulp_export_directories(server, pulp_import_export_paths):
_, export_paths = pulp_import_export_paths
for path in export_paths:
assert server.file(path).is_directory

@pytest.mark.parametrize("container", ["pulp-api", "pulp-content", "pulp-worker-1"])
def test_pulp_import_export_volume_mounts(server, container, pulp_import_export_paths):
import_paths, export_paths = pulp_import_export_paths
result = server.run(f"podman inspect {container} --format '{{{{json .Mounts}}}}'")
assert result.succeeded
mounts = json.loads(result.stdout)
destinations = [mount['Destination'] for mount in mounts]

for path in import_paths + export_paths:
assert path in destinations, f"expected {path} to be mounted as a volume in {container}"
Loading