Remove "logger" field from config.json (#5268)

File logging removed as part of improvement to server logging.

config.json format updated to version 21.

Fixes #5176

cmd/common-main.go

@@ -37,22 +37,6 @@ func checkUpdate(mode string) {
 	}
 }
 
-func enableLoggers() {
-	fileLogTarget := globalServerConfig.Logger.GetFile()
-	if fileLogTarget.Enable {
-		err := InitFileLogger(&fileLogTarget)
-		fatalIf(err, "Unable to initialize file logger")
-		log.AddTarget(fileLogTarget)
-	}
-
-	consoleLogTarget := globalServerConfig.Logger.GetConsole()
-	if consoleLogTarget.Enable {
-		InitConsoleLogger(&consoleLogTarget)
-	}
-
-	log.SetConsoleTarget(consoleLogTarget)
-}
-
 func initConfig() {
 	// Config file does not exist, we create it fresh and return upon success.
 	if isFile(getConfigFile()) {

cmd/config-current.go

@@ -36,9 +36,9 @@ import (
 // 6. Make changes in config-current_test.go for any test change
 
 // Config version
-const serverConfigVersion = "20"
+const serverConfigVersion = "21"
 
-type serverConfig = serverConfigV20
+type serverConfig = serverConfigV21
 
 var (
 	// globalServerConfig server config.
@@ -71,7 +71,7 @@ func (s *serverConfig) GetRegion() string {
 	return s.Region
 }
 
-// SetCredentials set new credentials. SetCredential returns the previous credential.
+// SetCredential sets new credential and returns the previous credential.
 func (s *serverConfig) SetCredential(creds auth.Credentials) (prevCred auth.Credentials) {
 	s.Lock()
 	defer s.Unlock()
@@ -126,13 +126,9 @@ func newServerConfig() *serverConfig {
 		Credential: auth.MustGetNewCredentials(),
 		Region:     globalMinioDefaultRegion,
 		Browser:    true,
-		Logger:     &loggers{},
 		Notify:     &notifier{},
 	}
 
-	// Enable console logger by default on a fresh run.
-	srvCfg.Logger.Console = NewConsoleLogger()
-
 	// Make sure to initialize notification configs.
 	srvCfg.Notify.AMQP = make(map[string]amqpNotify)
 	srvCfg.Notify.AMQP["1"] = amqpNotify{}
@@ -274,11 +270,6 @@ func getValidConfig() (*serverConfig, error) {
 		return nil, errors.New("invalid credential in config file " + configFile)
 	}
 
-	// Validate logger field
-	if err = srvCfg.Logger.Validate(); err != nil {
-		return nil, err
-	}
-
 	// Validate notify field
 	if err = srvCfg.Notify.Validate(); err != nil {
 		return nil, err

cmd/config-current_test.go

@@ -79,7 +79,6 @@ func TestServerConfig(t *testing.T) {
 		t.Errorf("Expecting Webhook config %#v found %#v", webhookNotify{}, savedNotifyCfg5)
 	}
 
-	// Set new console logger.
 	// Set new MySQL notification id.
 	globalServerConfig.Notify.SetMySQLByID("2", mySQLNotify{})
 	savedNotifyCfg6 := globalServerConfig.Notify.GetMySQLByID("2")
@@ -87,7 +86,6 @@ func TestServerConfig(t *testing.T) {
 		t.Errorf("Expecting Webhook config %#v found %#v", mySQLNotify{}, savedNotifyCfg6)
 	}
 
-	// Set new console logger.
 	// Set new MQTT notification id.
 	globalServerConfig.Notify.SetMQTTByID("2", mqttNotify{})
 	savedNotifyCfg7 := globalServerConfig.Notify.GetMQTTByID("2")
@@ -95,27 +93,6 @@ func TestServerConfig(t *testing.T) {
 		t.Errorf("Expecting Webhook config %#v found %#v", mqttNotify{}, savedNotifyCfg7)
 	}
 
-	consoleLogger := NewConsoleLogger()
-	globalServerConfig.Logger.SetConsole(consoleLogger)
-	consoleCfg := globalServerConfig.Logger.GetConsole()
-	if !reflect.DeepEqual(consoleCfg, consoleLogger) {
-		t.Errorf("Expecting console logger config %#v found %#v", consoleLogger, consoleCfg)
-	}
-	// Set new console logger.
-	consoleLogger.Enable = false
-	globalServerConfig.Logger.SetConsole(consoleLogger)
-
-	// Set new file logger.
-	fileLogger := NewFileLogger("test-log-file")
-	globalServerConfig.Logger.SetFile(fileLogger)
-	fileCfg := globalServerConfig.Logger.GetFile()
-	if !reflect.DeepEqual(fileCfg, fileLogger) {
-		t.Errorf("Expecting file logger config %#v found %#v", fileLogger, fileCfg)
-	}
-	// Set new file logger.
-	fileLogger.Enable = false
-	globalServerConfig.Logger.SetFile(fileLogger)
-
 	// Match version.
 	if globalServerConfig.GetVersion() != serverConfigVersion {
 		t.Errorf("Expecting version %s found %s", globalServerConfig.GetVersion(), serverConfigVersion)
@@ -273,58 +250,55 @@ func TestValidateConfig(t *testing.T) {
 		// Test 10 - duplicated json keys
 		{`{"version": "` + v + `", "browser": "on", "browser": "on", "region":"us-east-1", "credential" : {"accessKey":"minio", "secretKey":"minio123"}}`, false},
 
-		// Test 11 - empty filename field in File
-		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "logger": { "file": { "enable": true, "filename": "" } }}`, false},
-
-		// Test 12 - Test AMQP
+		// Test 11 - Test AMQP
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "amqp": { "1": { "enable": true, "url": "", "exchange": "", "routingKey": "", "exchangeType": "", "mandatory": false, "immediate": false, "durable": false, "internal": false, "noWait": false, "autoDeleted": false }}}}`, false},
 
-		// Test 13 - Test NATS
+		// Test 12 - Test NATS
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "nats": { "1": { "enable": true, "address": "", "subject": "", "username": "", "password": "", "token": "", "secure": false, "pingInterval": 0, "streaming": { "enable": false, "clusterID": "", "clientID": "", "async": false, "maxPubAcksInflight": 0 } } }}}`, false},
 
-		// Test 14 - Test ElasticSearch
+		// Test 13 - Test ElasticSearch
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "elasticsearch": { "1": { "enable": true, "url": "", "index": "" } }}}`, false},
 
-		// Test 15 - Test Redis
+		// Test 14 - Test Redis
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "address": "", "password": "", "key": "" } }}}`, false},
 
-		// Test 16 - Test PostgreSQL
+		// Test 15 - Test PostgreSQL
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "postgresql": { "1": { "enable": true, "connectionString": "", "table": "", "host": "", "port": "", "user": "", "password": "", "database": "" }}}}`, false},
 
