Merge pull request #7589 from hashicorp/no_fslock_solaris

fix flock build errors on solaris by shadowing the pkg

common/filelock/filelock.go

@@ -0,0 +1,11 @@
+// +build !solaris
+
+package filelock
+
+import "github.com/gofrs/flock"
+
+type Flock = flock.Flock
+
+func New(path string) *Flock {
+	return flock.New(path)
+}

common/filelock/filelock_solaris.go

@@ -0,0 +1,11 @@
+// build solaris
+
+package filelock
+
+// Flock is a noop on solaris for now.
+// TODO(azr): PR github.com/gofrs/flock for this.
+type Flock = Noop
+
+func New(string) *Flock {
+	return &Flock{}
+}

common/filelock/noop.go

@@ -0,0 +1,8 @@
+package filelock
+
+// this lock does nothing
+type Noop struct{}
+
+func (_ *Noop) Lock() (bool, error)    { return true, nil }
+func (_ *Noop) TryLock() (bool, error) { return true, nil }
+func (_ *Noop) Unlock() error          { return nil }

common/net/configure_port.go

@@ -7,9 +7,10 @@ import (
 	"math/rand"
 	"net"
 	"strconv"
+	"time"
 
-	"github.com/gofrs/flock"
-
+	"github.com/hashicorp/packer/common/filelock"
+	"github.com/hashicorp/packer/common/retry"
 	"github.com/hashicorp/packer/packer"
 )
 
