oc cluster up: work around docker attach race condition #12223
Conversation
| // stdout/stderr to avoid race condition in (at least) docker 1.10-1.14-dev: | ||
| // https://github.com/docker/docker/issues/29285 | ||
| glog.V(5).Infof("Attaching to container %q", id) | ||
| err = h.client.AttachToContainer(docker.AttachToContainerOptions{ |
There was a problem hiding this comment.
Just a question ... why use AttachToContainer and not Logs ?
There was a problem hiding this comment.
Only because that's what was there before -- now changed to h.client.Logs(), thanks!
jim-minter
force-pushed
the
dockerhang
branch
from
a147d61 to
46d539f
Compare
|
LGTM [merge] |
|
Evaluated for origin test up to 46d539f |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12273/) (Base Commit: 9ee2ff6) |
|
#12236 |
|
Evaluated for origin merge up to 46d539f |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12336/) (Base Commit: 850a41b) (Image: devenv-rhel7_5535) |
|
@jim-minter I think this is also causing problems in our postcommit hook logic when the postcommit hook operation is very short. Any chance you can do a PR to fsouza/go-dockerclient to patch the log streaming code there too? (Assuming i'm correct) here's where we're running the container and streaming the logs: |
|
@bparees You might create a new issue for this. Do you have an example/replicator?
|
|
@jim-minter opened #12587 the recreate its pretty simple. define a buildconfig w/ a postcommit hook. make the postcommit hook be something that's very quick like an echo command. @spadgett was able to recreate it consistently, i haven't tried myself. |
Fixes #9470
Alternative to #12189
[test]
@csrwng FYI