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
configbaker container #9574
configbaker container #9574
Conversation
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.
Just some initial feedback. I ran it with no arguments and got the help ok.
To make use of the setup scripts inside containers, the target URL must be configurable and not hardcoded to localhost:8080. This commit simply creates a variable and uses the old static URL as default if it has not been set by the environment before.
This commit enhances any reference to other files (scripts or data) to be relative to the script currently running instead of the working directory from where the script is called from. This removes the requirement to change into the directory with setup-all.sh 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.
Looks fine but I haven't tested it.
export DATAVERSE_URL | ||
SOLR_URL=${SOLR_URL:-"http://localhost:8983"} | ||
|
||
SCRIPT_PATH="$(dirname "$0")" |
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.
SCRIPT_PATH="$(dirname "$0")" | |
# scripts/api when called from the root of the source tree | |
SCRIPT_PATH="$(dirname "$0")" |
Copying the default configset of the requested Solr version and adding our schema and config creates a marvelous template. To be reused during execution of a vanilla Solr image.
By building the image at the same time as the application, we can ensure both match. It also keeps compatibility between the compose file to start things and the Maven build.
With adding bootstrap.sh, a Dataverse instance may now be initialized. Using the changed setup scripts, we can reach out to a Dataverse container and set it up. By using a concept of personas, it is easy to add and select other initializations as necessary. This will be extended in the future.
Now an instance can be bootstrapped automatically.
051d132
to
ceddf70
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.
Just a quick question.
@@ -27,9 +34,7 @@ done | |||
command -v jq >/dev/null 2>&1 || { echo >&2 '`jq` ("sed for JSON") is required, but not installed. Download the binary for your platform from http://stedolan.github.io/jq/ and make sure it is in your $PATH (/usr/bin/jq is fine) and executable with `sudo chmod +x /usr/bin/jq`. On Mac, you can install it with `brew install jq` if you use homebrew: http://brew.sh . Aborting.'; exit 1; } | |||
|
|||
echo "deleting all data from Solr" | |||
curl http://localhost:8983/solr/collection1/update/json?commit=true -H "Content-type: application/json" -X POST -d "{\"delete\": { \"query\":\"*:*\"}}" | |||
|
|||
SERVER=http://localhost:8080/api |
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.
Is it strictly necessary to remove the hard coded 8080 in this and other setup scripts?
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 awfully sorry me lad, but yes it is.
When you run this script inside a container which is not the application server itself, localhost
is that container and not the app server. It's like a different machine. If you run this on the host, this works because of the mapped ports, but this will not work on a Windows host.
Sorry, hardcoding it never was a good idea, same as the paths not being relative to the script location.
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.
BTW Jenkins made it to the end, so I'm pretty sure I didn't break the scripts 😄
I tried this as of 36a43ac and it worked great! It's now literally a one liner to spin up a Dataverse dev environment:
Great work, @poikilotherm! 🎉 🚀 |
This was added long ago in 42692f5 when we were still indexing users! We don't need it anymore. If people have junk in Solr they can clear it out manually before installing Dataverse.
Co-authored-by: Philip Durbin <philipdurbin@gmail.com>
/push-image Test what happens with the configbaker image...? |
📦 Could not push preview image 😞. See log for details. |
/push-image Try again to push the images after fixing the registry target in |
📦 Pushed preview application image as
🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry 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.
Looks good!
As I mentioned in standup, setup-all has been updated but I'm not too worried because it is exercised by Jenkins.
What this PR does / why we need it:
This pull request will add a new container to be built and pushed alongside the application itself, but as a standalone thing to pull from a registry. This image contains lots of tools and scripts necessary to successfully start and configure a Dataverse instance to a) avoid repeating these instructions in different flavors over and over again plus b) encapsulate the Linux scripts in a container so it can be executed on any platform.
Which issue(s) this PR closes:
Special notes for your reviewer:
None yet. This is a WIP.
Suggestions on how to test this:
docker run -it --rm gdcc/configbaker:unstable
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope.
Is there a release notes update needed for this change?:
Ahem. Dunno. Maybe.
Additional documentation:
None yet.