Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

weave script wait_for_status can error if container dies #600

Closed
bboreham opened this issue Apr 23, 2015 · 1 comment · Fixed by #603
Closed

weave script wait_for_status can error if container dies #600

bboreham opened this issue Apr 23, 2015 · 1 comment · Fixed by #603
Assignees
Labels
Milestone

Comments

@bboreham
Copy link
Contributor

If the container dies just before the docker inspect at the beginning of wait_for_status then TARGET_IP can be blank, which causes an error.

To reproduce: insert docker stop $1 in the script at the beginning of wait_for_status:

$ sudo weave --local launch
weave
/usr/local/bin/weave: 467: shift: can't shift that many
@rade rade added the bug label Apr 23, 2015
@rade
Copy link
Member

rade commented Apr 23, 2015

I struggled to reproduce this. In particular, I do not get the error with weave launch sans --local; I get "Container weave died", as expected. And then it dawned on me that we are running different shells. On my ubuntu box weave --local runs dash. Without --local the shell that gets run is whatever is in the weavexec image, which is busybox's shell, which is some flavour of ash.

@dpw discovered that POSIX says "If the n operand is invalid or is greater than "$#", this may be considered a syntax error and a non-interactive shell may exit; if the shell does not exit in this case, a non-zero exit status shall be returned. Otherwise, zero shall be returned."

So locally the invalid shift causes the shell to exit, whereas in weavexec it just causes the http_call_ip function to return a non-zero status. Which is good since that makes this a less critical bug.

@rade rade self-assigned this Apr 23, 2015
@rade rade closed this as completed in #603 Apr 23, 2015
rade added a commit that referenced this issue Apr 23, 2015
guard against early container death in wait_for_status

Fixes #600.
@rade rade modified the milestone: 0.11.0 May 12, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants