-
Notifications
You must be signed in to change notification settings - Fork 590
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
docker-compose: Bugfix: #1598
docker-compose: Bugfix: #1598
Conversation
kennychennetman
commented
Dec 11, 2020
- fix status determination for more than one docker-compose instance
- fix status determination for more than one docker-compose instance
Can one of the admins verify this patch? |
heartbeat/docker-compose
Outdated
# Version: 1.1.2 | ||
# Date: 2020-06-24 | ||
|
||
# Version: 1.1.1 |
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 increase the version nr, not decrease 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.
fixed
- add determination for docker daemon status - improved accuracy for multiple instance
@@ -36,7 +36,7 @@ | |||
. ${OCF_FUNCTIONS_DIR}/ocf-shellfuncs | |||
|
|||
# Defaults | |||
OCF_RESKEY_binpath_default=/usr/bin/docker-compose | |||
OCF_RESKEY_binpath_default=/usr/local/bin/docker-compose |
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 shouldnt change the default path for it. The setting is there so you can set what the path is on your system when it's not in the default 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.
I want to change the default path aligned to Ubuntu distribution.
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 be able to use get_release_id() for that (see ocf-distro file for logic).
cd $DIR | ||
$COMMAND up -d || { | ||
ocf_log err "Error. docker-compose returned error $?." | ||
cd /tmp |
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 shouldnt need to run cd /tmp
in the agent.
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.
fixed
@@ -185,19 +196,25 @@ docker_compose_status() | |||
|
|||
docker_compose_start() | |||
{ | |||
|
|||
sleep 5 |
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 shouldnt need to sleep here.
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.
yes, agree. I am still working on it... it could be improved in the future version.
docker_compose_validate_all | ||
docker_compose_status >/dev/null 2>&1 | ||
retVal=$? | ||
# return success if docker service is running | ||
[ $retVal -eq $OCF_SUCCESS ] && exit $OCF_SUCCESS | ||
|
||
sleep 10 |
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.
If you need to wait here you need to add a loop checking every 1-2s (and fails when Pacemaker timeouts) to avoid waiting too much in general, and also work for cases where it takes more than 10s.
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.
yes, agree. I am still working on it... it could be improved in the future version.
…P process. Using one command instead.
re-commit via #1694 |