@@ -26,7 +27,7 @@ type Listener struct {
 	net.Listener
 	Port    int
 	Address string
-	lock    *flock.Flock
+	lock    *filelock.Flock
 }
 
 func (l *Listener) Close() error {
@@ -40,7 +41,7 @@ func (l *Listener) Close() error {
 // ListenRangeConfig contains options for listening to a free address [Min,Max)
 // range. ListenRangeConfig wraps a net.ListenConfig.
 type ListenRangeConfig struct {
-	// tcp", "udp"
+	// like "tcp" or "udp". defaults to "tcp".
 	Network  string
 	Addr     string
 	Min, Max int
@@ -51,49 +52,72 @@ type ListenRangeConfig struct {
 // until ctx is cancelled.
 // Listen uses net.ListenConfig.Listen internally.
 func (lc ListenRangeConfig) Listen(ctx context.Context) (*Listener, error) {
+	if lc.Network == "" {
+		lc.Network = "tcp"
+	}
 	portRange := lc.Max - lc.Min
-	for {
-		if err := ctx.Err(); err != nil {
-			return nil, err
-		}
 
+	var listener *Listener
+
+	err := retry.Config{
+		RetryDelay: func() time.Duration { return 1 * time.Millisecond },
+	}.Run(ctx, func(context.Context) error {
 		port := lc.Min
 		if portRange > 0 {
 			port += rand.Intn(portRange)
 		}
 
-		log.Printf("Trying port: %d", port)
-
 		lockFilePath, err := packer.CachePath("port", strconv.Itoa(port))
 		if err != nil {
-			return nil, err
+			return err
 		}
 
-		lock := flock.New(lockFilePath)
+		lock := filelock.New(lockFilePath)
 		locked, err := lock.TryLock()
 		if err != nil {
-			return nil, err
+			return err
 		}
 		if !locked {
-			continue // this port seems to be locked by another packer goroutine
+			return ErrPortFileLocked(port)
 		}
 
 		l, err := lc.ListenConfig.Listen(ctx, lc.Network, fmt.Sprintf("%s:%d", lc.Addr, port))
 		if err != nil {
 			if err := lock.Unlock(); err != nil {
-				log.Printf("Could not unlock file lock for port %d: %v", port, err)
+				log.Fatalf("Could not unlock file lock for port %d: %v", port, err)
+			}
+			return &ErrPortBusy{
+				Port: port,
+				Err:  err,
 			}
-
-			continue // this port is most likely already open
 		}
 
 		log.Printf("Found available port: %d on IP: %s", port, lc.Addr)
-		return &Listener{
+		listener = &Listener{
 			Address:  lc.Addr,
 			Port:     port,
 			Listener: l,
 			lock:     lock,
-		}, err
+		}
+		return nil
+	})
+	return listener, err
+}
+
+type ErrPortFileLocked int
+
+func (port ErrPortFileLocked) Error() string {
+	return fmt.Sprintf("Port %d is file locked", port)
+}
+
+type ErrPortBusy struct {
+	Port int
+	Err  error
+}
 
+func (err *ErrPortBusy) Error() string {
+	if err == nil {
+		return "<nil>"
 	}
+	return fmt.Sprintf("port %d cannot be opened: %v", err.Port, err.Err)
 }

common/net/configure_port_test.go

@@ -0,0 +1,167 @@
+package net
+
+import (
+	"context"
+	"net"
+	"testing"
+	"time"
+)
+
+func TestListenRangeConfig_Listen(t *testing.T) {
+
+	topCtx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	var err error
+	var lockedListener *Listener
+	{ // open a random port in range
+		ctx, cancel := context.WithTimeout(topCtx, time.Second*5)
+
+		lockedListener, err = ListenRangeConfig{
+			Min:  800,
+			Max:  10000,
+			Addr: "localhost",
+		}.Listen(ctx)
+		if err != nil {
+			t.Fatalf("could not open first port")
+		}
+		cancel()
+		defer lockedListener.Close() // in case
+	}
+
+	{ // open a second random port in range
+		ctx, cancel := context.WithTimeout(topCtx, time.Second*5)
+
+		listener, err := ListenRangeConfig{
+			Min:  800,
+			Max:  10000,
+			Addr: "localhost",
+		}.Listen(ctx)
+		if err != nil {
+			t.Fatalf("could not open first port")
+		}
+		cancel()
+		if err := listener.Close(); err != nil { // in case
+			t.Fatal("failed to close second random port")
+		}
+	}
+
+	{ // test that opened port cannot be openned using min/max
+		ctx, cancel := context.WithTimeout(topCtx, 250*time.Millisecond)
+
+		l, err := ListenRangeConfig{
+			Min: lockedListener.Port,
+			Max: lockedListener.Port,
+		}.Listen(ctx)
+		if err == nil {
+			l.Close()
+			t.Fatal("port should be taken, this should fail")
+		}
+		if p := int(err.(ErrPortFileLocked)); p != lockedListener.Port {
+			t.Fatalf("wrong fileport: %d", p)
+		}
+		cancel()
+	}
+
+	{ // test that opened port cannot be openned using min only
+		ctx, cancel := context.WithTimeout(topCtx, 250*time.Millisecond)
+
+		l, err := ListenRangeConfig{
+			Min: lockedListener.Port,
+		}.Listen(ctx)
+		if err == nil {
+			l.Close()
+			t.Fatalf("port should be taken, this should timeout.")
+		}
+		if p := int(err.(ErrPortFileLocked)); p != lockedListener.Port {
+			t.Fatalf("wrong fileport: %d", p)
+		}
+		cancel()
+	}
+
+	err = lockedListener.Close() // close port and release lock file
+	if err != nil {
+		t.Fatalf("could not release lockfile or port: %v", err)
+	}
+
+	{ // test that closed port can be reopenned.
+		ctx, cancel := context.WithTimeout(topCtx, 250*time.Millisecond)
+
+		lockedListener, err = ListenRangeConfig{
+			Min: lockedListener.Port,
+		}.Listen(ctx)
+		if err != nil {
+			t.Fatalf("port should have been freed: %v", err)
+		}
+		cancel()
+		defer lockedListener.Close() // in case
+	}
+
+	err = lockedListener.Listener.Close() // close listener, keep lockfile only
+	if err != nil {
+		t.Fatalf("could not release lockfile or port: %v", err)
+	}
+
+	{ // test that file locked port cannot be opened
+		ctx, cancel := context.WithTimeout(topCtx, 250*time.Millisecond)
+
+		l, err := ListenRangeConfig{
+			Min: lockedListener.Port,
+		}.Listen(ctx)
+		if err == nil {
+			l.Close()
+			t.Fatalf("port should be file locked, this should timeout")
+		}
+		cancel()
+	}
+
+	var netListener net.Listener
+	{ // test that the closed network port can be reopened using net.Listen
+		netListener, err = net.Listen("tcp", lockedListener.Addr().String())
+		if err != nil {
+			t.Fatalf("listen on freed port failed: %v", err)
+		}
+
+	}
+
+	if err := lockedListener.lock.Unlock(); err != nil {
+		t.Fatalf("error closing port: %v", err)
+	}
+
+	{ // test that busy port cannot be opened
+		ctx, cancel := context.WithTimeout(topCtx, 250*time.Millisecond)
+
+		l, err := ListenRangeConfig{
+			Min: lockedListener.Port,
+		}.Listen(ctx)
+		if err == nil {
+			l.Close()
+			t.Fatalf("port should be file locked, this should timeout")
+		}
+		busyErr := err.(*ErrPortBusy)
+		if busyErr.Port != lockedListener.Port {
+			t.Fatal("wrong port")
+		}
+		// error types vary depending on OS and it might get quickly
+		// complicated to test for the error we want.
+		cancel()
+	}
+
+	if err := netListener.Close(); err != nil { // free port
+		t.Fatalf("close failed: %v", err)
+	}
+
+	{ // test that freed port can be opened
+		ctx, cancel := context.WithTimeout(topCtx, 250*time.Minute)
+
+		lockedListener, err = ListenRangeConfig{
+			Min: lockedListener.Port,
+		}.Listen(ctx)
+		if err != nil {
+			t.Fatalf("port should have been freed: %v", err)
+		}
+		cancel()
+		defer lockedListener.Close() // in case
+	}
+
+}

common/step_download.go

@@ -10,9 +10,9 @@ import (
 	"runtime"
 	"strings"
 
-	"github.com/gofrs/flock"
 	getter "github.com/hashicorp/go-getter"
 	urlhelper "github.com/hashicorp/go-getter/helper/url"
+	"github.com/hashicorp/packer/common/filelock"
 	"github.com/hashicorp/packer/helper/multistep"
 	"github.com/hashicorp/packer/packer"
 )
@@ -146,7 +146,7 @@ func (s *StepDownload) download(ctx context.Context, ui packer.Ui, source string
 	lockFile := targetPath + ".lock"
 
 	log.Printf("Acquiring lock for: %s (%s)", u.String(), lockFile)
-	lock := flock.New(lockFile)
+	lock := filelock.New(lockFile)
 	lock.Lock()
 	defer lock.Unlock()