Repository: NetApp/harvest
Pull request #239 (2021-06-24 18:44:17)

fix: poller struct should union defaults


Patch:
diff --git a/cmd/exporters/prometheus/README.md b/cmd/exporters/prometheus/README.md
index c570c864..62aa84e1 100644
--- a/cmd/exporters/prometheus/README.md
+++ b/cmd/exporters/prometheus/README.md
@@ -4,38 +4,85 @@
 
 ## Overview
 
-The Prometheus Exporter will format metrics into the Prometheus [line protocol](https://prometheus.io/docs/instrumenting/exposition_formats/) (a.k.a. *open metric format*) and expose it on an HTTP port (`http://<ADDR>:<PORT>/metrics`). Additionally, it serves a basic overview of available metrics and collectors on its root address (`http://<ADDR>:<PORT>`).
+The Prometheus exporter is responsible for:
+- formatting metrics into the Prometheus [line protocol](https://prometheus.io/docs/instrumenting/exposition_formats/) 
+- creating a web-endpoint on `http://<ADDR>:<PORT>/metrics` for Prometheus to scrape 
+  
+A web end-point is required because Prometheus scrapes Harvest by polling that end-point.
 
-Don't forget to [update your Prometheus configuration](#configure-prometheus-to-scrape-from-harvest) and add a new target for each of the ports defined in your Harvest configuration.
+In addition to the `/metrics` end-point, the Prometheus exporter also serves an overview of all metrics and collectors available on its root address `http://<ADDR>:<PORT>/`.
+
+Because Prometheus polls Harvest, don't forget to [update your Prometheus configuration](#configure-prometheus-to-scrape-from-harvest) and add a new target for each Prometheus based poller.
 
 ## Design
 
-The Exporter has two concurrent components that makes it possible to simultaneously serve push requests by collectors and pull requests by Prometheus scrapers.
+The Exporter runs two goroutines that simultaneously serve requests by collectors and Prometheus scrapers.
 
 <img src="prometheus.png" width="100%" align="center">
 
+There are two ways to configure the Prometheus exporter: using a `port range` or individual `port`s. 
 
+The `port range` is more flexible and should be used when you want multiple pollers all exporting to the same instance of Prometheus. Both options are explained below.
 ## Parameters
 
-All parameters of the exporter are defined in the `Exporters` section of `harvest.yml`. We are planning to integrate the exporter with one of Prometheus' service-discovery options to simplify this in the future.
-
+All parameters of the exporter are defined in the `Exporters` section of `harvest.yml`. 
 
 An overview of all parameters:
 
-
-| parameter              | type         | description                                      | default                |
-|------------------------|--------------|--------------------------------------------------|------------------------|
-| `port`                 | int, required | port of the HTTP server                          |                        |
-| `local_http_addr`      | string, optional	| address of the HTTP server Harvest starts for Prometheus to scrape:<br />use `localhost` to serve only on the local machine<br />use `0.0.0.0` (default) if Prometheus is scrapping from another machine | `0.0.0.0` |
-| `global_prefix`         | string, optional | add a prefix to all metrics (e.g. `netapp_`) |                        |
-| `allow_addrs`          | list of strings, optional | allow access only if host matches any of the provided addresses | |
-| `allow_addrs_regex`	 | list of strings, optional | allow access only if host address matches at least one of the regular expressions | |
-| `cache_max_keep`       | string (Go duration format), optional | maximum amount of time metrics are cached (in case Prometheus does not timely collect the metrics) | `180s` |
-| `add_meta_tags` |	bool, optional | add `HELP` and `TYPE` [metatags](https://prometheus.io/docs/instrumenting/exposition_formats/#comments-help-text-and-type-information) to metrics (currently no useful information, but required by some tools) | `false`	|
+| parameter           | type                                           | description                                                                                                                                                                                                     | default   |
+| ------------------- | ---------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- |
+| `port_range`        | int-int (range), overrides `port` if specified | lower port to upper port (inclusive) of the HTTP end-point to create when a poller specifies this exporter. Starting at lower port, each free port will be tried sequentially up to the upper port.             |           |
+| `port`              | int, required if port_range is not specified   | port of the HTTP end-point                                                                                                                                                                                      |           |
+| `local_http_addr`   | string, optional                               | address of the HTTP server Harvest starts for Prometheus to scrape:<br />use `localhost` to serve only on the local machine<br />use `0.0.0.0` (default) if Prometheus is scrapping from another machine        | `0.0.0.0` |
+| `global_prefix`     | string, optional                               | add a prefix to all metrics (e.g. `netapp_`)                                                                                                                                                                    |           |
+| `allow_addrs`       | list of strings, optional                      | allow access only if host matches any of the provided addresses                                                                                                                                                 |           |
+| `allow_addrs_regex` | list of strings, optional                      | allow access only if host address matches at least one of the regular expressions                                                                                                                               |           |
+| `cache_max_keep`    | string (Go duration format), optional          | maximum amount of time metrics are cached (in case Prometheus does not timely collect the metrics)                                                                                                              | `180s`    |
+| `add_meta_tags`     | bool, optional                                 | add `HELP` and `TYPE` [metatags](https://prometheus.io/docs/instrumenting/exposition_formats/#comments-help-text-and-type-information) to metrics (currently no useful information, but required by some tools) | `false`   |
 
 
 A few examples:
 
+#### port_range
+
+```yaml
+Exporters:
+  prom-prod:
+    exporter: Prometheus
+    addr: 0.0.0.0
+    port_range: 2000-2030
+Pollers:
+  cluster-01:
+    exporters:
+    - prom-prod
+  cluster-02:
+    exporters:
+    - prom-prod
+  cluster-03:
+    exporters:
+    - prom-prod
+  ...
+  cluster-16:
+    exporters:
+    - prom-prod
+```
+
+Sixteen pollers will collect metrics from 16 clusters and make those metrics available to a single instance of Prometheus named `prom-prod`. Sixteen web end-points will be created on the first 16 available free ports between 2000 and 2030 (inclusive). 
+
+After staring the pollers in the example above, running `bin/harvest status` shows the following. Note that ports 2000 and 2003 were not available so the next free port in the range was selected. If no free port can be found an error will be logged.
+
+```
+Datacenter   Poller       PID     PromPort  Status              
+++++++++++++ ++++++++++++ +++++++ +++++++++ ++++++++++++++++++++
+DC-01        cluster-01   2339    2001      running         
+DC-01        cluster-02   2343    2002      running         
+DC-01        cluster-03   2351    2004      running         
+...
+DC-01        cluster-14   2405    2015      running         
+DC-01        cluster-15   2502    2016      running         
+DC-01        cluster-16   2514    2017      running         
+```
+
 #### allow_addrs
 
 ```yaml
@@ -47,7 +94,6 @@ Exporters:
 ```
 will only allow access from exactly these two addresses.
 
-
 #### allow_addrs_regex
 
 ```yaml
@@ -60,7 +106,7 @@ will only allow access from the IP4 range `192.168.0.0`-`192.168.0.255`.
 
 ## Configure Prometheus to scrape from Harvest
 
-As an example, if we defined four prometheus exporters at ports: 12990, 12991, 14567, and 14568 you need to add four sections to your `prometheus.yml`.
+If we define four prometheus exporters at ports: 12990, 12991, 14567, and 14568 you need to add four sections to your `prometheus.yml`.
 
 ```bash
 $ vim /etc/prometheus/prometheus.yml
diff --git a/cmd/tools/zapi/zapi.go b/cmd/tools/zapi/zapi.go
index 71f8b2b6..06abce42 100644
--- a/cmd/tools/zapi/zapi.go
+++ b/cmd/tools/zapi/zapi.go
@@ -53,6 +53,7 @@ type Args struct {
 	MaxRecords int
 	// additional parameters to add to the ZAPI request, in "key:value" format
 	Parameters []string
+	Config     string // filepath of Harvest config (defaults to "harvest.yml") can be relative or absolute path
 }
 
 var ZapiCmd = &cobra.Command{
@@ -127,16 +128,13 @@ func validateArgs(strings []string) {
 
 func doCmd(cmd string) {
 	var (
-		err               error
-		item, params      *node.Node
-		connection        *client.Client
-		harvestConfigPath string
+		err          error
+		item, params *node.Node
+		connection   *client.Client
 	)
 
-	harvestConfigPath, err = conf.GetDefaultHarvestConfigPath()
-
 	// connect to cluster and retrieve system version
-	if params, err = conf.GetPoller(harvestConfigPath, args.Poller); err != nil {
+	if params, err = conf.GetPoller(args.Config, args.Poller); err != nil {
 		fmt.Println(err)
 		os.Exit(1)
 	}
@@ -320,16 +318,20 @@ func getData(c *client.Client, args *Args) (*node.Node, error) {
 var args = &Args{}
 
 func init() {
+	configPath, _ := conf.GetDefaultHarvestConfigPath()
+
 	ZapiCmd.AddCommand(showCmd, exportCmd)
-	ZapiCmd.PersistentFlags().StringVarP(&args.Poller, "poller", "p", "", "name of poller (cluster), as defined in your harvest config")
+	flags := ZapiCmd.PersistentFlags()
+	flags.StringVarP(&args.Poller, "poller", "p", "", "name of poller (cluster), as defined in your harvest config")
 	_ = ZapiCmd.MarkPersistentFlagRequired("poller")
 
-	ZapiCmd.PersistentFlags().StringVarP(&args.Api, "api", "a", "", "ZAPI query to show")
-	ZapiCmd.PersistentFlags().StringVarP(&args.Attr, "attr", "t", "", "ZAPI attribute to show")
-	ZapiCmd.PersistentFlags().StringVarP(&args.Object, "object", "o", "", "ZapiPerf object to show")
-	ZapiCmd.PersistentFlags().StringVarP(&args.Counter, "counter", "c", "", "ZapiPerf counter to show")
-	ZapiCmd.PersistentFlags().IntVarP(&args.MaxRecords, "max", "m", 100, "max-records: max instances per API request")
-	ZapiCmd.PersistentFlags().StringSliceVarP(&args.Parameters, "parameters", "r", []string{}, "parameter to add to the ZAPI query")
+	flags.StringVarP(&args.Api, "api", "a", "", "ZAPI query to show")
+	flags.StringVarP(&args.Attr, "attr", "t", "", "ZAPI attribute to show")
+	flags.StringVarP(&args.Object, "object", "o", "", "ZapiPerf object to show")
+	flags.StringVarP(&args.Counter, "counter", "c", "", "ZapiPerf counter to show")
+	flags.IntVarP(&args.MaxRecords, "max", "m", 100, "max-records: max instances per API request")
+	flags.StringSliceVarP(&args.Parameters, "parameters", "r", []string{}, "parameter to add to the ZAPI query")
+	flags.StringVar(&args.Config, "config", configPath, "harvest config file path")
 
 	showCmd.SetUsageTemplate("item to show should be one of: " + strings.Join(validShowArgs, ", "))
 
diff --git a/pkg/conf/conf.go b/pkg/conf/conf.go
index e8595942..acfa39f2 100644
--- a/pkg/conf/conf.go
+++ b/pkg/conf/conf.go
@@ -127,7 +127,7 @@ func GetPollerNames(configFp string) ([]string, error) {
 	return pollerNames, nil
 }
 
-func GetPollers2(configFp string) (map[string]Poller, error) {
+func GetPollers2(configFp string) (map[string]*Poller, error) {
 	err := LoadHarvestConfig(configFp)
 	if err != nil {
 		return nil, err
@@ -175,7 +175,7 @@ func GetPoller2(configFp, pollerName string) (*Poller, error) {
 	if !ok {
 		return nil, errors.New(errors.ERR_CONFIG, "poller ["+pollerName+"] not found")
 	}
-	return &poller, nil
+	return poller, nil
 }
 
 func GetPoller(configFp, pollerName string) (*node.Node, error) {
@@ -230,8 +230,11 @@ func GetPrometheusExporterPorts(pollerName string) (int, error) {
 	if len(promPortRangeMapping) == 0 {
 		loadPrometheusExporterPortRangeMapping()
 	}
-	exporters := (*Config.Pollers)[pollerName].Exporters
-
+	poller := (*Config.Pollers)[pollerName]
+	if poller == nil {
+		return 0, errors.New(errors.ERR_CONFIG, "Poller does not exist "+pollerName)
+	}
+	exporters := poller.Exporters
 	if exporters != nil && len(*exporters) > 0 {
 		for _, e := range *exporters {
 			exporter := (*Config.Exporters)[e]
@@ -251,8 +254,6 @@ func GetPrometheusExporterPorts(pollerName string) (int, error) {
 			}
 			continue
 		}
-	} else {
-		return 0, errors.New(errors.ERR_CONFIG, "Poller does not exist "+pollerName)
 	}
 	if port == 0 && isPrometheusExporterConfigured {
 		return port, errors.New(errors.ERR_CONFIG, "No free port found for poller "+pollerName)
@@ -470,7 +471,7 @@ type OrderedConfig struct {
 type HarvestConfig struct {
 	Tools          *Tools               `yaml:"Tools,omitempty"`
 	Exporters      *map[string]Exporter `yaml:"Exporters,omitempty"`
-	Pollers        *map[string]Poller   `yaml:"Pollers,omitempty"`
+	Pollers        *map[string]*Poller  `yaml:"Pollers,omitempty"`
 	Defaults       *Poller              `yaml:"Defaults,omitempty"`
 	PollersOrdered []string             // poller names in same order as yaml config
 }
diff --git a/pkg/util/util.go b/pkg/util/util.go
index c78bbc31..b87d3e7b 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -73,10 +73,6 @@ func readProcFile(path string) (string, error) {
 	return result, nil
 }
 
-func GetEnviron(pid int) (string, error) {
-	return readProcFile(fmt.Sprintf("/proc/%d/environ", pid))
-}
-
 func GetCmdLine(pid int) (string, error) {
 	if runtime.GOOS == "darwin" {
 		return darwinGetCmdLine(pid)
@@ -168,23 +164,8 @@ func GetPids(search string) ([]int, error) {
 		}
 
 		// Validate this is a Harvest process
-		if runtime.GOOS == "darwin" {
-			// env check does not work on darwin
-			result = append(result, p)
-		} else {
-			environ, err := GetEnviron(p)
-			if err != nil {
-				if errors.As(err, &pe) {
-					// permission denied, no need to log
-					continue
-				}
-				fmt.Printf("err reading environ for search=%s pid=%d err=%+v\n", search, p, err)
-				continue
-			}
-			if strings.Contains(environ, HarvestTag) {
-				result = append(result, p)
-			}
-		}
+		// env check does not work on Darwin or Unix when running as non-root
+		result = append(result, p)
 	}
 	return result, err
 }


Test patch:
diff --git a/cmd/tools/doctor/testdata/testConfig.yml b/cmd/tools/doctor/testdata/testConfig.yml
index bf6cfd12..26fe7589 100644
--- a/cmd/tools/doctor/testdata/testConfig.yml
+++ b/cmd/tools/doctor/testdata/testConfig.yml
@@ -40,6 +40,8 @@ Defaults:
   collectors:
     - Zapi
     - ZapiPerf
+  exporters:
+    - prometheusrange
   use_insecure_tls: false
   username: myuser
   password: 123#abc
diff --git a/pkg/conf/conf_test.go b/pkg/conf/conf_test.go
index 3869a2d6..43b2ec12 100644
--- a/pkg/conf/conf_test.go
+++ b/pkg/conf/conf_test.go
@@ -2,6 +2,7 @@ package conf
 
 import (
 	"fmt"
+	"reflect"
 	"testing"
 )
 
@@ -41,3 +42,57 @@ func TestGetPrometheusExporterPorts(t *testing.T) {
 		})
 	}
 }
+
+func TestPollerStructDefaults(t *testing.T) {
+	path := "../../cmd/tools/doctor/testdata/testConfig.yml"
+	err := LoadHarvestConfig(path)
+	if err != nil {
+		panic(err)
+	}
+	t.Run("poller exporters", func(t *testing.T) {
+		poller, err := GetPoller2(path, "zeros")
+		if err != nil {
+			panic(err)
+		}
+		// the poller does not define exporters but defaults does
+		if poller.Exporters == nil {
+			t.Fatalf(`expected exporters to not be nil, but it was`)
+		}
+		if len(*poller.Exporters) != 1 {
+			t.Fatalf(`expected 1 exporters but got %v`, *poller.Exporters)
+		}
+		expected := []string{"prometheusrange"}
+		if !reflect.DeepEqual(*poller.Exporters, expected) {
+			t.Fatalf(`expected collectors to be %v but was %v`, expected, *poller.Exporters)
+		}
+	})
+
+	t.Run("poller collector", func(t *testing.T) {
+		poller, err := GetPoller2(path, "cluster-01")
+		if err != nil {
+			panic(err)
+		}
+		// the poller does not define collectors but defaults does
+		if poller.Collectors == nil {
+			t.Fatalf(`expected collectors to not be nil, but it was`)
+		}
+		if len(*poller.Collectors) != 2 {
+			t.Fatalf(`expected 2 collectors but got %v`, *poller.Collectors)
+		}
+		expected := []string{"Zapi", "ZapiPerf"}
+		if !reflect.DeepEqual(*poller.Collectors, expected) {
+			t.Fatalf(`expected collectors to be %v but was %v`, expected, *poller.Collectors)
+		}
+	})
+
+	t.Run("poller username", func(t *testing.T) {
+		poller, err := GetPoller2(path, "zeros")
+		if err != nil {
+			panic(err)
+		}
+		// the poller does not define a username but defaults does
+		if *poller.Username != "myuser" {
+			t.Fatalf(`expected username to be [myuser] but was [%v]`, *poller.Username)
+		}
+	})
+}