Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 64b75dc

Browse files
author
Jim Minter
committedMar 2, 2017
Work around docker race condition when running build post commit hooks.
1 parent f1dd3c7 commit 64b75dc

File tree

3 files changed

+27
-8
lines changed

3 files changed

+27
-8
lines changed
 

‎pkg/build/builder/common.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,11 @@ func execPostCommitHook(client DockerClient, postCommitSpec api.BuildPostCommitS
150150
Memory: limits.MemoryLimitBytes,
151151
MemorySwap: limits.MemorySwap,
152152
},
153-
}, docker.LogsOptions{
153+
}, docker.AttachToContainerOptions{
154154
// Stream logs to stdout and stderr.
155155
OutputStream: os.Stdout,
156156
ErrorStream: os.Stderr,
157-
Follow: true,
157+
Stream: true,
158158
Stdout: true,
159159
Stderr: true,
160160
})

‎pkg/build/builder/dockerutil.go

+22-6
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ var (
3939
// DockerClient is an interface to the Docker client that contains
4040
// the methods used by the common builder
4141
type DockerClient interface {
42+
AttachToContainerNonBlocking(opts docker.AttachToContainerOptions) (docker.CloseWaiter, error)
4243
BuildImage(opts docker.BuildImageOptions) error
4344
PushImage(opts docker.PushImageOptions, auth docker.AuthConfiguration) error
4445
RemoveImage(name string) error
@@ -169,7 +170,7 @@ func tagImage(dockerClient DockerClient, image, name string) error {
169170
// dockerRun mimics the 'docker run --rm' CLI command. It uses the Docker Remote
170171
// API to create and start a container and stream its logs. The container is
171172
// removed after it terminates.
172-
func dockerRun(client DockerClient, createOpts docker.CreateContainerOptions, logsOpts docker.LogsOptions) error {
173+
func dockerRun(client DockerClient, createOpts docker.CreateContainerOptions, attachOpts docker.AttachToContainerOptions) error {
173174
// Create a new container.
174175
glog.V(4).Infof("Creating container with options {Name:%q Config:%+v HostConfig:%+v} ...", createOpts.Name, createOpts.Config, createOpts.HostConfig)
175176
c, err := client.CreateContainer(createOpts)
@@ -188,17 +189,32 @@ func dockerRun(client DockerClient, createOpts docker.CreateContainerOptions, lo
188189
}
189190
}
190191
startWaitContainer := func() error {
192+
// Changed to use attach call instead of logs call to stream stdout/stderr
193+
// during execution to avoid race condition
194+
// https://github.com/docker/docker/issues/31323 .
195+
// Using attach call is also racy in docker versions which don't carry
196+
// https://github.com/docker/docker/pull/30446 .
197+
// In RHEL, docker >= 1.12.6-10.el7.x86_64 should be OK.
198+
199+
// Attach to the container.
200+
attachOpts.Container = c.ID
201+
glog.V(4).Infof("Attaching to container %q with options %+v ...", containerName, attachOpts)
202+
wc, err := client.AttachToContainerNonBlocking(attachOpts)
203+
if err != nil {
204+
return fmt.Errorf("attach container %q: %v", containerName, err)
205+
}
206+
defer wc.Close()
207+
191208
// Start the container.
192209
glog.V(4).Infof("Starting container %q ...", containerName)
193210
if err := client.StartContainer(c.ID, nil); err != nil {
194211
return fmt.Errorf("start container %q: %v", containerName, err)
195212
}
196213

197-
// Stream container logs.
198-
logsOpts.Container = c.ID
199-
glog.V(4).Infof("Streaming logs of container %q with options %+v ...", containerName, logsOpts)
200-
if err := client.Logs(logsOpts); err != nil {
201-
return fmt.Errorf("streaming logs of %q: %v", containerName, err)
214+
// Wait for streaming to finish.
215+
glog.V(4).Infof("Waiting for streaming to finish ...")
216+
if err := wc.Wait(); err != nil {
217+
return fmt.Errorf("container %q streaming: %v", containerName, err)
202218
}
203219

204220
// Return an error if the exit code of the container is non-zero.

‎pkg/build/builder/dockerutil_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ func (d *FakeDocker) WaitContainer(id string) (int, error) {
9494
func (d *FakeDocker) Logs(opts docker.LogsOptions) error {
9595
return nil
9696
}
97+
func (d *FakeDocker) AttachToContainerNonBlocking(opts docker.AttachToContainerOptions) (docker.CloseWaiter, error) {
98+
return nil, nil
99+
}
97100
func (d *FakeDocker) TagImage(name string, opts docker.TagImageOptions) error {
98101
d.callLog = append(d.callLog, methodCall{"TagImage", []interface{}{name, opts}})
99102
return nil

0 commit comments

Comments
 (0)
Please sign in to comment.