extend server config.yaml to support per pool set drive count (#19663)

This is to support deployments migrating from a multi-pooled wider stripe to lower stripe. MINIO_STORAGE_CLASS_STANDARD is still expected to be same for all pools. So you can satisfy adding custom drive count based pools by adjusting the storage class value. ``` version: v2 address: ':9000' rootUser: 'minioadmin' rootPassword: 'minioadmin' console-address: ':9001' pools: # Specify the nodes and drives with pools - args: - 'node{11...14}.example.net/data{1...4}' - args: - 'node{15...18}.example.net/data{1...4}' - args: - 'node{19...22}.example.net/data{1...4}' - args: - 'node{23...34}.example.net/data{1...10}' set-drive-count: 6 ```
2024-05-03 08:54:03 -07:00 · 2024-05-03 08:54:03 -07:00 · 1526e7ece3
parent 6c07bfee8a
commit 1526e7ece3
5 changed files with 172 additions and 82 deletions
--- a/cmd/endpoint-ellipses.go
+++ b/cmd/endpoint-ellipses.go
@ -23,7 +23,6 @@ import (
 	"net/url"
 	"runtime"
 	"sort"
-	"strconv"
 	"strings"

 	"github.com/cespare/xxhash/v2"
@ -134,7 +133,7 @@ func possibleSetCountsWithSymmetry(setCounts []uint64, argPatterns []ellipses.Ar
 // on each index, this function also determines the final set size
 // The final set size has the affinity towards choosing smaller
 // indexes (total sets)
-func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint64, argPatterns []ellipses.ArgPattern) (setIndexes [][]uint64, err error) {
+func getSetIndexes(args []string, totalSizes []uint64, setDriveCount uint64, argPatterns []ellipses.ArgPattern) (setIndexes [][]uint64, err error) {
 	if len(totalSizes) == 0 || len(args) == 0 {
 		return nil, errInvalidArgument
 	}
@ -142,7 +141,7 @@ func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint6
 	setIndexes = make([][]uint64, len(totalSizes))
 	for _, totalSize := range totalSizes {
 		// Check if totalSize has minimum range upto setSize
-		if totalSize < setSizes[0] || totalSize < customSetDriveCount {
+		if totalSize < setSizes[0] || totalSize < setDriveCount {
 			msg := fmt.Sprintf("Incorrect number of endpoints provided %s", args)
 			return nil, config.ErrInvalidNumberOfErasureEndpoints(nil).Msg(msg)
 		}
@ -167,11 +166,11 @@ func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint6
 	var setSize uint64
 	// Custom set drive count allows to override automatic distribution.
 	// only meant if you want to further optimize drive distribution.
-	if customSetDriveCount > 0 {
+	if setDriveCount > 0 {
 		msg := fmt.Sprintf("Invalid set drive count. Acceptable values for %d number drives are %d", commonSize, setCounts)
 		var found bool
 		for _, ss := range setCounts {
-			if ss == customSetDriveCount {
+			if ss == setDriveCount {
 				found = true
 			}
 		}
@ -180,8 +179,7 @@ func getSetIndexes(args []string, totalSizes []uint64, customSetDriveCount uint6
 		}

 		// No automatic symmetry calculation expected, user is on their own
-		setSize = customSetDriveCount
-		globalCustomErasureDriveCount = true
+		setSize = setDriveCount
 	} else {
 		// Returns possible set counts with symmetry.
 		setCounts = possibleSetCountsWithSymmetry(setCounts, argPatterns)
@ -256,7 +254,7 @@ func getTotalSizes(argPatterns []ellipses.ArgPattern) []uint64 {
 // Parses all arguments and returns an endpointSet which is a collection
 // of endpoints following the ellipses pattern, this is what is used
 // by the object layer for initializing itself.
-func parseEndpointSet(customSetDriveCount uint64, args ...string) (ep endpointSet, err error) {
+func parseEndpointSet(setDriveCount uint64, args ...string) (ep endpointSet, err error) {
 	argPatterns := make([]ellipses.ArgPattern, len(args))
 	for i, arg := range args {
 		patterns, perr := ellipses.FindEllipsesPatterns(arg)
@ -266,7 +264,7 @@ func parseEndpointSet(customSetDriveCount uint64, args ...string) (ep endpointSe
 		argPatterns[i] = patterns
 	}

-	ep.setIndexes, err = getSetIndexes(args, getTotalSizes(argPatterns), customSetDriveCount, argPatterns)
+	ep.setIndexes, err = getSetIndexes(args, getTotalSizes(argPatterns), setDriveCount, argPatterns)
 	if err != nil {
 		return endpointSet{}, config.ErrInvalidErasureEndpoints(nil).Msg(err.Error())
 	}
@ -281,23 +279,14 @@ func parseEndpointSet(customSetDriveCount uint64, args ...string) (ep endpointSe
 // specific set size.
 // For example: {1...64} is divided into 4 sets each of size 16.
 // This applies to even distributed setup syntax as well.
-func GetAllSets(args ...string) ([][]string, error) {
-	var customSetDriveCount uint64
-	if v := env.Get(EnvErasureSetDriveCount, ""); v != "" {
-		driveCount, err := strconv.Atoi(v)
-		if err != nil {
-			return nil, config.ErrInvalidErasureSetSize(err)
-		}
-		customSetDriveCount = uint64(driveCount)
-	}
-
+func GetAllSets(setDriveCount uint64, args ...string) ([][]string, error) {
 	var setArgs [][]string
 	if !ellipses.HasEllipses(args...) {
 		var setIndexes [][]uint64
 		// Check if we have more one args.
 		if len(args) > 1 {
 			var err error
-			setIndexes, err = getSetIndexes(args, []uint64{uint64(len(args))}, customSetDriveCount, nil)
+			setIndexes, err = getSetIndexes(args, []uint64{uint64(len(args))}, setDriveCount, nil)
 			if err != nil {
 				return nil, err
 			}
@ -311,7 +300,7 @@ func GetAllSets(args ...string) ([][]string, error) {
 		}
 		setArgs = s.Get()
 	} else {
-		s, err := parseEndpointSet(customSetDriveCount, args...)
+		s, err := parseEndpointSet(setDriveCount, args...)
 		if err != nil {
 			return nil, err
 		}
@ -336,8 +325,6 @@ const (
 	EnvErasureSetDriveCount = "MINIO_ERASURE_SET_DRIVE_COUNT"
 )

-var globalCustomErasureDriveCount = false
-
 type node struct {
 	nodeName string
 	disks    []string
@ -366,8 +353,13 @@ func (el *endpointsList) add(arg string) error {
 	return nil
 }

+type poolArgs struct {
+	args          []string
+	setDriveCount uint64
+}
+
 // buildDisksLayoutFromConfFile supports with and without ellipses transparently.
-func buildDisksLayoutFromConfFile(pools [][]string) (layout disksLayout, err error) {
+func buildDisksLayoutFromConfFile(pools []poolArgs) (layout disksLayout, err error) {
 	if len(pools) == 0 {
 		return layout, errInvalidArgument
 	}
@ -375,7 +367,7 @@ func buildDisksLayoutFromConfFile(pools [][]string) (layout disksLayout, err err
 	for _, list := range pools {
 		var endpointsList endpointsList

-		for _, arg := range list {
+		for _, arg := range list.args {
 			switch {
 			case ellipses.HasList(arg):
 				patterns, err := ellipses.FindListPatterns(arg)
@ -436,7 +428,7 @@ func buildDisksLayoutFromConfFile(pools [][]string) (layout disksLayout, err err
 			}
 		}

-		setArgs, err := GetAllSets(eps...)
+		setArgs, err := GetAllSets(list.setDriveCount, eps...)
 		if err != nil {
 			return layout, err
 		}
@ -469,9 +461,15 @@ func mergeDisksLayoutFromArgs(args []string, ctxt *serverCtxt) (err error) {

 	var setArgs [][]string

+	v, err := env.GetInt(EnvErasureSetDriveCount, 0)
+	if err != nil {
+		return err
+	}
+	setDriveCount := uint64(v)
+
 	// None of the args have ellipses use the old style.
 	if ok {
-		setArgs, err = GetAllSets(args...)
+		setArgs, err = GetAllSets(setDriveCount, args...)
 		if err != nil {
 			return err
 		}
@ -487,7 +485,7 @@ func mergeDisksLayoutFromArgs(args []string, ctxt *serverCtxt) (err error) {
 			// TODO: support SNSD deployments to be decommissioned in future
 			return fmt.Errorf("all args must have ellipses for pool expansion (%w) args: %s", errInvalidArgument, args)
 		}
-		setArgs, err = GetAllSets(arg)
+		setArgs, err = GetAllSets(setDriveCount, arg)
 		if err != nil {
 			return err
 		}
--- a/cmd/format-erasure.go
+++ b/cmd/format-erasure.go
@ -443,11 +443,8 @@ func checkFormatErasureValues(formats []*formatErasureV3, disks []StorageAPI, se
 			return fmt.Errorf("%s drive is already being used in another erasure deployment. (Number of drives specified: %d but the number of drives found in the %s drive's format.json: %d)",
 				disks[i], len(formats), humanize.Ordinal(i+1), len(formatErasure.Erasure.Sets)*len(formatErasure.Erasure.Sets[0]))
 		}
-		// Only if custom erasure drive count is set, verify if the
-		// set_drive_count was manually set - we need to honor what is
-		// present on the drives.
-		if globalCustomErasureDriveCount && len(formatErasure.Erasure.Sets[0]) != setDriveCount {
-			return fmt.Errorf("%s drive is already formatted with %d drives per erasure set. This cannot be changed to %d, please revert your MINIO_ERASURE_SET_DRIVE_COUNT setting", disks[i], len(formatErasure.Erasure.Sets[0]), setDriveCount)
+		if len(formatErasure.Erasure.Sets[0]) != setDriveCount {
+			return fmt.Errorf("%s drive is already formatted with %d drives per erasure set. This cannot be changed to %d", disks[i], len(formatErasure.Erasure.Sets[0]), setDriveCount)
 		}
 	}
 	return nil
--- a/cmd/server-main.go
+++ b/cmd/server-main.go
@ -18,6 +18,7 @@
 package cmd

 import (
+	"bytes"
 	"context"
 	"encoding/hex"
 	"errors"
@ -287,23 +288,7 @@ func serverCmdArgs(ctx *cli.Context) []string {
 	return strings.Fields(v)
 }

-func mergeServerCtxtFromConfigFile(configFile string, ctxt *serverCtxt) error {
-	rd, err := Open(configFile)
-	if err != nil {
-		return err
-	}
-	defer rd.Close()
-
-	cf := &config.ServerConfig{}
-	dec := yaml.NewDecoder(rd)
-	dec.SetStrict(true)
-	if err = dec.Decode(cf); err != nil {
-		return err
-	}
-	if cf.Version != "v1" {
-		return fmt.Errorf("unexpected version: %s", cf.Version)
-	}
-
+func configCommonToSrvCtx(cf config.ServerConfigCommon, ctxt *serverCtxt) {
 	ctxt.RootUser = cf.RootUser
 	ctxt.RootPwd = cf.RootPwd

@ -331,8 +316,75 @@ func mergeServerCtxtFromConfigFile(configFile string, ctxt *serverCtxt) error {
 	if cf.Options.SFTP.SSHPrivateKey != "" {
 		ctxt.SFTP = append(ctxt.SFTP, fmt.Sprintf("ssh-private-key=%s", cf.Options.SFTP.SSHPrivateKey))
 	}
+}

-	ctxt.Layout, err = buildDisksLayoutFromConfFile(cf.Pools)
+func mergeServerCtxtFromConfigFile(configFile string, ctxt *serverCtxt) error {
+	rd, err := xioutil.ReadFile(configFile)
+	if err != nil {
+		return err
+	}
+
+	cfReader := bytes.NewReader(rd)
+
+	cv := config.ServerConfigVersion{}
+	if err = yaml.Unmarshal(rd, &cv); err != nil {
+		return err
+	}
+
+	switch cv.Version {
+	case "v1", "v2":
+	default:
+		return fmt.Errorf("unexpected version: %s", cv.Version)
+	}
+
+	cfCommon := config.ServerConfigCommon{}
+	if err = yaml.Unmarshal(rd, &cfCommon); err != nil {
+		return err
+	}
+
+	configCommonToSrvCtx(cfCommon, ctxt)
+
+	v, err := env.GetInt(EnvErasureSetDriveCount, 0)
+	if err != nil {
+		return err
+	}
+	setDriveCount := uint64(v)
+
+	var pools []poolArgs
+	switch cv.Version {
+	case "v1":
+		cfV1 := config.ServerConfigV1{}
+		if err = yaml.Unmarshal(rd, &cfV1); err != nil {
+			return err
+		}
+
+		pools = make([]poolArgs, 0, len(cfV1.Pools))
+		for _, list := range cfV1.Pools {
+			pools = append(pools, poolArgs{
+				args:          list,
+				setDriveCount: setDriveCount,
+			})
+		}
+	case "v2":
+		cf := config.ServerConfig{}
+		cfReader.Seek(0, io.SeekStart)
+		if err = yaml.Unmarshal(rd, &cf); err != nil {
+			return err
+		}
+
+		pools = make([]poolArgs, 0, len(cf.Pools))
+		for _, list := range cf.Pools {
+			driveCount := list.SetDriveCount
+			if setDriveCount > 0 {
+				driveCount = setDriveCount
+			}
+			pools = append(pools, poolArgs{
+				args:          list.Args,
+				setDriveCount: driveCount,
+			})
+		}
+	}
+	ctxt.Layout, err = buildDisksLayoutFromConfFile(pools)
 	return err
 }

--- a/docs/distributed/CONFIG.md
+++ b/docs/distributed/CONFIG.md
@ -15,34 +15,57 @@ minio server --config config.yaml
 Lets you start MinIO server with all inputs to start MinIO server provided via this configuration file, once the configuration file is provided all other pre-existing values on disk for configuration are overridden by the new values set in this configuration file.

 Following is an example YAML configuration structure.
-```
-version: v1
-address: ':9000'
-rootUser: 'minioadmin'
-rootPassword: 'pBU94AGAY85e'
-console-address: ':9001'
-certs-dir: '/home/user/.minio/certs/'
+```yaml
+version: v2
+address: ":9000"
+rootUser: "minioadmin"
+rootPassword: "minioadmin"
+console-address: ":9001"
+certs-dir: "/home/user/.minio/certs/"
 pools: # Specify the nodes and drives with pools
-  -
-	- 'https://server-example-pool1:9000/mnt/disk{1...4}/'
-	- 'https://server{1...2}-pool1:9000/mnt/disk{1...4}/'
-	- 'https://server3-pool1:9000/mnt/disk{1...4}/'
-	- 'https://server4-pool1:9000/mnt/disk{1...4}/'
-  -
-	- 'https://server-example-pool2:9000/mnt/disk{1...4}/'
-	- 'https://server{1...2}-pool2:9000/mnt/disk{1...4}/'
-	- 'https://server3-pool2:9000/mnt/disk{1...4}/'
-	- 'https://server4-pool2:9000/mnt/disk{1...4}/'
-
-...
+  - args:
+      - "https://server-example-pool1:9000/mnt/disk{1...4}/"
+      - "https://server{1...2}-pool1:9000/mnt/disk{1...4}/"
+      - "https://server3-pool1:9000/mnt/disk{1...4}/"
+      - "https://server4-pool1:9000/mnt/disk{1...4}/"
+  - args:
+      - "https://server-example-pool2:9000/mnt/disk{1...4}/"
+      - "https://server{1...2}-pool2:9000/mnt/disk{1...4}/"
+      - "https://server3-pool2:9000/mnt/disk{1...4}/"
+      - "https://server4-pool2:9000/mnt/disk{1...4}/"
+  # more args

 options:
  ftp: # settings for MinIO to act as an ftp server
-	address: ':8021'
-	passive-port-range: '30000-40000'
+    address: ":8021"
+    passive-port-range: "30000-40000"
  sftp: # settings for MinIO to act as an sftp server
-	address: ':8022'
-	ssh-private-key: '/home/user/.ssh/id_rsa'
+    address: ":8022"
+    ssh-private-key: "/home/user/.ssh/id_rsa"
+```
+
+If you are using the config `v1` YAML you should migrate your `pools:` field values to the following format
+
+`v1` format
+```yaml
+pools: # Specify the nodes and drives with pools
+  -
+    - "https://server-example-pool1:9000/mnt/disk{1...4}/"
+    - "https://server{1...2}-pool1:9000/mnt/disk{1...4}/"
+    - "https://server3-pool1:9000/mnt/disk{1...4}/"
+    - "https://server4-pool1:9000/mnt/disk{1...4}/"
+```
+
+to `v2` format
+
+```yaml
+pools:
+  - args:
+      - "https://server-example-pool1:9000/mnt/disk{1...4}/"
+      - "https://server{1...2}-pool1:9000/mnt/disk{1...4}/"
+      - "https://server3-pool1:9000/mnt/disk{1...4}/"
+      - "https://server4-pool1:9000/mnt/disk{1...4}/"
+    set-drive-count: 4 # Advanced option, must be used under guidance from MinIO team.
 ```

 ### Things to know
--- a/internal/config/server.go
+++ b/internal/config/server.go
@ -29,14 +29,34 @@ type Opts struct {
 	} `yaml:"sftp"`
 }

+// ServerConfigVersion struct is used to extract the version
+type ServerConfigVersion struct {
+	Version string `yaml:"version"`
+}
+
+// ServerConfigCommon struct for server config common options
+type ServerConfigCommon struct {
+	RootUser    string `yaml:"rootUser"`
+	RootPwd     string `yaml:"rootPassword"`
+	Addr        string `yaml:"address"`
+	ConsoleAddr string `yaml:"console-address"`
+	CertsDir    string `yaml:"certs-dir"`
+	Options     Opts   `yaml:"options"`
+}
+
+// ServerConfigV1 represents a MinIO configuration file v1
+type ServerConfigV1 struct {
+	ServerConfigVersion
+	ServerConfigCommon
+	Pools [][]string `yaml:"pools"`
+}
+
 // ServerConfig represents a MinIO configuration file
 type ServerConfig struct {
-	Version     string     `yaml:"version"`
-	RootUser    string     `yaml:"rootUser"`
-	RootPwd     string     `yaml:"rootPassword"`
-	Addr        string     `yaml:"address"`
-	ConsoleAddr string     `yaml:"console-address"`
-	CertsDir    string     `yaml:"certs-dir"`
-	Pools       [][]string `yaml:"pools"`
-	Options     Opts       `yaml:"options"`
+	ServerConfigVersion
+	ServerConfigCommon
+	Pools []struct {
+		Args          []string `yaml:"args"`
+		SetDriveCount uint64   `yaml:"set-drive-count"`
+	} `yaml:"pools"`
 }