Merge pull request #711 from docker/fix_aci_500

ACI tests : Wait for http status and retry if necessary.

tests/aci-e2e/e2e-aci_test.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"os"
@@ -31,8 +30,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/docker/compose-cli/aci/convert"
-
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/icmd"
@@ -43,6 +40,7 @@ import (
 	"github.com/Azure/go-autorest/autorest/to"
 
 	"github.com/docker/compose-cli/aci"
+	"github.com/docker/compose-cli/aci/convert"
 	"github.com/docker/compose-cli/aci/login"
 	"github.com/docker/compose-cli/api/containers"
 	"github.com/docker/compose-cli/context/store"
@@ -252,12 +250,8 @@ func TestContainerRunVolume(t *testing.T) {
 	})
 
 	t.Run("http get", func(t *testing.T) {
-		r, err := HTTPGetWithRetry(endpoint, 3)
+		output := HTTPGetWithRetry(t, endpoint, http.StatusOK, 2*time.Second, 20*time.Second)
-		assert.NilError(t, err)
+		assert.Assert(t, strings.Contains(output, testFileContent), "Actual content: "+output)
-		assert.Equal(t, r.StatusCode, http.StatusOK)
-		b, err := ioutil.ReadAll(r.Body)
-		assert.NilError(t, err)
-		assert.Assert(t, strings.Contains(string(b), testFileContent), "Actual content: "+string(b))
 	})
 
 	t.Run("logs", func(t *testing.T) {
@@ -499,17 +493,12 @@ func TestComposeUpUpdate(t *testing.T) {
 		assert.Assert(t, is.Len(containerInspect.Ports, 1))
 		endpoint := fmt.Sprintf("http://%s:%d", containerInspect.Ports[0].HostIP, containerInspect.Ports[0].HostPort)
 
-		r, err := HTTPGetWithRetry(endpoint+"/words/noun", 3)
+		output := HTTPGetWithRetry(t, endpoint+"/words/noun", http.StatusOK, 2*time.Second, 20*time.Second)
-		assert.NilError(t, err)
+
-		assert.Equal(t, r.StatusCode, http.StatusOK)
+		assert.Assert(t, strings.Contains(output, `"word":`))
-		b, err := ioutil.ReadAll(r.Body)
-		assert.NilError(t, err)
-		assert.Assert(t, strings.Contains(string(b), `"word":`))
 
 		endpoint = fmt.Sprintf("http://%s:%d", fqdn, containerInspect.Ports[0].HostPort)
-		r, err = HTTPGetWithRetry(endpoint+"/words/noun", 3)
+		HTTPGetWithRetry(t, endpoint+"/words/noun", http.StatusOK, 2*time.Second, 20*time.Second)
-		assert.NilError(t, err)
-		assert.Equal(t, r.StatusCode, http.StatusOK)
 	})
 
 	t.Run("compose ps", func(t *testing.T) {
@@ -577,14 +566,7 @@ func TestComposeUpUpdate(t *testing.T) {
 				assert.Equal(t, containerInspect.Ports[0].HostPort, uint32(8080))
 				assert.Equal(t, containerInspect.Ports[0].ContainerPort, uint32(8080))
 			}
-			checkUp := func(t poll.LogT) poll.Result {
+			HTTPGetWithRetry(t, endpoint+route, http.StatusOK, 1*time.Second, 60*time.Second)
-				r, _ := http.Get(endpoint + route)
-				if r != nil && r.StatusCode == http.StatusOK {
-					return poll.Success()
-				}
-				return poll.Continue("Waiting for container to serve request")
-			}
-			poll.WaitOn(t, checkUp, poll.WithDelay(1*time.Second), poll.WithTimeout(60*time.Second))
 
 			res = c.RunDockerCmd("ps")
 			p := containerInspect.Ports[0]

tests/framework/e2e.go

@@ -34,6 +34,7 @@ import (
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 	"gotest.tools/v3/icmd"
+	"gotest.tools/v3/poll"
 
 	"github.com/docker/compose-cli/api/containers"
 )
@@ -201,19 +202,28 @@ func ParseContainerInspect(stdout string) (*containers.Container, error) {
 }
 
 // HTTPGetWithRetry performs an HTTP GET on an `endpoint`.
-// In the case of an error it retries the same request after a 5 second sleep,
+// In the case of an error or the response status is not the expeted one, it retries the same request,
-// returning the error if count of `tries` is reached
+// returning the response body as a string (empty if we could not reach it)
-func HTTPGetWithRetry(endpoint string, tries int) (*http.Response, error) {
+func HTTPGetWithRetry(t *testing.T, endpoint string, expectedStatus int, retryDelay time.Duration, timeout time.Duration) string {
 	var (
 		r   *http.Response
 		err error
 	)
-	for t := 0; t < tries; t++ {
+	checkUp := func(t poll.LogT) poll.Result {
 		r, err = http.Get(endpoint)
-		if err == nil || t == tries-1 {
+		if err != nil {
-			break
+			return poll.Continue("reaching %q: Error %s", endpoint, err.Error())
 		}
-		time.Sleep(5 * time.Second)
+		if r.StatusCode == expectedStatus {
+			return poll.Success()
+		}
+		return poll.Continue("reaching %q: %d != %d", endpoint, r.StatusCode, expectedStatus)
 	}
-	return r, err
+	poll.WaitOn(t, checkUp, poll.WithDelay(retryDelay), poll.WithTimeout(timeout))
+	if r != nil {
+		b, err := ioutil.ReadAll(r.Body)
+		assert.NilError(t, err)
+		return string(b)
+	}
+	return ""
 }