diff --git a/common/filelock/filelock.go b/common/filelock/filelock.go new file mode 100644 index 000000000..c83816ee6 --- /dev/null +++ b/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) +} diff --git a/common/filelock/filelock_solaris.go b/common/filelock/filelock_solaris.go new file mode 100644 index 000000000..06685254c --- /dev/null +++ b/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{} +} diff --git a/common/filelock/noop.go b/common/filelock/noop.go new file mode 100644 index 000000000..ebf8f1967 --- /dev/null +++ b/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 } diff --git a/common/net/configure_port.go b/common/net/configure_port.go index dbce7ee8f..3de38be9c 100644 --- a/common/net/configure_port.go +++ b/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 "" + } + return fmt.Sprintf("port %d cannot be opened: %v", err.Port, err.Err) } diff --git a/common/net/configure_port_test.go b/common/net/configure_port_test.go new file mode 100644 index 000000000..e39173e7c --- /dev/null +++ b/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 + } + +} diff --git a/common/step_download.go b/common/step_download.go index e562b619c..af7708e6e 100644 --- a/common/step_download.go +++ b/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()