Start all listeners when a given host resolves to multiple IPs. (#3145)

Default golang net.Listen only listens on the first IP when host resolves to multiple IPs. This change addresses a problem for example your ``/etc/hosts`` has entries as following ``` 127.0.1.1 minio1 192.168.1.10 minio1 ``` Trying to start minio as ``` minio server --address "minio1:9001" ~/Photos ``` Causes the minio server to be bound only to "127.0.1.1" which is an incorrect behavior since we are generally interested in `192.168.1.10` as well. This patch addresses this issue if the hostname is resolvable and gives back list of addresses associated with that hostname we just bind on all of them as it is the expected behavior.
8 years ago · 9bb799462e
parent 8bffa78f7f
commit 9bb799462e
3 changed files with 175 additions and 54 deletions
--- a/cmd/server-main.go
+++ b/cmd/server-main.go
@ -96,12 +96,6 @@ EXAMPLES:
      $ minio {{.Name}} http://192.168.1.11/mnt/export/ http://192.168.1.12/mnt/export/ \
          http://192.168.1.13/mnt/export/ http://192.168.1.14/mnt/export/

-  7. Start minio server on a 4 node distributed setup. Type the following command on all the 4 nodes exactly.
-      $ minio {{.Name}} http://minio:miniostorage@192.168.1.11/mnt/export/ \
-          http://minio:miniostorage@192.168.1.12/mnt/export/ \
-          http://minio:miniostorage@192.168.1.13/mnt/export/ \
-          http://minio:miniostorage@192.168.1.14/mnt/export/
-
 `,
 }

--- a/cmd/server-mux.go
+++ b/cmd/server-mux.go
@ -167,7 +167,7 @@ func (l *ListenerMux) Close() error {
 // ServerMux - the main mux server
 type ServerMux struct {
 	http.Server
-	listener        *ListenerMux
+	listeners       []*ListenerMux
 	WaitGroup       *sync.WaitGroup
 	GracefulTimeout time.Duration
 	mu              sync.Mutex // guards closed, conns, and listener
@ -199,6 +199,51 @@ func NewServerMux(addr string, handler http.Handler) *ServerMux {
 	return m
 }

+// Initialize listeners on all ports.
+func initListeners(serverAddr string, tls *tls.Config) ([]*ListenerMux, error) {
+	host, port, err := net.SplitHostPort(serverAddr)
+	if err != nil {
+		return nil, err
+	}
+	var listeners []*ListenerMux
+	if host == "" {
+		var listener net.Listener
+		listener, err = net.Listen("tcp", serverAddr)
+		if err != nil {
+			return nil, err
+		}
+		listeners = append(listeners, &ListenerMux{
+			Listener: listener,
+			config:   tls,
+		})
+		return listeners, nil
+	}
+	var addrs []string
+	if net.ParseIP(host) != nil {
+		addrs = append(addrs, host)
+	} else {
+		addrs, err = net.LookupHost(host)
+		if err != nil {
+			return nil, err
+		}
+		if len(addrs) == 0 {
+			return nil, errUnexpected
+		}
+	}
+	for _, addr := range addrs {
+		var listener net.Listener
+		listener, err = net.Listen("tcp", net.JoinHostPort(addr, port))
+		if err != nil {
+			return nil, err
+		}
+		listeners = append(listeners, &ListenerMux{
+			Listener: listener,
+			config:   tls,
+		})
+	}
+	return listeners, nil
+}
+
 // ListenAndServeTLS - similar to the http.Server version. However, it has the
 // ability to redirect http requests to the correct HTTPS url if the client
 // mistakenly initiates a http connection over the https port
@ -215,18 +260,21 @@ func (m *ServerMux) ListenAndServeTLS(certFile, keyFile string) (err error) {

 	go m.handleServiceSignals()

-	listener, err := net.Listen("tcp", m.Server.Addr)
+	listeners, err := initListeners(m.Server.Addr, config)
 	if err != nil {
 		return err
 	}

-	listenerMux := &ListenerMux{Listener: listener, config: config}
-
 	m.mu.Lock()
-	m.listener = listenerMux
+	m.listeners = listeners
 	m.mu.Unlock()

-	err = http.Serve(listenerMux,
+	var wg = &sync.WaitGroup{}
+	for _, listener := range listeners {
+		wg.Add(1)
+		go func(listener *ListenerMux) {
+			defer wg.Done()
+			serr := http.Serve(listener,
 				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 					// We reach here when ListenerMux.ConnMux is not wrapped with tls.Server
 					if r.TLS == nil {
@ -246,51 +294,58 @@ func (m *ServerMux) ListenAndServeTLS(certFile, keyFile string) (err error) {
 					}
 				}),
 			)
-	if nerr, ok := err.(*net.OpError); ok {
-		if nerr.Op == "accept" && nerr.Net == "tcp" {
-			return nil
+			errorIf(serr, "Unable to serve incoming requests.")
+		}(listener)
 	}
-	}
-	return err
+	// Waits for all http.Serve's to return.
+	wg.Wait()
+	return nil
 }

 // ListenAndServe - Same as the http.Server version
 func (m *ServerMux) ListenAndServe() error {
 	go m.handleServiceSignals()

-	listener, err := net.Listen("tcp", m.Server.Addr)
+	listeners, err := initListeners(m.Server.Addr, &tls.Config{})
 	if err != nil {
 		return err
 	}

-	listenerMux := &ListenerMux{Listener: listener, config: &tls.Config{}}
-
 	m.mu.Lock()
-	m.listener = listenerMux
+	m.listeners = listeners
 	m.mu.Unlock()

-	err = m.Server.Serve(listenerMux)
-	if nerr, ok := err.(*net.OpError); ok {
-		if nerr.Op == "accept" && nerr.Net == "tcp" {
+	var wg = &sync.WaitGroup{}
+	for _, listener := range listeners {
+		wg.Add(1)
+		go func(listener *ListenerMux) {
+			defer wg.Done()
+			serr := m.Server.Serve(listener)
+			errorIf(serr, "Unable to serve incoming requests.")
+		}(listener)
+	}
+	// Wait for all the http.Serve to finish.
+	wg.Wait()
 	return nil
-		}
-	}
-	return err
 }

 // Close initiates the graceful shutdown
 func (m *ServerMux) Close() error {
 	m.mu.Lock()
 	if m.closed {
+		m.mu.Unlock()
 		return errors.New("Server has been closed")
 	}
 	// Closed completely.
 	m.closed = true

-	// Close the listener.
-	if err := m.listener.Close(); err != nil {
+	// Close the listeners.
+	for _, listener := range m.listeners {
+		if err := listener.Close(); err != nil {
+			m.mu.Unlock()
 			return err
 		}
+	}

 	m.SetKeepAlivesEnabled(false)
 	// Force close any idle and new connections. Waiting for other connections
--- a/cmd/server-mux_test.go
+++ b/cmd/server-mux_test.go
@ -31,11 +31,69 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"runtime"
 	"sync"
 	"testing"
 	"time"
 )

+// Tests initalizing listeners.
+func TestInitListeners(t *testing.T) {
+	testCases := []struct {
+		serverAddr string
+		shouldPass bool
+	}{
+		// Test 1 with ip and port.
+		{
+			serverAddr: "127.0.0.1:" + getFreePort(),
+			shouldPass: true,
+		},
+		// Test 2 only port.
+		{
+			serverAddr: ":" + getFreePort(),
+			shouldPass: true,
+		},
+		// Test 3 with no port error.
+		{
+			serverAddr: "127.0.0.1",
+			shouldPass: false,
+		},
+		// Test 4 with 'foobar' host not resolvable.
+		{
+			serverAddr: "foobar:9000",
+			shouldPass: false,
+		},
+	}
+	for i, testCase := range testCases {
+		listeners, err := initListeners(testCase.serverAddr, &tls.Config{})
+		if testCase.shouldPass {
+			if err != nil {
+				t.Fatalf("Test %d: Unable to initialize listeners %s", i+1, err)
+			}
+			for _, listener := range listeners {
+				if err = listener.Close(); err != nil {
+					t.Fatalf("Test %d: Unable to close listeners %s", i+1, err)
+				}
+			}
+		}
+		if err == nil && !testCase.shouldPass {
+			t.Fatalf("Test %d: Should fail but is successful", i+1)
+		}
+	}
+	// Windows doesn't have 'localhost' hostname.
+	if runtime.GOOS != "windows" {
+		listeners, err := initListeners("localhost:"+getFreePort(), &tls.Config{})
+		if err != nil {
+			t.Fatalf("Test 3: Unable to initialize listeners %s", err)
+		}
+		for _, listener := range listeners {
+			if err = listener.Close(); err != nil {
+				t.Fatalf("Test 3: Unable to close listeners %s", err)
+			}
+		}
+	}
+}
+
 func TestClose(t *testing.T) {
 	// Create ServerMux
 	m := NewServerMux("", nil)
@ -43,6 +101,11 @@ func TestClose(t *testing.T) {
 	if err := m.Close(); err != nil {
 		t.Error("Server errored while trying to Close", err)
 	}
+
+	// Closing again should return an error.
+	if err := m.Close(); err.Error() != "Server has been closed" {
+		t.Error("Unexepcted error expected \"Server has been closed\", got", err)
+	}
 }

 func TestServerMux(t *testing.T) {
@ -63,7 +126,7 @@ func TestServerMux(t *testing.T) {
 		Listener: ts.Listener,
 		config:   &tls.Config{},
 	}
-	m.listener = lm
+	m.listeners = []*ListenerMux{lm}

 	client := http.Client{}
 	res, err := client.Get(ts.URL)
@ -116,7 +179,7 @@ func TestServerCloseBlocking(t *testing.T) {
 		Listener: ts.Listener,
 		config:   &tls.Config{},
 	}
-	m.listener = lm
+	m.listeners = []*ListenerMux{lm}

 	dial := func() net.Conn {
 		c, cerr := net.Dial("tcp", ts.Listener.Addr().String())
@ -245,14 +308,23 @@ func TestListenAndServeTLS(t *testing.T) {
 			Timeout:   time.Millisecond * 10,
 			Transport: tr,
 		}
-		ok := false
-		for !ok {
+		okTLS := false
+		for !okTLS {
 			res, _ := client.Get("https://" + addr)
 			if res != nil && res.StatusCode == http.StatusOK {
-				ok = true
+				okTLS = true
 			}
 		}

+		okNoTLS := false
+		for !okNoTLS {
+			res, _ := client.Get("http://" + addr)
+			// Without TLS we expect a re-direction from http to https
+			// And also the request is not rejected.
+			if res != nil && res.StatusCode == http.StatusOK && res.Request.URL.Scheme == "https" {
+				okNoTLS = true
+			}
+		}
 		wg.Done()
 	}()