From 66459a4ce007afc9532de5b8ec5871e173a4b2f9 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Thu, 8 Sep 2016 09:35:13 -0700 Subject: [PATCH] Add unit-tests for formatting disks during initialization (#2635) * Add unit-tests for formatting disks during initialization - Fixed corresponding code at places where it was deviating from the tabular spec. * Added more test cases and simplified algo ... based on feedback from ``go test -coverprofile``. --- cmd/prepare-storage.go | 22 ++++-- cmd/prepare-storage_test.go | 153 ++++++++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 cmd/prepare-storage_test.go diff --git a/cmd/prepare-storage.go b/cmd/prepare-storage.go index c4dd08629..98ae06cbb 100644 --- a/cmd/prepare-storage.go +++ b/cmd/prepare-storage.go @@ -60,7 +60,7 @@ func init() { | Quorum | | "Heal via minioctl" | | | Quorum UnFormatted | | +----------+--------------------------+-----------------------+ - | No | | Wait till enough | + | No | | Wait till enough | | Quorum | _ | nodes are online and | | | | one of the above | | | | sections apply | @@ -121,22 +121,28 @@ func prepForInit(disks []string, sErrs []error, diskCount int) InitActions { return FormatDisks } return WaitForFormatting - } else if (disksUnformatted >= quorum) && (disksUnformatted+disksOffline == diskCount) { - return WaitForAll + } else if disksUnformatted >= quorum { + if disksUnformatted+disksOffline == diskCount { + return WaitForAll + } + // Some disks possibly corrupted. + return WaitForHeal } // Already formatted, proceed to initialization of object layer. if disksFormatted == diskCount { return InitObjectLayer - } else if disksFormatted >= quorum && disksFormatted+disksOffline == diskCount { - return InitObjectLayer } else if disksFormatted >= quorum { - // TODO: Print minioctl heal command - return InitObjectLayer + if (disksFormatted+disksOffline == diskCount) || + (disksFormatted+disksUnformatted == diskCount) { + return InitObjectLayer + } + // Some disks possibly corrupted. + return WaitForHeal } // No Quorum. - if disksOffline > quorum { + if disksOffline >= quorum { return WaitForQuorum } diff --git a/cmd/prepare-storage_test.go b/cmd/prepare-storage_test.go new file mode 100644 index 000000000..bdeedd43b --- /dev/null +++ b/cmd/prepare-storage_test.go @@ -0,0 +1,153 @@ +/* + * Minio Cloud Storage, (C) 2016 Minio, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package cmd + +import ( + "runtime" + "testing" +) + +func (action InitActions) String() string { + switch action { + case InitObjectLayer: + return "InitObjectLayer" + case FormatDisks: + return "FormatDisks" + case WaitForFormatting: + return "WaitForFormatting" + case WaitForHeal: + return "WaitForHeal" + case WaitForAll: + return "WaitForAll" + case WaitForQuorum: + return "WaitForQuorum" + case Abort: + return "Abort" + default: + return "Unknown" + } +} +func TestPrepForInit(t *testing.T) { + var disks []string + if runtime.GOOS == "windows" { + disks = []string{ + `c:\mnt\disk1`, + `c:\mnt\disk2`, + `c:\mnt\disk3`, + `c:\mnt\disk4`, + `c:\mnt\disk5`, + `c:\mnt\disk6`, + `c:\mnt\disk7`, + `c:\mnt\disk8`, + } + } else { + disks = []string{ + "/mnt/disk1", + "/mnt/disk2", + "/mnt/disk3", + "/mnt/disk4", + "/mnt/disk5", + "/mnt/disk6", + "/mnt/disk7", + "/mnt/disk8", + } + } + // Building up disks that resolve to localhost and remote w.r.t isLocalStorage(). + var ( + disksLocal []string + disksRemote []string + ) + for i := range disks { + disksLocal = append(disksLocal, "localhost:"+disks[i]) + } + // Using 4.4.4.4 as a known non-local address. + for i := range disks { + disksRemote = append(disksRemote, "4.4.4.4:"+disks[i]) + } + // All disks are unformatted, a fresh setup. + allUnformatted := []error{ + errUnformattedDisk, errUnformattedDisk, errUnformattedDisk, errUnformattedDisk, + errUnformattedDisk, errUnformattedDisk, errUnformattedDisk, errUnformattedDisk, + } + // All disks are formatted, possible restart of a node in a formatted setup. + allFormatted := []error{ + nil, nil, nil, nil, + nil, nil, nil, nil, + } + // Quorum number of disks are formatted and rest are offline. + quorumFormatted := []error{ + nil, nil, nil, nil, + nil, errDiskNotFound, errDiskNotFound, errDiskNotFound, + } + // Minority disks are corrupted, can be healed. + minorityCorrupted := []error{ + errCorruptedFormat, errCorruptedFormat, errCorruptedFormat, nil, + nil, nil, nil, nil, + } + // Majority disks are corrupted, pretty bad setup. + majorityCorrupted := []error{ + errCorruptedFormat, errCorruptedFormat, errCorruptedFormat, errCorruptedFormat, + errCorruptedFormat, nil, nil, nil, + } + // Quorum disks are unformatted, remaining yet to come online. + quorumUnformatted := []error{ + errUnformattedDisk, errUnformattedDisk, errUnformattedDisk, errUnformattedDisk, + errUnformattedDisk, errDiskNotFound, errDiskNotFound, errDiskNotFound, + } + quorumUnformattedSomeCorrupted := []error{ + errUnformattedDisk, errUnformattedDisk, errUnformattedDisk, errUnformattedDisk, + errUnformattedDisk, errCorruptedFormat, errCorruptedFormat, errDiskNotFound, + } + // Quorum number of disks not online yet. + noQuourm := []error{ + errDiskNotFound, errDiskNotFound, errDiskNotFound, errDiskNotFound, + errDiskNotFound, nil, nil, nil, + } + + testCases := []struct { + // Params for prepForInit(). + disks []string + errs []error + diskCount int + action InitActions + }{ + // Local disks. + {disksLocal, allFormatted, 8, InitObjectLayer}, + {disksLocal, quorumFormatted, 8, InitObjectLayer}, + {disksLocal, allUnformatted, 8, FormatDisks}, + {disksLocal, quorumUnformatted, 8, WaitForAll}, + {disksLocal, quorumUnformattedSomeCorrupted, 8, WaitForHeal}, + {disksLocal, noQuourm, 8, WaitForQuorum}, + {disksLocal, minorityCorrupted, 8, WaitForHeal}, + {disksLocal, majorityCorrupted, 8, Abort}, + // Remote disks. + {disksRemote, allFormatted, 8, InitObjectLayer}, + {disksRemote, quorumFormatted, 8, InitObjectLayer}, + {disksRemote, allUnformatted, 8, WaitForFormatting}, + {disksRemote, quorumUnformatted, 8, WaitForAll}, + {disksRemote, quorumUnformattedSomeCorrupted, 8, WaitForHeal}, + {disksRemote, noQuourm, 8, WaitForQuorum}, + {disksRemote, minorityCorrupted, 8, WaitForHeal}, + {disksRemote, majorityCorrupted, 8, Abort}, + } + for i, test := range testCases { + actual := prepForInit(test.disks, test.errs, test.diskCount) + if actual != test.action { + t.Errorf("Test %d expected %s but receieved %s\n", i+1, test.action, actual) + } + } +}