Merge pull request #9048 from matejvasek/apiv2_wait

Fix Docker APIv2 container wait endpoint

cmd/podman/containers/wait.go

@@ -95,10 +95,11 @@ func wait(cmd *cobra.Command, args []string) error {
 		return errors.New("--latest and containers cannot be used together")
 	}
 
-	waitOptions.Condition, err = define.StringToContainerStatus(waitCondition)
+	cond, err := define.StringToContainerStatus(waitCondition)
 	if err != nil {
 		return err
 	}
+	waitOptions.Condition = []define.ContainerStatus{cond}
 
 	responses, err := registry.ContainerEngine().ContainerWait(context.Background(), args, waitOptions)
 	if err != nil {

libpod/container_api.go

@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
+	"sync"
 	"time"
 
 	"github.com/containers/podman/v2/libpod/define"
@@ -478,13 +479,13 @@ func (c *Container) RemoveArtifact(name string) error {
 }
 
 // Wait blocks until the container exits and returns its exit code.
-func (c *Container) Wait() (int32, error) {
-	return c.WaitWithInterval(DefaultWaitInterval)
+func (c *Container) Wait(ctx context.Context) (int32, error) {
+	return c.WaitWithInterval(ctx, DefaultWaitInterval)
 }
 
 // WaitWithInterval blocks until the container to exit and returns its exit
 // code. The argument is the interval at which checks the container's status.
-func (c *Container) WaitWithInterval(waitTimeout time.Duration) (int32, error) {
+func (c *Container) WaitWithInterval(ctx context.Context, waitTimeout time.Duration) (int32, error) {
 	if !c.valid {
 		return -1, define.ErrCtrRemoved
 	}
@@ -495,41 +496,111 @@ func (c *Container) WaitWithInterval(waitTimeout time.Duration) (int32, error) {
 	}
 	chWait := make(chan error, 1)
 
-	defer close(chWait)
+	go func() {
+		<-ctx.Done()
+		chWait <- define.ErrCanceled
+	}()
 
 	for {
-		// ignore errors here, it is only used to avoid waiting
+		// ignore errors here (with exception of cancellation), it is only used to avoid waiting
 		// too long.
-		_, _ = WaitForFile(exitFile, chWait, waitTimeout)
+		_, e := WaitForFile(exitFile, chWait, waitTimeout)
+		if e == define.ErrCanceled {
+			return -1, define.ErrCanceled
+		}
 
-		stopped, err := c.isStopped()
+		stopped, code, err := c.isStopped()
 		if err != nil {
 			return -1, err
 		}
 		if stopped {
-			return c.state.ExitCode, nil
+			return code, nil
 		}
 	}
 }
 
-func (c *Container) WaitForConditionWithInterval(waitTimeout time.Duration, condition define.ContainerStatus) (int32, error) {
+type waitResult struct {
+	code int32
+	err  error
+}
+
+func (c *Container) WaitForConditionWithInterval(ctx context.Context, waitTimeout time.Duration, conditions ...define.ContainerStatus) (int32, error) {
 	if !c.valid {
 		return -1, define.ErrCtrRemoved
 	}
-	if condition == define.ContainerStateStopped || condition == define.ContainerStateExited {
-		return c.WaitWithInterval(waitTimeout)
+
+	if len(conditions) == 0 {
+		panic("at least one condition should be passed")
 	}
-	for {
-		state, err := c.State()
-		if err != nil {
-			return -1, err
+
+	ctx, cancelFn := context.WithCancel(ctx)
+	defer cancelFn()
+
+	resultChan := make(chan waitResult)
+	waitForExit := false
+	wantedStates := make(map[define.ContainerStatus]bool, len(conditions))
+
+	for _, condition := range conditions {
+		if condition == define.ContainerStateStopped || condition == define.ContainerStateExited {
+			waitForExit = true
+			continue
 		}
-		if state == condition {
-			break
+		wantedStates[condition] = true
+	}
+
+	trySend := func(code int32, err error) {
+		select {
+		case resultChan <- waitResult{code, err}:
+		case <-ctx.Done():
 		}
-		time.Sleep(waitTimeout)
 	}
-	return -1, nil
+
+	var wg sync.WaitGroup
+
+	if waitForExit {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			code, err := c.WaitWithInterval(ctx, waitTimeout)
+			trySend(code, err)
+		}()
+	}
+
+	if len(wantedStates) > 0 {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			for {
+				state, err := c.State()
+				if err != nil {
+					trySend(-1, err)
+					return
+				}
+				if _, found := wantedStates[state]; found {
+					trySend(-1, nil)
+					return
+				}
+				select {
+				case <-ctx.Done():
+					return
+				case <-time.After(waitTimeout):
+					continue
+				}
+			}
+		}()
+	}
+
+	var result waitResult
+	select {
+	case result = <-resultChan:
+		cancelFn()
+	case <-ctx.Done():
+		result = waitResult{-1, define.ErrCanceled}
+	}
+	wg.Wait()
+	return result.code, result.err
 }
 
 // Cleanup unmounts all mount points in container and cleans up container storage

libpod/container_internal.go

@@ -754,17 +754,17 @@ func (c *Container) getArtifactPath(name string) string {
 }
 
 // Used with Wait() to determine if a container has exited
-func (c *Container) isStopped() (bool, error) {
+func (c *Container) isStopped() (bool, int32, error) {
 	if !c.batched {
 		c.lock.Lock()
 		defer c.lock.Unlock()
 	}
 	err := c.syncContainer()
 	if err != nil {
-		return true, err
+		return true, -1, err
 	}
 
-	return !c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused, define.ContainerStateStopping), nil
+	return !c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused, define.ContainerStateStopping), c.state.ExitCode, nil
 }
 
 // save container state to the database

libpod/define/errors.go

@@ -198,4 +198,8 @@ var (
 	// ErrSecurityAttribute indicates that an error processing security attributes
 	// for the container
 	ErrSecurityAttribute = fmt.Errorf("%w: unable to process security attribute", ErrOCIRuntime)
+
+	// ErrCanceled indicates that an operation has been cancelled by a user.
+	// Useful for potentially long running tasks.
+	ErrCanceled = errors.New("cancelled by user")
 )

pkg/api/handlers/compat/containers.go

@@ -23,10 +23,8 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/go-connections/nat"
 	"github.com/docker/go-units"
-	"github.com/gorilla/mux"
 	"github.com/gorilla/schema"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 func RemoveContainer(w http.ResponseWriter, r *http.Request) {
@@ -233,8 +231,11 @@ func KillContainer(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		if sig == 0 || syscall.Signal(sig) == syscall.SIGKILL {
-			if _, err := utils.WaitContainer(w, r); err != nil {
-
+			opts := entities.WaitOptions{
+				Condition: []define.ContainerStatus{define.ContainerStateExited, define.ContainerStateStopped},
+				Interval:  time.Millisecond * 250,
+			}
+			if _, err := containerEngine.ContainerWait(r.Context(), []string{name}, opts); err != nil {
 				utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
 				return
 			}
@@ -245,26 +246,8 @@ func KillContainer(w http.ResponseWriter, r *http.Request) {
 }
 
 func WaitContainer(w http.ResponseWriter, r *http.Request) {
-	var msg string
 	// /{version}/containers/(name)/wait
-	exitCode, err := utils.WaitContainer(w, r)
-	if err != nil {
-		if errors.Cause(err) == define.ErrNoSuchCtr {
-			logrus.Warnf("container not found %q: %v", utils.GetName(r), err)
-			return
-		}
-		logrus.Warnf("failed to wait on container %q: %v", mux.Vars(r)["name"], err)
-		return
-	}
-
-	utils.WriteResponse(w, http.StatusOK, handlers.ContainerWaitOKBody{
-		StatusCode: int(exitCode),
-		Error: struct {
-			Message string
-		}{
-			Message: msg,
-		},
-	})
+	utils.WaitContainerDocker(w, r)
 }
 
 func LibpodToContainer(l *libpod.Container, sz bool) (*handlers.Container, error) {

pkg/api/handlers/libpod/containers.go

@@ -4,7 +4,6 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
-	"strconv"
 
 	"github.com/containers/podman/v2/libpod"
 	"github.com/containers/podman/v2/libpod/define"
@@ -146,17 +145,7 @@ func GetContainer(w http.ResponseWriter, r *http.Request) {
 }
 
 func WaitContainer(w http.ResponseWriter, r *http.Request) {
-	exitCode, err := utils.WaitContainer(w, r)
-	if err != nil {
-		name := utils.GetName(r)
-		if errors.Cause(err) == define.ErrNoSuchCtr {
-			utils.ContainerNotFound(w, name, err)
-			return
-		}
-		logrus.Warnf("failed to wait on container %q: %v", name, err)
-		return
-	}
-	utils.WriteResponse(w, http.StatusOK, strconv.Itoa(int(exitCode)))
+	utils.WaitContainerLibpod(w, r)
 }
 
 func UnmountContainer(w http.ResponseWriter, r *http.Request) {