diff --git a/cmd/sonobuoy/app/worker.go b/cmd/sonobuoy/app/worker.go index b85f03247..cfb208b2e 100644 --- a/cmd/sonobuoy/app/worker.go +++ b/cmd/sonobuoy/app/worker.go @@ -104,8 +104,8 @@ func loadAndValidateConfig() (*plugin.WorkerConfig, error) { } var errlst []string - if cfg.MasterURL == "" { - errlst = append(errlst, "MasterURL not set") + if cfg.AggregatorURL == "" { + errlst = append(errlst, "AggregatorURL not set") } if cfg.ResultsDir == "" { errlst = append(errlst, "ResultsDir not set") @@ -163,13 +163,13 @@ func runGather(global bool) error { return errors.Wrap(err, "getting HTTP client") } - resultURL, err := url.Parse(cfg.MasterURL) + resultURL, err := url.Parse(cfg.AggregatorURL) if err != nil { - return errors.Wrap(err, "parsing MasterURL") + return errors.Wrap(err, "parsing AggregatorURL") } - progressURL, err := url.Parse(cfg.MasterURL) + progressURL, err := url.Parse(cfg.AggregatorURL) if err != nil { - return errors.Wrap(err, "parsing MasterURL") + return errors.Wrap(err, "parsing AggregatorURL") } if global { diff --git a/pkg/config/loader.go b/pkg/config/loader.go index b6365d67d..8b01bb653 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -26,6 +26,7 @@ import ( "github.com/vmware-tanzu/sonobuoy/pkg/buildinfo" "github.com/vmware-tanzu/sonobuoy/pkg/plugin" pluginloader "github.com/vmware-tanzu/sonobuoy/pkg/plugin/loader" + "github.com/pkg/errors" uuid "github.com/satori/go.uuid" ) diff --git a/pkg/plugin/driver/base.go b/pkg/plugin/driver/base.go index ffa79ad28..39ae000a8 100644 --- a/pkg/plugin/driver/base.go +++ b/pkg/plugin/driver/base.go @@ -168,7 +168,7 @@ func (b *Base) workerEnvironment(hostname string, cert *tls.Certificate, progres Value: b.GetName(), }, { - Name: "MASTER_URL", + Name: "AGGREGATOR_URL", Value: hostname, }, { diff --git a/pkg/plugin/driver/base_test.go b/pkg/plugin/driver/base_test.go index 92e50f260..15f23f9ef 100644 --- a/pkg/plugin/driver/base_test.go +++ b/pkg/plugin/driver/base_test.go @@ -195,7 +195,7 @@ func TestCreateWorkerContainerDefinition(t *testing.T) { Value: b.GetName(), }, { - Name: "MASTER_URL", + Name: "AGGREGATOR_URL", Value: aggregatorURL, }, { diff --git a/pkg/plugin/interface.go b/pkg/plugin/interface.go index 31733a45c..b4a6e86e7 100644 --- a/pkg/plugin/interface.go +++ b/pkg/plugin/interface.go @@ -136,8 +136,8 @@ type AggregationConfig struct { // WorkerConfig is the file given to the sonobuoy worker to configure it to phone home. type WorkerConfig struct { - // MasterURL is the URL we talk to the aggregator pod on for submitting results - MasterURL string `json:"masterurl,omitempty" mapstructure:"masterurl"` + // AggregatorURL is the URL we talk to the aggregator pod on for submitting results + AggregatorURL string `json:"aggregatorurl,omitempty" mapstructure:"aggregatorurl"` // NodeName is the node name we should call ourselves when sending results NodeName string `json:"nodename,omitempty" mapstructure:"nodename"` diff --git a/pkg/worker/config.go b/pkg/worker/config.go index 42e8b3c4a..a2bf15f07 100644 --- a/pkg/worker/config.go +++ b/pkg/worker/config.go @@ -32,13 +32,22 @@ func setConfigDefaults(ac *plugin.WorkerConfig) { ac.ProgressUpdatesPort = defaultProgressUpdatesPort } +func processDeprecatedVariables() { + // Default to using deprecated "masterurl" key if "aggregatorurl" key is not set. + // Remove in v0.19.0 + viper.BindEnv("masterurl", "MASTER_URL") + if viper.Get("masterurl") != nil && viper.Get("aggregatorurl") == nil { + viper.Set("aggregatorurl", viper.Get("masterurl")) + } +} + // LoadConfig loads the configuration for the sonobuoy worker from environment // variables, returning a plugin.WorkerConfig struct with defaults applied func LoadConfig() (*plugin.WorkerConfig, error) { config := &plugin.WorkerConfig{} var err error - viper.BindEnv("masterurl", "MASTER_URL") + viper.BindEnv("aggregatorurl", "AGGREGATOR_URL") viper.BindEnv("nodename", "NODE_NAME") viper.BindEnv("resultsdir", "RESULTS_DIR") viper.BindEnv("resulttype", "RESULT_TYPE") @@ -50,6 +59,8 @@ func LoadConfig() (*plugin.WorkerConfig, error) { setConfigDefaults(config) + processDeprecatedVariables() + if err = viper.Unmarshal(config); err != nil { return nil, errors.WithStack(err) } diff --git a/pkg/worker/config_test.go b/pkg/worker/config_test.go new file mode 100644 index 000000000..872927c9a --- /dev/null +++ b/pkg/worker/config_test.go @@ -0,0 +1,85 @@ +package worker + +import ( + "github.com/spf13/viper" + "github.com/vmware-tanzu/sonobuoy/pkg/plugin" + "os" + "reflect" + "testing" +) + +func TestLoadConfig(t *testing.T) { + testCases := []struct { + desc string + expectedCfg *plugin.WorkerConfig + env map[string]string + }{ + { + desc: "No environment variables results in default config values", + expectedCfg: &plugin.WorkerConfig{ + ResultsDir: "/tmp/results", + ProgressUpdatesPort: "8099", + }, + }, + { + desc: "Aggregator URL is set in config if env var is set", + expectedCfg: &plugin.WorkerConfig{ + AggregatorURL: "aggregator", + ResultsDir: plugin.ResultsDir, + ProgressUpdatesPort: defaultProgressUpdatesPort, + }, + env: map[string]string{ + "AGGREGATOR_URL": "aggregator", + }, + }, + { + desc: "Aggregator URL is set in config if only deprecated master url env var is set", + expectedCfg: &plugin.WorkerConfig{ + AggregatorURL: "master", + ResultsDir: plugin.ResultsDir, + ProgressUpdatesPort: defaultProgressUpdatesPort, + }, + env: map[string]string{ + "MASTER_URL": "master", + }, + }, + { + desc: "Aggregator URL env var takes precedence if both new and deprecated env vars set", + expectedCfg: &plugin.WorkerConfig{ + AggregatorURL: "aggregator", + ResultsDir: plugin.ResultsDir, + ProgressUpdatesPort: defaultProgressUpdatesPort, + }, + env: map[string]string{ + "MASTER_URL": "master", + "AGGREGATOR_URL": "aggregator", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + viper.Reset() + for k, v := range tc.env { + if err := os.Setenv(k, v); err != nil { + t.Fatalf("unable to set environment variable %q to value %q", k, v) + } + } + defer func() { + for k := range tc.env { + if err := os.Unsetenv(k); err != nil { + t.Fatalf("unable to unset environment variable %q", k) + } + } + }() + + cfg, err := LoadConfig() + if err != nil { + t.Fatalf("unexepected err from LoadConfig %q", err) + } + if !reflect.DeepEqual(cfg, tc.expectedCfg) { + t.Fatalf("expected config to be %q, got %q", tc.expectedCfg, cfg) + } + }) + } +}