From 2244adff07392b8cd9a30b45964c4218391059a1 Mon Sep 17 00:00:00 2001 From: "A. Elleuch" Date: Thu, 28 Dec 2017 18:32:48 +0100 Subject: [PATCH] fix: Better printing of XL config init error (#5284) --- cmd/admin-handlers_test.go | 2 +- cmd/format-config-v1_test.go | 4 +-- cmd/prepare-storage-msg.go | 11 ------- cmd/prepare-storage-msg_test.go | 6 ---- cmd/prepare-storage.go | 47 ++++++++++++++++++++++------- cmd/prepare-storage_test.go | 41 ++++++++++++++++++++++++- cmd/test-utils_test.go | 19 +++++++----- cmd/utils.go | 12 +++++--- cmd/utils_test.go | 36 ++++++++++++++++++++++ cmd/web-handlers_test.go | 4 +-- cmd/xl-v1-common_test.go | 2 +- cmd/xl-v1-healing-common_test.go | 4 +-- cmd/xl-v1-list-objects-heal_test.go | 4 +-- cmd/xl-v1-multipart_test.go | 6 ++-- cmd/xl-v1-object_test.go | 12 ++++---- cmd/xl-v1_test.go | 2 +- 16 files changed, 151 insertions(+), 61 deletions(-) diff --git a/cmd/admin-handlers_test.go b/cmd/admin-handlers_test.go index 35a544033..719a6cd0b 100644 --- a/cmd/admin-handlers_test.go +++ b/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 } diff --git a/cmd/format-config-v1_test.go b/cmd/format-config-v1_test.go index 13e555e60..cedebb786 100644 --- a/cmd/format-config-v1_test.go +++ b/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) } diff --git a/cmd/prepare-storage-msg.go b/cmd/prepare-storage-msg.go index 94169ee9e..faa3b1ee1 100644 --- a/cmd/prepare-storage-msg.go +++ b/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. diff --git a/cmd/prepare-storage-msg_test.go b/cmd/prepare-storage-msg_test.go index a8ddbf5d9..7f096f50e 100644 --- a/cmd/prepare-storage-msg_test.go +++ b/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) - } } } diff --git a/cmd/prepare-storage.go b/cmd/prepare-storage.go index a30ba0199..3745e763b 100644 --- a/cmd/prepare-storage.go +++ b/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: diff --git a/cmd/prepare-storage_test.go b/cmd/prepare-storage_test.go index 8c384cb5a..fbccea720 100644 --- a/cmd/prepare-storage_test.go +++ b/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{ diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 4a59d480d..4a97f2cb1 100644 --- a/cmd/test-utils_test.go +++ b/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) } diff --git a/cmd/utils.go b/cmd/utils.go index c4d656f01..2171297fe 100644 --- a/cmd/utils.go +++ b/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 diff --git a/cmd/utils_test.go b/cmd/utils_test.go index bcd7c86d6..9d7980f73 100644 --- a/cmd/utils_test.go +++ b/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) + } + } +} diff --git a/cmd/web-handlers_test.go b/cmd/web-handlers_test.go index d3787ee76..cc668f418 100644 --- a/cmd/web-handlers_test.go +++ b/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) } diff --git a/cmd/xl-v1-common_test.go b/cmd/xl-v1-common_test.go index f6b79dcc6..d0c8f729b 100644 --- a/cmd/xl-v1-common_test.go +++ b/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.") } diff --git a/cmd/xl-v1-healing-common_test.go b/cmd/xl-v1-healing-common_test.go index 87964397b..c93ef352d 100644 --- a/cmd/xl-v1-healing-common_test.go +++ b/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) } diff --git a/cmd/xl-v1-list-objects-heal_test.go b/cmd/xl-v1-list-objects-heal_test.go index de4541aec..316b66694 100644 --- a/cmd/xl-v1-list-objects-heal_test.go +++ b/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) } diff --git a/cmd/xl-v1-multipart_test.go b/cmd/xl-v1-multipart_test.go index 0dcbf3045..bd1c791cf 100644 --- a/cmd/xl-v1-multipart_test.go +++ b/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) } diff --git a/cmd/xl-v1-object_test.go b/cmd/xl-v1-object_test.go index 26e67757b..b6d3ca499 100644 --- a/cmd/xl-v1-object_test.go +++ b/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) } diff --git a/cmd/xl-v1_test.go b/cmd/xl-v1_test.go index 981ea8f68..51d3fe429 100644 --- a/cmd/xl-v1_test.go +++ b/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.") }