-
Notifications
You must be signed in to change notification settings - Fork 9
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
ACC-119: allow docker image deployment #34
base: master
Are you sure you want to change the base?
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.
Invalidated as it needs more tests.
Their is also no apache configuration to expose the container. IMO at least a http / https and ldap protected / unprotected version should be need
env_var PRODUCT_BRANCH `expr "${PRODUCT_VERSION}" : '\([0-9]*\.[0-9]*\).*'`".x" | ||
env_var PRODUCT_MAJOR_BRANCH `expr "${PRODUCT_VERSION}" : '\([0-9]*\).*'`".x" | ||
configurable_env_var "INSTANCE_ID" "" | ||
if [[ -v PRODUCT_VERSION ]]; 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.
The -v option is not available on macos. Is it not possible to use a classical test with -z
or -n
?
Why not using a test on PRODUCT_NAME == docker ?
fi | ||
# docker image should have INSTANCE_KEY | ||
if [ -n "${DEPLOYMENT_DOCKER_IMAGE}" ]; then | ||
env_var "INSTANCE_KEY" "docker-${DEPLOYMENT_DOCKER_IMAGE}-${DOCKER_IMAGE_VERSION}" |
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.
You should not lose the INSTANCE_ID
parameter if we want to be able to deploy several time the same image
|
||
# validate additional parameters | ||
case "${ACTION}" in | ||
start | stop | restart | undeploy | deploy | download-dataset) | ||
# Mandatory env vars. They need to be defined before launching the script | ||
validate_env_var "PRODUCT_NAME" | ||
validate_env_var "PRODUCT_VERSION" | ||
configurable_env_var "PRODUCT_VERSION" "" |
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.
The validate should be moved on the previous test to ensure it's defined
@@ -1258,6 +1273,8 @@ do_deploy() { | |||
esac | |||
# Hack | |||
do_configure_chat | |||
|
|||
fi |
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.
The indentation looks weird
fi | ||
# docker image should have INSTANCE_KEY | ||
if [ -n "${DEPLOYMENT_DOCKER_IMAGE}" ]; 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.
In a plf deployment this variable is not set, the test will failed will failed
7e2ade5
to
ae38ca8
Compare
No description provided.