-
Notifications
You must be signed in to change notification settings - Fork 16
Added run_container.sh script, formatted README #250
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
base: main
Are you sure you want to change the base?
Conversation
d0bcd1b to
34b7cfd
Compare
himdel
left a comment
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 not sure a change like this is desirable here.
Pulp already has good docs on how to run their containers, and those already include the UI.
For local development, my goal is to provide instructions on how to get things working on a linux and a mac, so that people can do development without knowing anything about how to set up pulp ... but I feel like providing scripts for all the possible docker/podman/rancher configs is out of scope for me.
(At least, this still won't work on macs if you just brew install podman and don't run the VM by default. And rancher may do things yet differently.)
But, if you're willing to consistently maintain it, it does look like a good thing to have 👍 :)
tests/run_container.sh
Outdated
| if [ "${CONTAINER_FILE:+x}" ] | ||
| then | ||
| IMAGE_TAG="ephemeral-build" | ||
| "${CONTAINER_RUNTIME}" build --file "${BASEPATH}/assets/${CONTAINER_FILE}" --build-arg FROM_TAG="${FROM_TAG}" --tag ghcr.io/pulp/pulp:"${IMAGE_TAG}" . |
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.
--tag ghcr.io/pulp/pulp:"${IMAGE_TAG}"
This looks wrong, nothing in this repo should be building a pulp/pulp image.
And it very clearly doesn't come from ghcr.io, so maybe not that either? Just name it pulp-ui or something?
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.
Removed from run_container.sh script.
tests/run_container.sh
Outdated
| if [ "${CONTAINER_FILE:+x}" ] | ||
| then | ||
| IMAGE_TAG="ephemeral-build" | ||
| "${CONTAINER_RUNTIME}" build --file "${BASEPATH}/assets/${CONTAINER_FILE}" --build-arg FROM_TAG="${FROM_TAG}" --tag ghcr.io/pulp/pulp:"${IMAGE_TAG}" . |
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.
BASEPATH points to pulp-ui/tests/, which is empty except for run_container.sh
Which /assets/ are we talking about?
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.
Removed from run_container.sh script.
| ${PULP_API_ROOT:+--env PULP_API_ROOT} \ | ||
| --detach \ | ||
| --name "pulp-ephemeral" \ | ||
| --volume "${BASEPATH}/settings:/etc/pulp${SELINUX:+: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 here, there is no pulp-ui/tests/settings)
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 was my mistake, I've added settings.py and changed .gitignore to exclude the tests/settings/certs/ directory.
tests/run_container.sh
Outdated
| --volume "${BASEPATH}/settings:/etc/pulp${SELINUX:+:Z}" \ | ||
| --publish "8080:80" \ | ||
| --network "bridge" \ | ||
| "ghcr.io/pulp/pulp:${IMAGE_TAG}" |
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 here, ghcr.io/pulp/pulp is not the right name)
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've changed this to docker.io
tests/run_container.sh
Outdated
| "${CONTAINER_RUNTIME}" build --file "${BASEPATH}/assets/${CONTAINER_FILE}" --build-arg FROM_TAG="${FROM_TAG}" --tag ghcr.io/pulp/pulp:"${IMAGE_TAG}" . | ||
| fi | ||
|
|
||
| if [ "$(getenforce)" = "Enforcing" ]; then |
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.
bash: getenforce: command not found
you will need to check for its existence first
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.
Add a check to determine if getenforce is present
tests/run_container.sh
Outdated
| # Set admin password | ||
| "${CONTAINER_RUNTIME}" exec "pulp-ephemeral" pulpcore-manager reset-admin-password --password password | ||
|
|
||
| if [ -d "${BASEPATH}/container_setup.d/" ] |
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 here, there is no pulp-ui/tests/container_setup.d/)
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.
Removed from run_container.sh script.
tests/run_container.sh
Outdated
| @@ -0,0 +1,97 @@ | |||
| #!/bin/sh | |||
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.
| #!/bin/sh | |
| #!/bin/bash |
given the rest of the script, this has to be bash not sh - at least $() won't work in plain sh, and I'm not sure about all those ${:+} either.
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 suspect the original author wrote this with sh as a symlink to bash. Have updated it point directly to bash.
| #### change password: | ||
|
|
||
| ```sh | ||
| podman exec -it pulp pulpcore-manager reset-admin-password --password admin | ||
| ``` | ||
| ```sh | ||
| docker exec -it compose-pulp_api-1 pulpcore-manager reset-admin-password --password admin | ||
| ``` | ||
|
|
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.
Keep this please. This is where I look when I need to change the backend password, it's not so useful when hidden inside a script.
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.
Added back to readme
README.md
Outdated
|
|
||
| You can follow the [pulp-oci-images quickstart](https://pulpproject.org/pulp-oci-images/docs/admin/tutorials/quickstart/), | ||
| TLDR: | ||
| The `tests/run_container.sh` script is provided and allows you to run a command with a [Pulp in one](https://pulpproject.org/pulp-in-one-container/) container active. |
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.
Your link does not work. Keep the one that does please.
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.
Have re-added original setup documentation
| #### setup: | ||
|
|
||
| ```sh | ||
| mkdir -p ~/pulp-backend-oci/{settings/certs,pulp_storage,pgsql,containers} | ||
| cd ~/pulp-backend-oci/ | ||
| echo " | ||
| CONTENT_ORIGIN='http://$(hostname):8080' | ||
| ANSIBLE_API_HOSTNAME='http://$(hostname):8080' | ||
| ANSIBLE_CONTENT_HOSTNAME='http://$(hostname):8080/pulp/content' | ||
| " >> settings/settings.py | ||
| ``` | ||
|
|
||
| #### run: | ||
|
|
||
| ```sh | ||
| cd ~/pulp-backend-oci/ | ||
| podman run --publish 8080:80 \ | ||
| --replace --name pulp \ | ||
| --volume "$(pwd)/settings":/etc/pulp \ | ||
| --volume "$(pwd)/pulp_storage":/var/lib/pulp \ | ||
| --volume "$(pwd)/pgsql":/var/lib/pgsql \ | ||
| --volume "$(pwd)/containers":/var/lib/containers \ | ||
| docker.io/pulp/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.
keep the minimal setup documented please
It's more useful as 20 lines of docs, than 100 lines of script.
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.
Have re-added original setup documentation
| run-parts --regex '^[0-9]+-[-_[:alnum:]]*\.sh$' "${BASEPATH}/container_setup.d/" | ||
| fi | ||
|
|
||
| PULP_LOGGING="${CONTAINER_RUNTIME}" "$@" |
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.
| PULP_LOGGING="${CONTAINER_RUNTIME}" "$@" | |
| echo Press ^C to stop the container 1>&2 | |
| sleep inf |
and we don't need to document any params for it :)
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 did think of this. But one way this script can be use is running it with test software, Squeezer (ansible module) does it like this:
./tests/run_container.sh make livetest
This allows a user to start a pulp oci container, run tests against it and automatically have it remove itself when complete.
This script could be used in the same way for this repository in the future.
.gitignore
Outdated
| dist/ | ||
| node_modules/ | ||
| npm-debug.log* | ||
| tests/settings |
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.
squeezer does actually have tests/settings, which are NOT gitignored .. sounds like a meaningful difference given https://github.com/pulp/pulp-ui/pull/250/files#r2231553649
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 was my mistake, I've added tests/testtings/settings.py and changed .gitignore to exclude the tests/settings/certs/ directory.
|
Going to address a bunch of the script questions here. Originally, I was looking for a easy way to start a consistent pulp environment while working on this project. I ended up using the Initially I was reluctant to make any changes to the contents because I assumed that However, I've compared the script against the other repositories listed in it's header and there are differences between them. So I'll update/rewrite this script to make it more compatible with this repository. |
This commit adds the
tests/run_container.shscript from: https://github.com/pulp/squeezer/blob/develop/tests/run_container.shIt provides an easy way to start a pulp oci container.