Repository: NetApp/harvest
Pull request #218 (2021-06-23 09:39:40)

port range test cases and harvest doctor validation


Patch:
diff --git a/README.md b/README.md
index 75b27e95..a039290a 100644
--- a/README.md
+++ b/README.md
@@ -137,11 +137,6 @@ bin/harvest version
 If you're building on a Mac use `GOOS=darwin make build`
 
 Checkout the `Makefile` for other targets of interest.
-
-## Build Binaries
-Refer [Build Binaries](jenkins/artifacts/README.MD)
-
-
 # Quick start
 
 ## 1. Configuration file
diff --git a/cmd/poller/collector/collector.go b/cmd/poller/collector/collector.go
index 16f6c9e4..284aebad 100644
--- a/cmd/poller/collector/collector.go
+++ b/cmd/poller/collector/collector.go
@@ -382,7 +382,7 @@ func (me *AbstractCollector) Start(wg *sync.WaitGroup) {
 		if nd := me.Schedule.NextDue(); nd > 0 {
 			me.Logger.Debug().Msgf("sleeping %s until next poll", nd.String()) //DEBUG
 			me.Schedule.Sleep()
-		} else if nd.Milliseconds() > -30000 { // log if lagging by more than 30 secs
+		} else if nd.Milliseconds() > -50 { // avoid warning for small delays
 			me.Logger.Warn().Msgf("lagging behind schedule %s", (-nd).String())
 		}
 	}
