server: Fix a regression in printing startup banner. (#4100)

Octect based sorting was lost in the previous commit

de204a0a52

This PR fixes a regression - fixes #4099

cmd/net.go

@@ -77,18 +77,67 @@ func getHostIP4(host string) (ipList set.StringSet, err error) {
 	return ipList, err
 }
 
+// byLastOctetValue implements sort.Interface used in sorting a list
+// of ip address by their last octet value in descending order.
+type byLastOctetValue []net.IP
+
+func (n byLastOctetValue) Len() int      { return len(n) }
+func (n byLastOctetValue) Swap(i, j int) { n[i], n[j] = n[j], n[i] }
+func (n byLastOctetValue) Less(i, j int) bool {
+	// This case is needed when all ips in the list
+	// have same last octets, Following just ensures that
+	// 127.0.0.1 is moved to the end of the list.
+	if n[i].String() == "127.0.0.1" {
+		return false
+	}
+	if n[j].String() == "127.0.0.1" {
+		return true
+	}
+	return []byte(n[i].To4())[3] > []byte(n[j].To4())[3]
+}
+
+// sortIPs - sort ips based on higher octects.
+// The logic to sort by last octet is implemented to
+// prefer CIDRs with higher octects, this in-turn skips the
+// localhost/loopback address to be not preferred as the
+// first ip on the list. Subsequently this list helps us print
+// a user friendly message with appropriate values.
+func sortIPs(ipList []string) []string {
+	if len(ipList) == 1 {
+		return ipList
+	}
+
+	var ipV4s []net.IP
+	var nonIPs []string
+	for _, ip := range ipList {
+		nip := net.ParseIP(ip)
+		if nip != nil {
+			ipV4s = append(ipV4s, nip)
+		} else {
+			nonIPs = append(nonIPs, ip)
+		}
+	}
+
+	sort.Sort(byLastOctetValue(ipV4s))
+
+	var ips []string
+	for _, ip := range ipV4s {
+		ips = append(ips, ip.String())
+	}
+
+	return append(nonIPs, ips...)
+}
+
 func getAPIEndpoints(serverAddr string) (apiEndpoints []string) {
 	host, port := mustSplitHostPort(serverAddr)
 
 	var ipList []string
 	if host == "" {
-		ipList = localIP4.ToSlice()
+		ipList = sortIPs(localIP4.ToSlice())
 	} else {
 		ipList = []string{host}
 	}
 
-	sort.Strings(ipList)
-
 	scheme := httpScheme
 	if globalIsSSL {
 		scheme = httpsScheme

cmd/net_test.go

@@ -19,6 +19,7 @@ package cmd
 import (
 	"fmt"
 	"net"
+	"reflect"
 	"runtime"
 	"testing"
 
@@ -52,6 +53,64 @@ func TestMustSplitHostPort(t *testing.T) {
 	}
 }
 
+func TestSortIPs(t *testing.T) {
+	testCases := []struct {
+		ipList       []string
+		sortedIPList []string
+	}{
+		// Default case of two ips one with higher octet moves
+		// to the beginning of the list.
+		{
+			ipList:       []string{"127.0.0.1", "10.0.0.13"},
+			sortedIPList: []string{"10.0.0.13", "127.0.0.1"},
+		},
+		// With multiple types of octet, chooses a higher octet.
+		{
+			ipList:       []string{"127.0.0.1", "172.0.21.1", "192.168.1.106"},
+			sortedIPList: []string{"192.168.1.106", "172.0.21.1", "127.0.0.1"},
+		},
+		// With different ip along with localhost.
+		{
+			ipList:       []string{"127.0.0.1", "192.168.1.106"},
+			sortedIPList: []string{"192.168.1.106", "127.0.0.1"},
+		},
+		// With a list of only one element nothing to sort.
+		{
+			ipList:       []string{"hostname"},
+			sortedIPList: []string{"hostname"},
+		},
+		// With a list of only one element nothing to sort.
+		{
+			ipList:       []string{"127.0.0.1"},
+			sortedIPList: []string{"127.0.0.1"},
+		},
+		// Non parsable ip is assumed to be hostame and gets preserved
+		// as the left most elements, regardless of IP based sorting.
+		{
+			ipList:       []string{"hostname", "127.0.0.1", "192.168.1.106"},
+			sortedIPList: []string{"hostname", "192.168.1.106", "127.0.0.1"},
+		},
+		// Non parsable ip is assumed to be hostname, with a mixed input of ip and hostname.
+		// gets preserved and moved into left most elements, regardless of
+		// IP based sorting.
+		{
+			ipList:       []string{"hostname1", "10.0.0.13", "hostname2", "127.0.0.1", "192.168.1.106"},
+			sortedIPList: []string{"hostname1", "hostname2", "192.168.1.106", "10.0.0.13", "127.0.0.1"},
+		},
+		// With same higher octets, preferentially move the localhost.
+		{
+			ipList:       []string{"127.0.0.1", "10.0.0.1", "192.168.0.1"},
+			sortedIPList: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1"},
+		},
+	}
+	for i, testCase := range testCases {
+		gotIPList := sortIPs(testCase.ipList)
+		if !reflect.DeepEqual(testCase.sortedIPList, gotIPList) {
+			t.Errorf("Test %d: Expected %s, got %s", i+1, testCase.sortedIPList, gotIPList)
+		}
+	}
+}
+
 func TestMustGetLocalIP4(t *testing.T) {
 	testCases := []struct {
 		expectedIPList set.StringSet

cmd/server-startup-msg.go

@@ -42,6 +42,7 @@ func getFormatStr(strLen int, padding int) string {
 
 // Prints the formatted startup message.
 func printStartupMessage(apiEndPoints []string) {
+
 	// Prints credential, region and browser access.
 	printServerCommonMsg(apiEndPoints)