From 85c6bb98099345502d6eceb0f2c92b3d91402434 Mon Sep 17 00:00:00 2001
From: Krishnan Parthasarathi <krisis@users.noreply.github.com>
Date: Tue, 20 Dec 2016 06:34:31 +0530
Subject: [PATCH] server: Sort disk arguments for consistent ordering (#3469)

This is important in a distributed setup, where the server hosting the
first disk formats a fresh setup. Sorting ensures that all servers
arrive at the same 'first' server.

Note: This change doesn't protect against different disk arguments
with some disks being same across servers.
---
 cmd/server-main.go   |  7 ++++
 cmd/url-sort.go      | 34 +++++++++++++++
 cmd/url-sort_test.go | 99 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+)
 create mode 100644 cmd/url-sort.go
 create mode 100644 cmd/url-sort_test.go

diff --git a/cmd/server-main.go b/cmd/server-main.go
index c020e240b..23ba3cac1 100644
--- a/cmd/server-main.go
+++ b/cmd/server-main.go
@@ -24,6 +24,7 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"sort"
 	"strings"
 
 	"runtime"
@@ -429,6 +430,12 @@ func serverMain(c *cli.Context) {
 		fatalIf(errInvalidArgument, "None of the disks passed as command line args are local to this server.")
 	}
 
+	// Sort endpoints for consistent ordering across multiple
+	// nodes in a distributed setup. This is to avoid format.json
+	// corruption if the disks aren't supplied in the same order
+	// on all nodes.
+	sort.Sort(byHostPath(endpoints))
+
 	storageDisks, err := initStorageDisks(endpoints)
 	fatalIf(err, "Unable to initialize storage disk(s).")
 
diff --git a/cmd/url-sort.go b/cmd/url-sort.go
new file mode 100644
index 000000000..a1f3f306c
--- /dev/null
+++ b/cmd/url-sort.go
@@ -0,0 +1,34 @@
+/*
+ * 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 "net/url"
+
+type byHostPath []*url.URL
+
+func (s byHostPath) Swap(i, j int) {
+	s[i], s[j] = s[j], s[i]
+}
+
+func (s byHostPath) Len() int {
+	return len(s)
+}
+
+// Note: Host in url.URL includes the port too.
+func (s byHostPath) Less(i, j int) bool {
+	return (s[i].Host + s[i].Path) < (s[j].Host + s[j].Path)
+}
diff --git a/cmd/url-sort_test.go b/cmd/url-sort_test.go
new file mode 100644
index 000000000..fb1b6e5c1
--- /dev/null
+++ b/cmd/url-sort_test.go
@@ -0,0 +1,99 @@
+/*
+ * 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 (
+	"net/url"
+	"reflect"
+	"sort"
+	"testing"
+)
+
+// TestSortByHostPath - tests if ordering of urls are based on
+// host+path concatenated.
+func TestSortByHostPath(t *testing.T) {
+	testCases := []struct {
+		given    []string
+		expected []*url.URL
+	}{
+		{
+			given: []string{
+				"http://abcd.com/a/b/d",
+				"http://abcd.com/a/b/c",
+				"http://abcd.com/a/b/e",
+			},
+			expected: []*url.URL{
+				{
+					Scheme: "http",
+					Host:   "abcd.com:9000",
+					Path:   "/a/b/c",
+				},
+				{
+					Scheme: "http",
+					Host:   "abcd.com:9000",
+					Path:   "/a/b/d",
+				},
+				{
+					Scheme: "http",
+					Host:   "abcd.com:9000",
+					Path:   "/a/b/e",
+				},
+			},
+		},
+		{
+			given: []string{
+				"http://defg.com/a/b/c",
+				"http://abcd.com/a/b/c",
+				"http://hijk.com/a/b/c",
+			},
+			expected: []*url.URL{
+				{
+					Scheme: "http",
+					Host:   "abcd.com:9000",
+					Path:   "/a/b/c",
+				},
+				{
+					Scheme: "http",
+					Host:   "defg.com:9000",
+					Path:   "/a/b/c",
+				},
+				{
+					Scheme: "http",
+					Host:   "hijk.com:9000",
+					Path:   "/a/b/c",
+				},
+			},
+		},
+	}
+
+	saveGlobalPort := globalMinioPort
+	globalMinioPort = "9000"
+	for i, test := range testCases {
+		eps, err := parseStorageEndpoints(test.given)
+		if err != nil {
+			t.Fatalf("Test %d - Failed to parse storage endpoint %v", i+1, err)
+		}
+		sort.Sort(byHostPath(eps))
+		if !sort.IsSorted(byHostPath(eps)) {
+			t.Errorf("Test %d - Expected order %v but got %v", i+1, test.expected, eps)
+		}
+		if !reflect.DeepEqual(eps, test.expected) {
+			t.Errorf("Test %d - Expected order %v but got %v", i+1, test.expected, eps)
+		}
+	}
+	globalMinioPort = saveGlobalPort
+}