Validate XL sets on format (#8779)

When formatting a set validate if a host failure will likely lead to data loss.

While we don't know what config will be set in the future 
evaluate to our best knowledge, assuming default settings.

cmd/endpoint.go

@@ -29,7 +29,6 @@ import (
 	"time"
 
 	humanize "github.com/dustin/go-humanize"
-	"github.com/minio/cli"
 	"github.com/minio/minio-go/v6/pkg/set"
 	"github.com/minio/minio/cmd/config"
 	"github.com/minio/minio/cmd/logger"
@@ -459,28 +458,6 @@ func NewEndpoints(args ...string) (endpoints Endpoints, err error) {
 	return endpoints, nil
 }
 
-func checkEndpointsSubOptimal(ctx *cli.Context, setupType SetupType, endpointZones EndpointZones) (err error) {
-	// Validate sub optimal ordering only for distributed setup.
-	if setupType != DistXLSetupType {
-		return nil
-	}
-	var endpointOrder int
-	err = fmt.Errorf("Too many disk args are local, input is in sub-optimal order. Please review input args: %s", ctx.Args())
-	for _, endpoints := range endpointZones {
-		for _, endpoint := range endpoints.Endpoints {
-			if endpoint.IsLocal {
-				endpointOrder++
-			} else {
-				endpointOrder--
-			}
-			if endpointOrder >= 2 {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
 // Checks if there are any cross device mounts.
 func checkCrossDeviceMounts(endpoints Endpoints) (err error) {
 	var absPaths []string

cmd/endpoint_test.go

@@ -17,7 +17,6 @@
 package cmd
 
 import (
-	"flag"
 	"fmt"
 	"net"
 	"net/url"
@@ -25,47 +24,9 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/minio/cli"
 	"github.com/minio/minio-go/v6/pkg/set"
 )
 
-func TestSubOptimalEndpointInput(t *testing.T) {
-	args1 := []string{"http://localhost/d1", "http://localhost/d2", "http://localhost/d3", "http://localhost/d4"}
-	args2 := []string{"http://example.org/d1", "http://example.com/d1", "http://example.net/d1", "http://example.edu/d1"}
-
-	tests := []struct {
-		setupType SetupType
-		ctx       *cli.Context
-		endpoints EndpointZones
-		isErr     bool
-	}{
-		{
-			setupType: DistXLSetupType,
-			ctx:       cli.NewContext(cli.NewApp(), flag.NewFlagSet("", flag.ContinueOnError), nil),
-			endpoints: mustGetZoneEndpoints(args1...),
-			isErr:     false,
-		},
-		{
-			setupType: DistXLSetupType,
-			ctx:       cli.NewContext(cli.NewApp(), flag.NewFlagSet("", flag.ContinueOnError), nil),
-			endpoints: mustGetZoneEndpoints(args2...),
-			isErr:     false,
-		},
-	}
-	for i, test := range tests {
-		test := test
-		t.Run(fmt.Sprintf("Test%d", i+1), func(t *testing.T) {
-			err := checkEndpointsSubOptimal(test.ctx, test.setupType, test.endpoints)
-			if test.isErr && err == nil {
-				t.Error("expected err but found nil")
-			}
-			if !test.isErr && err != nil {
-				t.Errorf("expected err nil but found an err %s", err)
-			}
-		})
-	}
-}
-
 func TestNewEndpoint(t *testing.T) {
 	u2, _ := url.Parse("https://example.org/path")
 	u4, _ := url.Parse("http://192.168.253.200/path")

cmd/erasure-encode_test.go

@@ -44,6 +44,10 @@ func (a badDisk) CreateFile(volume, path string, size int64, reader io.Reader) e
 	return errFaultyDisk
 }
 
+func (badDisk) Hostname() string {
+	return ""
+}
+
 const oneMiByte = 1 * humanize.MiByte
 
 var erasureEncodeTests = []struct {

cmd/format-xl.go

@@ -19,15 +19,17 @@ package cmd
 import (
 	"bytes"
 	"context"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"reflect"
-
+	"sync"
-	"encoding/hex"
 
 	humanize "github.com/dustin/go-humanize"
+	"github.com/minio/minio/cmd/config/storageclass"
 	"github.com/minio/minio/cmd/logger"
+	"github.com/minio/minio/pkg/color"
 	"github.com/minio/minio/pkg/sync/errgroup"
 	sha256 "github.com/minio/sha256-simd"
 )
@@ -753,16 +755,43 @@ func fixFormatXLV3(storageDisks []StorageAPI, endpoints Endpoints, formats []*fo
 func initFormatXL(ctx context.Context, storageDisks []StorageAPI, setCount, drivesPerSet int, deploymentID string) (*formatXLV3, error) {
 	format := newFormatXLV3(setCount, drivesPerSet)
 	formats := make([]*formatXLV3, len(storageDisks))
+	wantAtMost := ecDrivesNoConfig(drivesPerSet)
 
+	logger.Info("Formatting zone, %v set(s), %v drives per set.", setCount, drivesPerSet)
 	for i := 0; i < setCount; i++ {
+		hostCount := make(map[string]int, drivesPerSet)
 		for j := 0; j < drivesPerSet; j++ {
+			disk := storageDisks[i*drivesPerSet+j]
 			newFormat := format.Clone()
 			newFormat.XL.This = format.XL.Sets[i][j]
 			if deploymentID != "" {
 				newFormat.ID = deploymentID
 			}
+			hostCount[disk.Hostname()]++
 			formats[i*drivesPerSet+j] = newFormat
 		}
+		if len(hostCount) > 0 {
+			var once sync.Once
+			for host, count := range hostCount {
+				if count > wantAtMost {
+					if host == "" {
+						host = "local"
+					}
+					once.Do(func() {
+						if len(hostCount) == 1 {
+							return
+						}
+						logger.Info(" * Set %v:", i+1)
+						for j := 0; j < drivesPerSet; j++ {
+							disk := storageDisks[i*drivesPerSet+j]
+							logger.Info("   - Drive: %s", disk.String())
+						}
+					})
+					logger.Info(color.Yellow("WARNING:")+" Host %v has more than %v drives of set. "+
+						"A host failure will result in data becoming unavailable.", host, wantAtMost)
+				}
+			}
+		}
 	}
 
 	// Save formats `format.json` across all disks.
@@ -773,6 +802,22 @@ func initFormatXL(ctx context.Context, storageDisks []StorageAPI, setCount, driv
 	return getFormatXLInQuorum(formats)
 }
 
+// ecDrivesNoConfig returns the erasure coded drives in a set if no config has been set.
+// It will attempt to read it from env variable and fall back to drives/2.
+func ecDrivesNoConfig(drivesPerSet int) int {
+	ecDrives := globalStorageClass.GetParityForSC(storageclass.STANDARD)
+	if ecDrives == 0 {
+		cfg, err := storageclass.LookupConfig(nil, drivesPerSet)
+		if err == nil {
+			ecDrives = cfg.Standard.Parity
+		}
+		if ecDrives == 0 {
+			ecDrives = drivesPerSet / 2
+		}
+	}
+	return ecDrives
+}
+
 // Make XL backend meta volumes.
 func makeFormatXLMetaVolumes(disk StorageAPI) error {
 	// Attempt to create MinIO internal buckets.

cmd/naughty-disk_test.go

@@ -53,6 +53,10 @@ func (d *naughtyDisk) IsOnline() bool {
 	return d.disk.IsOnline()
 }
 
+func (*naughtyDisk) Hostname() string {
+	return ""
+}
+
 func (d *naughtyDisk) LastError() (err error) {
 	return nil
 }

cmd/posix-diskid-check.go

@@ -42,6 +42,10 @@ func (p *posixDiskIDCheck) CrawlAndGetDataUsage(endCh <-chan struct{}) (DataUsag
 	return p.storage.CrawlAndGetDataUsage(endCh)
 }
 
+func (p *posixDiskIDCheck) Hostname() string {
+	return p.storage.Hostname()
+}
+
 func (p *posixDiskIDCheck) LastError() error {
 	return p.storage.LastError()
 }

cmd/posix.go

@@ -309,6 +309,10 @@ func (s *posix) String() string {
 	return s.diskPath
 }
 
+func (*posix) Hostname() string {
+	return ""
+}
+
 func (s *posix) LastError() error {
 	if atomic.LoadInt32(&s.ioErrCount) > maxAllowedIOError {
 		return errFaultyDisk

cmd/prepare-storage.go

@@ -160,6 +160,7 @@ func validateXLFormats(format *formatXLV3, formats []*formatXLV3, endpoints Endp
 	if len(format.XL.Sets[0]) != drivesPerSet {
 		return fmt.Errorf("Current backend format is inconsistent with input args (%s), Expected drive count per set %d, got %d", endpoints, len(format.XL.Sets[0]), drivesPerSet)
 	}
+
 	return nil
 }
 

cmd/server-main.go

@@ -127,10 +127,6 @@ func serverHandleCmdArgs(ctx *cli.Context) {
 	}
 	logger.FatalIf(err, "Invalid command line arguments")
 
-	if err = checkEndpointsSubOptimal(ctx, setupType, globalEndpoints); err != nil {
-		logger.Info("Optimal endpoint check failed %s", err)
-	}
-
 	// On macOS, if a process already listens on LOCALIPADDR:PORT, net.Listen() falls back
 	// to IPv6 address ie minio will start listening on IPv6 address whereas another
 	// (non-)minio process is listening on IPv4 of given port.

cmd/storage-interface.go

@@ -26,7 +26,8 @@ type StorageAPI interface {
 	String() string
 
 	// Storage operations.
-	IsOnline() bool // Returns true if disk is online.
+	IsOnline() bool   // Returns true if disk is online.
+	Hostname() string // Returns host name if remote host.
 	LastError() error
 	Close() error
 	SetDiskID(id string)

cmd/storage-rest-client.go

@@ -148,6 +148,10 @@ func (client *storageRESTClient) IsOnline() bool {
 	return atomic.LoadInt32(&client.connected) == 1
 }
 
+func (client *storageRESTClient) Hostname() string {
+	return client.endpoint.Host
+}
+
 func (client *storageRESTClient) CrawlAndGetDataUsage(endCh <-chan struct{}) (DataUsageInfo, error) {
 	respBody, err := client.call(storageRESTMethodCrawlAndGetDataUsage, nil, nil, -1)
 	defer http.DrainBody(respBody)