diff --git a/cmd/tools/doctor/doctor.go b/cmd/tools/doctor/doctor.go
index 94f09cd2..718d8232 100644
--- a/cmd/tools/doctor/doctor.go
+++ b/cmd/tools/doctor/doctor.go
@@ -121,7 +121,8 @@ func checkUniquePromPorts(config conf.HarvestConfig) validation {
 	// map of portNum -> list of names
 	seen := make(map[int][]string)
 	for name, exporter := range *config.Exporters {
-		if exporter.Port == nil || exporter.Type == nil || *exporter.Type != "Prometheus" {
+		// ignore configuration with both port and portrange defined. PortRange takes precedence
+		if exporter.Port == nil || exporter.Type == nil || *exporter.Type != "Prometheus" || exporter.PortRange != nil {
 			continue
 		}
 		previous := seen[*exporter.Port]
@@ -129,6 +130,21 @@ func checkUniquePromPorts(config conf.HarvestConfig) validation {
 		seen[*exporter.Port] = previous
 	}
 
+	// Update PortRanges
+	for name, exporter := range *config.Exporters {
+		if exporter.PortRange == nil || exporter.Type == nil || *exporter.Type != "Prometheus" {
+			continue
+		}
+		portRange := exporter.PortRange
+		start := portRange.Min
+		end := portRange.Max
+		for i := start; i <= end; i++ {
+			previous := seen[i]
+			previous = append(previous, name)
+			seen[i] = previous
+		}
+	}
+
 	valid := validation{isValid: true}
 	for _, exporterNames := range seen {
 		if len(exporterNames) == 1 {
diff --git a/cmd/tools/generate/generate.go b/cmd/tools/generate/generate.go
index db83f282..71a20c46 100644
--- a/cmd/tools/generate/generate.go
+++ b/cmd/tools/generate/generate.go
@@ -61,7 +61,7 @@ func generateDockerCompose(path string) {
 	if err != nil {
 		panic(err)
 	}
-	conf.IsDocker = true
+	conf.ValidatePortInUse = true
 	for _, v := range conf.Config.PollersOrdered {
 		port, _ := conf.GetPrometheusExporterPorts(v)
 		pollerTemplate.Pollers = append(pollerTemplate.Pollers, PollerPort{v, port, absPath})
diff --git a/grafana/prometheus/harvest_dashboard_node.json b/grafana/prometheus/harvest_dashboard_node.json
index 223f5517..9e19559c 100644
--- a/grafana/prometheus/harvest_dashboard_node.json
+++ b/grafana/prometheus/harvest_dashboard_node.json
@@ -491,7 +491,7 @@
       "targets": [
         {
           "exemplar": false,
-          "expr": "avg by(node) (volume_avg_latency{datacenter=\"$Datacenter\",cluster=\"$Cluster\",node=~\"$Node\"})",
+          "expr": "avg by(node) (volume_avg_latency{datacenter=\"$Datacenter\",cluster=\"$Cluster\"})",
           "interval": "",
           "legendFormat": "{{node}}",
           "refId": "A"
@@ -595,7 +595,7 @@
       "targets": [
         {
           "exemplar": false,
-          "expr": "sum by (node) (volume_write_data+volume_read_data{datacenter=\"$Datacenter\",cluster=\"$Cluster\",node=~\"$Node\"})",
+          "expr": "sum by (node) (volume_write_data+volume_read_data{datacenter=\"$Datacenter\",cluster=\"$Cluster\"})",
           "interval": "",
           "legendFormat": "{{node}}",
           "refId": "A"
@@ -699,7 +699,7 @@
       "targets": [
         {
           "exemplar": false,
-          "expr": "sum by(node) (volume_total_ops{datacenter=\"$Datacenter\",cluster=\"$Cluster\",node=~\"$Node\"})",
+          "expr": "sum by(node) (volume_total_ops{datacenter=\"$Datacenter\",cluster=\"$Cluster\"})",
           "interval": "",
           "legendFormat": "{{node}}",
           "refId": "A"
@@ -3029,7 +3029,7 @@
         "allValue": null,
         "current": {},
         "datasource": "${DS_PROMETHEUS}",
-        "definition": "label_values(node_labels, datacenter)",
+        "definition": "label_values(datacenter)",
         "description": null,
         "error": null,
         "hide": 0,
@@ -3039,7 +3039,7 @@
         "name": "Datacenter",
         "options": [],
         "query": {
-          "query": "label_values(node_labels, datacenter)",
+          "query": "label_values(datacenter)",
           "refId": "Prometheus-Datacenter-Variable-Query"
         },
         "refresh": 1,
@@ -3056,7 +3056,7 @@
         "allValue": null,
         "current": {},
         "datasource": "${DS_PROMETHEUS}",
-        "definition": "label_values(node_labels{datacenter=\"$Datacenter\"}, cluster)",
+        "definition": "label_values(cluster)",
         "description": null,
         "error": null,
         "hide": 0,
@@ -3066,7 +3066,7 @@
         "name": "Cluster",
         "options": [],
         "query": {
-          "query": "label_values(node_labels{datacenter=\"$Datacenter\"}, cluster)",
+          "query": "label_values(cluster)",
           "refId": "Prometheus-Cluster-Variable-Query"
         },
         "refresh": 1,
@@ -3083,7 +3083,7 @@
         "allValue": null,
         "current": {},
         "datasource": "${DS_PROMETHEUS}",
-        "definition": "label_values(node_labels{datacenter=\"$Datacenter\", cluster=\"$Cluster\"}, node)",
+        "definition": "label_values(node_uptime{cluster=\"$Cluster\"}, node)",
         "description": null,
         "error": null,
         "hide": 0,
@@ -3093,7 +3093,7 @@
         "name": "Node",
         "options": [],
         "query": {
-          "query": "label_values(node_labels{datacenter=\"$Datacenter\", cluster=\"$Cluster\"}, node)",
+          "query": "label_values(node_uptime{cluster=\"$Cluster\"}, node)",
           "refId": "StandardVariableQuery"
         },
         "refresh": 1,
diff --git a/jenkins/artifacts/README.MD b/jenkins/artifacts/README.MD
deleted file mode 100644
index 002ddb9a..00000000
--- a/jenkins/artifacts/README.MD
+++ /dev/null
@@ -1,50 +0,0 @@
-## Version Names
-Harvest releases use a calendar-based numbering scheme. Example: `YY.MM.bb`, where `YY` is the last two digits of the year of the release, `MM` is the month of release, and `bb` is the bugfix release.
-
-There are four release types:
-* Major release: `YY.MM` is updated to match year and month of release
-* Bugfix, patch, or point-release: `bb` is incremented for each point release
-* Release candidates (aka pre-releases): These are similar to point-releases, but with `-rc.num` suffixed to the point-release pattern
-* Nightly: `YY.MM.ddHH-nightly`, where `dd` is day-of-month and `HH` is the hour-of-day (0-23)
-
-Examples:
-
-| Type    | Version              | Notes                                                            |
-| :------ | :------------------- | :--------------------------------------------------------------- |
-| Release | `21.05.0`            | a release in May 2021                                            |
-| Release | `21.11.0`            | a release in Nov 2021                                            |
-| Bugfix  | `21.11.1`            | `21.11.0 had a defect, this version fixes it                     |
-| RC      | `21.05.0-rc1`        | First release candidate of `21.05.0`, 2nd would be `21.05.0-rc2` |
-| Nightly | `21.05.1813-nightly` | Build of `21.05.0` release created on the 18th at 1 pm           |
-
-
-### RPM and Debian Names
-The RPM and Debian file formats require `Version` and `Release` fields. The [RPM packaging guide](https://rpm-packaging-guide.github.io/) says `Release` is
-> The number of times this version of the software was released. Reset to 1 when a new Version of the software is built.
-
-When the Harvest version changes, per the rules above, the `RPM Version` will follow in lock-step and in most cases the `RPM Release` will be 1. The exceptions are release candidates and nightly builds which will include that information in the `RPM Release` field as shown below.
-
-Examples of RPM version/release and filename values based on the Harvest's version names.
-
-| Harvest            | RPM Version | RPM Release | File name                             |
-| :----------------- | :---------- | :---------- | :------------------------------------ |
-| `21.05.0`          | `21.05.0`   | 1           | `harvest-21.05.0-1.x86_64.rpm`        |
-| `21.11.1`          | `21.11.1`   | 1           | `harvest-21.11.1-1.x86_64.rpm`        |
-| `21.05.0-rc1`      | `21.05.0`   | rc1         | `harvest-21.05.0-rc1.x86_64.rpm`      |
-| `21.05.13-nightly` | `21.05.13`  | nightly     | `harvest-21.05.13-nightly.x86_64.rpm` |
-
-Every release includes the whole project.
-
-### Build RPM
-
-Refer [RPM and Debian Names](#rpm%20and%20debian%20names) for $VERSION $RELEASE
-
-`./package.sh rpm x86_64 $VERSION $RELEASE`
-
-### Build DEB
-
-`./package.sh deb x86_64 $VERSION $RELEASE`
-
-### Tar gz
-
-`make package VERSION=$VERSION RELEASE=$RELEASE`
\ No newline at end of file
diff --git a/pkg/conf/conf.go b/pkg/conf/conf.go
index 4ded3b04..338118c8 100644
--- a/pkg/conf/conf.go
+++ b/pkg/conf/conf.go
@@ -56,7 +56,7 @@ func LoadConfig(configPath string) (*node.Node, error) {
 
 var Config = HarvestConfig{}
 var configRead = false
-var IsDocker = false
+var ValidatePortInUse = false
 
 func LoadHarvestConfig(configPath string) error {
 	if configRead {
@@ -192,7 +192,6 @@ func GetHarvestLogPath() string {
 
 /*
 GetPrometheusExporterPorts returns port configured in prometheus exporter for given poller
-If there are more than 1 exporter configured for a poller then return string will have ports as comma seperated
 */
 func GetPrometheusExporterPorts(pollerName string) (int, error) {
 	var port int
@@ -206,7 +205,7 @@ func GetPrometheusExporterPorts(pollerName string) (int, error) {
 	if exporters != nil && len(*exporters) > 0 {
 		for _, e := range *exporters {
 			exporter := (*Config.Exporters)[e]
-			if *exporter.Type == "Prometheus" {
+			if exporter.Type != nil && *exporter.Type == "Prometheus" {
 				isPrometheusExporterConfigured = true
 				if exporter.PortRange != nil {
 					ports := promPortRangeMapping[e]
@@ -222,6 +221,8 @@ 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)
@@ -242,11 +243,11 @@ func PortMapFromRange(address string, portRange *IntRange) PortMap {
 	end := portRange.Max
 	for i := start; i <= end; i++ {
 		portMap.portSet = append(portMap.portSet, i)
-		if IsDocker {
+		if ValidatePortInUse {
 			portMap.freePorts[i] = struct{}{}
 		}
 	}
-	if !IsDocker {
+	if !ValidatePortInUse {
 		portMap.freePorts = util.CheckFreePorts(address, portMap.portSet)
 	}
 	return portMap


Test patch:
diff --git a/cmd/tools/doctor/testdata/testConfig.yml b/cmd/tools/doctor/testdata/testConfig.yml
index 94f2f434..bf6cfd12 100644
--- a/cmd/tools/doctor/testdata/testConfig.yml
+++ b/cmd/tools/doctor/testdata/testConfig.yml
@@ -3,6 +3,10 @@ Tools:
   token: [ 'aaa-bbb-ccc-ddd' ]
 
 Exporters:
+  prometheusrange:
+    exporter: Prometheus
+    addr: 0.0.0.0
+    port_range: 2000-2000
   prometheus:
     exporter: Prometheus
     addr: 0.0.0.0
@@ -85,6 +89,25 @@ Pollers:
     addr: 10.0.0.2
     collectors: [ Zapi ]
 
+  unix-01:
+    datacenter: local
+    addr: localhost
+    collectors:
+      - Unix
+    exporters:
+      - prometheusrange
+
+  # add your Cdot / 7Mode pollers below, example:
+  cluster-02:
+    datacenter: DC-01
+    addr: 1.1.1.1
+    username: username
+    password: password
+    collectors:
+      - Zapi
+    exporters:
+      - prometheusrange
+
   sim-0001:
     is_kfs: true
 
diff --git a/pkg/conf/conf_test.go b/pkg/conf/conf_test.go
new file mode 100644
index 00000000..3869a2d6
--- /dev/null
+++ b/pkg/conf/conf_test.go
@@ -0,0 +1,43 @@
+package conf
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestGetPrometheusExporterPorts(t *testing.T) {
+	path := "../../cmd/tools/doctor/testdata/testConfig.yml"
+	err := LoadHarvestConfig(path)
+	// Test without checking
+	ValidatePortInUse = true
+	if err != nil {
+		panic(err)
+	}
+	type args struct {
+		pollerName string
+	}
+
+	type test struct {
+		name    string
+		args    args
+		wantErr bool
+	}
+	tests := []test{
+		{"success", args{pollerName: "unix-01"}, false},
+		{"failure_no_free_port", args{pollerName: "cluster-02"}, true},
+		{"failure_poller_name_does_not_exist", args{pollerName: "test1"}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := GetPrometheusExporterPorts(tt.args.pollerName)
+			fmt.Println(got)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("GetPrometheusExporterPorts() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if err == nil && got == 0 {
+				t.Errorf("GetPrometheusExporterPorts() got = %v, want %s", got, "non zero value")
+			}
+		})
+	}
+}