Log warning on unused config keys (#3458)

* Log warning on unused config keys * Comments * Remove unused config * Remove unused config
armadaproject · Mar 14, 2024 · 2b9a81c · 2b9a81c
1 parent be3b40c
commit 2b9a81c
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 24 deletions.
diff --git a/cmd/armada/main.go b/cmd/armada/main.go
@@ -39,8 +39,7 @@ func main() {
 	common.ConfigureLogging()
 	common.BindCommandlineArguments()
 
-	// TODO Load relevant config in one place: don't use viper here and in the config package
-	// (currently in common).
+	// TODO Load relevant config in one place: don't use viper here and in LoadConfig.
 	var config configuration.ArmadaConfig
 	userSpecifiedConfigs := viper.GetStringSlice(CustomConfigLocation)
 	common.LoadConfig(&config, "./config/armada", userSpecifiedConfigs)

diff --git a/config/armada/config.yaml b/config/armada/config.yaml
@@ -98,17 +98,10 @@ submission:
   defaultActiveDeadline: "72h" # 3 days.
   defaultActiveDeadlineByResourceRequest:
     nvidia.com/gpu: "336h" # 14 days.
-eventRetention:
-  expiryEnabled: true
-  retentionDuration: 336h
-metrics:
-  refreshInterval: 5m
 pulsar:
   URL: "pulsar://pulsar:6650"
   jobsetEventsTopic: "events"
   redisFromPulsarSubscription: "RedisFromPulsar"
-  hostnameSuffix: "svc"
-  certNameSuffix: "ingress-tls-certificate"
   dedupTable: pulsar_submit_dedup
   maxConnectionsPerBroker: 1
   compressionType: zlib

diff --git a/config/binoculars/config.yaml b/config/binoculars/config.yaml
@@ -9,8 +9,6 @@ cordon:
     armadaproject.io/cordon-reason: "binoculars"
     armadaproject.io/cordon-user: "<user>"
 auth:
-  basicAuth:
-    enableAuthentication: false
   anonymousAuth: true
 impersonateUsers: false
 kubernetes:

diff --git a/config/lookoutv2/config.yaml b/config/lookoutv2/config.yaml
@@ -8,9 +8,6 @@ tls:
   certPath: /certs/tls.crt
   keyPath: /certs/tls.key
 postgres:
-  maxOpenConns: 100
-  maxIdleConns: 25
-  connMaxLifetime: 30m
   connection:
     host: postgres
     port: 5433

diff --git a/config/scheduler/config.yaml b/config/scheduler/config.yaml
@@ -1,7 +1,6 @@
 cyclePeriod: 1s
 schedulePeriod: 10s
 maxSchedulingDuration: 5s
-maxJobsLeasedPerCall: 1000
 executorTimeout: 1h
 databaseFetchSize: 1000
 pulsarSendTimeout: 5s

diff --git a/config/scheduleringester/config.yaml b/config/scheduleringester/config.yaml
@@ -1,25 +1,19 @@
 postgres:
-  maxOpenConns: 10
-  maxIdleConns: 1
-  connMaxLifetime: 30m
   connection:
     host: localhost
     port: 5432
     user: postgres
     password: psw
     dbname: postgres
     sslmode: disable
-
 metrics:
   port: 9003
-
 pulsar:
   URL: "pulsar://localhost:6650"
   jobsetEventsTopic: "events"
   receiveTimeout: 5s
   backoffTime: 1s
   receiverQueueSize: 100
-
 subscriptionName: "scheduler-ingester"
 batchSize: 10000
 batchDuration: 500ms

diff --git a/internal/common/startup.go b/internal/common/startup.go
@@ -11,6 +11,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/mitchellh/mapstructure"
+	"golang.org/x/exp/slices"
+
 	"github.com/armadaproject/armada/internal/common/certs"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -39,7 +42,7 @@ func BindCommandlineArguments() {
 }
 
 // TODO Move code relating to config out of common into a new package internal/serverconfig
-func LoadConfig(config interface{}, defaultPath string, overrideConfigs []string) *viper.Viper {
+func LoadConfig(config any, defaultPath string, overrideConfigs []string) *viper.Viper {
 	v := viper.NewWithOptions(viper.KeyDelimiter("::"))
 	v.SetConfigName(baseConfigFileName)
 	v.AddConfigPath(defaultPath)
@@ -48,6 +51,9 @@ func LoadConfig(config interface{}, defaultPath string, overrideConfigs []string
 		os.Exit(-1)
 	}
 	log.Infof("Read base config from %s", v.ConfigFileUsed())
+	if len(overrideConfigs) > 0 {
+		log.Infof("Read override config from %v", overrideConfigs)
+	}
 
 	for _, overrideConfig := range overrideConfigs {
 		v.SetConfigFile(overrideConfig)
@@ -63,12 +69,26 @@ func LoadConfig(config interface{}, defaultPath string, overrideConfigs []string
 	v.SetEnvPrefix("ARMADA")
 	v.AutomaticEnv()
 
-	err := v.Unmarshal(config, commonconfig.CustomHooks...)
-	if err != nil {
+	var metadata mapstructure.Metadata
+	customHooks := append(slices.Clone(commonconfig.CustomHooks), func(c *mapstructure.DecoderConfig) { c.Metadata = &metadata })
+	if err := v.Unmarshal(config, customHooks...); err != nil {
 		log.Error(err)
 		os.Exit(-1)
 	}
 
+	// Log a warning if there are config keys that don't match a config item in the struct the yaml is decoded into.
+	// Since such unused keys indicate there's a typo in the config.
+	// Also log set and unset keys at a debug level.
+	if len(metadata.Keys) > 0 {
+		log.Debugf("Decoded keys: %v", metadata.Keys)
+	}
+	if len(metadata.Unused) > 0 {
+		log.Warnf("Unused keys: %v", metadata.Unused)
+	}
+	if len(metadata.Unset) > 0 {
+		log.Debugf("Unset keys: %v", metadata.Unset)
+	}
+
 	return v
 }