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

Add pulp-ui to pulp image #677

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Add pulp-ui to pulp image #677

merged 1 commit into from
Nov 5, 2024

Conversation

gerrod3
Copy link
Collaborator

@gerrod3 gerrod3 commented Oct 11, 2024

There's many different ways to serve these files, here's one way I chose. Would like some feedback on this approach.

TODO:

  • Add logic in CI to find ui url
  • Add support for nightly image??
  • Add support for templating a config file for values like API_ROOT (maybe in a future PR once we decide how we will do this in the ui)
  • Add issue changelog

Copy link
Member

@mdellweg mdellweg left a comment

Choose a reason for hiding this comment

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

I like it a lot.


RUN <<EOF
if [[ "$PULP_UI_URL" ]]; then
mkdir -p /usr/share/nginx/static/pulp_ui
Copy link
Member

Choose a reason for hiding this comment

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

Is this the new /var/www ?
It's OK...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Where should these files live? I was thinking I could put them in the pulp static directory (default: /var/lib/operator/static/), but I was worried if the user ran collectstatic with --clean then they would lose their UI as well.


ENV PULP_UI=true
Copy link
Member

Choose a reason for hiding this comment

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

This should probably depend on PULP_UI_URL... Or let's just make PULP_UI_URL the env var?

@@ -105,6 +105,17 @@ http {

include pulp/*.conf;

{% if ui | default(false) -%}
Copy link
Member

Choose a reason for hiding this comment

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

This could be if ui_url != ""?

Comment on lines 45 to 50
RUN <<EOF
if [[ "$PULP_UI_URL" ]]; then
mkdir -p "${PULP_STATIC_ROOT}pulp_ui"
curl -Ls $PULP_UI_URL | tar -xzv -C "${PULP_STATIC_ROOT}pulp_ui"
fi
EOF
Copy link
Member

Choose a reason for hiding this comment

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

I haven't seen this "here document" thing in a container file before.
Can you try with \ on each lines end instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I couldn't get it to work with \.

Copy link
Collaborator Author

@gerrod3 gerrod3 Oct 31, 2024

Choose a reason for hiding this comment

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

Ok I got a version with \ to work, had to switch to using single [ for the if check.

@gerrod3 gerrod3 force-pushed the pulp-ui branch 11 times, most recently from 4bfb1f9 to bc878aa Compare November 1, 2024 03:53
Copy link
Contributor

@ggainey ggainey left a comment

Choose a reason for hiding this comment

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

This all looks reasonable now, nice work.

@gerrod3 gerrod3 marked this pull request as ready for review November 1, 2024 19:18
@gerrod3 gerrod3 changed the title WIP Add pulp-ui to pulp image Add pulp-ui to pulp image Nov 1, 2024
@gerrod3 gerrod3 linked an issue Nov 1, 2024 that may be closed by this pull request
@mdellweg mdellweg merged commit d74b7bb into pulp:latest Nov 5, 2024
11 checks passed
@gerrod3 gerrod3 deleted the pulp-ui branch November 5, 2024 21:36
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.

Add Pulp UI to All-In-One image
3 participants