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

Work around docker race condition when running build post commit hooks. #13100

Merged
merged 1 commit into from Mar 4, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/build/builder/common.go
Expand Up @@ -150,11 +150,11 @@ func execPostCommitHook(client DockerClient, postCommitSpec api.BuildPostCommitS
Memory: limits.MemoryLimitBytes,
MemorySwap: limits.MemorySwap,
},
}, docker.LogsOptions{
}, docker.AttachToContainerOptions{
// Stream logs to stdout and stderr.
OutputStream: os.Stdout,
ErrorStream: os.Stderr,
Follow: true,
Stream: true,
Stdout: true,
Stderr: true,
})
Expand Down
37 changes: 31 additions & 6 deletions pkg/build/builder/dockerutil.go
Expand Up @@ -39,6 +39,7 @@ var (
// DockerClient is an interface to the Docker client that contains
// the methods used by the common builder
type DockerClient interface {
AttachToContainerNonBlocking(opts docker.AttachToContainerOptions) (docker.CloseWaiter, error)
BuildImage(opts docker.BuildImageOptions) error
PushImage(opts docker.PushImageOptions, auth docker.AuthConfiguration) error
RemoveImage(name string) error
Expand Down Expand Up @@ -169,7 +170,7 @@ func tagImage(dockerClient DockerClient, image, name string) error {
// dockerRun mimics the 'docker run --rm' CLI command. It uses the Docker Remote
// API to create and start a container and stream its logs. The container is
// removed after it terminates.
func dockerRun(client DockerClient, createOpts docker.CreateContainerOptions, logsOpts docker.LogsOptions) error {
func dockerRun(client DockerClient, createOpts docker.CreateContainerOptions, attachOpts docker.AttachToContainerOptions) error {
// Create a new container.
glog.V(4).Infof("Creating container with options {Name:%q Config:%+v HostConfig:%+v} ...", createOpts.Name, createOpts.Config, createOpts.HostConfig)
c, err := client.CreateContainer(createOpts)
Expand All @@ -188,17 +189,41 @@ func dockerRun(client DockerClient, createOpts docker.CreateContainerOptions, lo
}
}
startWaitContainer := func() error {
// Changed to use attach call instead of logs call to stream stdout/stderr
// during execution to avoid race condition
// https://github.com/docker/docker/issues/31323 .
// Using attach call is also racy in docker versions which don't carry
// https://github.com/docker/docker/pull/30446 .
// In RHEL, docker >= 1.12.6-10.el7.x86_64 should be OK.

// Attach to the container.
success := make(chan struct{})
attachOpts.Container = c.ID
attachOpts.Success = success
glog.V(4).Infof("Attaching to container %q with options %+v ...", containerName, attachOpts)
wc, err := client.AttachToContainerNonBlocking(attachOpts)
if err != nil {
return fmt.Errorf("attach container %q: %v", containerName, err)
}
defer wc.Close()

select {
case <-success:
close(success)
case <-time.After(120 * time.Second):
return fmt.Errorf("attach container %q: timeout waiting for success signal", containerName)
}

// Start the container.
glog.V(4).Infof("Starting container %q ...", containerName)
if err := client.StartContainer(c.ID, nil); err != nil {
return fmt.Errorf("start container %q: %v", containerName, err)
}

// Stream container logs.
logsOpts.Container = c.ID
glog.V(4).Infof("Streaming logs of container %q with options %+v ...", containerName, logsOpts)
if err := client.Logs(logsOpts); err != nil {
return fmt.Errorf("streaming logs of %q: %v", containerName, err)
// Wait for streaming to finish.
glog.V(4).Infof("Waiting for streaming to finish ...")
if err := wc.Wait(); err != nil {
return fmt.Errorf("container %q streaming: %v", containerName, err)
}

// Return an error if the exit code of the container is non-zero.
Expand Down
3 changes: 3 additions & 0 deletions pkg/build/builder/dockerutil_test.go
Expand Up @@ -94,6 +94,9 @@ func (d *FakeDocker) WaitContainer(id string) (int, error) {
func (d *FakeDocker) Logs(opts docker.LogsOptions) error {
return nil
}
func (d *FakeDocker) AttachToContainerNonBlocking(opts docker.AttachToContainerOptions) (docker.CloseWaiter, error) {
return nil, nil
}
func (d *FakeDocker) TagImage(name string, opts docker.TagImageOptions) error {
d.callLog = append(d.callLog, methodCall{"TagImage", []interface{}{name, opts}})
return nil
Expand Down
4 changes: 2 additions & 2 deletions test/extended/util/framework.go
Expand Up @@ -592,9 +592,9 @@ func WaitForAnImageStream(client client.ImageStreamInterface,
}

// WaitForAnImageStreamTag waits until an image stream with given name has non-empty history for given tag.
// Defaults to waiting for 60 seconds
// Defaults to waiting for 300 seconds
func WaitForAnImageStreamTag(oc *CLI, namespace, name, tag string) error {
return TimedWaitForAnImageStreamTag(oc, namespace, name, tag, time.Second*60)
return TimedWaitForAnImageStreamTag(oc, namespace, name, tag, time.Second*300)
}

// TimedWaitForAnImageStreamTag waits until an image stream with given name has non-empty history for given tag.
Expand Down