heal: Print heal command appropriately without export path. (#3208)

Fixes #3204

cmd/prepare-storage-msg.go

@@ -18,6 +18,7 @@ package cmd
 
 import (
 	"fmt"
+	"net"
 	"net/url"
 	"sync"
 
@@ -49,21 +50,55 @@ func printOnceFn() printOnceFunc {
 }
 
 // Prints custom message when healing is required for XL and Distributed XL backend.
-func printHealMsg(firstEndpoint string, storageDisks []StorageAPI, fn printOnceFunc) {
+func printHealMsg(endpoints []*url.URL, storageDisks []StorageAPI, fn printOnceFunc) {
-	msg := getHealMsg(firstEndpoint, storageDisks)
+	msg := getHealMsg(endpoints, storageDisks)
 	fn(msg)
 }
 
+// Heal endpoint constructs the final endpoint URL for control heal command.
+// Disk heal endpoint needs to be just a URL and no special paths.
+// This function constructs the right endpoint under various conditions
+// for single node XL, distributed XL and when minio server is bound
+// to a specific ip:port.
+func getHealEndpoint(tls bool, firstEndpoint *url.URL) (cEndpoint *url.URL) {
+	scheme := "http"
+	if tls {
+		scheme = "https"
+	}
+	cEndpoint = &url.URL{
+		Scheme: scheme,
+	}
+	// Bind to `--address host:port` was specified.
+	if globalMinioHost != "" {
+		cEndpoint.Host = net.JoinHostPort(globalMinioHost, globalMinioPort)
+		return cEndpoint
+	}
+	// For distributed XL setup.
+	if firstEndpoint.Host != "" {
+		cEndpoint.Host = firstEndpoint.Host
+		return cEndpoint
+	}
+	// For single node XL setup, we need to find the endpoint.
+	cEndpoint.Host = globalMinioAddr
+	// Fetch all the listening ips. For single node XL we
+	// just use the first host.
+	hosts, _, err := getListenIPs(cEndpoint.Host)
+	if err == nil {
+		cEndpoint.Host = net.JoinHostPort(hosts[0], globalMinioPort)
+	}
+	return cEndpoint
+}
+
 // Constructs a formatted heal message, when cluster is found to be in state where it requires healing.
 // healing is optional, server continues to initialize object layer after printing this message.
 // it is upto the end user to perform a heal if needed.
-func getHealMsg(firstEndpoint string, storageDisks []StorageAPI) string {
+func getHealMsg(endpoints []*url.URL, storageDisks []StorageAPI) string {
 	msg := fmt.Sprintln("\nData volume requires HEALING. Please run the following command:")
 	msg += "MINIO_ACCESS_KEY=%s "
 	msg += "MINIO_SECRET_KEY=%s "
 	msg += "minio control heal %s"
 	creds := serverConfig.GetCredential()
-	msg = fmt.Sprintf(msg, creds.AccessKeyID, creds.SecretAccessKey, firstEndpoint)
+	msg = fmt.Sprintf(msg, creds.AccessKeyID, creds.SecretAccessKey, getHealEndpoint(isSSL(), endpoints[0]))
 	disksInfo, _, _ := getDisksInfo(storageDisks)
 	for i, info := range disksInfo {
 		if storageDisks[i] == nil {
@@ -72,7 +107,7 @@ func getHealMsg(firstEndpoint string, storageDisks []StorageAPI) string {
 		msg += fmt.Sprintf(
 			"\n[%s] %s - %s %s",
 			int2Str(i+1, len(storageDisks)),
-			storageDisks[i],
+			endpoints[i],
 			humanize.IBytes(uint64(info.Total)),
 			func() string {
 				if info.Total > 0 {

cmd/prepare-storage-msg_test.go

@@ -18,9 +18,59 @@ package cmd
 
 import (
 	"net/url"
+	"reflect"
 	"testing"
 )
 
+// Tests and validates the output for heal endpoint.
+func TestGetHealEndpoint(t *testing.T) {
+	// Test for a SSL scheme.
+	tls := true
+	hURL := getHealEndpoint(tls, &url.URL{
+		Scheme: "http",
+		Host:   "localhost:9000",
+	})
+	sHURL := &url.URL{
+		Scheme: "https",
+		Host:   "localhost:9000",
+	}
+	if !reflect.DeepEqual(hURL, sHURL) {
+		t.Fatalf("Expected %#v, but got %#v", sHURL, hURL)
+	}
+
+	// Test a non-TLS scheme.
+	tls = false
+	hURL = getHealEndpoint(tls, &url.URL{
+		Scheme: "https",
+		Host:   "localhost:9000",
+	})
+	sHURL = &url.URL{
+		Scheme: "http",
+		Host:   "localhost:9000",
+	}
+	if !reflect.DeepEqual(hURL, sHURL) {
+		t.Fatalf("Expected %#v, but got %#v", sHURL, hURL)
+	}
+
+	// FIXME(GLOBAL): purposefully Host is left empty because
+	// we need to bring in safe handling on global values
+	// add a proper test case here once that happens.
+	/*
+		tls = false
+		hURL = getHealEndpoint(tls, &url.URL{
+			Path: "/export",
+		})
+		sHURL = &url.URL{
+			Scheme: "http",
+			Host:   "",
+		}
+		globalMinioAddr = ""
+		if !reflect.DeepEqual(hURL, sHURL) {
+			t.Fatalf("Expected %#v, but got %#v", sHURL, hURL)
+		}
+	*/
+}
+
 // Tests heal message to be correct and properly formatted.
 func TestHealMsg(t *testing.T) {
 	rootPath, err := newTestConfig("us-east-1")
@@ -69,7 +119,7 @@ func TestHealMsg(t *testing.T) {
 		},
 	}
 	for i, testCase := range testCases {
-		msg := getHealMsg(testCase.endPoints[0].String(), testCase.storageDisks)
+		msg := getHealMsg(testCase.endPoints, testCase.storageDisks)
 		if msg == "" {
 			t.Fatalf("Test: %d Unable to get heal message.", i+1)
 		}

cmd/prepare-storage.go

@@ -190,10 +190,6 @@ func retryFormattingDisks(firstDisk bool, endpoints []*url.URL, storageDisks []S
 	if len(endpoints) == 0 {
 		return errInvalidArgument
 	}
-	firstEndpoint := endpoints[0]
-	if firstEndpoint == nil {
-		return errInvalidArgument
-	}
 	if storageDisks == nil {
 		return errInvalidArgument
 	}
@@ -239,7 +235,7 @@ func retryFormattingDisks(firstDisk bool, endpoints []*url.URL, storageDisks []S
 					// Validate formats load before proceeding forward.
 					err := genericFormatCheck(formatConfigs, sErrs)
 					if err == nil {
-						printHealMsg(firstEndpoint.String(), storageDisks, printOnceFn())
+						printHealMsg(endpoints, storageDisks, printOnceFn())
 					}
 					return err
 				case WaitForQuorum:

cmd/server-main.go

@@ -149,9 +149,9 @@ func parseStorageEndpoints(eps []string) (endpoints []*url.URL, err error) {
 }
 
 // getListenIPs - gets all the ips to listen on.
-func getListenIPs(httpServerConf *http.Server) (hosts []string, port string, err error) {
+func getListenIPs(serverAddr string) (hosts []string, port string, err error) {
 	var host string
-	host, port, err = net.SplitHostPort(httpServerConf.Addr)
+	host, port, err = net.SplitHostPort(serverAddr)
 	if err != nil {
 		return nil, port, fmt.Errorf("Unable to parse host address %s", err)
 	}
@@ -180,7 +180,7 @@ func finalizeEndpoints(tls bool, apiServer *http.Server) (endPoints []string) {
 	}
 
 	// Get list of listen ips and port.
-	hosts, port, err := getListenIPs(apiServer)
+	hosts, port, err := getListenIPs(apiServer.Addr)
 	fatalIf(err, "Unable to get list of ips to listen on")
 
 	// Construct proper endpoints.

cmd/server-main_test.go

@@ -44,9 +44,7 @@ func TestGetListenIPs(t *testing.T) {
 		if test.port != "" {
 			addr = test.addr + ":" + test.port
 		}
-		hosts, port, err := getListenIPs(&http.Server{
+		hosts, port, err := getListenIPs(addr)
-			Addr: addr,
-		})
 		if !test.shouldPass && err == nil {
 			t.Fatalf("Test should fail but succeeded %s", err)
 		}