Skip to content
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

Get only the first container on node to call command #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gonzalo
Copy link

@gonzalo gonzalo commented Apr 26, 2019

We've been using your extremely useful helpers for a year (thanks!) and just found a small issue. When there are multiple replicas of the same service in the same node the command fails because it tries to invoke it with both container names and takes the second one as the parameter.
Example:

  • dockernodeps my_service -> node1: er234dse325 sdr234wsf3
  • dockerserviceexec my_service my_command -> ssh -t node1 docker exec -it er234dse325 sdr234wsf3 my_command (and then it fails)

My proposal is to slice PAIR string to catch only the first container. I did it in a roughly way (despite it works) because I don't know too much about parameter expansion but probably it could be done in a more elegant way.

@rdxmb
Copy link
Owner

rdxmb commented Aug 8, 2019

@gonzalo thanks for feedback and your contribution! I haven't expected anyone would be interested ...

I have to checkout the set -f . Maybe an awk would be the better way here. Let me see.

@@ -39,6 +39,11 @@ fi
if PAIR=$(dockernodeps $SERVICE) ; then
NODE=${PAIR%%:*}
CONTAINER=${PAIR#*:}
#this sequence takes the first container in case there are more of one in the node
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PAIR=${PAIR%% *} would also fix that. Could you test that, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants