-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
add pulp services #34
Conversation
There are few things which I don't understand clearly yet, first how can we ensure dependencies here like we have |
This could be handled via systemd
We are using podman secrets right now, and I recently added to the repository a naming scheme for how to define the secrets (https://github.com/ehelms/foreman-quadlet?tab=readme-ov-file#naming-convention). For services that need the same secret, we can define the secret once with podman secrets and then it can be declared within each quadlet file. See https://github.com/ehelms/foreman-quadlet/blob/master/roles/candlepin/tasks/main.yml#L59-L71
I am not sure about this. If you ssh into the VM and try to start it manually what happens? |
|
62017a0
to
66de065
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all Pulp containers depend on PostgreSQL. How do they know where to connect to?
We'll want to drop the all-in-one Pulp container /service https://github.com/theforeman/foreman-quadlet/pull/34/files#diff-6d531cd8425f805f75bfb941463407fb86c42731bd3036def1633d683def9cf0R34 |
This fixed some issues for me: diff --git a/roles/pulp/tasks/main.yaml b/roles/pulp/tasks/main.yaml
index c7b019c..87c5e23 100644
--- a/roles/pulp/tasks/main.yaml
+++ b/roles/pulp/tasks/main.yaml
@@ -31,20 +31,6 @@
name: pulp-settings-py
data: "{{ lookup('ansible.builtin.template', 'settings.py.j2') }}"
-- name: Deploy Pulp Container
diff --git a/roles/pulp/tasks/main.yaml b/roles/pulp/tasks/main.yaml
index c7b019c..87c5e23 100644
--- a/roles/pulp/tasks/main.yaml
+++ b/roles/pulp/tasks/main.yaml
@@ -31,20 +31,6 @@
name: pulp-settings-py
data: "{{ lookup('ansible.builtin.template', 'settings.py.j2') }}"
-- name: Deploy Pulp Container
- containers.podman.podman_container:
- name: "{{ pulp_container_name }}"
- image: "{{ pulp_image }}"
- state: quadlet
- ports: "{{ pulp_ports }}"
- volumes: "{{ pulp_volumes }}"
- secrets:
- - 'pulp-settings-py,type=mount,target=/etc/pulp/settings.py'
- quadlet_options:
- - |
- [Install]
- WantedBy=default.target
-
- name: Deploy Pulp API Container
containers.podman.podman_container:
name: "{{ pulp_api_container_name }}"
@@ -60,28 +46,6 @@
[Install]
WantedBy=default.target
-- name: Run daemon reload to make Quadlet create the service files
- ansible.builtin.systemd:
- daemon_reload: true
-
-- name: Start the Pulp Service
- ansible.builtin.systemd:
- name: pulp
- enabled: true
- state: restarted
-
-- name: Wait for Pulp service to be accessible
- ansible.builtin.wait_for:
- host: "{{ ansible_hostname }}"
- port: 8080
- timeout: 300
-
-- name: Wait for Pulp API service to be accessible
- ansible.builtin.wait_for:
- host: "{{ ansible_hostname }}"
- port: 24817
- timeout: 600
-
- name: Deploy Pulp Content Container
containers.podman.podman_container:
name: "{{ pulp_content_container_name }}"
@@ -133,6 +97,12 @@
enabled: true
state: started
+- name: Wait for Pulp API service to be accessible
+ ansible.builtin.wait_for:
+ host: "{{ ansible_hostname }}"
+ port: 24817
+ timeout: 600
+
- name: Wait for Pulp Content service to be accessible
ansible.builtin.wait_for:
host: "{{ ansible_hostname }}" The
|
I did some config setup to resolve it, but I still see the pulp-api.service up and then failing due to the same issue, I'm not sure why this is happening, Can anyone point me why this is still persist? |
8a0ec40
to
97ebc9d
Compare
roles/pulp/tasks/main.yaml
Outdated
- name: Generate database symmetric key | ||
ansible.builtin.command: "bash -c 'openssl rand -base64 32 | tr \"+/\" \"-_\" > /var/lib/pulp/database_fields.symmetric.key'" | ||
args: | ||
creates: /var/lib/pulp/database_fields.symmetric.key | ||
|
||
- name: Create database symmetric key secret | ||
containers.podman.podman_secret: | ||
state: present | ||
name: pulp-symmetric-key | ||
data: "{{ lookup('file', '/var/lib/pulp/database_fields.symmetric.key') }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stores the secret an additional time, which I'd prefer to avoid. Can't you change Generate database symmetric key
to output something on stdout and store that here? Or would that not be idempotent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be not idempotent, yeah :(
@@ -1,11 +1,11 @@ | |||
import json | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This empty line is common in Python. It separates the built in modules (json
) from third party ones (pytest
). isort will place it back.
I think I understand the issue now. When you look at the whole output (via
So it tries to reach the DB ("Waiting on postgresql to start..."), and while doing so it fails and also fails to load the secret we're mounting. Especially, the "permission denied" is confusing, right? After a bit of poking, I realized that the permission error is because we both try to have a secret, but also actually mount things on-top of
Once I drop this line, and add network: host to the container definitions it can actually reach the database and the pulp services start.The DB is still not getting migrated and that's what breaks pulp from actually working, but at least things start) (and the |
roles/pulp/defaults/main.yaml
Outdated
- /var/lib/pulp/pulp_storage:/var/lib/pulp | ||
- /var/lib/pulp/containers:/var/lib/containers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the containers mount is needed anymore with minimal containers and we only really need storage.
At that point, should we just expose /var/lib/pulp
as /var/lib/pulp
? And I still think you need the SELinux label.
- /var/lib/pulp/pulp_storage:/var/lib/pulp | |
- /var/lib/pulp/containers:/var/lib/containers | |
- /var/lib/pulp:/var/lib/pulp:Z |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I still think you need the SELinux label.
I wish. It didn't work with :Z in my tests :/
That's we set label=disable
on the containers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And yes, you're right, we don't need the containers mount, see https://discourse.pulpproject.org/t/what-is-var-lib-containers-used-for-when-deploing-oci/1782
roles/pulp/templates/settings.py.j2
Outdated
@@ -1,7 +1,20 @@ | |||
CONTENT_ORIGIN="http://{{ ansible_hostname }}:8080" | |||
CONTENT_ORIGIN="http://{{ ansible_hostname }}:24816" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK this should be wherever Apache (httpd) presents the content, so end user clients talk to. They shouldn't talk to the internal endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand, would you mind expanding a little more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quoting https://pulpproject.org/pulpcore/docs/admin/reference/settings/#content_origin
A required string containing the protocol, fqdn, and port where the content app is reachable by users. This is used by pulpcore and various plugins when referring users to the content app. For example if the API should refer users to content at using http to pulp.example.com on port 24816, (the content default port), you would set: https://pulp.example.com:24816.
In our deployment we have httpd in front of Pulp which presents it at on https://{{ ansible_hostname }}
. In other words, this was always wrong, but I'm noticing it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ekohl so we basically have to set it up without port? if we want user to point at the right place? to this CONTENT_ORIGIN="https://{{ ansible_hostname }}"
(sorry pinging on this again, i'm not good at this part yet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what we do in puppet-pulpcore, yeah:
https://github.com/theforeman/puppet-pulpcore/blob/master/templates/settings.py.erb#L20
You could also argue, that this sort-of blurs the lines between the roles (the plain, port 443, HTTPS is not available w/o the httpd
role), and the pulp
role should have a default for the content origin being http://{{ ansible_fqdn }}:port
, and then we override this in deploy.yaml
with https://{{ ansible_fqdn }}
as that's the value the overall deployment uses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it should be a variable on the role that the playbook sets (because that knows both httpd and pulp)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/variable/default/, but yeah, that's how I'd do it.
ca29d61
to
7d3b249
Compare
7565d3c
to
f87525f
Compare
- "24817:80" | ||
pulp_content_ports: | ||
- "24816:80" | ||
pulp_worker_count: 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also unused, but I think this could still be needed to tune the worker count.
roles/pulp/templates/settings.py.j2
Outdated
'USER': 'pulp', | ||
'PASSWORD': '{{ pulp_db_password }}', | ||
'HOST': 'localhost', | ||
'PORT': '5432', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.djangoproject.com/en/5.1/ref/settings/#std-setting-PORT says an empty string (default) means the default port. IMHO there's no value in hardcoding the default port
'PORT': '5432', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit unsure why my suggestion wasn't followed. IMHO the config should be as short as possible and not copy all the defaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, i did updated it 3d1f220
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion was to remove the line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, i understand now! sorry, updating it in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these variables are no longer used in the playbook due to the host network configuration, which eliminates the need for explicit port mappings.
implementation of pulp services(api, content and worker)