Add proper contexts with timeouts for etcd operations (#7097)

This fixes an issue of perceived hang when incorrect
unreachable URLs are specified in MINIO_ETCD_ENDPOINTS
variable.

Fixes #7096
This commit is contained in:
Harshavardhana 2019-01-18 23:06:45 +05:30 committed by kannappanr
parent 730ac5381c
commit 74c2048ea9
5 changed files with 56 additions and 33 deletions

View File

@ -242,6 +242,7 @@ func handleCommonEnvVars() {
return &cert, terr return &cert, terr
} }
} }
globalEtcdClient, err = etcd.New(etcd.Config{ globalEtcdClient, err = etcd.New(etcd.Config{
Endpoints: etcdEndpoints, Endpoints: etcdEndpoints,
DialTimeout: defaultDialTimeout, DialTimeout: defaultDialTimeout,

View File

@ -20,6 +20,7 @@ import (
"bytes" "bytes"
"context" "context"
"errors" "errors"
"fmt"
etcd "github.com/coreos/etcd/clientv3" etcd "github.com/coreos/etcd/clientv3"
"github.com/minio/minio/cmd/logger" "github.com/minio/minio/cmd/logger"
@ -60,8 +61,15 @@ func deleteConfig(ctx context.Context, objAPI ObjectLayer, configFile string) er
} }
func saveConfigEtcd(ctx context.Context, client *etcd.Client, configFile string, data []byte) error { func saveConfigEtcd(ctx context.Context, client *etcd.Client, configFile string, data []byte) error {
_, err := client.Put(ctx, configFile, string(data)) timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
return err defer cancel()
_, err := client.Put(timeoutCtx, configFile, string(data))
if err == context.DeadlineExceeded {
return fmt.Errorf("etcd setup is unreachable, please check your endpoints %s", client.Endpoints())
} else if err != nil {
return fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s", err, client.Endpoints())
}
return nil
} }
func saveConfig(ctx context.Context, objAPI ObjectLayer, configFile string, data []byte) error { func saveConfig(ctx context.Context, objAPI ObjectLayer, configFile string, data []byte) error {
@ -75,9 +83,14 @@ func saveConfig(ctx context.Context, objAPI ObjectLayer, configFile string, data
} }
func readConfigEtcd(ctx context.Context, client *etcd.Client, configFile string) ([]byte, error) { func readConfigEtcd(ctx context.Context, client *etcd.Client, configFile string) ([]byte, error) {
resp, err := client.Get(ctx, configFile) timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
defer cancel()
resp, err := client.Get(timeoutCtx, configFile)
if err != nil { if err != nil {
return nil, err if err == context.DeadlineExceeded {
return nil, fmt.Errorf("etcd setup is unreachable, please check your endpoints %s", client.Endpoints())
}
return nil, fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s", err, client.Endpoints())
} }
if resp.Count == 0 { if resp.Count == 0 {
return nil, errConfigNotFound return nil, errConfigNotFound
@ -93,7 +106,9 @@ func readConfigEtcd(ctx context.Context, client *etcd.Client, configFile string)
// watchConfig - watches for changes on `configFile` on etcd and loads them. // watchConfig - watches for changes on `configFile` on etcd and loads them.
func watchConfig(objAPI ObjectLayer, configFile string, loadCfgFn func(ObjectLayer) error) { func watchConfig(objAPI ObjectLayer, configFile string, loadCfgFn func(ObjectLayer) error) {
if globalEtcdClient != nil { if globalEtcdClient != nil {
for watchResp := range globalEtcdClient.Watch(context.Background(), configFile) { ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout)
defer cancel()
for watchResp := range globalEtcdClient.Watch(ctx, configFile) {
for _, event := range watchResp.Events { for _, event := range watchResp.Events {
if event.IsModify() || event.IsCreate() { if event.IsModify() || event.IsCreate() {
loadCfgFn(objAPI) loadCfgFn(objAPI)
@ -104,9 +119,14 @@ func watchConfig(objAPI ObjectLayer, configFile string, loadCfgFn func(ObjectLay
} }
func checkConfigEtcd(ctx context.Context, client *etcd.Client, configFile string) error { func checkConfigEtcd(ctx context.Context, client *etcd.Client, configFile string) error {
resp, err := globalEtcdClient.Get(ctx, configFile) timeoutCtx, cancel := context.WithTimeout(ctx, defaultContextTimeout)
defer cancel()
resp, err := client.Get(timeoutCtx, configFile)
if err != nil { if err != nil {
return err if err == context.DeadlineExceeded {
return fmt.Errorf("etcd setup is unreachable, please check your endpoints %s", client.Endpoints())
}
return fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s", err, client.Endpoints())
} }
if resp.Count == 0 { if resp.Count == 0 {
return errConfigNotFound return errConfigNotFound

View File

@ -24,7 +24,6 @@ import (
"path" "path"
"runtime" "runtime"
"strings" "strings"
"time"
"github.com/minio/minio/cmd/logger" "github.com/minio/minio/cmd/logger"
"github.com/minio/minio/pkg/quick" "github.com/minio/minio/pkg/quick"
@ -52,10 +51,7 @@ func saveServerConfig(ctx context.Context, objAPI ObjectLayer, config *serverCon
configFile := path.Join(minioConfigPrefix, minioConfigFile) configFile := path.Join(minioConfigPrefix, minioConfigFile)
if globalEtcdClient != nil { if globalEtcdClient != nil {
timeoutCtx, cancel := context.WithTimeout(ctx, 5*time.Minute) return saveConfigEtcd(ctx, globalEtcdClient, configFile, data)
_, err = globalEtcdClient.Put(timeoutCtx, configFile, string(data))
defer cancel()
return err
} }
// Create a backup of the current config // Create a backup of the current config
@ -163,19 +159,17 @@ func initConfig(objAPI ObjectLayer) error {
} }
if globalEtcdClient != nil { if globalEtcdClient != nil {
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) if err := checkConfigEtcd(context.Background(), globalEtcdClient, getConfigFile()); err != nil {
resp, err := globalEtcdClient.Get(ctx, getConfigFile()) return err
cancel() }
if err == nil && resp.Count > 0 { // Migrates all configs at old location.
if err = migrateConfig(); err != nil { if err := migrateConfig(); err != nil {
return err return err
} }
// Migrates etcd ${HOME}/.minio/config.json to '/config/config.json' // Migrates etcd ${HOME}/.minio/config.json to '/config/config.json'
if err := migrateConfigToMinioSys(objAPI); err != nil { if err := migrateConfigToMinioSys(objAPI); err != nil {
return err return err
} }
}
} else { } else {
if isFile(getConfigFile()) { if isFile(getConfigFile()) {
if err := migrateConfig(); err != nil { if err := migrateConfig(); err != nil {

View File

@ -464,13 +464,13 @@ func (sys *IAMSys) IsAllowed(args iampolicy.Args) bool {
return args.IsOwner return args.IsOwner
} }
var defaultContextTimeout = 5 * time.Minute var defaultContextTimeout = 30 * time.Second
// Similar to reloadUsers but updates users, policies maps from etcd server, // Similar to reloadUsers but updates users, policies maps from etcd server,
func reloadEtcdUsers(prefix string, usersMap map[string]auth.Credentials, policyMap map[string]string) error { func reloadEtcdUsers(prefix string, usersMap map[string]auth.Credentials, policyMap map[string]string) error {
ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout) ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout)
r, err := globalEtcdClient.Get(ctx, prefix, etcd.WithPrefix(), etcd.WithKeysOnly())
defer cancel() defer cancel()
r, err := globalEtcdClient.Get(ctx, prefix, etcd.WithPrefix(), etcd.WithKeysOnly())
if err != nil { if err != nil {
return err return err
} }
@ -536,8 +536,8 @@ func reloadEtcdUsers(prefix string, usersMap map[string]auth.Credentials, policy
func reloadEtcdPolicies(prefix string, cannedPolicyMap map[string]iampolicy.Policy) error { func reloadEtcdPolicies(prefix string, cannedPolicyMap map[string]iampolicy.Policy) error {
ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout) ctx, cancel := context.WithTimeout(context.Background(), defaultContextTimeout)
r, err := globalEtcdClient.Get(ctx, prefix, etcd.WithPrefix(), etcd.WithKeysOnly())
defer cancel() defer cancel()
r, err := globalEtcdClient.Get(ctx, prefix, etcd.WithPrefix(), etcd.WithKeysOnly())
if err != nil { if err != nil {
return err return err
} }

View File

@ -138,18 +138,26 @@ func saveFileConfigEtcd(filename string, clnt *etcd.Client, v interface{}) error
dataBytes = []byte(strings.Replace(string(dataBytes), "\n", "\r\n", -1)) dataBytes = []byte(strings.Replace(string(dataBytes), "\n", "\r\n", -1))
} }
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
_, err = clnt.Put(ctx, filename, string(dataBytes))
defer cancel() defer cancel()
return err _, err = clnt.Put(ctx, filename, string(dataBytes))
if err == context.DeadlineExceeded {
return fmt.Errorf("etcd setup is unreachable, please check your endpoints %s", clnt.Endpoints())
} else if err != nil {
return fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s", err, clnt.Endpoints())
}
return nil
} }
func loadFileConfigEtcd(filename string, clnt *etcd.Client, v interface{}) error { func loadFileConfigEtcd(filename string, clnt *etcd.Client, v interface{}) error {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
resp, err := clnt.Get(ctx, filename)
defer cancel() defer cancel()
resp, err := clnt.Get(ctx, filename)
if err != nil { if err != nil {
return err if err == context.DeadlineExceeded {
return fmt.Errorf("etcd setup is unreachable, please check your endpoints %s", clnt.Endpoints())
}
return fmt.Errorf("unexpected error %s returned by etcd setup, please check your endpoints %s", err, clnt.Endpoints())
} }
if resp.Count == 0 { if resp.Count == 0 {
return dns.ErrNoEntriesFound return dns.ErrNoEntriesFound