Replace docker check #3270
Closed
Replace docker check #3270
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
magiconair
force-pushed
the
replace-docker-check
branch
from
95a8597
to
28be66f
Compare
magiconair
force-pushed
the
replace-docker-check
branch
from
28be66f
to
796d4f7
Compare
magiconair
force-pushed
the
replace-docker-check
branch
from
796d4f7
to
f159498
Compare
magiconair
force-pushed
the
replace-docker-check
branch
from
f159498
to
fa84b3e
Compare
magiconair
force-pushed
the
replace-docker-check
branch
from
fa84b3e
to
db947ed
Compare
magiconair
force-pushed
the
replace-docker-check
branch
from
db947ed
to
83de060
Compare
magiconair
force-pushed
the
replace-docker-check
branch
from
83de060
to
5b71afc
Compare
magiconair
force-pushed
the
replace-docker-check
branch
from
5b71afc
to
b514187
Compare
slackpad
reviewed
Jul 18, 2017
agent/check_test.go
Outdated
| }, | ||
| "POST /exec/456/start": func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(200) | ||
| fmt.Fprint(w, "01234567890123456789OK") // more than 10 bytes |
There was a problem hiding this comment.
You probably meant "20 bytes" here, since that's the output buffer size for this test.
There was a problem hiding this comment.
Yes, was playing with 10 bytes but that bit me somewhere else. will fix.
agent/docker.go
Outdated
| host = DefaultDockerHost | ||
| } | ||
| p := strings.SplitN(host, "://", 2) | ||
| if len(p) == 1 { |
There was a problem hiding this comment.
Since this can come from the outside I'd assert that it's equal to 2. If we have no split (len(p) == 0) this will panic down below.
There was a problem hiding this comment.
I don't think that len(p) can ever be zero unless you specify 0 as the last argument. If the split string isn't found you get host back.
agent/docker.go
Outdated
| Cmd: cmd, | ||
| } | ||
|
|
||
| uri := fmt.Sprintf("/containers/%s/exec", containerID) |
There was a problem hiding this comment.
Maybe URL encode the containerID to prevent shenanigans?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR drops unused dependencies and rewrites the Docker health check without using the docker client. This removes about 1/3rd of our code base.
See #3254 #3257