fix: Better printing of XL config init error (#5284)

cmd/admin-handlers_test.go

@@ -193,7 +193,7 @@ func (atb *adminXLTestBed) TearDown() {
 // initTestObjLayer - Helper function to initialize an XL-based object
 // layer and set globalObjectAPI.
 func initTestXLObjLayer() (ObjectLayer, []string, error) {
-	objLayer, xlDirs, xlErr := prepareXL()
+	objLayer, xlDirs, xlErr := prepareXL16()
 	if xlErr != nil {
 		return nil, nil, xlErr
 	}

cmd/format-config-v1_test.go

@@ -317,7 +317,7 @@ func TestFormatXLHealFreshDisks(t *testing.T) {
 // a given disk to test healing a corrupted disk
 func TestFormatXLHealCorruptedDisks(t *testing.T) {
 	// Create an instance of xl backend.
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -391,7 +391,7 @@ func TestFormatXLHealCorruptedDisks(t *testing.T) {
 // some of format.json
 func TestFormatXLReorderByInspection(t *testing.T) {
 	// Create an instance of xl backend.
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/prepare-storage-msg.go

@@ -125,17 +125,6 @@ func printFormatMsg(endpoints EndpointList, storageDisks []StorageAPI, fn printO
 	fn(msg)
 }
 
-func printConfigErrMsg(storageDisks []StorageAPI, sErrs []error, fn printOnceFunc) {
-	msg := getConfigErrMsg(storageDisks, sErrs)
-	fn(msg)
-}
-
-// Generate a formatted message when cluster is misconfigured.
-func getConfigErrMsg(storageDisks []StorageAPI, sErrs []error) string {
-	msg := colorBlue("\nDetected configuration inconsistencies in the cluster. Please fix following servers.")
-	return msg + combineDiskErrs(storageDisks, sErrs)
-}
-
 // Combines each disk errors in a newline formatted string.
 // this is a helper function in printing messages across
 // all disks.

cmd/prepare-storage-msg_test.go

@@ -52,8 +52,6 @@ func TestHealMsg(t *testing.T) {
 		{endpoints, storageDisks, errs},
 		// Test - 2 for one of the disks is nil.
 		{endpoints, nilDisks, errs},
-		// Test - 3 for one of the errs is authentication.
-		{endpoints, nilDisks, authErrs},
 	}
 
 	for i, testCase := range testCases {
@@ -65,10 +63,6 @@ func TestHealMsg(t *testing.T) {
 		if msg == "" {
 			t.Fatalf("Test: %d Unable to get regular message.", i+1)
 		}
-		msg = getConfigErrMsg(testCase.storageDisks, testCase.serrs)
-		if msg == "" {
-			t.Fatalf("Test: %d Unable to get config error message.", i+1)
-		}
 	}
 }
 

cmd/prepare-storage.go

@@ -98,23 +98,48 @@ const (
 	Abort
 )
 
+// configErrs contains the list of configuration errors.
+var configErrs = []error{
+	errInvalidAccessKeyID,
+	errAuthentication,
+	errServerVersionMismatch,
+	errServerTimeMismatch,
+}
+
 // Quick error to actions converts looking for specific errors
 // which need to be returned quickly and server should wait instead.
 func quickErrToActions(errMap map[error]int) InitActions {
 	var action InitActions
-	switch {
-	case errMap[errInvalidAccessKeyID] > 0:
-		fallthrough
-	case errMap[errAuthentication] > 0:
-		fallthrough
-	case errMap[errServerVersionMismatch] > 0:
-		fallthrough
-	case errMap[errServerTimeMismatch] > 0:
-		action = WaitForConfig
+	for _, configErr := range configErrs {
+		if errMap[configErr] > 0 {
+			action = WaitForConfig
+			break
+		}
 	}
 	return action
 }
 
+// reduceInitXLErrs reduces errors found in distributed XL initialization
+func reduceInitXLErrs(storageDisks []StorageAPI, sErrs []error) error {
+	var foundErrs int
+	for i := range sErrs {
+		if sErrs[i] != nil {
+			foundErrs++
+		}
+	}
+	if foundErrs == 0 {
+		return nil
+	}
+	// Early quit if there is a config error
+	for i := range sErrs {
+		if contains(configErrs, sErrs[i]) {
+			return fmt.Errorf("%s: %s", storageDisks[i], sErrs[i])
+		}
+	}
+	// Combine all disk errors otherwise for user inspection
+	return fmt.Errorf("%s", combineDiskErrs(storageDisks, sErrs))
+}
+
 // Preparatory initialization stage for XL validates known errors.
 // Converts them into specific actions. These actions have special purpose
 // which caller decides on what needs to be done.
@@ -262,7 +287,7 @@ func retryFormattingXLDisks(firstDisk bool, endpoints EndpointList, storageDisks
 			// Check if this is a XL or distributed XL, anything > 1 is considered XL backend.
 			switch prepForInitXL(firstDisk, sErrs, len(storageDisks)) {
 			case Abort:
-				return fmt.Errorf("%s", combineDiskErrs(storageDisks, sErrs))
+				return reduceInitXLErrs(storageDisks, sErrs)
 			case FormatDisks:
 				console.Eraseline()
 				printFormatMsg(endpoints, storageDisks, printOnceFn())
@@ -289,7 +314,7 @@ func retryFormattingXLDisks(firstDisk bool, endpoints EndpointList, storageDisks
 				)
 			case WaitForConfig:
 				// Print configuration errors.
-				printConfigErrMsg(storageDisks, sErrs, printOnceFn())
+				return reduceInitXLErrs(storageDisks, sErrs)
 			case WaitForAll:
 				console.Printf("Initializing data volume for first time. Waiting for other servers to come online (elapsed %s)\n", getElapsedTime())
 			case WaitForFormatting:

cmd/prepare-storage_test.go

@@ -16,7 +16,10 @@
 
 package cmd
 
-import "testing"
+import (
+	"os"
+	"testing"
+)
 
 func (action InitActions) String() string {
 	switch action {
@@ -41,6 +44,42 @@ func (action InitActions) String() string {
 	}
 }
 
+func TestReduceInitXLErrs(t *testing.T) {
+	_, fsDirs, err := prepareXL(4)
+	if err != nil {
+		t.Fatalf("Unable to initialize 'XL' object layer.")
+	}
+
+	// Remove all dirs.
+	for _, dir := range fsDirs {
+		defer os.RemoveAll(dir)
+	}
+
+	storageDisks, err := initStorageDisks(mustGetNewEndpointList(fsDirs...))
+	if err != nil {
+		t.Fatal("Unexpected error: ", err)
+	}
+
+	testCases := []struct {
+		sErrs       []error
+		expectedErr string
+	}{
+		{[]error{nil, nil, nil, nil}, ""},
+		{[]error{errUnformattedDisk, nil, nil, nil}, "\n[01/04] " + storageDisks[0].String() + " : unformatted disk found"},
+		{[]error{errUnformattedDisk, errUnformattedDisk, nil, nil}, "\n[01/04] " + storageDisks[0].String() + " : unformatted disk found" + "\n[02/04] " + storageDisks[1].String() + " : unformatted disk found"},
+		{[]error{errUnformattedDisk, errUnformattedDisk, errServerVersionMismatch, nil}, storageDisks[2].String() + ": Server versions do not match"},
+	}
+	for i, test := range testCases {
+		actual := reduceInitXLErrs(storageDisks, test.sErrs)
+		if test.expectedErr == "" && actual != nil {
+			t.Errorf("Test %d expected no error but received `%s`", i+1, actual.Error())
+		}
+		if test.expectedErr != "" && actual.Error() != test.expectedErr {
+			t.Errorf("Test %d expected `%s` but received `%s`", i+1, test.expectedErr, actual.Error())
+		}
+	}
+}
+
 func TestPrepForInitXL(t *testing.T) {
 	// All disks are unformatted, a fresh setup.
 	allUnformatted := []error{

cmd/test-utils_test.go

@@ -169,8 +169,7 @@ func prepareFS() (ObjectLayer, string, error) {
 	return obj, fsDirs[0], nil
 }
 
-func prepareXL() (ObjectLayer, []string, error) {
-	nDisks := 16
+func prepareXL(nDisks int) (ObjectLayer, []string, error) {
 	fsDirs, err := getRandomDisks(nDisks)
 	if err != nil {
 		return nil, nil, err
@@ -183,6 +182,10 @@ func prepareXL() (ObjectLayer, []string, error) {
 	return obj, fsDirs, nil
 }
 
+func prepareXL16() (ObjectLayer, []string, error) {
+	return prepareXL(16)
+}
+
 // Initialize FS objects.
 func initFSObjects(disk string, t *testing.T) (obj ObjectLayer) {
 	newTestConfig(globalMinioDefaultRegion)
@@ -1748,7 +1751,7 @@ func prepareTestBackend(instanceType string) (ObjectLayer, []string, error) {
 	switch instanceType {
 	// Total number of disks for XL backend is set to 16.
 	case XLTestStr:
-		return prepareXL()
+		return prepareXL16()
 	default:
 		// return FS backend by default.
 		obj, disk, err := prepareFS()
@@ -1955,7 +1958,7 @@ func ExecObjectLayerAPITest(t *testing.T, objAPITest objAPITestType, endpoints [
 	// Executing the object layer tests for single node setup.
 	objAPITest(objLayer, FSTestStr, bucketFS, fsAPIRouter, credentials, t)
 
-	objLayer, xlDisks, err := prepareXL()
+	objLayer, xlDisks, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Initialization of object layer failed for XL setup: %s", err)
 	}
@@ -2001,7 +2004,7 @@ func ExecObjectLayerTest(t TestErrHandler, objTest objTestType) {
 	// Executing the object layer tests for single node setup.
 	objTest(objLayer, FSTestStr, t)
 
-	objLayer, fsDirs, err := prepareXL()
+	objLayer, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Initialization of object layer failed for XL setup: %s", err)
 	}
@@ -2026,7 +2029,7 @@ func ExecObjectLayerTestWithDirs(t TestErrHandler, objTest objTestTypeWithDirs)
 		t.Fatalf("Initialization of object layer failed for single node setup: %s", err)
 	}
 
-	objLayer, fsDirs, err := prepareXL()
+	objLayer, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Initialization of object layer failed for XL setup: %s", err)
 	}
@@ -2044,7 +2047,7 @@ func ExecObjectLayerDiskAlteredTest(t *testing.T, objTest objTestDiskNotFoundTyp
 	}
 	defer os.RemoveAll(configPath)
 
-	objLayer, fsDirs, err := prepareXL()
+	objLayer, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Initialization of object layer failed for XL setup: %s", err)
 	}
@@ -2255,7 +2258,7 @@ func StartTestS3PeerRPCServer(t TestErrHandler) (TestServer, []string) {
 	testRPCServer.SecretKey = credentials.SecretKey
 
 	// init disks
-	objLayer, fsDirs, err := prepareXL()
+	objLayer, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("%s", err)
 	}

cmd/utils.go

@@ -29,6 +29,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"reflect"
 	"strings"
 	"time"
 
@@ -132,10 +133,13 @@ func isMaxPartID(partID int) bool {
 	return partID > globalMaxPartID
 }
 
-func contains(stringList []string, element string) bool {
-	for _, e := range stringList {
-		if e == element {
-			return true
+func contains(slice interface{}, elem interface{}) bool {
+	v := reflect.ValueOf(slice)
+	if v.Kind() == reflect.Slice {
+		for i := 0; i < v.Len(); i++ {
+			if v.Index(i).Interface() == elem {
+				return true
+			}
 		}
 	}
 	return false

cmd/utils_test.go

@@ -18,6 +18,7 @@ package cmd
 
 import (
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/url"
 	"reflect"
@@ -312,3 +313,38 @@ func TestToS3ETag(t *testing.T) {
 		}
 	}
 }
+
+// Test contains
+func TestContains(t *testing.T) {
+
+	testErr := errors.New("test err")
+
+	testCases := []struct {
+		slice interface{}
+		elem  interface{}
+		found bool
+	}{
+		{nil, nil, false},
+		{"1", "1", false},
+		{nil, "1", false},
+		{[]string{"1"}, nil, false},
+		{[]string{}, "1", false},
+		{[]string{"1"}, "1", true},
+		{[]string{"2"}, "1", false},
+		{[]string{"1", "2"}, "1", true},
+		{[]string{"2", "1"}, "1", true},
+		{[]string{"2", "1", "3"}, "1", true},
+		{[]int{1, 2, 3}, "1", false},
+		{[]int{1, 2, 3}, 2, true},
+		{[]int{1, 2, 3, 4, 5, 6}, 7, false},
+		{[]error{errors.New("new err")}, testErr, false},
+		{[]error{errors.New("new err"), testErr}, testErr, true},
+	}
+
+	for i, testCase := range testCases {
+		found := contains(testCase.slice, testCase.elem)
+		if found != testCase.found {
+			t.Fatalf("Test %v: expected: %v, got: %v", i+1, testCase.found, found)
+		}
+	}
+}

cmd/web-handlers_test.go

@@ -1363,7 +1363,7 @@ func testWebSetBucketPolicyHandler(obj ObjectLayer, instanceType string, t TestE
 // TestWebCheckAuthorization - Test Authorization for all web handlers
 func TestWebCheckAuthorization(t *testing.T) {
 	// Prepare XL backend
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Initialization of object layer failed for XL setup: %s", err)
 	}
@@ -1542,7 +1542,7 @@ func TestWebObjectLayerFaultyDisks(t *testing.T) {
 	defer os.RemoveAll(root)
 
 	// Prepare XL backend
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Initialization of object layer failed for XL setup: %s", err)
 	}

cmd/xl-v1-common_test.go

@@ -30,7 +30,7 @@ func TestXLParentDirIsObject(t *testing.T) {
 	}
 	defer os.RemoveAll(rootPath)
 
-	obj, fsDisks, err := prepareXL()
+	obj, fsDisks, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Unable to initialize 'XL' object layer.")
 	}

cmd/xl-v1-healing-common_test.go

@@ -130,7 +130,7 @@ func TestListOnlineDisks(t *testing.T) {
 	}
 	defer os.RemoveAll(rootPath)
 
-	obj, disks, err := prepareXL()
+	obj, disks, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Prepare XL backend failed - %v", err)
 	}
@@ -339,7 +339,7 @@ func TestDisksWithAllParts(t *testing.T) {
 	}
 	defer os.RemoveAll(rootPath)
 
-	obj, disks, err := prepareXL()
+	obj, disks, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Prepare XL backend failed - %v", err)
 	}

cmd/xl-v1-list-objects-heal_test.go

@@ -38,7 +38,7 @@ func TestListObjectsHeal(t *testing.T) {
 	defer os.RemoveAll(rootPath)
 
 	// Create an instance of xl backend
-	xl, fsDirs, err := prepareXL()
+	xl, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -155,7 +155,7 @@ func TestListUploadsHeal(t *testing.T) {
 	defer os.RemoveAll(rootPath)
 
 	// Create an instance of XL backend.
-	xl, fsDirs, err := prepareXL()
+	xl, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/xl-v1-multipart_test.go

@@ -34,7 +34,7 @@ func TestXLCleanupMultipartUploadsInRoutine(t *testing.T) {
 	defer os.RemoveAll(root)
 
 	// Create an instance of xl backend
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -84,7 +84,7 @@ func TestXLCleanupMultipartUpload(t *testing.T) {
 	defer os.RemoveAll(root)
 
 	// Create an instance of xl backend
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -128,7 +128,7 @@ func TestUpdateUploadJSON(t *testing.T) {
 	defer os.RemoveAll(root)
 
 	// Create an instance of xl backend
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/xl-v1-object_test.go

@@ -35,7 +35,7 @@ func TestRepeatPutObjectPart(t *testing.T) {
 	var disks []string
 	var err error
 
-	objLayer, disks, err = prepareXL()
+	objLayer, disks, err = prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -81,7 +81,7 @@ func TestXLDeleteObjectBasic(t *testing.T) {
 	}
 
 	// Create an instance of xl backend
-	xl, fsDirs, err := prepareXL()
+	xl, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -113,7 +113,7 @@ func TestXLDeleteObjectBasic(t *testing.T) {
 
 func TestXLDeleteObjectDiskNotFound(t *testing.T) {
 	// Create an instance of xl backend.
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -164,7 +164,7 @@ func TestXLDeleteObjectDiskNotFound(t *testing.T) {
 
 func TestGetObjectNoQuorum(t *testing.T) {
 	// Create an instance of xl backend.
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -216,7 +216,7 @@ func TestGetObjectNoQuorum(t *testing.T) {
 
 func TestPutObjectNoQuorum(t *testing.T) {
 	// Create an instance of xl backend.
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -273,7 +273,7 @@ func TestHealing(t *testing.T) {
 	}
 	defer os.RemoveAll(rootPath)
 
-	obj, fsDirs, err := prepareXL()
+	obj, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/xl-v1_test.go

@@ -27,7 +27,7 @@ import (
 
 // TestStorageInfo - tests storage info.
 func TestStorageInfo(t *testing.T) {
-	objLayer, fsDirs, err := prepareXL()
+	objLayer, fsDirs, err := prepareXL16()
 	if err != nil {
 		t.Fatalf("Unable to initialize 'XL' object layer.")
 	}