From 6a4ef2e48e7d870a6fb198abb83c3e0ddd559ab6 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Tue, 8 Oct 2019 23:11:15 -0700 Subject: [PATCH] Initialize configs correctly, move notification config (#8367) This PR also removes deprecated tests, adds checks to avoid races reproduced on CI/CD. --- .travis.yml | 2 +- Makefile | 6 +- cmd/config-current.go | 88 +-------------- cmd/config-current_test.go | 130 ----------------------- cmd/config-versions.go | 40 +------ cmd/config/notify/config.go | 66 ++++++++++++ cmd/config/storageclass/storage-class.go | 12 +-- cmd/lock-rest-client.go | 54 +++------- cmd/lock-rest-client_test.go | 2 +- cmd/lock-rest-server.go | 2 +- cmd/logger/config.go | 24 +++++ cmd/peer-rest-client.go | 19 ++-- cmd/storage-rest-client.go | 21 ++-- go.mod | 2 +- go.sum | 2 + 15 files changed, 149 insertions(+), 321 deletions(-) create mode 100644 cmd/config/notify/config.go diff --git a/.travis.yml b/.travis.yml index f509df47d..96996c5fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,7 +31,7 @@ matrix: - make - diff -au <(gofmt -s -d cmd) <(printf "") - diff -au <(gofmt -s -d pkg) <(printf "") - - for d in $(go list ./... | grep -v browser); do CGO_ENABLED=1 go test -v -race --timeout 15m "$d"; done + - for d in $(go list ./... | grep -v browser); do CGO_ENABLED=1 go test -v -race --timeout 20m "$d"; done - make verifiers - make crosscompile - make verify diff --git a/Makefile b/Makefile index 59ce5c840..fcbd5b9ad 100644 --- a/Makefile +++ b/Makefile @@ -64,10 +64,12 @@ test: verifiers build @echo "Running unit tests" @GO111MODULE=on CGO_ENABLED=0 go test -tags kqueue ./... 1>/dev/null -# Verify minio binary, enable races as well +# Verify minio binary +# TODO: enable races as well +# @GO111MODULE=on CGO_ENABLED=1 go build -race -tags kqueue --ldflags $(BUILD_LDFLAGS) -o $(PWD)/minio 1>/dev/null verify: @echo "Verifying build" - @GO111MODULE=on CGO_ENABLED=1 go build -race -tags kqueue --ldflags $(BUILD_LDFLAGS) -o $(PWD)/minio 1>/dev/null + @GO111MODULE=on CGO_ENABLED=1 go build -tags kqueue --ldflags $(BUILD_LDFLAGS) -o $(PWD)/minio 1>/dev/null @(env bash $(PWD)/buildscripts/verify-build.sh) coverage: build diff --git a/cmd/config-current.go b/cmd/config-current.go index 20c80117e..065086d93 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "reflect" "strings" "sync" @@ -28,6 +27,7 @@ import ( "github.com/minio/minio/cmd/config/cache" "github.com/minio/minio/cmd/config/compress" xldap "github.com/minio/minio/cmd/config/ldap" + "github.com/minio/minio/cmd/config/notify" "github.com/minio/minio/cmd/config/storageclass" "github.com/minio/minio/cmd/crypto" xhttp "github.com/minio/minio/cmd/http" @@ -465,56 +465,6 @@ func (s *serverConfig) TestNotificationTargets() error { return nil } -// Returns the string describing a difference with the given -// configuration object. If the given configuration object is -// identical, an empty string is returned. -func (s *serverConfig) ConfigDiff(t *serverConfig) string { - switch { - case t == nil: - return "Given configuration is empty" - case s.Credential != t.Credential: - return "Credential configuration differs" - case s.Region != t.Region: - return "Region configuration differs" - case s.StorageClass != t.StorageClass: - return "StorageClass configuration differs" - case !reflect.DeepEqual(s.Cache, t.Cache): - return "Cache configuration differs" - case !reflect.DeepEqual(s.Compression, t.Compression): - return "Compression configuration differs" - case !reflect.DeepEqual(s.Notify.AMQP, t.Notify.AMQP): - return "AMQP Notification configuration differs" - case !reflect.DeepEqual(s.Notify.NATS, t.Notify.NATS): - return "NATS Notification configuration differs" - case !reflect.DeepEqual(s.Notify.NSQ, t.Notify.NSQ): - return "NSQ Notification configuration differs" - case !reflect.DeepEqual(s.Notify.Elasticsearch, t.Notify.Elasticsearch): - return "ElasticSearch Notification configuration differs" - case !reflect.DeepEqual(s.Notify.Redis, t.Notify.Redis): - return "Redis Notification configuration differs" - case !reflect.DeepEqual(s.Notify.PostgreSQL, t.Notify.PostgreSQL): - return "PostgreSQL Notification configuration differs" - case !reflect.DeepEqual(s.Notify.Kafka, t.Notify.Kafka): - return "Kafka Notification configuration differs" - case !reflect.DeepEqual(s.Notify.Webhook, t.Notify.Webhook): - return "Webhook Notification configuration differs" - case !reflect.DeepEqual(s.Notify.MySQL, t.Notify.MySQL): - return "MySQL Notification configuration differs" - case !reflect.DeepEqual(s.Notify.MQTT, t.Notify.MQTT): - return "MQTT Notification configuration differs" - case !reflect.DeepEqual(s.Logger, t.Logger): - return "Logger configuration differs" - case !reflect.DeepEqual(s.KMS, t.KMS): - return "KMS configuration differs" - case reflect.DeepEqual(s, t): - return "" - default: - // This case will not happen unless this comparison - // function has become stale. - return "Configuration differs" - } -} - func newServerConfig() *serverConfig { cred, err := auth.GetNewCredentials() logger.FatalIf(err, "") @@ -534,47 +484,15 @@ func newServerConfig() *serverConfig { MaxUse: globalCacheMaxUse, }, KMS: crypto.KMSConfig{}, - Notify: notifier{}, + Notify: notify.NewConfig(), Compression: compress.Config{ Enabled: false, Extensions: globalCompressExtensions, MimeTypes: globalCompressMimeTypes, }, + Logger: logger.NewConfig(), } - // Make sure to initialize notification configs. - srvCfg.Notify.AMQP = make(map[string]target.AMQPArgs) - srvCfg.Notify.AMQP["1"] = target.AMQPArgs{} - srvCfg.Notify.MQTT = make(map[string]target.MQTTArgs) - srvCfg.Notify.MQTT["1"] = target.MQTTArgs{} - srvCfg.Notify.Elasticsearch = make(map[string]target.ElasticsearchArgs) - srvCfg.Notify.Elasticsearch["1"] = target.ElasticsearchArgs{} - srvCfg.Notify.Redis = make(map[string]target.RedisArgs) - srvCfg.Notify.Redis["1"] = target.RedisArgs{} - srvCfg.Notify.NATS = make(map[string]target.NATSArgs) - srvCfg.Notify.NATS["1"] = target.NATSArgs{} - srvCfg.Notify.NSQ = make(map[string]target.NSQArgs) - srvCfg.Notify.NSQ["1"] = target.NSQArgs{} - srvCfg.Notify.PostgreSQL = make(map[string]target.PostgreSQLArgs) - srvCfg.Notify.PostgreSQL["1"] = target.PostgreSQLArgs{} - srvCfg.Notify.MySQL = make(map[string]target.MySQLArgs) - srvCfg.Notify.MySQL["1"] = target.MySQLArgs{} - srvCfg.Notify.Kafka = make(map[string]target.KafkaArgs) - srvCfg.Notify.Kafka["1"] = target.KafkaArgs{} - srvCfg.Notify.Webhook = make(map[string]target.WebhookArgs) - srvCfg.Notify.Webhook["1"] = target.WebhookArgs{} - - srvCfg.Cache.Drives = make([]string, 0) - srvCfg.Cache.Exclude = make([]string, 0) - srvCfg.Cache.Expiry = globalCacheExpiry - srvCfg.Cache.MaxUse = globalCacheMaxUse - - // Console logging is on by default - srvCfg.Logger.Console.Enabled = true - // Create an example of HTTP logger - srvCfg.Logger.HTTP = make(map[string]logger.HTTP) - srvCfg.Logger.HTTP["target1"] = logger.HTTP{Endpoint: "https://username:password@example.com/api"} - return srvCfg } diff --git a/cmd/config-current_test.go b/cmd/config-current_test.go index 3350fff02..21743877d 100644 --- a/cmd/config-current_test.go +++ b/cmd/config-current_test.go @@ -21,11 +21,6 @@ import ( "os" "path" "testing" - - "github.com/minio/minio/cmd/config/storageclass" - "github.com/minio/minio/cmd/logger" - "github.com/minio/minio/pkg/auth" - "github.com/minio/minio/pkg/event/target" ) func TestServerConfig(t *testing.T) { @@ -182,128 +177,3 @@ func TestValidateConfig(t *testing.T) { } } - -func TestConfigDiff(t *testing.T) { - testCases := []struct { - s, t *serverConfig - diff string - }{ - // 1 - {&serverConfig{}, nil, "Given configuration is empty"}, - // 2 - { - &serverConfig{Credential: auth.Credentials{ - AccessKey: "u1", - SecretKey: "p1", - Expiration: timeSentinel, - }}, - &serverConfig{Credential: auth.Credentials{ - AccessKey: "u1", - SecretKey: "p2", - Expiration: timeSentinel, - }}, - "Credential configuration differs", - }, - // 3 - {&serverConfig{Region: "us-east-1"}, &serverConfig{Region: "us-west-1"}, "Region configuration differs"}, - // 4 - { - &serverConfig{StorageClass: storageclass.Config{ - Standard: storageclass.StorageClass{ - Parity: 8, - }, - RRS: storageclass.StorageClass{ - Parity: 6, - }, - }}, - &serverConfig{StorageClass: storageclass.Config{ - Standard: storageclass.StorageClass{ - Parity: 8, - }, - RRS: storageclass.StorageClass{ - Parity: 4, - }, - }}, - "StorageClass configuration differs", - }, - // 5 - { - &serverConfig{Notify: notifier{AMQP: map[string]target.AMQPArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{AMQP: map[string]target.AMQPArgs{"1": {Enable: false}}}}, - "AMQP Notification configuration differs", - }, - // 6 - { - &serverConfig{Notify: notifier{NATS: map[string]target.NATSArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{NATS: map[string]target.NATSArgs{"1": {Enable: false}}}}, - "NATS Notification configuration differs", - }, - // 7 - { - &serverConfig{Notify: notifier{NSQ: map[string]target.NSQArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{NSQ: map[string]target.NSQArgs{"1": {Enable: false}}}}, - "NSQ Notification configuration differs", - }, - // 8 - { - &serverConfig{Notify: notifier{Elasticsearch: map[string]target.ElasticsearchArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{Elasticsearch: map[string]target.ElasticsearchArgs{"1": {Enable: false}}}}, - "ElasticSearch Notification configuration differs", - }, - // 9 - { - &serverConfig{Notify: notifier{Redis: map[string]target.RedisArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{Redis: map[string]target.RedisArgs{"1": {Enable: false}}}}, - "Redis Notification configuration differs", - }, - // 10 - { - &serverConfig{Notify: notifier{PostgreSQL: map[string]target.PostgreSQLArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{PostgreSQL: map[string]target.PostgreSQLArgs{"1": {Enable: false}}}}, - "PostgreSQL Notification configuration differs", - }, - // 11 - { - &serverConfig{Notify: notifier{Kafka: map[string]target.KafkaArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{Kafka: map[string]target.KafkaArgs{"1": {Enable: false}}}}, - "Kafka Notification configuration differs", - }, - // 12 - { - &serverConfig{Notify: notifier{Webhook: map[string]target.WebhookArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{Webhook: map[string]target.WebhookArgs{"1": {Enable: false}}}}, - "Webhook Notification configuration differs", - }, - // 13 - { - &serverConfig{Notify: notifier{MySQL: map[string]target.MySQLArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{MySQL: map[string]target.MySQLArgs{"1": {Enable: false}}}}, - "MySQL Notification configuration differs", - }, - // 14 - { - &serverConfig{Notify: notifier{MQTT: map[string]target.MQTTArgs{"1": {Enable: true}}}}, - &serverConfig{Notify: notifier{MQTT: map[string]target.MQTTArgs{"1": {Enable: false}}}}, - "MQTT Notification configuration differs", - }, - // 15 - { - &serverConfig{Logger: logger.Config{ - Console: logger.Console{Enabled: false}, - HTTP: map[string]logger.HTTP{"1": {Endpoint: "http://address1"}}, - }}, - &serverConfig{Logger: logger.Config{ - Console: logger.Console{Enabled: false}, - HTTP: map[string]logger.HTTP{"1": {Endpoint: "http://address2"}}, - }}, - "Logger configuration differs", - }, - } - - for i, testCase := range testCases { - got := testCase.s.ConfigDiff(testCase.t) - if got != testCase.diff { - t.Errorf("Test %d: got %s expected %s", i+1, got, testCase.diff) - } - } -} diff --git a/cmd/config-versions.go b/cmd/config-versions.go index c9f63044f..428d91bca 100644 --- a/cmd/config-versions.go +++ b/cmd/config-versions.go @@ -23,6 +23,7 @@ import ( "github.com/minio/minio/cmd/config/cache" "github.com/minio/minio/cmd/config/compress" xldap "github.com/minio/minio/cmd/config/ldap" + "github.com/minio/minio/cmd/config/notify" "github.com/minio/minio/cmd/config/storageclass" "github.com/minio/minio/cmd/crypto" "github.com/minio/minio/cmd/logger" @@ -271,7 +272,7 @@ type serverConfigV7 struct { Notify notifierV1 `json:"notify"` } -// serverConfigV8 server configuration version '8'. Adds NATS notifier +// serverConfigV8 server configuration version '8'. Adds NATS notify.Config // configuration. type serverConfigV8 struct { Version string `json:"version"` @@ -288,7 +289,7 @@ type serverConfigV8 struct { } // serverConfigV9 server configuration version '9'. Adds PostgreSQL -// notifier configuration. +// notify.Config configuration. type serverConfigV9 struct { Version string `json:"version"` @@ -562,9 +563,6 @@ type serverConfigV21 struct { // serverConfigV22 is just like version '21' with added support // for StorageClass. -// -// IMPORTANT NOTE: When updating this struct make sure that -// serverConfig.ConfigDiff() is updated as necessary. type serverConfigV22 struct { Version string `json:"version"` @@ -582,9 +580,6 @@ type serverConfigV22 struct { } // serverConfigV23 is just like version '22' with addition of cache field. -// -// IMPORTANT NOTE: When updating this struct make sure that -// serverConfig.ConfigDiff() is updated as necessary. type serverConfigV23 struct { Version string `json:"version"` @@ -606,9 +601,6 @@ type serverConfigV23 struct { // serverConfigV24 is just like version '23', we had to revert // the changes which were made in 6fb06045028b7a57c37c60a612c8e50735279ab4 -// -// IMPORTANT NOTE: When updating this struct make sure that -// serverConfig.ConfigDiff() is updated as necessary. type serverConfigV24 struct { Version string `json:"version"` @@ -630,9 +622,6 @@ type serverConfigV24 struct { // serverConfigV25 is just like version '24', stores additionally // worm variable. -// -// IMPORTANT NOTE: When updating this struct make sure that -// serverConfig.ConfigDiff() is updated as necessary. type serverConfigV25 struct { quick.Config `json:"-"` // ignore interfaces @@ -681,9 +670,6 @@ type serverConfigV26 struct { // serverConfigV27 is just like version '26', stores additionally // the logger field -// -// IMPORTANT NOTE: When updating this struct make sure that -// serverConfig.ConfigDiff() is updated as necessary. type serverConfigV27 struct { quick.Config `json:"-"` // ignore interfaces @@ -711,9 +697,6 @@ type serverConfigV27 struct { // serverConfigV28 is just like version '27', additionally // storing KMS config -// -// IMPORTANT NOTE: When updating this struct make sure that -// serverConfig.ConfigDiff() is updated as necessary. type serverConfigV28 struct { quick.Config `json:"-"` // ignore interfaces @@ -814,19 +797,6 @@ type serverConfigV31 struct { } `json:"policy"` } -type notifier struct { - AMQP map[string]target.AMQPArgs `json:"amqp"` - Elasticsearch map[string]target.ElasticsearchArgs `json:"elasticsearch"` - Kafka map[string]target.KafkaArgs `json:"kafka"` - MQTT map[string]target.MQTTArgs `json:"mqtt"` - MySQL map[string]target.MySQLArgs `json:"mysql"` - NATS map[string]target.NATSArgs `json:"nats"` - NSQ map[string]target.NSQArgs `json:"nsq"` - PostgreSQL map[string]target.PostgreSQLArgs `json:"postgresql"` - Redis map[string]target.RedisArgs `json:"redis"` - Webhook map[string]target.WebhookArgs `json:"webhook"` -} - // serverConfigV32 is just like version '31' with added nsq notifer. type serverConfigV32 struct { Version string `json:"version"` @@ -846,7 +816,7 @@ type serverConfigV32 struct { KMS crypto.KMSConfig `json:"kms"` // Notification queue configuration. - Notify notifier `json:"notify"` + Notify notify.Config `json:"notify"` // Logger configuration Logger logger.Config `json:"logger"` @@ -890,7 +860,7 @@ type serverConfigV33 struct { KMS crypto.KMSConfig `json:"kms"` // Notification queue configuration. - Notify notifier `json:"notify"` + Notify notify.Config `json:"notify"` // Logger configuration Logger logger.Config `json:"logger"` diff --git a/cmd/config/notify/config.go b/cmd/config/notify/config.go new file mode 100644 index 000000000..ef9472e69 --- /dev/null +++ b/cmd/config/notify/config.go @@ -0,0 +1,66 @@ +/* + * MinIO Cloud Storage, (C) 2019 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package notify + +import "github.com/minio/minio/pkg/event/target" + +// Config - notification target configuration structure, holds +// information about various notification targets. +type Config struct { + AMQP map[string]target.AMQPArgs `json:"amqp"` + Elasticsearch map[string]target.ElasticsearchArgs `json:"elasticsearch"` + Kafka map[string]target.KafkaArgs `json:"kafka"` + MQTT map[string]target.MQTTArgs `json:"mqtt"` + MySQL map[string]target.MySQLArgs `json:"mysql"` + NATS map[string]target.NATSArgs `json:"nats"` + NSQ map[string]target.NSQArgs `json:"nsq"` + PostgreSQL map[string]target.PostgreSQLArgs `json:"postgresql"` + Redis map[string]target.RedisArgs `json:"redis"` + Webhook map[string]target.WebhookArgs `json:"webhook"` +} + +const ( + defaultTarget = "1" +) + +// NewConfig - initialize notification config. +func NewConfig() Config { + // Make sure to initialize notification targets + cfg := Config{ + NSQ: make(map[string]target.NSQArgs), + AMQP: make(map[string]target.AMQPArgs), + MQTT: make(map[string]target.MQTTArgs), + NATS: make(map[string]target.NATSArgs), + Redis: make(map[string]target.RedisArgs), + MySQL: make(map[string]target.MySQLArgs), + Kafka: make(map[string]target.KafkaArgs), + Webhook: make(map[string]target.WebhookArgs), + PostgreSQL: make(map[string]target.PostgreSQLArgs), + Elasticsearch: make(map[string]target.ElasticsearchArgs), + } + cfg.NSQ[defaultTarget] = target.NSQArgs{} + cfg.AMQP[defaultTarget] = target.AMQPArgs{} + cfg.MQTT[defaultTarget] = target.MQTTArgs{} + cfg.NATS[defaultTarget] = target.NATSArgs{} + cfg.Redis[defaultTarget] = target.RedisArgs{} + cfg.MySQL[defaultTarget] = target.MySQLArgs{} + cfg.Kafka[defaultTarget] = target.KafkaArgs{} + cfg.Webhook[defaultTarget] = target.WebhookArgs{} + cfg.PostgreSQL[defaultTarget] = target.PostgreSQLArgs{} + cfg.Elasticsearch[defaultTarget] = target.ElasticsearchArgs{} + return cfg +} diff --git a/cmd/config/storageclass/storage-class.go b/cmd/config/storageclass/storage-class.go index 472f0583c..aaa4a6930 100644 --- a/cmd/config/storageclass/storage-class.go +++ b/cmd/config/storageclass/storage-class.go @@ -205,9 +205,9 @@ func LookupConfig(cfg Config, drivesPerSet int) (Config, error) { if err != nil { return cfg, err } - if cfg.Standard.Parity == 0 { - cfg.Standard.Parity = drivesPerSet / 2 - } + } + if cfg.Standard.Parity == 0 { + cfg.Standard.Parity = drivesPerSet / 2 } if rrsc := env.Get(RRSEnv, cfg.RRS.String()); rrsc != "" { @@ -215,9 +215,9 @@ func LookupConfig(cfg Config, drivesPerSet int) (Config, error) { if err != nil { return cfg, err } - if cfg.RRS.Parity == 0 { - cfg.RRS.Parity = defaultRRSParity - } + } + if cfg.RRS.Parity == 0 { + cfg.RRS.Parity = defaultRRSParity } // Validation is done after parsing both the storage classes. This is needed because we need one diff --git a/cmd/lock-rest-client.go b/cmd/lock-rest-client.go index bdf4ecfcd..808cc5b4b 100644 --- a/cmd/lock-rest-client.go +++ b/cmd/lock-rest-client.go @@ -21,7 +21,7 @@ import ( "crypto/tls" "errors" "io" - "sync" + "sync/atomic" "time" "net/url" @@ -35,12 +35,10 @@ import ( // lockRESTClient is authenticable lock REST client type lockRESTClient struct { - lockSync sync.RWMutex host *xnet.Host restClient *rest.Client serverURL *url.URL - connected bool - timer *time.Timer + connected int32 } func toLockError(err error) error { @@ -67,42 +65,11 @@ func (client *lockRESTClient) ServiceEndpoint() string { return client.serverURL.Path } -// check if the host is up or if it is fine -// to make a call to the lock rest server. -func (client *lockRESTClient) isHostUp() bool { - client.lockSync.Lock() - defer client.lockSync.Unlock() - - if client.connected { - return true - } - select { - case <-client.timer.C: - client.connected = true - client.timer = nil - return true - default: - } - return false -} - -// Mark the host as down if there is a Network error. -func (client *lockRESTClient) markHostDown() { - client.lockSync.Lock() - defer client.lockSync.Unlock() - - if !client.connected { - return - } - client.connected = false - client.timer = time.NewTimer(defaultRetryUnit * 5) -} - // Wrapper to restClient.Call to handle network errors, in case of network error the connection is marked disconnected // permanently. The only way to restore the connection is at the xl-sets layer by xlsets.monitorAndConnectEndpoints() // after verifying format.json func (client *lockRESTClient) call(method string, values url.Values, body io.Reader, length int64) (respBody io.ReadCloser, err error) { - if !client.isHostUp() { + if !client.IsOnline() { return nil, errors.New("Lock rest server node is down") } @@ -116,7 +83,12 @@ func (client *lockRESTClient) call(method string, values url.Values, body io.Rea } if isNetworkError(err) { - client.markHostDown() + time.AfterFunc(defaultRetryUnit*5, func() { + // After 5 seconds, take this lock client + // online for a retry. + atomic.StoreInt32(&client.connected, 1) + }) + atomic.StoreInt32(&client.connected, 0) } return nil, toLockError(err) @@ -129,12 +101,12 @@ func (client *lockRESTClient) String() string { // IsOnline - returns whether REST client failed to connect or not. func (client *lockRESTClient) IsOnline() bool { - return client.connected + return atomic.LoadInt32(&client.connected) == 1 } // Close - marks the client as closed. func (client *lockRESTClient) Close() error { - client.connected = false + atomic.StoreInt32(&client.connected, 0) client.restClient.Close() return nil } @@ -217,8 +189,8 @@ func newlockRESTClient(peer *xnet.Host) *lockRESTClient { if err != nil { logger.LogIf(context.Background(), err) - return &lockRESTClient{serverURL: serverURL, host: peer, restClient: restClient, connected: false, timer: time.NewTimer(defaultRetryUnit * 5)} + return &lockRESTClient{serverURL: serverURL, host: peer, restClient: restClient, connected: 0} } - return &lockRESTClient{serverURL: serverURL, host: peer, restClient: restClient, connected: true} + return &lockRESTClient{serverURL: serverURL, host: peer, restClient: restClient, connected: 1} } diff --git a/cmd/lock-rest-client_test.go b/cmd/lock-rest-client_test.go index 05612848f..7a34bf201 100644 --- a/cmd/lock-rest-client_test.go +++ b/cmd/lock-rest-client_test.go @@ -30,7 +30,7 @@ func TestLockRESTlient(t *testing.T) { t.Fatalf("unexpected error %v", err) } lkClient := newlockRESTClient(host) - if lkClient.connected == false { + if lkClient.connected == 0 { t.Fatalf("unexpected error. connection failed") } diff --git a/cmd/lock-rest-server.go b/cmd/lock-rest-server.go index 77c940876..53bd14805 100644 --- a/cmd/lock-rest-server.go +++ b/cmd/lock-rest-server.go @@ -196,7 +196,7 @@ func (l *lockRESTServer) lockMaintenance(interval time.Duration) { continue } c := newlockRESTClient(host) - if !c.connected { + if !c.IsOnline() { continue } diff --git a/cmd/logger/config.go b/cmd/logger/config.go index ba61bba96..45a3afcf9 100644 --- a/cmd/logger/config.go +++ b/cmd/logger/config.go @@ -51,6 +51,30 @@ const ( defaultTarget = "_" ) +// NewConfig - initialize new logger config. +func NewConfig() Config { + cfg := Config{ + // Console logging is on by default + Console: Console{ + Enabled: true, + }, + HTTP: make(map[string]HTTP), + Audit: make(map[string]HTTP), + } + + // Create an example HTTP logger + cfg.HTTP[defaultTarget] = HTTP{ + Endpoint: "https://username:password@example.com/api", + } + + // Create an example Audit logger + cfg.Audit[defaultTarget] = HTTP{ + Endpoint: "https://username:password@example.com/api/audit", + } + + return cfg +} + // LookupConfig - lookup logger config, override with ENVs if set. func LookupConfig(cfg Config) (Config, error) { envs := env.List(EnvLoggerHTTPEndpoint) diff --git a/cmd/peer-rest-client.go b/cmd/peer-rest-client.go index dd50ac050..bc33a0124 100644 --- a/cmd/peer-rest-client.go +++ b/cmd/peer-rest-client.go @@ -25,6 +25,7 @@ import ( "math/rand" "net/url" "strconv" + "sync/atomic" "time" "github.com/minio/minio/cmd/http" @@ -42,14 +43,12 @@ import ( type peerRESTClient struct { host *xnet.Host restClient *rest.Client - connected bool + connected int32 } // Reconnect to a peer rest server. func (client *peerRESTClient) reConnect() error { - // correct (intelligent) retry logic will be - // implemented in subsequent PRs. - client.connected = true + atomic.StoreInt32(&client.connected, 1) return nil } @@ -64,7 +63,7 @@ func (client *peerRESTClient) call(method string, values url.Values, body io.Rea // permanently. The only way to restore the connection is at the xl-sets layer by xlsets.monitorAndConnectEndpoints() // after verifying format.json func (client *peerRESTClient) callWithContext(ctx context.Context, method string, values url.Values, body io.Reader, length int64) (respBody io.ReadCloser, err error) { - if !client.connected { + if !client.IsOnline() { err := client.reConnect() logger.LogIf(ctx, err) if err != nil { @@ -82,7 +81,7 @@ func (client *peerRESTClient) callWithContext(ctx context.Context, method string } if isNetworkError(err) { - client.connected = false + atomic.StoreInt32(&client.connected, 0) } return nil, err @@ -95,12 +94,12 @@ func (client *peerRESTClient) String() string { // IsOnline - returns whether RPC client failed to connect or not. func (client *peerRESTClient) IsOnline() bool { - return client.connected + return atomic.LoadInt32(&client.connected) == 1 } // Close - marks the client as closed. func (client *peerRESTClient) Close() error { - client.connected = false + atomic.StoreInt32(&client.connected, 0) client.restClient.Close() return nil } @@ -733,8 +732,8 @@ func newPeerRESTClient(peer *xnet.Host) (*peerRESTClient, error) { restClient, err := rest.NewClient(serverURL, tlsConfig, rest.DefaultRESTTimeout, newAuthToken) if err != nil { - return &peerRESTClient{host: peer, restClient: restClient, connected: false}, err + return &peerRESTClient{host: peer, restClient: restClient, connected: 0}, err } - return &peerRESTClient{host: peer, restClient: restClient, connected: true}, nil + return &peerRESTClient{host: peer, restClient: restClient, connected: 1}, nil } diff --git a/cmd/storage-rest-client.go b/cmd/storage-rest-client.go index 1e99e216c..5c74e9318 100644 --- a/cmd/storage-rest-client.go +++ b/cmd/storage-rest-client.go @@ -1,5 +1,5 @@ /* - * MinIO Cloud Storage, (C) 2018 MinIO, Inc. + * MinIO Cloud Storage, (C) 2018-2019 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,6 +27,7 @@ import ( "net/url" "path" "strconv" + "sync/atomic" "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/rest" @@ -107,7 +108,7 @@ func toStorageErr(err error) error { type storageRESTClient struct { endpoint Endpoint restClient *rest.Client - connected bool + connected int32 lastError error instanceID string // REST server's instanceID which is sent with every request for validation. } @@ -116,7 +117,7 @@ type storageRESTClient struct { // permanently. The only way to restore the storage connection is at the xl-sets layer by xlsets.monitorAndConnectEndpoints() // after verifying format.json func (client *storageRESTClient) call(method string, values url.Values, body io.Reader, length int64) (respBody io.ReadCloser, err error) { - if !client.connected { + if !client.IsOnline() { return nil, errDiskNotFound } if values == nil { @@ -129,7 +130,7 @@ func (client *storageRESTClient) call(method string, values url.Values, body io. } client.lastError = err if isNetworkError(err) { - client.connected = false + atomic.StoreInt32(&client.connected, 0) } return nil, toStorageErr(err) @@ -142,7 +143,7 @@ func (client *storageRESTClient) String() string { // IsOnline - returns whether RPC client failed to connect or not. func (client *storageRESTClient) IsOnline() bool { - return client.connected + return atomic.LoadInt32(&client.connected) == 1 } // LastError - returns the network error if any. @@ -460,7 +461,7 @@ func (client *storageRESTClient) VerifyFile(volume, path string, size int64, alg // Close - marks the client as closed. func (client *storageRESTClient) Close() error { - client.connected = false + atomic.StoreInt32(&client.connected, 0) client.restClient.Close() return nil } @@ -496,7 +497,11 @@ func newStorageRESTClient(endpoint Endpoint) (*storageRESTClient, error) { if err != nil { return nil, err } - client := &storageRESTClient{endpoint: endpoint, restClient: restClient, connected: true} - client.connected = client.getInstanceID() == nil + client := &storageRESTClient{endpoint: endpoint, restClient: restClient, connected: 1} + if client.getInstanceID() == nil { + client.connected = 1 + } else { + client.connected = 0 + } return client, nil } diff --git a/go.mod b/go.mod index 0c8a8b1a7..a22945ce1 100644 --- a/go.mod +++ b/go.mod @@ -44,7 +44,7 @@ require ( github.com/minio/lsync v1.0.1 github.com/minio/mc v0.0.0-20190924013003-643835013047 github.com/minio/minio-go v0.0.0-20190327203652-5325257a208f - github.com/minio/minio-go/v6 v6.0.38 + github.com/minio/minio-go/v6 v6.0.39 github.com/minio/parquet-go v0.0.0-20190318185229-9d767baf1679 github.com/minio/sha256-simd v0.1.1 github.com/minio/sio v0.2.0 diff --git a/go.sum b/go.sum index 3c6cad69b..97ac3b753 100644 --- a/go.sum +++ b/go.sum @@ -438,6 +438,8 @@ github.com/minio/minio-go/v6 v6.0.37 h1:rqot4cO9+mLpf56q+yumA0xZlncbkFpqa4A8jw1Y github.com/minio/minio-go/v6 v6.0.37/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg= github.com/minio/minio-go/v6 v6.0.38 h1:zd3yagckaBVAMJT+HsbpURx9ndqYQp/N/udc1UVS72E= github.com/minio/minio-go/v6 v6.0.38/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg= +github.com/minio/minio-go/v6 v6.0.39 h1:9qmKCTBpQpMdGlDAbs3mbb4mmL45/lwRUvHL1VLhYUk= +github.com/minio/minio-go/v6 v6.0.39/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg= github.com/minio/parquet-go v0.0.0-20190318185229-9d767baf1679 h1:OMKaN/82sBHUZPvjYNBFituHExa1OGY63eACDGtetKs= github.com/minio/parquet-go v0.0.0-20190318185229-9d767baf1679/go.mod h1:J+goXSuzlte5imWMqb6cUWC/tbYYysUHctwmKXomYzM= github.com/minio/sha256-simd v0.0.0-20190131020904-2d45a736cd16/go.mod h1:2FMWW+8GMoPweT6+pI63m9YE3Lmw4J71hV56Chs1E/U=