Add registry-loader-lite docker image, which is a simple passthrough …#54
Add registry-loader-lite docker image, which is a simple passthrough …#54tloubrieu-jpl merged 5 commits intomainfrom
Conversation
|
|
||
| # Install curl | ||
| RUN apt-get update -y | ||
| RUN apt-get install curl -y |
There was a problem hiding this comment.
These two RUN commands can be combined into one with && and will save a layer in the image. (--yes? 😉)
| # Set following arguments with compatible versions | ||
| ARG harvest_package_path=https://github.com/NASA-PDS/harvest/releases/download/v5.0.0/harvest-5.0.0-bin.tar.gz | ||
| ARG reg_manager_package_path=https://github.com/NASA-PDS/registry-mgr/releases/download/v6.0.0/registry-manager-6.0.0-bin.tar.gz | ||
| ARG test_data_url=https://pds.nasa.gov/data/pds4/test-data/registry/custom-datasets.tar.gz |
There was a problem hiding this comment.
The test_data_url isn't used anywhere else in the DockerfileLite; can it be removed?
| set -euo pipefail | ||
|
|
||
|
|
||
| exec "$@" |
There was a problem hiding this comment.
First, this script has two "shebangs", #!? Shouldn't just one be necessary?
Also, since this script just does an exec of its argument, would it maybe be simpler to just have the ENTRYPOINT in the Dockerfile?
Actually, the README expects callers to provide the command directly, so there might not be a need for an ENTRYPOINT at all.
There was a problem hiding this comment.
Thanks @nutjob4life , that is an interesting simplification, that I will try.
| Alternatively you can create a simplified docker image without the integration test logic which is preferred to be handled in the registry overarching repository. | ||
|
|
||
| ``` | ||
| docker image build -t nasapds/registry-loader-lite -f docker/DockerfileLite --build-arg harvest_package_path=harvest/target/harvest-5.0.0-SNAPSHOT-bin.tar.gz --build-arg reg_manager_package_path=manager/target/registry-manager-6.0.0-SNAPSHOT-bin.tar.gz . |
There was a problem hiding this comment.
In READMEs, I prefer to see the --long form of command-line arguments like --tag and --file, since they're self-documenting. But that's just my personal taste; feel free to ignore this.
|
|
||
| Call one of the registry-loader tools (registry-manager or harvest) using the following command. Below is an example to create a registry using the registry-manager tool: | ||
|
|
||
| docker run --rm -it -v {where your configuration files are}:/config nasapds/registry-loader-lite registry-manager create-registry -auth /config/es-admin-auth.cfg -registry file:///config/registry-connection.xml |
There was a problem hiding this comment.
Same thing here; --volume instead of -v, and docker container run to be extra-explicit.
Also: why is -it (interactive and tty) necessary? Does this require user interaction?
| ADD $harvest_package_path /app/ | ||
| ADD $reg_manager_package_path /app/ | ||
| # One extraction step for all archives | ||
| RUN sh -c 'ls /app/*.tar.gz 2>/dev/null | grep -q . && for f in /app/*.tar.gz; do tar -xzf "$f" -C /app; done || echo "No .tar.gz files to extract"' \ |
There was a problem hiding this comment.
There are extra spaces after the \ meaning it's not a continuation line.
|
|
Thanks @nutjob4life , I applied your change requests. |
2 participants
…to call registry-loader tools
🗒️ Summary
Add a simplified docker image without the test logic.
This is used by the new registry deployment automation which handles the test logic.
🤖 AI Assistance Disclosure
Estimated % of code influenced by AI: _50 %
⚙️ Test Data and/or Report
Used with PR NASA-PDS/registry#475
♻️ Related Issues
🤓 Reviewer Checklist
Reviewers: Please verify the following before approving this pull request.
Documentation and PR Content
Security & Quality
Testing & Validation
Maintenance