-		// Test 17 - Test Kafka
+		// Test 16 - Test Kafka
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "kafka": { "1": { "enable": true, "brokers": null, "topic": "" } }}}`, false},
 
-		// Test 18 - Test Webhook
+		// Test 17 - Test Webhook
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "webhook": { "1": { "enable": true, "endpoint": "" } }}}`, false},
 
-		// Test 20 - Test MySQL
+		// Test 18 - Test MySQL
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "mysql": { "1": { "enable": true, "dsnString": "",  "table": "", "host": "", "port": "", "user": "", "password": "", "database": "" }}}}`, false},
 
-		// Test 21 - Test Format for MySQL
+		// Test 19 - Test Format for MySQL
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "mysql": { "1": { "enable": true, "dsnString": "",  "format": "invalid", "table": "xxx", "host": "10.0.0.1", "port": "3306", "user": "abc", "password": "pqr", "database": "test1" }}}}`, false},
 
-		// Test 22 - Test valid Format for MySQL
+		// Test 20 - Test valid Format for MySQL
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "mysql": { "1": { "enable": true, "dsnString": "",  "format": "namespace", "table": "xxx", "host": "10.0.0.1", "port": "3306", "user": "abc", "password": "pqr", "database": "test1" }}}}`, true},
 
-		// Test 23 - Test Format for PostgreSQL
+		// Test 21 - Test Format for PostgreSQL
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "postgresql": { "1": { "enable": true, "connectionString": "", "format": "invalid", "table": "xxx", "host": "myhost", "port": "5432", "user": "abc", "password": "pqr", "database": "test1" }}}}`, false},
 
