Skip to content
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

Use new entrypoint cmd per guidance from Pulp team #2105

Merged

Conversation

rooftopcellist
Copy link
Member

What is this PR doing:

In this pulpcore PR, the entrypoint for api and content containers as changed.

In the changelog, they advise:

Starting with this release, it is highly recommended to start the api and content processes by
the newly provided entrypoints (pulpcore-api and pulpcore-content) instead of calling
gunicorn directly.

I have removed the --worker-class flag as it is now handled automatically in the entrypoint script (here).

This PR is a best effort, as I don't know the galaxy_ng project as well. Please feel free to close and replace with your own PR or request changes. Thanks!

Reviewers must know:

PR Author & Reviewers: Keep or remove backport labels per Backporting Guidelines
Reviewers: Look for sound code, no code smells, docs & test coverage
Merger: When merging, include the Jira issue link in the squashed commit

@rochacbruno
Copy link
Member

/retest

@drodowic
Copy link
Collaborator

Do these changes require a newer version of pulp? Because the pods are failing with:
/usr/local/bin/start-api: line 33: /usr/local/bin/pulpcore-api: No such file or directory or
/usr/local/bin/start-content-app: line 12: /usr/local/bin/pulpcore-content: No such file or directory

@dsavineau
Copy link

Do these changes require a newer version of pulp? Because the pods are failing with: /usr/local/bin/start-api: line 33: /usr/local/bin/pulpcore-api: No such file or directory or /usr/local/bin/start-content-app: line 12: /usr/local/bin/pulpcore-content: No such file or directory

those changes are needed because of f80ef95#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7R115

However, the cli path is probably wrong in this PR because pulpcore should be installed in the virtual environmant under /venv right ?

@rooftopcellist rooftopcellist force-pushed the use-new-pulpcore-api-entrypoint branch from 0a278f8 to b28a993 Compare March 25, 2024 20:52
@rooftopcellist
Copy link
Member Author

rooftopcellist commented Mar 25, 2024

I just updated the PR with the correct path to pulpcore-api. It now fails with this error in the test playbook check.

setuptools.extern.packaging.version.InvalidVersion: Invalid version: ''"

Maybe we need to re-trigger CI to see if it is flake? I don't have permission to do so.

@rochacbruno
Copy link
Member

/retest

@rooftopcellist rooftopcellist force-pushed the use-new-pulpcore-api-entrypoint branch from b28a993 to 033954e Compare March 26, 2024 13:53
@rooftopcellist
Copy link
Member Author

@rochacbruno it was failing when deploying with the galaxy-operator with this error:

Error: No such option: --limit-request-field_size (Possible options: --limit-request-field-size, --limit-request-fields, --limit-request-line)

So with the new changes, it needs to be:

--limit-request-field-size 32768

as per:

This latest push should fix it.

@rooftopcellist rooftopcellist force-pushed the use-new-pulpcore-api-entrypoint branch 2 times, most recently from 63e466a to 131f9c2 Compare March 26, 2024 14:38
@rooftopcellist
Copy link
Member Author

I just built a galaxy-ng image from this branch and tested it out with the operator and it now works:

NAME                                                  READY   STATUS    RESTARTS   AGE
galaxy-api-69bdf7b687-k5h6d                           1/1     Running   0          12m
galaxy-content-5669c9558f-fnsll                       1/1     Running   0          13m
galaxy-content-5669c9558f-wrxpr                       1/1     Running   0          13m
galaxy-operator-controller-manager-657f4b5466-rrfdf   2/2     Running   0          14m
galaxy-postgres-13-0                                  1/1     Running   0          13m
galaxy-redis-54b6b744db-wqwxd                         1/1     Running   0          12m
galaxy-web-55bf78bcbd-wc45v                           1/1     Running   0          13m
galaxy-worker-869f4987c9-m9pcz                        1/1     Running   0          12m

Caveat: I had to remove this flag because the pulpcore-api script didn't allow it, I think the pulp team may need to add a click option for this, I'll talk with them about this.

--limit-request-field-size 32768

No-Issue

Signed-off-by: Christian M. Adams <[email protected]>
@rooftopcellist rooftopcellist force-pushed the use-new-pulpcore-api-entrypoint branch from 131f9c2 to 05c407f Compare March 26, 2024 16:49
@rooftopcellist
Copy link
Member Author

@rochacbruno I think we should merge this PR as is. I expect the tests to pass shortly.

While testing this, I ran in to another bug, pulpcore-api does not correctly handle the --limit-request-field_size flag for gunicorn. This is a simple fix in pulpcore, but will require them to cut a new release and for the galaxy_ng project to bump the pulpcore dependency. So once that new pulpcore versionis available, in a separate PR, we should:

  • bump the pulpcore version to the latest with the fix
  • remove the comment and add back the --limit-request-field_size flag

@rochacbruno rochacbruno merged commit 59f21ec into ansible:master Mar 26, 2024
25 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants