go: [go1.8beta2] Investigating bad bad break in docker test suite

What version of Go are you using (go version)?

1.8beta2

What operating system and processor architecture are you using (go env)?

linux/amd64

What did you do?

Testing 1.8beta on docker’s test suite

What did you expect to see?

Tests actually run

What did you see instead?

All of our integration tests are erroring.

This only happens with beta2. Beta1 had a couple of failures (1 which was fixed). Beta2 has broken our entire integration suite. We are still investigating the potential cause, but are turning up short at the moment as there’s not much to go on.

I have not reproduced any issue with manual testing, but using the test suite any and every test errors during cleanup with a response from the docker daemon of page not found

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Comments: 22 (13 by maintainers)

Commits related to this issue

Most upvoted comments

For the record, 7db996ee77b20b68ee583c65d59be1d81cef9090 (CL 29852) changed shouldRedirectPost from only redirecting StatusFound, StatusSeeOther (302 & 303) to also including StatusMovedPermanently (301) in the new func redirectBehavior, to fix #9348 and other bugs.

https://tools.ietf.org/html/rfc7231#section-6.4.2 says:

6.4.2.  301 Moved Permanently

   The 301 (Moved Permanently) status code indicates that the target
   resource has been assigned a new permanent URI and any future
   references to this resource ought to use one of the enclosed URIs.
   Clients with link-editing capabilities ought to automatically re-link
   references to the effective request URI to one or more of the new
   references sent by the server, where possible.

   The server SHOULD generate a Location header field in the response
   containing a preferred URI reference for the new permanent URI.  The
   user agent MAY use the Location field value for automatic
   redirection.  The server's response payload usually contains a short
   hypertext note with a hyperlink to the new URI(s).

      Note: For historical reasons, a user agent MAY change the request
      method from POST to GET for the subsequent request.  If this
      behavior is undesired, the 307 (Temporary Redirect) status code
      can be used instead.

And from the discussion in #9348, it seems the new behavior is the desired and correct behavior.

This program changed from Go 1.7 to Go 1.8 and is (I believe) the source of docker’s problems.

package main

import (
	"bytes"
	"fmt"
	"io"
	"log"
	"net/http"
	"net/http/httptest"
	"os"
)

func main() {
	http.DefaultClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
		fmt.Printf("redirect %s", req.URL)
		for _, v := range via {
			fmt.Printf(" via %s", v.URL)
		}
		fmt.Printf("\n")
		return nil
	}
	http.HandleFunc("/", show)
	srv := httptest.NewServer(nil)
	r, err := http.Post(srv.URL+"/x//z", "text/plain", bytes.NewReader(nil))
	if err != nil {
		log.Fatal(err)
	}
	if r.StatusCode != 200 {
		log.Fatal("status: " + r.Status)
	}
	io.Copy(os.Stdout, r.Body)
}

func show(w http.ResponseWriter, req *http.Request) {
	fmt.Fprintf(w, "%s %s\n", req.Method, req.URL)
}

Before the POST got a redirect and returned that error. Now it follows the redirect, translates to GET, and gets 404. I guess Docker was swallowing the 301 but is now choking on the 404.

r$ go1.7 run /tmp/x.go
2016/12/20 10:29:59 status: 301 Moved Permanently
exit status 1
r$ go run /tmp/x.go
redirect http://127.0.0.1:52118/x/z via http://127.0.0.1:52118/x//z
GET /x/z
r$