From ee7dcc2903477e8340dfaf2270093768549df68d Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 24 Jan 2019 00:40:59 +0530 Subject: [PATCH] Handle errs returned with etcd properly for config init and migration (#7134) Returning unexpected errors can cause problems for config handling, which is what led gateway deployments with etcd to misbehave and had stopped working properly --- cmd/admin-handlers_test.go | 2 +- cmd/admin-router.go | 10 ++++++---- cmd/config-migrate.go | 11 +++++++---- cmd/config.go | 35 +++++++++++++---------------------- cmd/gateway-main.go | 9 ++++++--- cmd/routers.go | 2 +- pkg/quick/encoding.go | 5 ++--- 7 files changed, 36 insertions(+), 38 deletions(-) diff --git a/cmd/admin-handlers_test.go b/cmd/admin-handlers_test.go index dc69355fd..c50620e6c 100644 --- a/cmd/admin-handlers_test.go +++ b/cmd/admin-handlers_test.go @@ -278,7 +278,7 @@ func prepareAdminXLTestBed() (*adminXLTestBed, error) { // Setup admin mgmt REST API handlers. adminRouter := mux.NewRouter() - registerAdminRouter(adminRouter, true) + registerAdminRouter(adminRouter, true, true) return &adminXLTestBed{ xlDirs: xlDirs, diff --git a/cmd/admin-router.go b/cmd/admin-router.go index 175b77f13..acef0035b 100644 --- a/cmd/admin-router.go +++ b/cmd/admin-router.go @@ -31,7 +31,7 @@ type adminAPIHandlers struct { } // registerAdminRouter - Add handler functions for each service REST API routes. -func registerAdminRouter(router *mux.Router, enableIAM bool) { +func registerAdminRouter(router *mux.Router, enableConfigOps, enableIAMOps bool) { adminAPI := adminAPIHandlers{} // Admin router @@ -73,8 +73,7 @@ func registerAdminRouter(router *mux.Router, enableIAM bool) { adminV1Router.Methods(http.MethodGet).Path("/profiling/download").HandlerFunc(httpTraceAll(adminAPI.DownloadProfilingHandler)) /// Config operations - - if enableIAM { + if enableConfigOps { // Update credentials adminV1Router.Methods(http.MethodPut).Path("/config/credential").HandlerFunc(httpTraceHdrs(adminAPI.UpdateAdminCredentialsHandler)) // Get config @@ -86,11 +85,14 @@ func registerAdminRouter(router *mux.Router, enableIAM bool) { adminV1Router.Methods(http.MethodGet).Path("/config-keys").HandlerFunc(httpTraceHdrs(adminAPI.GetConfigKeysHandler)) // Set config keys/values adminV1Router.Methods(http.MethodPut).Path("/config-keys").HandlerFunc(httpTraceHdrs(adminAPI.SetConfigKeysHandler)) + } + if enableIAMOps { // -- IAM APIs -- // Add policy IAM - adminV1Router.Methods(http.MethodPut).Path("/add-canned-policy").HandlerFunc(httpTraceHdrs(adminAPI.AddCannedPolicy)).Queries("name", "{name:.*}") + adminV1Router.Methods(http.MethodPut).Path("/add-canned-policy").HandlerFunc(httpTraceHdrs(adminAPI.AddCannedPolicy)).Queries("name", + "{name:.*}") // Add user IAM adminV1Router.Methods(http.MethodPut).Path("/add-user").HandlerFunc(httpTraceHdrs(adminAPI.AddUser)).Queries("accessKey", "{accessKey:.*}") diff --git a/cmd/config-migrate.go b/cmd/config-migrate.go index 5175f03cd..799a5c52b 100644 --- a/cmd/config-migrate.go +++ b/cmd/config-migrate.go @@ -27,7 +27,6 @@ import ( "github.com/minio/minio/cmd/crypto" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/auth" - "github.com/minio/minio/pkg/dns" "github.com/minio/minio/pkg/event" "github.com/minio/minio/pkg/event/target" "github.com/minio/minio/pkg/iam/policy" @@ -235,7 +234,7 @@ func purgeV1() error { cv1 := &configV1{} _, err := Load(configFile, cv1) - if os.IsNotExist(err) || err == dns.ErrNoEntriesFound { + if os.IsNotExist(err) { return nil } else if err != nil { return fmt.Errorf("Unable to load config version ‘1’. %v", err) @@ -2454,10 +2453,14 @@ func migrateConfigToMinioSys(objAPI ObjectLayer) (err error) { } // Read from deprecate file as well if necessary. if _, err = Load(getConfigFile()+".deprecated", config); err != nil { - return err + if !os.IsNotExist(err) { + return err + } + // If all else fails simply initialize the server config. + return newSrvConfig(objAPI) } - } + } return saveServerConfig(context.Background(), objAPI, config) } diff --git a/cmd/config.go b/cmd/config.go index 8799c263f..a6d43c63a 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -20,7 +20,6 @@ import ( "bytes" "context" "encoding/json" - "os" "path" "runtime" "strings" @@ -160,15 +159,18 @@ func initConfig(objAPI ObjectLayer) error { if globalEtcdClient != nil { if err := checkConfigEtcd(context.Background(), globalEtcdClient, getConfigFile()); err != nil { - return err - } - // Migrates all configs at old location. - if err := migrateConfig(); err != nil { - return err - } - // Migrates etcd ${HOME}/.minio/config.json to '/config/config.json' - if err := migrateConfigToMinioSys(objAPI); err != nil { - return err + if err == errConfigNotFound { + // Migrates all configs at old location. + if err = migrateConfig(); err != nil { + return err + } + // Migrates etcd ${HOME}/.minio/config.json to '/config/config.json' + if err = migrateConfigToMinioSys(objAPI); err != nil { + return err + } + } else { + return err + } } } else { if isFile(getConfigFile()) { @@ -179,7 +181,7 @@ func initConfig(objAPI ObjectLayer) error { // Migrates ${HOME}/.minio/config.json or config.json.deprecated // to '/.minio.sys/config/config.json' // ignore if the file doesn't exist. - if err := migrateConfigToMinioSys(objAPI); err != nil && !os.IsNotExist(err) { + if err := migrateConfigToMinioSys(objAPI); err != nil { return err } } @@ -189,17 +191,6 @@ func initConfig(objAPI ObjectLayer) error { // Watch config for changes and reloads them in-memory. go watchConfig(objAPI, configFile, loadConfig) - if err := checkConfig(context.Background(), objAPI, configFile); err != nil { - if err == errConfigNotFound { - // Config file does not exist, we create it fresh and return upon success. - if err = newSrvConfig(objAPI); err != nil { - return err - } - } else { - return err - } - } - if err := migrateMinioSysConfig(objAPI); err != nil { return err } diff --git a/cmd/gateway-main.go b/cmd/gateway-main.go index d22856c00..43cb15f0a 100644 --- a/cmd/gateway-main.go +++ b/cmd/gateway-main.go @@ -155,9 +155,12 @@ func StartGateway(ctx *cli.Context, gw Gateway) { registerSTSRouter(router) } + enableConfigOps := globalEtcdClient != nil && gatewayName == "nas" + enableIAMOps := globalEtcdClient != nil + // Enable IAM admin APIs if etcd is enabled, if not just enable basic // operations such as profiling, server info etc. - registerAdminRouter(router, globalEtcdClient != nil) + registerAdminRouter(router, enableConfigOps, enableIAMOps) // Add healthcheck router registerHealthCheckRouter(router) @@ -215,7 +218,7 @@ func StartGateway(ctx *cli.Context, gw Gateway) { logger.FatalIf(err, "Unable to initialize gateway backend") } - if globalEtcdClient != nil && gatewayName == "nas" { + if enableConfigOps { // Create a new config system. globalConfigSys = NewConfigSys() @@ -242,7 +245,7 @@ func StartGateway(ctx *cli.Context, gw Gateway) { // Create new IAM system. globalIAMSys = NewIAMSys() - if globalEtcdClient != nil { + if enableIAMOps { // Initialize IAM sys. _ = globalIAMSys.Init(newObject) } diff --git a/cmd/routers.go b/cmd/routers.go index 3fa047d87..877272401 100644 --- a/cmd/routers.go +++ b/cmd/routers.go @@ -105,7 +105,7 @@ func configureServerHandler(endpoints EndpointList) (http.Handler, error) { registerSTSRouter(router) // Add Admin router, all APIs are enabled in server mode. - registerAdminRouter(router, true) + registerAdminRouter(router, true, true) // Add healthcheck router registerHealthCheckRouter(router) diff --git a/pkg/quick/encoding.go b/pkg/quick/encoding.go index 562abbdd0..8e8bd7664 100644 --- a/pkg/quick/encoding.go +++ b/pkg/quick/encoding.go @@ -31,7 +31,6 @@ import ( "time" etcd "github.com/coreos/etcd/clientv3" - dns "github.com/minio/minio/pkg/dns" yaml "gopkg.in/yaml.v2" ) @@ -160,7 +159,7 @@ func loadFileConfigEtcd(filename string, clnt *etcd.Client, v interface{}) error return fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s", err, clnt.Endpoints()) } if resp.Count == 0 { - return dns.ErrNoEntriesFound + return os.ErrNotExist } for _, ev := range resp.Kvs { @@ -173,7 +172,7 @@ func loadFileConfigEtcd(filename string, clnt *etcd.Client, v interface{}) error return toUnmarshaller(filepath.Ext(filename))(fileData, v) } } - return dns.ErrNoEntriesFound + return os.ErrNotExist } // loadFileConfig unmarshals the file's content with the right