From a887624e945244da75b7b1cbe2029a53751269b3 Mon Sep 17 00:00:00 2001 From: Chris Grindstaff Date: Mon, 18 Sep 2023 11:41:35 -0400 Subject: [PATCH 1/2] fix: read harvest.yml from `HARVEST_CONF` when set --- cmd/admin/admin.go | 2 +- cmd/collectors/unix/main.go | 2 +- cmd/exporters/influxdb/influxdb_test.go | 2 +- cmd/harvest/harvest.go | 3 ++- cmd/poller/poller.go | 23 +++++++++++++++----- cmd/tools/generate/generate.go | 6 ++--- cmd/tools/grafana/grafana.go | 4 ++-- cmd/tools/rest/rest.go | 2 +- cmd/tools/zapi/zapi.go | 2 +- integration/test/metric_test.go | 2 +- integration/test/utils/utils.go | 2 +- pkg/conf/conf.go | 29 +++++++++++++------------ pkg/conf/conf_test.go | 20 +++++++++++++++++ pkg/conf/testdata/harvest.yml | 6 +++++ 14 files changed, 72 insertions(+), 33 deletions(-) create mode 100644 pkg/conf/testdata/harvest.yml diff --git a/cmd/admin/admin.go b/cmd/admin/admin.go index e1d976c6e..d8eed9f0d 100644 --- a/cmd/admin/admin.go +++ b/cmd/admin/admin.go @@ -223,7 +223,7 @@ func doTLS(_ *cobra.Command, _ []string) { func doAdmin(c *cobra.Command, _ []string) { var configPath = c.Root().PersistentFlags().Lookup("config").Value.String() - err := conf.LoadHarvestConfig(configPath) + _, err := conf.LoadHarvestConfig(configPath) if err != nil { return } diff --git a/cmd/collectors/unix/main.go b/cmd/collectors/unix/main.go index badd567cd..8739c23cc 100644 --- a/cmd/collectors/unix/main.go +++ b/cmd/collectors/unix/main.go @@ -280,7 +280,7 @@ func (u *Unix) PollInstance() (map[string]*matrix.Matrix, error) { currInstances := set.NewFrom(mat.GetInstanceKeys()) currSize := currInstances.Size() - err := conf.LoadHarvestConfig(u.Options.Config) + _, err := conf.LoadHarvestConfig(u.Options.Config) if err != nil { return nil, err } diff --git a/cmd/exporters/influxdb/influxdb_test.go b/cmd/exporters/influxdb/influxdb_test.go index d019e962f..9be1e48f8 100644 --- a/cmd/exporters/influxdb/influxdb_test.go +++ b/cmd/exporters/influxdb/influxdb_test.go @@ -15,7 +15,7 @@ func setupInfluxDB(t *testing.T, exporterName string) *InfluxDB { opts := options.New() opts.Debug = true - err := conf.LoadHarvestConfig("../../tools/doctor/testdata/testConfig.yml") + _, err := conf.LoadHarvestConfig("../../tools/doctor/testdata/testConfig.yml") if err != nil { panic(err) } diff --git a/cmd/harvest/harvest.go b/cmd/harvest/harvest.go index a2d786e6f..c0bf18911 100644 --- a/cmd/harvest/harvest.go +++ b/cmd/harvest/harvest.go @@ -101,7 +101,8 @@ func doManageCmd(cmd *cobra.Command, args []string) { // cmd.DebugFlags() // uncomment to print flags - if err = conf.LoadHarvestConfig(opts.config); err != nil { + _, err = conf.LoadHarvestConfig(opts.config) + if err != nil { if os.IsNotExist(err) { log.Fatalf("config [%s]: not found\n", opts.config) } diff --git a/cmd/poller/poller.go b/cmd/poller/poller.go index 3f24deef2..0726735ff 100644 --- a/cmd/poller/poller.go +++ b/cmd/poller/poller.go @@ -129,12 +129,16 @@ type Poller struct { // starts collectors and exporters func (p *Poller) Init() error { - var err error + var ( + err error + fileLoggingEnabled bool + consoleLoggingEnabled bool + configPath string + ) + p.options = opts.SetDefaults() p.name = opts.Poller - var fileLoggingEnabled bool - var consoleLoggingEnabled bool zeroLogLevel := logging.GetZerologLevel(p.options.LogLevel) // if we are a daemon, use file logging if p.options.Daemon { @@ -147,18 +151,24 @@ func (p *Poller) Init() error { logFileName = "poller_" + p.name + ".log" } - err = conf.LoadHarvestConfig(p.options.Config) + configPath, err = conf.LoadHarvestConfig(p.options.Config) if err != nil { // separate logger is not yet configured as it depends on setting logMaxMegaBytes, logMaxFiles later // Using default instance of logger which logs below error to harvest.log logging.Get().SubLogger("Poller", p.name).Error(). - Str("config", p.options.Config).Err(err).Msg("Unable to read config") + Str("config", p.options.Config). + Str("configPath", configPath). + Err(err). + Msg("Unable to read config") return err } p.params, err = conf.PollerNamed(p.name) if err != nil { logging.Get().SubLogger("Poller", p.name).Error(). - Str("config", p.options.Config).Err(err).Msg("Failed to find poller") + Str("config", p.options.Config). + Str("configPath", configPath). + Err(err). + Msg("Failed to find poller") return err } @@ -199,6 +209,7 @@ func (p *Poller) Init() error { logger.Info(). Str("logLevel", zeroLogLevel.String()). + Str("configPath", configPath). Str("version", strings.TrimSpace(version.String())). EmbedObject(p.options). Msg("Init") diff --git a/cmd/tools/generate/generate.go b/cmd/tools/generate/generate.go index 919982762..df69152e2 100644 --- a/cmd/tools/generate/generate.go +++ b/cmd/tools/generate/generate.go @@ -150,7 +150,7 @@ func generateDocker(path string, kind int) { opts.grafanaPort, opts.promPort, } - err := conf.LoadHarvestConfig(path) + _, err := conf.LoadHarvestConfig(path) if err != nil { logErrAndExit(err) } @@ -348,7 +348,7 @@ func silentClose(body io.ReadCloser) { func generateSystemd(path string) { var adminService string - err := conf.LoadHarvestConfig(path) + _, err := conf.LoadHarvestConfig(path) if err != nil { logErrAndExit(err) } @@ -430,7 +430,7 @@ func generateMetrics(path string) { zapiClient *zapi.Client ) - err = conf.LoadHarvestConfig(path) + _, err = conf.LoadHarvestConfig(path) if err != nil { logErrAndExit(err) } diff --git a/cmd/tools/grafana/grafana.go b/cmd/tools/grafana/grafana.go index 2268c66fd..c4aabf309 100644 --- a/cmd/tools/grafana/grafana.go +++ b/cmd/tools/grafana/grafana.go @@ -351,7 +351,7 @@ func newLabelVar(label string) []byte { func doImport(_ *cobra.Command, _ []string) { opts.command = "import" - err := conf.LoadHarvestConfig(opts.config) + _, err := conf.LoadHarvestConfig(opts.config) if err != nil { return } @@ -731,7 +731,7 @@ func checkToken(opts *options, ignoreConfig bool, tries int) error { configPath = opts.config - err = conf.LoadHarvestConfig(configPath) + _, err = conf.LoadHarvestConfig(configPath) if err != nil { return err } diff --git a/cmd/tools/rest/rest.go b/cmd/tools/rest/rest.go index 383735c4e..a5be563ee 100644 --- a/cmd/tools/rest/rest.go +++ b/cmd/tools/rest/rest.go @@ -112,7 +112,7 @@ func doShow(_ *cobra.Command, a []string) { if !c.isValid { return } - err := conf.LoadHarvestConfig(args.Config) + _, err := conf.LoadHarvestConfig(args.Config) if err != nil { log.Fatal(err) } diff --git a/cmd/tools/zapi/zapi.go b/cmd/tools/zapi/zapi.go index 3b583519b..b52d9c8df 100644 --- a/cmd/tools/zapi/zapi.go +++ b/cmd/tools/zapi/zapi.go @@ -129,7 +129,7 @@ func doCmd(cmd string) { connection *client.Client ) - err = conf.LoadHarvestConfig(args.Config) + _, err = conf.LoadHarvestConfig(args.Config) if err != nil { log.Fatal(err) } diff --git a/integration/test/metric_test.go b/integration/test/metric_test.go index eb1329012..631109314 100644 --- a/integration/test/metric_test.go +++ b/integration/test/metric_test.go @@ -25,7 +25,7 @@ var skipDuplicates = map[string]bool{ func TestPollerMetrics(t *testing.T) { utils.SkipIfMissing(t, utils.Regression) - err := conf.LoadHarvestConfig(installer.HarvestConfigFile) + _, err := conf.LoadHarvestConfig(installer.HarvestConfigFile) if err != nil { log.Fatal().Err(err).Msg("Unable to load harvest config") } diff --git a/integration/test/utils/utils.go b/integration/test/utils/utils.go index a87f21cac..e8d4c5170 100644 --- a/integration/test/utils/utils.go +++ b/integration/test/utils/utils.go @@ -248,7 +248,7 @@ func WriteToken(token string) { var err error filename := "harvest.yml" abs, _ := filepath.Abs(filename) - err = conf.LoadHarvestConfig(filename) + _, err = conf.LoadHarvestConfig(filename) PanicIfNotNil(err) tools := conf.Config.Tools if tools != nil { diff --git a/pkg/conf/conf.go b/pkg/conf/conf.go index f44068822..2f113d4d9 100644 --- a/pkg/conf/conf.go +++ b/pkg/conf/conf.go @@ -36,7 +36,7 @@ func TestLoadHarvestConfig(configPath string) { configRead = false Config = HarvestConfig{} promPortRangeMapping = make(map[string]PortMap) - err := LoadHarvestConfig(configPath) + _, err := LoadHarvestConfig(configPath) if err != nil { log.Fatalf("Failed to load config at=[%s] err=%+v\n", configPath, err) } @@ -44,36 +44,37 @@ func TestLoadHarvestConfig(configPath string) { func ConfigPath(path string) string { // Harvest uses the following precedence order. Each item takes precedence over the - // item below it: - // 1. --config command line flag - // 2. HARVEST_CONFIG environment variable + // item below it. All paths are relative to the `HARVEST_CONF` environment variable + // 1. `--config` command line flag + // 2. `HARVEST_CONFIG` environment variable // 3. no command line argument and no environment variable, use the default path (HarvestYML) + // If the environment variable, HARVEST_CONF is set, items three will be relative to it if path != HarvestYML && path != "./"+HarvestYML { - return path + return Path(path) } fp := os.Getenv("HARVEST_CONFIG") - if fp == "" { - return path + if fp != "" { + path = fp } - return fp + return Path(path) } -func LoadHarvestConfig(configPath string) error { +func LoadHarvestConfig(configPath string) (string, error) { + configPath = ConfigPath(configPath) if configRead { - return nil + return configPath, nil } - configPath = ConfigPath(configPath) contents, err := os.ReadFile(configPath) if err != nil { - return fmt.Errorf("error reading %s err=%w", configPath, err) + return "", fmt.Errorf("error reading %s err=%w", configPath, err) } err = DecodeConfig(contents) if err != nil { fmt.Printf("error unmarshalling config file=[%s] %+v\n", configPath, err) - return err + return "", err } - return nil + return configPath, nil } func DecodeConfig(contents []byte) error { diff --git a/pkg/conf/conf_test.go b/pkg/conf/conf_test.go index a003a1b66..5e5a49da2 100644 --- a/pkg/conf/conf_test.go +++ b/pkg/conf/conf_test.go @@ -281,3 +281,23 @@ func TestNodeToPoller(t *testing.T) { testArg(t, "30s", poller.ClientTimeout) testArg(t, "true", strconv.FormatBool(*poller.UseInsecureTLS)) } + +func TestReadHarvestConfigFromEnv(t *testing.T) { + t.Helper() + configRead = false + Config = HarvestConfig{} + t.Setenv(HomeEnvVar, "testdata") + cp, err := LoadHarvestConfig(HarvestYML) + if err != nil { + t.Errorf("Failed to load config at=[%s] err=%+v\n", HarvestYML, err) + return + } + wantCp := "testdata/harvest.yml" + if cp != wantCp { + t.Errorf("configPath got=%s want=%s", cp, wantCp) + } + poller := Config.Pollers["star"] + if poller == nil { + t.Errorf("check if star poller exists. got=nil want=poller") + } +} diff --git a/pkg/conf/testdata/harvest.yml b/pkg/conf/testdata/harvest.yml new file mode 100644 index 000000000..715be42e1 --- /dev/null +++ b/pkg/conf/testdata/harvest.yml @@ -0,0 +1,6 @@ + +Pollers: + star: + addr: localhost + collectors: + - Simple From c4646c8ea26cc2bcabfc305848a860a60e928c85 Mon Sep 17 00:00:00 2001 From: Chris Grindstaff Date: Mon, 18 Sep 2023 11:47:06 -0400 Subject: [PATCH 2/2] fix: read harvest.yml from `HARVEST_CONF` when set --- pkg/conf/conf.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/conf/conf.go b/pkg/conf/conf.go index 2f113d4d9..ae14956f0 100644 --- a/pkg/conf/conf.go +++ b/pkg/conf/conf.go @@ -44,11 +44,10 @@ func TestLoadHarvestConfig(configPath string) { func ConfigPath(path string) string { // Harvest uses the following precedence order. Each item takes precedence over the - // item below it. All paths are relative to the `HARVEST_CONF` environment variable + // item below it. All paths are relative to `HARVEST_CONF` environment variable // 1. `--config` command line flag // 2. `HARVEST_CONFIG` environment variable // 3. no command line argument and no environment variable, use the default path (HarvestYML) - // If the environment variable, HARVEST_CONF is set, items three will be relative to it if path != HarvestYML && path != "./"+HarvestYML { return Path(path) }