-		// Test 24 - Test valid Format for PostgreSQL
+		// Test 22 - Test valid Format for PostgreSQL
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "postgresql": { "1": { "enable": true, "connectionString": "", "format": "namespace", "table": "xxx", "host": "myhost", "port": "5432", "user": "abc", "password": "pqr", "database": "test1" }}}}`, true},
 
-		// Test 25 - Test Format for ElasticSearch
+		// Test 23 - Test Format for ElasticSearch
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "elasticsearch": { "1": { "enable": true, "format": "invalid", "url": "example.com", "index": "myindex" } }}}`, false},
 
-		// Test 26 - Test valid Format for ElasticSearch
+		// Test 24 - Test valid Format for ElasticSearch
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "elasticsearch": { "1": { "enable": true, "format": "namespace", "url": "example.com", "index": "myindex" } }}}`, true},
 
-		// Test 27 - Test Format for Redis
+		// Test 25 - Test Format for Redis
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "format": "invalid", "address": "example.com:80", "password": "xxx", "key": "key1" } }}}`, false},
 
-		// Test 28 - Test valid Format for Redis
+		// Test 26 - Test valid Format for Redis
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "redis": { "1": { "enable": true, "format": "namespace", "address": "example.com:80", "password": "xxx", "key": "key1" } }}}`, true},
 
-		// Test 29 - Test MQTT
+		// Test 27 - Test MQTT
 		{`{"version": "` + v + `", "credential": { "accessKey": "minio", "secretKey": "minio123" }, "region": "us-east-1", "browser": "on", "notify": { "mqtt": { "1": { "enable": true, "broker": "",  "topic": "", "qos": 0, "clientId": "", "username": "", "password": ""}}}}`, false},
 	}
 
@@ -337,7 +311,7 @@ func TestValidateConfig(t *testing.T) {
 			t.Errorf("Test %d, should pass but it failed with err = %v", i+1, verr)
 		}
 		if !testCase.shouldPass && verr == nil {
-			t.Errorf("Test %d, should fail but it succeed.", i+1)
+			t.Errorf("Test %d, should fail but it succeeded.", i+1)
 		}
 	}
 

cmd/config-migrate.go

@@ -143,7 +143,7 @@ func migrateConfig() error {
 		}
 		fallthrough
 	case "18":
-		// Migrate version '17' to '18'.
+		// Migrate version '18' to '19'.
 		if err = migrateV18ToV19(); err != nil {
 			return err
 		}
@@ -154,6 +154,11 @@ func migrateConfig() error {
 		}
 		fallthrough
 	case "20":
+		if err = migrateV20ToV21(); err != nil {
+			return err
+		}
+		fallthrough
+	case serverConfigVersion:
 		// No migration needed. this always points to current version.
 		err = nil
 	}
@@ -1500,7 +1505,7 @@ func migrateV19ToV20() error {
 	}
 
 	// Copy over fields from V19 into V20 config struct
-	srvConfig := &serverConfig{
+	srvConfig := &serverConfigV20{
 		Logger: &loggers{},
 		Notify: &notifier{},
 	}
@@ -1592,3 +1597,110 @@ func migrateV19ToV20() error {
 	log.Printf(configMigrateMSGTemplate, configFile, cv19.Version, srvConfig.Version)
 	return nil
 }
+
+func migrateV20ToV21() error {
+	configFile := getConfigFile()
+
+	cv20 := &serverConfigV20{}
+	_, err := quick.Load(configFile, cv20)
+	if os.IsNotExist(err) {
+		return nil
+	} else if err != nil {
+		return fmt.Errorf("Unable to load config version ‘20’. %v", err)
+	}
+	if cv20.Version != "20" {
+		return nil
+	}
+
+	// Copy over fields from V20 into V21 config struct
+	srvConfig := &serverConfigV21{
+		Notify: &notifier{},
+	}
+	srvConfig.Version = serverConfigVersion
+	srvConfig.Credential = cv20.Credential
+	srvConfig.Region = cv20.Region
+	if srvConfig.Region == "" {
+		// Region needs to be set for AWS Signature Version 4.
+		srvConfig.Region = globalMinioDefaultRegion
+	}
+
+	// check and set notifiers config
+	if len(cv20.Notify.AMQP) == 0 {
+		srvConfig.Notify.AMQP = make(map[string]amqpNotify)
+		srvConfig.Notify.AMQP["1"] = amqpNotify{}
+	} else {
+		// New deliveryMode parameter is added for AMQP,
+		// default value is already 0, so nothing to
+		// explicitly migrate here.
+		srvConfig.Notify.AMQP = cv20.Notify.AMQP
+	}
+	if len(cv20.Notify.ElasticSearch) == 0 {
+		srvConfig.Notify.ElasticSearch = make(map[string]elasticSearchNotify)
+		srvConfig.Notify.ElasticSearch["1"] = elasticSearchNotify{
+			Format: formatNamespace,
+		}
+	} else {
+		srvConfig.Notify.ElasticSearch = cv20.Notify.ElasticSearch
+	}
+	if len(cv20.Notify.Redis) == 0 {
+		srvConfig.Notify.Redis = make(map[string]redisNotify)
+		srvConfig.Notify.Redis["1"] = redisNotify{
+			Format: formatNamespace,
+		}
+	} else {
+		srvConfig.Notify.Redis = cv20.Notify.Redis
+	}
+	if len(cv20.Notify.PostgreSQL) == 0 {
+		srvConfig.Notify.PostgreSQL = make(map[string]postgreSQLNotify)
+		srvConfig.Notify.PostgreSQL["1"] = postgreSQLNotify{
+			Format: formatNamespace,
+		}
+	} else {
+		srvConfig.Notify.PostgreSQL = cv20.Notify.PostgreSQL
+	}
+	if len(cv20.Notify.Kafka) == 0 {
+		srvConfig.Notify.Kafka = make(map[string]kafkaNotify)
+		srvConfig.Notify.Kafka["1"] = kafkaNotify{}
+	} else {
+		srvConfig.Notify.Kafka = cv20.Notify.Kafka
+	}
+	if len(cv20.Notify.NATS) == 0 {
+		srvConfig.Notify.NATS = make(map[string]natsNotify)
+		srvConfig.Notify.NATS["1"] = natsNotify{}
+	} else {
+		srvConfig.Notify.NATS = cv20.Notify.NATS
+	}
+	if len(cv20.Notify.Webhook) == 0 {
+		srvConfig.Notify.Webhook = make(map[string]webhookNotify)
+		srvConfig.Notify.Webhook["1"] = webhookNotify{}
+	} else {
+		srvConfig.Notify.Webhook = cv20.Notify.Webhook
+	}
+	if len(cv20.Notify.MySQL) == 0 {
+		srvConfig.Notify.MySQL = make(map[string]mySQLNotify)
+		srvConfig.Notify.MySQL["1"] = mySQLNotify{
+			Format: formatNamespace,
+		}
+	} else {
+		srvConfig.Notify.MySQL = cv20.Notify.MySQL
+	}
+	if len(cv20.Notify.MQTT) == 0 {
+		srvConfig.Notify.MQTT = make(map[string]mqttNotify)
+		srvConfig.Notify.MQTT["1"] = mqttNotify{}
+	} else {
+		srvConfig.Notify.MQTT = cv20.Notify.MQTT
+	}
+
+	// Load browser config from existing config in the file.
+	srvConfig.Browser = cv20.Browser
+
+	// Load domain config from existing config in the file.
+	srvConfig.Domain = cv20.Domain
+
+	if err = quick.Save(configFile, srvConfig); err != nil {
+		return fmt.Errorf("Failed to migrate config from ‘%s’ to ‘%s’. %v", cv20.Version, srvConfig.Version, err)
+	}
+
+	log.Printf(configMigrateMSGTemplate, configFile, cv20.Version, srvConfig.Version)
+	return nil
+}

cmd/config-migrate_test.go

@@ -125,11 +125,16 @@ func TestServerConfigMigrateInexistentConfig(t *testing.T) {
 	if err := migrateV18ToV19(); err != nil {
 		t.Fatal("migrate v18 to v19 should succeed when no config file is found")
 	}
-
+	if err := migrateV19ToV20(); err != nil {
+		t.Fatal("migrate v19 to v20 should succeed when no config file is found")
+	}
+	if err := migrateV20ToV21(); err != nil {
+		t.Fatal("migrate v20 to v21 should succeed when no config file is found")
+	}
 }
 
-// Test if a config migration from v2 to v19 is successfully done
-func TestServerConfigMigrateV2toV19(t *testing.T) {
+// Test if a config migration from v2 to v21 is successfully done
+func TestServerConfigMigrateV2toV21(t *testing.T) {
 	rootPath, err := newTestConfig(globalMinioDefaultRegion)
 	if err != nil {
 		t.Fatalf("Init Test config failed")
@@ -252,6 +257,12 @@ func TestServerConfigMigrateFaultyConfig(t *testing.T) {
 	if err := migrateV18ToV19(); err == nil {
 		t.Fatal("migrateConfigV18ToV19() should fail with a corrupted json")
 	}
+	if err := migrateV19ToV20(); err == nil {
+		t.Fatal("migrateConfigV19ToV20() should fail with a corrupted json")
+	}
+	if err := migrateV20ToV21(); err == nil {
+		t.Fatal("migrateConfigV20ToV21() should fail with a corrupted json")
+	}
 }
 
 // Test if all migrate code returns error with corrupted config files

cmd/config-versions.go

@@ -418,6 +418,12 @@ type serverConfigV15 struct {
 	Notify *notifier `json:"notify"`
 }
 
+type loggers struct {
+	sync.RWMutex
+	Console ConsoleLogger `json:"console"`
+	File    FileLogger    `json:"file"`
+}
+
 // serverConfigV16 server configuration version '16' which is like
 // version '15' except it makes a change to logging configuration.
 type serverConfigV16 struct {
@@ -509,3 +515,18 @@ type serverConfigV20 struct {
 	// Notification queue configuration.
 	Notify *notifier `json:"notify"`
 }
+
+// serverConfigV21 is just like version '20' without logger field
+type serverConfigV21 struct {
+	sync.RWMutex
+	Version string `json:"version"`
+
+	// S3 API configuration.
+	Credential auth.Credentials `json:"credential"`
+	Region     string           `json:"region"`
+	Browser    BrowserFlag      `json:"browser"`
+	Domain     string           `json:"domain"`
+
+	// Notification queue configuration.
+	Notify *notifier `json:"notify"`
+}

cmd/gateway-main.go

@@ -142,9 +142,6 @@ func StartGateway(ctx *cli.Context, gw Gateway) {
 	// Initialize gateway config.
 	initConfig()
 
-	// Enable loggers as per configuration file.
-	enableLoggers()
-
 	// Init the error tracing module.
 	errors.Init(GOPATH, "github.com/minio/minio")
 

cmd/logger.go

@@ -22,7 +22,6 @@ import (
 	"path"
 	"runtime"
 	"strings"
-	"sync"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/minio/mc/pkg/console"
@@ -31,52 +30,6 @@ import (
 
 var log = NewLogger()
 
-type loggers struct {
-	sync.RWMutex
-	Console ConsoleLogger `json:"console"`
-	File    FileLogger    `json:"file"`
-}
-
-// Validate - Check whether loggers are valid or not.
-func (l *loggers) Validate() (err error) {
-	if l != nil {
-		fileLogger := l.GetFile()
-		if fileLogger.Enable && fileLogger.Filename == "" {
-			err = fmt.Errorf("Missing filename for enabled file logger")
-		}
-	}
-
-	return err
-}
-
-// SetFile set new file logger.
-func (l *loggers) SetFile(flogger FileLogger) {
-	l.Lock()
-	defer l.Unlock()
-	l.File = flogger
-}
-
-// GetFileLogger get current file logger.
-func (l *loggers) GetFile() FileLogger {
-	l.RLock()
-	defer l.RUnlock()
-	return l.File
-}
-
-// SetConsole set new console logger.
-func (l *loggers) SetConsole(clogger ConsoleLogger) {
-	l.Lock()
-	defer l.Unlock()
-	l.Console = clogger
-}
-
-// GetConsole get current console logger.
-func (l *loggers) GetConsole() ConsoleLogger {
-	l.RLock()
-	defer l.RUnlock()
-	return l.Console
-}
-
 // LogTarget - interface for log target.
 type LogTarget interface {
 	Fire(entry *logrus.Entry) error

cmd/server-main.go

@@ -147,9 +147,6 @@ func serverMain(ctx *cli.Context) {
 	// Initialize server config.
 	initConfig()
 
-	// Enable loggers as per configuration file.
-	enableLoggers()
-
 	// Init the error tracing module.
 	errors.Init(GOPATH, "github.com/minio/minio")