Repository: NetApp/harvest
Pull request #626 (2021-11-01 12:14:37)

refactor: clean-up conf


Patch:
diff --git a/.github/fetch-asup b/.github/fetch-asup
index c580aef4..daff3552 100755
--- a/.github/fetch-asup
+++ b/.github/fetch-asup
@@ -10,9 +10,9 @@ ASUP_BIN_VERSION="$2"
 BIN_PLATFORM="$3"
 mkdir -p "$ASUP_BIN_DIR"
 
-COUNT_REPO_FOLDERS=($(find $ASUP_BIN_DIR/github.com/!net!app -mindepth 1 -maxdepth 1 -type d  | wc -l))
-if [ $COUNT_REPO_FOLDERS -gt 5 ]; then
-  rm -rf $ASUP_BIN_DIR/*
+COUNT_REPO_FOLDERS=( "$(find "$ASUP_BIN_DIR"/github.com/!net!app -mindepth 1 -maxdepth 1 -type d  | wc -l)" )
+if [ "${COUNT_REPO_FOLDERS[*]}" -gt 5 ]; then
+  rm -rf "${ASUP_BIN_DIR:?}"/*
 fi
 GOMODCACHE=$ASUP_BIN_DIR go mod download github.com/NetApp/harvest-metrics@"$ASUP_BIN_VERSION"
 mod_bin=$(find "$ASUP_BIN_DIR" -name "*asup*" -name "*$BIN_PLATFORM*" -exec ls -t {} + | head -n 1)
diff --git a/cmd/collectors/zapi/collector/zapi.go b/cmd/collectors/zapi/collector/zapi.go
index 1e52b13b..91185ae8 100644
--- a/cmd/collectors/zapi/collector/zapi.go
+++ b/cmd/collectors/zapi/collector/zapi.go
@@ -14,6 +14,7 @@ import (
 	"goharvest2/cmd/collectors/zapiperf/plugins/nic"
 	"goharvest2/cmd/collectors/zapiperf/plugins/volume"
 	"goharvest2/cmd/poller/plugin"
+	"goharvest2/pkg/conf"
 	"sort"
 	"strconv"
 	"strings"
@@ -82,7 +83,7 @@ func (me *Zapi) InitVars() error {
 
 	var err error
 
-	if me.Client, err = client.New(me.Params); err != nil { // convert to connection error, so poller aborts
+	if me.Client, err = client.New(conf.ZapiPoller(me.Params)); err != nil { // convert to connection error, so poller aborts
 		return errors.New(errors.ERR_CONNECTION, err.Error())
 	}
 	me.Client.TraceLogSet(me.Name, me.Params)
diff --git a/cmd/collectors/zapi/plugins/quota/quota.go b/cmd/collectors/zapi/plugins/quota/quota.go
index 0802c4f0..3c20db30 100644
--- a/cmd/collectors/zapi/plugins/quota/quota.go
+++ b/cmd/collectors/zapi/plugins/quota/quota.go
@@ -1,12 +1,12 @@
-/*
- * Copyright NetApp Inc, 2021 All rights reserved
- */
+// Copyright NetApp Inc, 2021 All rights reserved
+
 package quota
 
 import (
 	"goharvest2/cmd/poller/collector"
 	"goharvest2/cmd/poller/plugin"
 	"goharvest2/pkg/api/ontapi/zapi"
+	"goharvest2/pkg/conf"
 	"goharvest2/pkg/dict"
 	"goharvest2/pkg/errors"
 	"goharvest2/pkg/matrix"
@@ -15,7 +15,7 @@ import (
 	"strings"
 )
 
-const QuotaBatchSize = "500"
+const BatchSize = "500"
 
 // Quota plugin is needed to match qtrees with quotas.
 type Quota struct {
@@ -40,7 +40,7 @@ func (my *Quota) Init() error {
 		return err
 	}
 
-	if my.client, err = zapi.New(my.ParentParams); err != nil {
+	if my.client, err = zapi.New(conf.ZapiPoller(my.ParentParams)); err != nil {
 		my.Logger.Error().Stack().Err(err).Msg("connecting")
 		return err
 	}
@@ -98,8 +98,8 @@ func (my *Quota) Init() error {
 				my.Logger.Info().Msgf("using batch-size [%s]", my.batchSize)
 			}
 		} else {
-			my.batchSize = QuotaBatchSize
-			my.Logger.Trace().Msgf("using default batch-size [%s]", QuotaBatchSize)
+			my.batchSize = BatchSize
+			my.Logger.Trace().Str("BatchSize", BatchSize).Msg("Using default batch-size")
 		}
 	}
 
diff --git a/cmd/collectors/zapi/plugins/shelf/shelf.go b/cmd/collectors/zapi/plugins/shelf/shelf.go
index 5fc673c2..c5932c44 100644
--- a/cmd/collectors/zapi/plugins/shelf/shelf.go
+++ b/cmd/collectors/zapi/plugins/shelf/shelf.go
@@ -7,6 +7,7 @@ import (
 	"goharvest2/cmd/poller/collector"
 	"goharvest2/cmd/poller/plugin"
 	"goharvest2/pkg/api/ontapi/zapi"
+	"goharvest2/pkg/conf"
 	"goharvest2/pkg/dict"
 	"goharvest2/pkg/errors"
 	"goharvest2/pkg/matrix"
@@ -35,7 +36,7 @@ func (my *Shelf) Init() error {
 		return err
 	}
 
-	if my.client, err = zapi.New(my.ParentParams); err != nil {
+	if my.client, err = zapi.New(conf.ZapiPoller(my.ParentParams)); err != nil {
 		my.Logger.Error().Stack().Err(err).Msg("connecting")
 		return err
 	}
@@ -141,7 +142,7 @@ func (my *Shelf) Run(data *matrix.Matrix) ([]*matrix.Matrix, error) {
 	}
 
 	// Set all global labels from zapi.go if already not exist
-	for a, _ := range my.instanceLabels {
+	for a := range my.instanceLabels {
 		my.data[a].SetGlobalLabels(data.GetGlobalLabels())
 	}
 
diff --git a/cmd/collectors/zapi/plugins/snapmirror/snapmirror.go b/cmd/collectors/zapi/plugins/snapmirror/snapmirror.go
index 1ef5208d..4c790016 100644
--- a/cmd/collectors/zapi/plugins/snapmirror/snapmirror.go
+++ b/cmd/collectors/zapi/plugins/snapmirror/snapmirror.go
@@ -6,6 +6,7 @@ package snapmirror
 import (
 	"goharvest2/cmd/poller/plugin"
 	"goharvest2/pkg/api/ontapi/zapi"
+	"goharvest2/pkg/conf"
 	"goharvest2/pkg/dict"
 	"goharvest2/pkg/matrix"
 	"goharvest2/pkg/tree/node"
@@ -34,7 +35,7 @@ func (my *SnapMirror) Init() error {
 		return err
 	}
 
-	if my.client, err = zapi.New(my.ParentParams); err != nil {
+	if my.client, err = zapi.New(conf.ZapiPoller(my.ParentParams)); err != nil {
 		my.Logger.Error().Stack().Err(err).Msg("connecting")
 		return err
 	}
diff --git a/cmd/exporters/prometheus/README.md b/cmd/exporters/prometheus/README.md
index 6375d283..ed4fa4f6 100644
--- a/cmd/exporters/prometheus/README.md
+++ b/cmd/exporters/prometheus/README.md
@@ -1,5 +1,4 @@
 
-
 # Prometheus Exporter
 
 ## Overview
@@ -125,7 +124,7 @@ To use HTTP service discovery you need to:
 
 Add the following to your `harvest.yml`
 
-```
+```yaml
 Admin:
   httpsd:
     listen: :8887
@@ -172,7 +171,7 @@ Edit your `prometheus.yml` and add the following section
 
 `$ vim /etc/prometheus/prometheus.yml`
 
-```
+```yaml
 scrape_configs:
   - job_name: harvest
     http_sd_configs:
@@ -181,7 +180,7 @@ scrape_configs:
  
 Harvest and Prometheus both support basic authentication for HTTP SD end-points. To enable basic auth, add the following to your Harvest config.
 
-```
+```yaml
 Admin:
   httpsd:
     listen: :8887
@@ -201,7 +200,7 @@ Notice that none of the pollers specify an exporter. Instead, all the pollers sh
 
 If you add or remove more clusters in the `Pollers` section, you do not have to change Prometheus since it dynamically pulls the targets from the Harvest admin node.
 
-```
+```yaml
 Admin:
   httpsd:
     listen: :8887
diff --git a/cmd/tools/zapi/zapi.go b/cmd/tools/zapi/zapi.go
index 6978e31e..721bbe62 100644
--- a/cmd/tools/zapi/zapi.go
+++ b/cmd/tools/zapi/zapi.go
@@ -145,11 +145,7 @@ func doCmd(cmd string) {
 	if poller, err = conf.PollerNamed(args.Poller); err != nil {
 		log.Fatal(err)
 	}
-	n, err := poller.AsNode(args.Poller)
-	if err != nil {
-		log.Fatal(err)
-	}
-	if connection, err = client.New(n); err != nil {
+	if connection, err = client.New(*poller); err != nil {
 		log.Fatal(err)
 	}
 
diff --git a/integration/Jenkinsfile b/integration/Jenkinsfile
index e5ec2c0a..3e03c76f 100644
--- a/integration/Jenkinsfile
+++ b/integration/Jenkinsfile
@@ -180,9 +180,8 @@ pipeline {
 
 def void stopAndRemoveDockers() {
     sh '''
-        for container_id in $(docker ps -a -q);do docker stop $container_id && docker rm $container_id;done
-        for image_id in $(docker images -q);do docker rmi $image_id;done
-        for volume_id in $(docker volume ls -q); do docker volume rm -f $volume_id;done
+        docker system prune --all --force --volumes
+
     '''
 }
 
diff --git a/jenkins/artifacts/jenkinsfile b/jenkins/artifacts/jenkinsfile
index 028d16f6..ea8ecddf 100644
--- a/jenkins/artifacts/jenkinsfile
+++ b/jenkins/artifacts/jenkinsfile
@@ -51,7 +51,7 @@ pipeline {
         stage('clean workspace') {
             steps {
                 cleanWs()
-                sh 'rm -rf $targetParentLocation$VERSION$RELEASE'
+                stopAndRemoveDockers()
             }
         }
         stage('Setup GO') {
@@ -198,6 +198,15 @@ pipeline {
     }
 }
 
+def void stopAndRemoveDockers() {
+    sh '''
+        rm -rf /opt/home/nightly/
+        mkdir -p /opt/home/nightly/
+        docker system prune --all --force --volumes
+    '''
+}
+
+
 def getBranchName(gitBranchName, paramBranchName) {
     gitBranchName = gitBranchName.replace('origin/', '')
     if (gitBranchName?.trim() && gitBranchName != "main") {
diff --git a/pkg/api/ontapi/zapi/client.go b/pkg/api/ontapi/zapi/client.go
index 0595970a..4bd2f9cb 100644
--- a/pkg/api/ontapi/zapi/client.go
+++ b/pkg/api/ontapi/zapi/client.go
@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"crypto/tls"
 	"fmt"
+	"goharvest2/pkg/conf"
 	"goharvest2/pkg/errors"
 	"goharvest2/pkg/logging"
 	"goharvest2/pkg/tree"
@@ -37,7 +38,7 @@ type Client struct {
 	logZapi    bool            // used to log ZAPI request/response
 }
 
-func New(config *node.Node) (*Client, error) {
+func New(poller conf.Poller) (*Client, error) {
 	var (
 		client         Client
 		httpclient     *http.Client
@@ -54,20 +55,20 @@ func New(config *node.Node) (*Client, error) {
 	client.Logger = logging.SubLogger("Zapi", "Client")
 
 	// check required & optional parameters
-	if client.apiVersion = config.GetChildContentS("api_version"); client.apiVersion == "" {
+	if client.apiVersion = poller.ApiVersion; client.apiVersion == "" {
 		client.apiVersion = DefaultApiVersion
 		client.Logger.Debug().Msgf("using default API version [%s]", DefaultApiVersion)
 	}
 
-	if client.vfiler = config.GetChildContentS("api_vfiler"); client.vfiler != "" {
+	if client.vfiler = poller.ApiVfiler; client.vfiler != "" {
 		client.Logger.Debug().Msgf("using vfiler tunneling [%s]", client.vfiler)
 	}
 
-	if addr = config.GetChildContentS("addr"); addr == "" {
+	if addr = poller.Addr; addr == "" {
 		return nil, errors.New(errors.MISSING_PARAM, "addr")
 	}
 
-	if config.GetChildContentS("is_kfs") == "true" {
+	if poller.IsKfs {
 		url = "https://" + addr + ":8443/servlets/netapp.servlets.admin.XMLrequest_filer"
 	} else {
 		url = "https://" + addr + ":443/servlets/netapp.servlets.admin.XMLrequest_filer"
@@ -81,19 +82,16 @@ func New(config *node.Node) (*Client, error) {
 	request.Header.Set("Charset", "utf-8")
 
 	// by default, enforce secure TLS, if not requested otherwise by user
-	if x := config.GetChildContentS("use_insecure_tls"); x != "" {
-		if useInsecureTLS, err = strconv.ParseBool(x); err != nil {
-			client.Logger.Error().Stack().Err(err).Msg("use_insecure_tls")
-		}
-	} else {
-		useInsecureTLS = false
+	useInsecureTLS = false
+	if poller.UseInsecureTls != nil {
+		useInsecureTLS = *poller.UseInsecureTls
 	}
 
 	// set authentication method
-	if config.GetChildContentS("auth_style") == "certificate_auth" {
+	if poller.AuthStyle == "certificate_auth" {
 
-		certPath := config.GetChildContentS("ssl_cert")
-		keyPath := config.GetChildContentS("ssl_key")
+		certPath := poller.SslCert
+		keyPath := poller.SslKey
 
 		if certPath == "" {
 			return nil, errors.New(errors.MISSING_PARAM, "ssl_cert")
@@ -111,16 +109,13 @@ func New(config *node.Node) (*Client, error) {
 		}
 	} else {
 
-		username := config.GetChildContentS("username")
-		password := config.GetChildContentS("password")
-
-		if username == "" {
+		if poller.Username == "" {
 			return nil, errors.New(errors.MISSING_PARAM, "username")
-		} else if password == "" {
+		} else if poller.Password == "" {
 			return nil, errors.New(errors.MISSING_PARAM, "password")
 		}
 
-		request.SetBasicAuth(username, password)
+		request.SetBasicAuth(poller.Username, poller.Password)
 		transport = &http.Transport{
 			Proxy:           http.ProxyFromEnvironment,
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: useInsecureTLS},
@@ -132,7 +127,7 @@ func New(config *node.Node) (*Client, error) {
 	httpclient = &http.Client{Transport: transport, Timeout: timeout}
 
 	client.client = httpclient
-	client.SetTimeout(config.GetChildContentS("client_timeout"))
+	client.SetTimeout(poller.ClientTimeout)
 	// ensure that we can change body dynamically
 	request.GetBody = func() (io.ReadCloser, error) {
 		r := bytes.NewReader(client.buffer.Bytes())
diff --git a/pkg/conf/conf.go b/pkg/conf/conf.go
index 55879fef..824d4626 100644
--- a/pkg/conf/conf.go
+++ b/pkg/conf/conf.go
@@ -9,7 +9,6 @@ import (
 	"github.com/imdario/mergo"
 	"goharvest2/pkg/constant"
 	"goharvest2/pkg/errors"
-	"goharvest2/pkg/tree"
 	"goharvest2/pkg/tree/node"
 	"goharvest2/pkg/util"
 	"gopkg.in/yaml.v3"
@@ -23,10 +22,14 @@ import (
 )
 
 var Config = HarvestConfig{}
-var cfp = ""
 var configRead = false
 var ValidatePortInUse = false
 
+const (
+	DefaultApiVersion = "1.3"
+	DefaultTimeout    = "10s"
+)
+
 // TestLoadHarvestConfig is used by testing code to reload a new config
 func TestLoadHarvestConfig(configPath string) {
 	configRead = false
@@ -41,7 +44,7 @@ func LoadHarvestConfig(configPath string) error {
 		return nil
 	}
 	contents, err := ioutil.ReadFile(configPath)
-	cfp = configPath
+
 	if err != nil {
 		fmt.Printf("error reading config file=[%s] %+v\n", configPath, err)
 		return err
@@ -302,66 +305,65 @@ func (p *Poller) Union(defaults *Poller) {
 	_ = mergo.Merge(p, defaults)
 }
 
-// TODO remove node code after changing bin/zapi to use Poller
-func loadConfig(configPath string) (*node.Node, error) {
-	configNode, err := tree.Import("yaml", configPath)
-	if configNode != nil {
-		// Load HarvestConfig to rewrite passwords - eventually all the code will be refactored to use HarvestConfig.
-		// This is needed because the current yaml parser does not handle password with special characters.
-		// E.g abc#123, that's because the # is interpreted as the beginning of a comment. The code below overwrites
-		// the incorrect password with the correct one by using a better yaml parser for each Poller and Default section
-		err := LoadHarvestConfig(configPath)
-		if err != nil {
-			return nil, err
-		}
-		pollers := configNode.GetChildS("Pollers")
-		if pollers != nil {
-			for _, poller := range pollers.GetChildren() {
-				password := poller.GetChildContentS("password")
-				pollerStruct := (Config.Pollers)[poller.GetNameS()]
-				if pollerStruct.Password != "" && pollerStruct.Password != password {
-					poller.SetChildContentS("password", pollerStruct.Password)
-				}
-			}
-		}
-		// Check Defaults also
-		defaultNode := configNode.GetChildS("Defaults")
-		if defaultNode != nil {
-			password := defaultNode.GetChildContentS("password")
-			defaultStruct := *Config.Defaults
-			if defaultStruct.Password != "" && defaultStruct.Password != password {
-				defaultNode.SetChildContentS("password", defaultStruct.Password)
-			}
-		}
+// ZapiPoller creates a poller out of a node, this is a bridge between the node and struct-based code
+// Used by ZAPI based code
+func ZapiPoller(n *node.Node) Poller {
+	var p Poller
+
+	if Config.Defaults != nil {
+		p = *Config.Defaults
+	} else {
+		p = Poller{}
 	}
-	return configNode, err
-}
 
-// AsNode converts a poller into a node based structure to bridge the struct and node-based code
-func (p *Poller) AsNode(name string) (*node.Node, error) {
-	if p == nil {
-		return nil, nil
+	if apiVersion := n.GetChildContentS("api_version"); apiVersion != "" {
+		p.ApiVersion = apiVersion
+	} else {
+		if p.ApiVersion == "" {
+			p.ApiVersion = DefaultApiVersion
+		}
 	}
-	cfg, err := loadConfig(cfp)
-	if err != nil {
-		return nil, err
+	if vfiler := n.GetChildContentS("api_vfiler"); vfiler != "" {
+		p.ApiVfiler = vfiler
 	}
-	var pollers, poller, defaults *node.Node
-
-	pollers = cfg.GetChildS("Pollers")
-	defaults = cfg.GetChildS("Defaults")
-
-	if pollers == nil {
-		err = errors.New(errors.ERR_CONFIG, "[Pollers] section not found")
-	} else if defaults != nil { // optional
-		for _, p := range pollers.GetChildren() {
-			p.Union(defaults)
+	if addr := n.GetChildContentS("addr"); addr != "" {
+		p.Addr = addr
+	}
+	isKfs := n.GetChildContentS("is_kfs")
+	if isKfs == "true" {
+		p.IsKfs = true
+	} else if isKfs == "false" {
+		p.IsKfs = false
+	}
+	if x := n.GetChildContentS("use_insecure_tls"); x != "" {
+		if insecureTLS, err := strconv.ParseBool(x); err == nil {
+			// err can be ignored since conf was already validated
+			p.UseInsecureTls = &insecureTLS
 		}
 	}
-	if poller = pollers.GetChildS(name); poller == nil {
-		err = errors.New(errors.ERR_CONFIG, "poller ["+name+"] not found")
+	if authStyle := n.GetChildContentS("auth_style"); authStyle != "" {
+		p.AuthStyle = authStyle
+	}
+	if sslCert := n.GetChildContentS("ssl_cert"); sslCert != "" {
+		p.SslCert = sslCert
+	}
+	if sslKey := n.GetChildContentS("ssl_key"); sslKey != "" {
+		p.SslKey = sslKey
+	}
+	if username := n.GetChildContentS("username"); username != "" {
+		p.Username = username
+	}
+	if password := n.GetChildContentS("password"); password != "" {
+		p.Password = password
+	}
+	if clientTimeout := n.GetChildContentS("client_timeout"); clientTimeout != "" {
+		p.ClientTimeout = clientTimeout
+	} else {
+		if p.ClientTimeout == "" {
+			p.ClientTimeout = DefaultTimeout
+		}
 	}
-	return poller, err
+	return p
 }
 
 type Exporter struct {


Test patch:
diff --git a/pkg/api/ontapi/zapi/client_test.go b/pkg/api/ontapi/zapi/client_test.go
index 863cf79b..245c2ff2 100644
--- a/pkg/api/ontapi/zapi/client_test.go
+++ b/pkg/api/ontapi/zapi/client_test.go
@@ -1,6 +1,7 @@
 package zapi
 
 import (
+	"goharvest2/pkg/conf"
 	"goharvest2/pkg/tree/node"
 	"testing"
 	"time"
@@ -50,7 +51,7 @@ func TestNew(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := New(tt.config)
+			_, err := New(conf.ZapiPoller(tt.config))
 			if (err != nil) != tt.wantErr {
 				t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr)
 				return
diff --git a/pkg/conf/conf_test.go b/pkg/conf/conf_test.go
index 80843e57..99c8ccdd 100644
--- a/pkg/conf/conf_test.go
+++ b/pkg/conf/conf_test.go
@@ -2,6 +2,7 @@ package conf
 
 import (
 	"fmt"
+	"goharvest2/pkg/tree/node"
 	"reflect"
 	"sort"
 	"strconv"
@@ -249,3 +250,27 @@ func TestCollectorConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestNodeToPoller(t *testing.T) {
+	testArg := func(t *testing.T, want, got string) {
+		if got != want {
+			t.Errorf("want=[%s] got=[%s]", want, got)
+		}
+	}
+
+	Config.Defaults = &Poller{
+		Username: "bob",
+		Password: "bob",
+	}
+
+	defaultNode := node.NewS("root")
+	defaultNode.NewChildS("password", "pass")
+	defaultNode.NewChildS("use_insecure_tls", "true")
+	poller := ZapiPoller(defaultNode)
+
+	testArg(t, DefaultApiVersion, poller.ApiVersion)
+	testArg(t, "bob", poller.Username)
+	testArg(t, "pass", poller.Password)
+	testArg(t, "10s", poller.ClientTimeout)
+	testArg(t, "true", strconv.FormatBool(*poller.UseInsecureTls))
+}