From 56bde5df31d34b4599005920f71de3207bb34019 Mon Sep 17 00:00:00 2001 From: Nitish Tiwari Date: Tue, 9 Jan 2018 11:56:13 +0530 Subject: [PATCH] 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 | 43 ++++++++++++++----------------------- cmd/storage-class.go | 49 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/cmd/config-current.go b/cmd/config-current.go index 53d51d39d..ea40c0f2d 100644 --- a/cmd/config-current.go +++ b/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.Scheme + strconv.Itoa(standardClass.Parity) - s.StorageClass.RRS = rrsClass.Scheme + strconv.Itoa(rrsClass.Parity) + s.StorageClass.Standard = standardClass + s.StorageClass.RRS = rrsClass } // 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, - Credential: auth.MustGetNewCredentials(), - Region: globalMinioDefaultRegion, - Browser: true, - Notify: ¬ifier{}, + Version: serverConfigVersion, + Credential: auth.MustGetNewCredentials(), + Region: globalMinioDefaultRegion, + Browser: true, + StorageClass: storageClassConfig{}, + Notify: ¬ifier{}, } // Make sure to initialize notification configs. diff --git a/cmd/storage-class.go b/cmd/storage-class.go index dc56ebb75..719390bc3 100644 --- a/cmd/storage-class.go +++ b/cmd/storage-class.go @@ -50,8 +50,8 @@ type storageClass struct { } type storageClassConfig struct { - Standard string `json:"standard"` - RRS string `json:"rrs"` + Standard storageClass `json:"standard"` + 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 { - return fmt.Errorf("Reduced redundancy storage class not supported for " + strconv.Itoa(len(globalEndpoints)) + " disk setup") + if disks == 4 && rrsParity != 0 { + 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 { - return fmt.Errorf("Reduced redundancy storage class parity disks should be less than " + strconv.Itoa(len(globalEndpoints)/2)) + if rrsParity >= disks/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 { - return fmt.Errorf("Standard storage class parity disks should be less than or equal to " + strconv.Itoa(len(globalEndpoints)/2)) + if ssParity > disks/2 { + return fmt.Errorf("Standard storage class parity disks should be less than or equal to " + strconv.Itoa(disks/2)) } return nil