Refactor storage class parsing for Gateway mode (#5331)

This PR updates the behaviour to print relevant error message
if storage class is set in config.json for gateway

This PR also fixes the case where storage class set via
environment variables is not parsed properly into config.json.

cmd/config-current.go

@@ -20,7 +20,6 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
-	"strconv"
 	"sync"
 
 	"github.com/minio/minio/pkg/auth"
@@ -108,46 +107,35 @@ func (s *serverConfig) SetStorageClass(standardClass, rrsClass storageClass) {
 	s.Lock()
 	defer s.Unlock()
 
-	// Set the values
+	s.StorageClass.Standard = standardClass
-	s.StorageClass.Standard = standardClass.Scheme + strconv.Itoa(standardClass.Parity)
+	s.StorageClass.RRS = rrsClass
-	s.StorageClass.RRS = rrsClass.Scheme + strconv.Itoa(rrsClass.Parity)
 }
 
 // GetStorageClass reads storage class fields from current config, parses and validates it.
 // It returns the standard and reduced redundancy storage class struct
-func (s *serverConfig) GetStorageClass() (ssc, rrsc storageClass) {
+func (s *serverConfig) GetStorageClass() (storageClass, storageClass) {
 	s.RLock()
 	defer s.RUnlock()
 
 	var err error
+	// Storage Class from config.json is already parsed and stored in s.StorageClass
+	// Now validate the storage class fields
+	ssc := s.StorageClass.Standard
+	rrsc := s.StorageClass.RRS
 
-	if s.StorageClass.Standard != "" {
-		// Parse the values read from config file into storageClass struct
-		ssc, err = parseStorageClass(s.StorageClass.Standard)
-		fatalIf(err, "Invalid value %s set in config.json", s.StorageClass.Standard)
-	}
-
-	if s.StorageClass.RRS != "" {
-		// Parse the values read from config file into storageClass struct
-		rrsc, err = parseStorageClass(s.StorageClass.RRS)
-		fatalIf(err, "Invalid value %s set in config.json", s.StorageClass.RRS)
-	}
-
-	// Validation is done after parsing both the storage classes. This is needed because we need one
-	// storage class value to deduce the correct value of the other storage class.
 	if rrsc.Scheme != "" {
 		err = validateRRSParity(rrsc.Parity, ssc.Parity)
-		fatalIf(err, "Invalid value %s set in config.json", s.StorageClass.RRS)
+		fatalIf(err, "Invalid value %s:%d set in config.json", rrsc.Scheme, rrsc.Parity)
 		globalIsStorageClass = true
 	}
 
 	if ssc.Scheme != "" {
 		err = validateSSParity(ssc.Parity, rrsc.Parity)
-		fatalIf(err, "Invalid value %s set in config.json", s.StorageClass.Standard)
+		fatalIf(err, "Invalid value %s:%d set in config.json", ssc.Scheme, ssc.Parity)
 		globalIsStorageClass = true
 	}
 
-	return
+	return s.StorageClass.Standard, s.StorageClass.RRS
 }
 
 // GetCredentials get current credentials.
@@ -169,11 +157,12 @@ func (s *serverConfig) Save() error {
 
 func newServerConfig() *serverConfig {
 	srvCfg := &serverConfig{
-		Version:    serverConfigVersion,
+		Version:      serverConfigVersion,
-		Credential: auth.MustGetNewCredentials(),
+		Credential:   auth.MustGetNewCredentials(),
-		Region:     globalMinioDefaultRegion,
+		Region:       globalMinioDefaultRegion,
-		Browser:    true,
+		Browser:      true,
-		Notify:     &notifier{},
+		StorageClass: storageClassConfig{},
+		Notify:       &notifier{},
 	}
 
 	// Make sure to initialize notification configs.

cmd/storage-class.go

@@ -50,8 +50,8 @@ type storageClass struct {
 }
 
 type storageClassConfig struct {
-	Standard string `json:"standard"`
+	Standard storageClass `json:"standard"`
-	RRS      string `json:"rrs"`
+	RRS      storageClass `json:"rrs"`
 }
 
 // Validate if storage class in metadata
@@ -60,6 +60,29 @@ func isValidStorageClassMeta(sc string) bool {
 	return sc == reducedRedundancyStorageClass || sc == standardStorageClass
 }
 
+func (sc *storageClass) UnmarshalText(b []byte) error {
+	scStr := string(b)
+	if scStr != "" {
+		s, err := parseStorageClass(scStr)
+		if err != nil {
+			return err
+		}
+		sc.Parity = s.Parity
+		sc.Scheme = s.Scheme
+	} else {
+		sc = &storageClass{}
+	}
+
+	return nil
+}
+
+func (sc *storageClass) MarshalText() ([]byte, error) {
+	if sc.Scheme != "" && sc.Parity != 0 {
+		return []byte(fmt.Sprintf("%s:%d", sc.Scheme, sc.Parity)), nil
+	}
+	return []byte(""), nil
+}
+
 // Parses given storageClassEnv and returns a storageClass structure.
 // Supported Storage Class format is "Scheme:Number of parity disks".
 // Currently only supported scheme is "EC".
@@ -94,10 +117,15 @@ func parseStorageClass(storageClassEnv string) (sc storageClass, err error) {
 
 // Validates the parity disks for Reduced Redundancy storage class
 func validateRRSParity(rrsParity, ssParity int) (err error) {
+	disks := len(globalEndpoints)
+	// disks < 4 means this is not a erasure coded setup and so storage class is not supported
+	if disks < 4 {
+		return fmt.Errorf("Setting storage class only allowed for erasure coding mode")
+	}
 
 	// Reduced redundancy storage class is not supported for 4 disks erasure coded setup.
-	if len(globalEndpoints) == 4 && rrsParity != 0 {
+	if disks == 4 && rrsParity != 0 {
-		return fmt.Errorf("Reduced redundancy storage class not supported for " + strconv.Itoa(len(globalEndpoints)) + " disk setup")
+		return fmt.Errorf("Reduced redundancy storage class not supported for " + strconv.Itoa(disks) + " disk setup")
 	}
 
 	// RRS parity disks should be greater than or equal to minimumParityDisks. Parity below minimumParityDisks is not recommended.
@@ -110,8 +138,8 @@ func validateRRSParity(rrsParity, ssParity int) (err error) {
 	// - less than StorageClass Parity, if Storage class parity is set.
 	switch ssParity {
 	case 0:
-		if rrsParity >= len(globalEndpoints)/2 {
+		if rrsParity >= disks/2 {
-			return fmt.Errorf("Reduced redundancy storage class parity disks should be less than " + strconv.Itoa(len(globalEndpoints)/2))
+			return fmt.Errorf("Reduced redundancy storage class parity disks should be less than " + strconv.Itoa(disks/2))
 		}
 	default:
 		if rrsParity >= ssParity {
@@ -124,6 +152,11 @@ func validateRRSParity(rrsParity, ssParity int) (err error) {
 
 // Validates the parity disks for Standard storage class
 func validateSSParity(ssParity, rrsParity int) (err error) {
+	disks := len(globalEndpoints)
+	// disks < 4 means this is not a erasure coded setup and so storage class is not supported
+	if disks < 4 {
+		return fmt.Errorf("Setting storage class only allowed for erasure coding mode")
+	}
 
 	// Standard storage class implies more parity than Reduced redundancy storage class. So, Standard storage parity disks should be
 	// - greater than or equal to 2, if RRS parity is not set.
@@ -140,8 +173,8 @@ func validateSSParity(ssParity, rrsParity int) (err error) {
 	}
 
 	// Standard storage class parity should be less than or equal to N/2
-	if ssParity > len(globalEndpoints)/2 {
+	if ssParity > disks/2 {
-		return fmt.Errorf("Standard storage class parity disks should be less than or equal to " + strconv.Itoa(len(globalEndpoints)/2))
+		return fmt.Errorf("Standard storage class parity disks should be less than or equal to " + strconv.Itoa(disks/2))
 	}
 
 	return nil