Repository: ClusterLabs/ha_cluster_exporter
Pull request #132 (2020-02-20 15:17:31)

Refactor Pacemaker collector
This is a 100% transparent refactoring, no functionality is changed, but it's propedeutic for fixing #114.

The Pacemaker collector is now stripped from the responsibility of parsing the XML collected via `cibadmin` and `crmmon`, which is now done in dedicated packages, and exposed with a couple of dedicated interfaces (`cib.Parser` and `crmmon.Parser`), with dedicated XML unmarshalling structs which can now evolve independently without name clashes.

stefanotorresi: @MalloZup good idea! I will update the design doc and ping you again.

Patch:
diff --git a/collector/pacemaker/cib/data.go b/collector/pacemaker/cib/data.go
new file mode 100644
index 0000000..8ff2dbf
--- /dev/null
+++ b/collector/pacemaker/cib/data.go
@@ -0,0 +1,64 @@
+package cib
+
+/*
+The Cluster Information Base (Root) is an XML representation of the cluster’s configuration and the state of all nodes and resources.
+The Root manager (pacemaker-based) keeps the Root synchronized across the cluster, and handles requests to modify it.
+
+https://clusterlabs.org/pacemaker/doc/en-US/Pacemaker/2.0/html-single/Pacemaker_Administration/index.html
+
+*/
+
+type Root struct {
+	Configuration struct {
+		Nodes []struct {
+			Id                 string     `xml:"id,attr"`
+			Uname              string     `xml:"uname,attr"`
+			InstanceAttributes Attributes `xml:"instance_attributes"`
+		} `xml:"nodes>node"`
+		Resources struct {
+			Primitives []Primitive `xml:"primitive"`
+			Masters    []Clone     `xml:"master"`
+			Clones     []Clone     `xml:"clone"`
+		} `xml:"resources"`
+		Constraints struct {
+			RscLocations []struct {
+				Id       string `xml:"id,attr"`
+				Node     string `xml:"node,attr"`
+				Resource string `xml:"rsc,attr"`
+				Role     string `xml:"role,attr"`
+				Score    string `xml:"score,attr"`
+			} `xml:"rsc_location"`
+		} `xml:"constraints"`
+	} `xml:"configuration"`
+}
+
+type Attributes struct {
+	Id      string `xml:"id,attr"`
+	NvPairs []struct {
+		Id    string `xml:"id,attr"`
+		Name  string `xml:"name,attr"`
+		Value string `xml:"value,attr"`
+	} `xml:"nvpair"`
+}
+
+type Primitive struct {
+	Id                 string     `xml:"id,attr"`
+	Class              string     `xml:"class,attr"`
+	Type               string     `xml:"type,attr"`
+	Provider           string     `xml:"provider,attr"`
+	InstanceAttributes Attributes `xml:"instance_attributes"`
+	MetaAttributes     Attributes `xml:"meta_attributes"`
+	Operations         []struct {
+		Id       string `xml:"id,attr"`
+		Name     string `xml:"name,attr"`
+		Role     string `xml:"role,attr"`
+		Interval int    `xml:"interval,attr"`
+		Timeout  int    `xml:"timeout,attr"`
+	} `xml:"operations>op"`
+}
+
+type Clone struct {
+	Id             string     `xml:"id,attr"`
+	MetaAttributes Attributes `xml:"meta_attributes"`
+	Primitive      Primitive  `xml:"primitive"`
+}
diff --git a/collector/pacemaker/cib/parser.go b/collector/pacemaker/cib/parser.go
new file mode 100644
index 0000000..01e46a2
--- /dev/null
+++ b/collector/pacemaker/cib/parser.go
@@ -0,0 +1,35 @@
+package cib
+
+import (
+	"encoding/xml"
+	"os/exec"
+
+	"github.com/pkg/errors"
+)
+
+type Parser interface {
+	Parse() (Root, error)
+}
+
+type cibAdminParser struct {
+	cibAdminPath string
+}
+
+func (p *cibAdminParser) Parse() (Root, error) {
+	var CIB Root
+	cibXML, err := exec.Command(p.cibAdminPath, "--query", "--local").Output()
+	if err != nil {
+		return CIB, errors.Wrap(err, "error while executing cibadmin")
+	}
+
+	err = xml.Unmarshal(cibXML, &CIB)
+	if err != nil {
+		return CIB, errors.Wrap(err, "could not parse cibadmin status from XML")
+	}
+
+	return CIB, nil
+}
+
+func NewCibAdminParser(cibAdminPath string) *cibAdminParser {
+	return &cibAdminParser{cibAdminPath}
+}
diff --git a/collector/pacemaker/crmmon/data.go b/collector/pacemaker/crmmon/data.go
new file mode 100644
index 0000000..c707f2f
--- /dev/null
+++ b/collector/pacemaker/crmmon/data.go
@@ -0,0 +1,64 @@
+package crmmon
+
+// *** crm_mon XML unserialization structures
+
+type Root struct {
+	Version string `xml:"version,attr"`
+	Summary struct {
+		Nodes struct {
+			Number int `xml:"number,attr"`
+		} `xml:"nodes_configured"`
+		LastChange struct {
+			Time string `xml:"time,attr"`
+		} `xml:"last_change"`
+		Resources struct {
+			Number   int `xml:"number,attr"`
+			Disabled int `xml:"disabled,attr"`
+			Blocked  int `xml:"blocked,attr"`
+		} `xml:"resources_configured"`
+		ClusterOptions struct {
+			StonithEnabled bool `xml:"stonith-enabled,attr"`
+		} `xml:"cluster_options"`
+	} `xml:"summary"`
+	Nodes       []Node `xml:"nodes>node"`
+	NodeHistory struct {
+		Node []struct {
+			Name            string `xml:"name,attr"`
+			ResourceHistory []struct {
+				Name               string `xml:"id,attr"`
+				MigrationThreshold int    `xml:"migration-threshold,attr"`
+				FailCount          int    `xml:"fail-count,attr"`
+			} `xml:"resource_history"`
+		} `xml:"node"`
+	} `xml:"node_history"`
+}
+
+type Node struct {
+	Name             string     `xml:"name,attr"`
+	ID               string     `xml:"id,attr"`
+	Online           bool       `xml:"online,attr"`
+	Standby          bool       `xml:"standby,attr"`
+	StandbyOnFail    bool       `xml:"standby_onfail,attr"`
+	Maintenance      bool       `xml:"maintenance,attr"`
+	Pending          bool       `xml:"pending,attr"`
+	Unclean          bool       `xml:"unclean,attr"`
+	Shutdown         bool       `xml:"shutdown,attr"`
+	ExpectedUp       bool       `xml:"expected_up,attr"`
+	DC               bool       `xml:"is_dc,attr"`
+	ResourcesRunning int        `xml:"resources_running,attr"`
+	Type             string     `xml:"type,attr"`
+	Resources        []Resource `xml:"resource"`
+}
+
+type Resource struct {
+	ID             string `xml:"id,attr"`
+	Agent          string `xml:"resource_agent,attr"`
+	Role           string `xml:"role,attr"`
+	Active         bool   `xml:"active,attr"`
+	Orphaned       bool   `xml:"orphaned,attr"`
+	Blocked        bool   `xml:"blocked,attr"`
+	Managed        bool   `xml:"managed,attr"`
+	Failed         bool   `xml:"failed,attr"`
+	FailureIgnored bool   `xml:"failure_ignored,attr"`
+	NodesRunningOn int    `xml:"nodes_running_on,attr"`
+}
diff --git a/collector/pacemaker/crmmon/parser.go b/collector/pacemaker/crmmon/parser.go
new file mode 100644
index 0000000..f9fd1e3
--- /dev/null
+++ b/collector/pacemaker/crmmon/parser.go
@@ -0,0 +1,34 @@
+package crmmon
+
+import (
+	"encoding/xml"
+	"os/exec"
+
+	"github.com/pkg/errors"
+)
+
+type Parser interface {
+	Parse() (Root, error)
+}
+
+type crmMonParser struct {
+	crmMonPath string
+}
+
+func (c *crmMonParser) Parse() (crmMon Root, err error) {
+	crmMonXML, err := exec.Command(c.crmMonPath, "-X", "--group-by-node", "--inactive").Output()
+	if err != nil {
+		return crmMon, errors.Wrap(err, "error while executing crm_mon")
+	}
+
+	err = xml.Unmarshal(crmMonXML, &crmMon)
+	if err != nil {
+		return crmMon, errors.Wrap(err, "error while parsing crm_mon XML output")
+	}
+
+	return crmMon, nil
+}
+
+func NewCrmMonParser(crmMonPath string) *crmMonParser {
+	return &crmMonParser{crmMonPath}
+}
diff --git a/collector/pacemaker/pacemaker.go b/collector/pacemaker/pacemaker.go
index 30a632a..4c676c8 100644
--- a/collector/pacemaker/pacemaker.go
+++ b/collector/pacemaker/pacemaker.go
@@ -1,101 +1,20 @@
 package pacemaker
 
 import (
-	"encoding/xml"
 	"math"
-	"os/exec"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/ClusterLabs/ha_cluster_exporter/collector"
+	"github.com/ClusterLabs/ha_cluster_exporter/collector/pacemaker/cib"
+	"github.com/ClusterLabs/ha_cluster_exporter/collector/pacemaker/crmmon"
 
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
 	log "github.com/sirupsen/logrus"
 )
 
-// *** crm_mon XML unserialization structures
-type pacemakerStatus struct {
-	Version string  `xml:"version,attr"`
-	Summary summary `xml:"summary"`
-	Nodes   struct {
-		Node []node `xml:"node"`
-	} `xml:"nodes"`
-	NodeHistory struct {
-		Node []struct {
-			Name            string `xml:"name,attr"`
-			ResourceHistory []struct {
-				Name               string `xml:"id,attr"`
-				MigrationThreshold int    `xml:"migration-threshold,attr"`
-				FailCount          int    `xml:"fail-count,attr"`
-			} `xml:"resource_history"`
-		} `xml:"node"`
-	} `xml:"node_history"`
-}
-
-type summary struct {
-	Nodes struct {
-		Number int `xml:"number,attr"`
-	} `xml:"nodes_configured"`
-	LastChange struct {
-		Time string `xml:"time,attr"`
-	} `xml:"last_change"`
-	Resources struct {
-		Number   int `xml:"number,attr"`
-		Disabled int `xml:"disabled,attr"`
-		Blocked  int `xml:"blocked,attr"`
-	} `xml:"resources_configured"`
-	ClusterOptions struct {
-		StonithEnabled bool `xml:"stonith-enabled,attr"`
-	} `xml:"cluster_options"`
-}
-
-type node struct {
-	Name             string     `xml:"name,attr"`
-	ID               string     `xml:"id,attr"`
-	Online           bool       `xml:"online,attr"`
-	Standby          bool       `xml:"standby,attr"`
-	StandbyOnFail    bool       `xml:"standby_onfail,attr"`
-	Maintenance      bool       `xml:"maintenance,attr"`
-	Pending          bool       `xml:"pending,attr"`
-	Unclean          bool       `xml:"unclean,attr"`
-	Shutdown         bool       `xml:"shutdown,attr"`
-	ExpectedUp       bool       `xml:"expected_up,attr"`
-	DC               bool       `xml:"is_dc,attr"`
-	ResourcesRunning int        `xml:"resources_running,attr"`
-	Type             string     `xml:"type,attr"`
-	Resources        []resource `xml:"resource"`
-}
-
-type resource struct {
-	ID             string `xml:"id,attr"`
-	Agent          string `xml:"resource_agent,attr"`
-	Role           string `xml:"role,attr"`
-	Active         bool   `xml:"active,attr"`
-	Orphaned       bool   `xml:"orphaned,attr"`
-	Blocked        bool   `xml:"blocked,attr"`
-	Managed        bool   `xml:"managed,attr"`
-	Failed         bool   `xml:"failed,attr"`
-	FailureIgnored bool   `xml:"failure_ignored,attr"`
-	NodesRunningOn int    `xml:"nodes_running_on,attr"`
-}
-
-// Pacemaker CIB is queried from cibadmin and unmarshaled from XML
-type CIB struct {
-	Configuration struct {
-		Constraints struct {
-			RscLocations []struct {
-				Id       string `xml:"id,attr"`
-				Node     string `xml:"node,attr"`
-				Resource string `xml:"rsc,attr"`
-				Role     string `xml:"role,attr"`
-				Score    string `xml:"score,attr"`
-			} `xml:"rsc_location"`
-		} `xml:"constraints"`
-	} `xml:"configuration"`
-}
-
 func NewCollector(crmMonPath string, cibAdminPath string) (*pacemakerCollector, error) {
 	err := collector.CheckExecutables(crmMonPath, cibAdminPath)
 	if err != nil {
@@ -104,8 +23,8 @@ func NewCollector(crmMonPath string, cibAdminPath string) (*pacemakerCollector,
 
 	c := &pacemakerCollector{
 		collector.NewDefaultCollector("pacemaker"),
-		crmMonPath,
-		cibAdminPath,
+		crmmon.NewCrmMonParser(crmMonPath),
+		cib.NewCibAdminParser(cibAdminPath),
 	}
 	c.SetDescriptor("nodes", "The nodes in the cluster; one line per name, per status", []string{"node", "type", "status"})
 	c.SetDescriptor("resources", "The resources in the cluster; one line per id, per status", []string{"node", "resource", "role", "managed", "status"})
@@ -120,65 +39,41 @@ func NewCollector(crmMonPath string, cibAdminPath string) (*pacemakerCollector,
 
 type pacemakerCollector struct {
 	collector.DefaultCollector
-	crmMonPath   string
-	cibAdminPath string
+	crmMonParser crmmon.Parser
+	cibParser    cib.Parser
 }
 
 func (c *pacemakerCollector) Collect(ch chan<- prometheus.Metric) {
 	log.Infoln("Collecting pacemaker metrics...")
 
-	pacemakerStatus, err := c.getPacemakerStatus()
+	crmMon, err := c.crmMonParser.Parse()
 	if err != nil {
 		log.Warnln(err)
 		return
 	}
 
-	cib, err := c.getCIB()
+	CIB, err := c.cibParser.Parse()
 	if err != nil {
 		log.Warnln(err)
 		return
 	}
 
 	var stonithEnabled float64
-	if pacemakerStatus.Summary.ClusterOptions.StonithEnabled {
+	if crmMon.Summary.ClusterOptions.StonithEnabled {
 		stonithEnabled = 1
 	}
 
 	ch <- c.MakeGaugeMetric("stonith_enabled", stonithEnabled)
 
-	c.recordNodeMetrics(pacemakerStatus, ch)
-	c.recordFailCountMetrics(pacemakerStatus, ch)
-	c.recordMigrationThresholdMetrics(pacemakerStatus, ch)
-	c.recordResourceAgentsChanges(pacemakerStatus, ch)
-	c.recordConstraintsMetrics(cib, ch)
-}
-
-func (c *pacemakerCollector) getPacemakerStatus() (pacemakerStatus, error) {
-	var pacemakerStatus pacemakerStatus
-	pacemakerStatusXML, err := exec.Command(c.crmMonPath, "-X", "--group-by-node", "--inactive").Output()
-	if err != nil {
-		return pacemakerStatus, errors.Wrap(err, "error while executing crm_mon")
-	}
-
-	pacemakerStatus, err = parsePacemakerStatus(pacemakerStatusXML)
-	if err != nil {
-		return pacemakerStatus, errors.Wrap(err, "error while parsing crm_mon XML output")
-	}
-
-	return pacemakerStatus, nil
-}
-
-func parsePacemakerStatus(pacemakerXMLRaw []byte) (pacemakerStatus, error) {
-	var pacemakerStatus pacemakerStatus
-	err := xml.Unmarshal(pacemakerXMLRaw, &pacemakerStatus)
-	if err != nil {
-		return pacemakerStatus, errors.Wrap(err, "could not parse Pacemaker status from XML")
-	}
-	return pacemakerStatus, nil
+	c.recordNodeMetrics(crmMon, ch)
+	c.recordFailCountMetrics(crmMon, ch)
+	c.recordMigrationThresholdMetrics(crmMon, ch)
+	c.recordResourceAgentsChanges(crmMon, ch)
+	c.recordConstraintsMetrics(CIB, ch)
 }
 
-func (c *pacemakerCollector) recordNodeMetrics(pacemakerStatus pacemakerStatus, ch chan<- prometheus.Metric) {
-	for _, node := range pacemakerStatus.Nodes.Node {
+func (c *pacemakerCollector) recordNodeMetrics(crmMon crmmon.Root, ch chan<- prometheus.Metric) {
+	for _, node := range crmMon.Nodes {
 		nodeStatuses := map[string]bool{
 			"online":         node.Online,
 			"standby":        node.Standby,
@@ -210,7 +105,7 @@ func (c *pacemakerCollector) recordNodeMetrics(pacemakerStatus pacemakerStatus,
 	}
 }
 
-func (c *pacemakerCollector) recordResourcesMetrics(node node, ch chan<- prometheus.Metric) {
+func (c *pacemakerCollector) recordResourcesMetrics(node crmmon.Node, ch chan<- prometheus.Metric) {
 	for _, resource := range node.Resources {
 		resourceStatuses := map[string]bool{
 			"active":          resource.Active,
@@ -234,8 +129,8 @@ func (c *pacemakerCollector) recordResourcesMetrics(node node, ch chan<- prometh
 	}
 }
 
-func (c *pacemakerCollector) recordFailCountMetrics(pacemakerStatus pacemakerStatus, ch chan<- prometheus.Metric) {
-	for _, node := range pacemakerStatus.NodeHistory.Node {
+func (c *pacemakerCollector) recordFailCountMetrics(crmMon crmmon.Root, ch chan<- prometheus.Metric) {
+	for _, node := range crmMon.NodeHistory.Node {
 		for _, resHistory := range node.ResourceHistory {
 			failCount := float64(resHistory.FailCount)
 
@@ -250,8 +145,8 @@ func (c *pacemakerCollector) recordFailCountMetrics(pacemakerStatus pacemakerSta
 	}
 }
 
-func (c *pacemakerCollector) recordResourceAgentsChanges(pacemakerStatus pacemakerStatus, ch chan<- prometheus.Metric) {
-	t, err := time.Parse(time.ANSIC, pacemakerStatus.Summary.LastChange.Time)
+func (c *pacemakerCollector) recordResourceAgentsChanges(crmMon crmmon.Root, ch chan<- prometheus.Metric) {
+	t, err := time.Parse(time.ANSIC, crmMon.Summary.LastChange.Time)
 	if err != nil {
 		log.Warnln(err)
 		return
@@ -260,31 +155,16 @@ func (c *pacemakerCollector) recordResourceAgentsChanges(pacemakerStatus pacemak
 	ch <- c.MakeCounterMetric("config_last_change", float64(t.Unix()))
 }
 
-func (c *pacemakerCollector) recordMigrationThresholdMetrics(pacemakerStatus pacemakerStatus, ch chan<- prometheus.Metric) {
-	for _, node := range pacemakerStatus.NodeHistory.Node {
+func (c *pacemakerCollector) recordMigrationThresholdMetrics(crmMon crmmon.Root, ch chan<- prometheus.Metric) {
+	for _, node := range crmMon.NodeHistory.Node {
 		for _, resHistory := range node.ResourceHistory {
 			ch <- c.MakeGaugeMetric("migration_threshold", float64(resHistory.MigrationThreshold), node.Name, resHistory.Name)
 		}
 	}
 }
 
-func (c *pacemakerCollector) getCIB() (CIB, error) {
-	var cib CIB
-	cibXML, err := exec.Command(c.cibAdminPath, "--query", "--local").Output()
-	if err != nil {
-		return cib, errors.Wrap(err, "error while executing cibadmin")
-	}
-
-	err = xml.Unmarshal(cibXML, &cib)
-	if err != nil {
-		return cib, errors.Wrap(err, "could not parse cibadmin status from XML")
-	}
-
-	return cib, nil
-}
-
-func (c *pacemakerCollector) recordConstraintsMetrics(cib CIB, ch chan<- prometheus.Metric) {
-	for _, constraint := range cib.Configuration.Constraints.RscLocations {
+func (c *pacemakerCollector) recordConstraintsMetrics(CIB cib.Root, ch chan<- prometheus.Metric) {
+	for _, constraint := range CIB.Configuration.Constraints.RscLocations {
 		var constraintScore float64
 		switch constraint.Score {
 		case "INFINITY":
diff --git a/doc/design.md b/doc/design.md
index 233532f..b8c0c08 100644
--- a/doc/design.md
+++ b/doc/design.md
@@ -1,10 +1,10 @@
 # Design Notes
 
-This document describes the rationale behind design decisions takend during the development of this project.
+This document describes the rationale behind design decisions taken during the development of this project.
 
 ## Goals
 
-- Export runtime statistics about the various HA cluster components from existing data sources, to be consumed in a Prometheus monitoring stack.
+- Export runtime statistics about the various ClusterLabs Linux HA cluster components from existing data sources, to be consumed by a Prometheus monitoring stack.
 
 ## Non-goals
 
@@ -14,17 +14,26 @@ This document describes the rationale behind design decisions takend during the
 ## Structure
 
 The project consist in a small HTTP application that exposes runtime data in a line protocol.
+  
+A series of "metric collectors" are consumed by the main application entry point, `ha_cluster_exporter.go`, where they are registered with the Prometheus client and then exposed via its HTTP handler.
 
-A series of `prometheus.Collector` implementations, one for each cluster component (that we'll call _subsystems_) are instantiated in the main application entry point, registered with the Prometheus client, and then exposed via its HTTP handler.
+Concurrency is handled internally by a worker pool provided by the Prometheus library, but this implementation detail is completely obfuscated to the consumers.
 
-Each collector `Collect` method will be called concurrently by the client itself in an internal worker goroutine.
+The data sources are read every time an HTTP request comes, and the collected metrics are not shared: their lifecycle corresponds with the request's.
 
-The sources hence are read every time an HTTP request comes, and the collected data is not shared: its lifecycle corresponds with the request's.
-
-To avoid concurrent reads of the same source, all `Collect` methods are serialized with a mutex.
+The `internal` package contains common code shared among all the other packages, but not intended for usage outside this projects.
 
 ## Collectors
 
-The collectors are very simple: they usually just invoke a bunch of system commands, then parse the output into bespoke data structures that can be used to build Prometheus metrics.
+Inside the `collector` package, you wil find the code of the main logic of the project: these are a number of [`prometheus.Collector`](https://github.com/prometheus/client_golang/blob/b25ce2693a6de99c3ea1a1471cd8f873301a452f/prometheus/collector.go#L16-L63) implementations, one for each cluster component (that we'll call _subsystems_), like Pacemaker, or Corosync.
+
+Common functionality is provided by composing the [`DefaultCollector`](../collector/default_collector.go). 
+
+Each subsystem collector has a dedicated package; some are very simple, some are little more nuanced. In general, they depend on external, globally available, system tools, to introspect the subsystems. 
+
+The collectors usually just invoke these system commands, parsing the output into bespoke data structures.
+When building these data structures involves a significant amount of code, for a better separation of concerns this responsibility is extracted in dedicated subpackages, like [`collector/pacemaker/cib`](../collector/pacemaker/cib).
+
+The data structures are then used by the collectors to build the Prometheus metrics. 
 
-More details about these metrics can be found in the [metrics specification document](metrics.md).
+More details about the metrics themselves can be found in the [metrics](metrics.md) document.


Test patch:
diff --git a/collector/pacemaker/cib/parser_test.go b/collector/pacemaker/cib/parser_test.go
new file mode 100644
index 0000000..28b19dd
--- /dev/null
+++ b/collector/pacemaker/cib/parser_test.go
@@ -0,0 +1,63 @@
+package cib
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConstructor(t *testing.T) {
+	p := NewCibAdminParser("foo")
+	assert.Equal(t, "foo", p.cibAdminPath)
+}
+
+func TestParse(t *testing.T) {
+	p := NewCibAdminParser("../../../test/fake_cibadmin.sh")
+	data, err := p.Parse()
+	assert.NoError(t, err)
+	assert.Equal(t, 2, len(data.Configuration.Nodes))
+	assert.Equal(t, "hana01", data.Configuration.Nodes[0].Uname)
+	assert.Equal(t, "hana02", data.Configuration.Nodes[1].Uname)
+	assert.Equal(t, 3, len(data.Configuration.Resources.Primitives))
+	assert.Equal(t, 1, len(data.Configuration.Resources.Masters))
+	assert.Equal(t, 1, len(data.Configuration.Resources.Clones))
+	assert.Equal(t, "stonith-sbd", data.Configuration.Resources.Primitives[0].Id)
+	assert.Equal(t, "stonith", data.Configuration.Resources.Primitives[0].Class)
+	assert.Equal(t, "external/sbd", data.Configuration.Resources.Primitives[0].Type)
+	assert.Equal(t, "stonith-sbd-instance_attributes", data.Configuration.Resources.Primitives[0].InstanceAttributes.Id)
+	assert.Equal(t, 1, len(data.Configuration.Resources.Primitives[0].InstanceAttributes.NvPairs))
+	assert.Equal(t, "pcmk_delay_max", data.Configuration.Resources.Primitives[0].InstanceAttributes.NvPairs[0].Name)
+	assert.Equal(t, "stonith-sbd-instance_attributes-pcmk_delay_max", data.Configuration.Resources.Primitives[0].InstanceAttributes.NvPairs[0].Id)
+	assert.Equal(t, "30s", data.Configuration.Resources.Primitives[0].InstanceAttributes.NvPairs[0].Value)
+	assert.Equal(t, "msl_SAPHana_PRD_HDB00", data.Configuration.Resources.Masters[0].Id)
+	assert.Equal(t, 3, len(data.Configuration.Resources.Masters[0].MetaAttributes.NvPairs))
+	assert.Equal(t, "rsc_SAPHana_PRD_HDB00", data.Configuration.Resources.Masters[0].Primitive.Id)
+	assert.Equal(t, 5, len(data.Configuration.Resources.Masters[0].Primitive.Operations))
+	assert.Equal(t, "rsc_SAPHana_PRD_HDB00-start-0", data.Configuration.Resources.Masters[0].Primitive.Operations[0].Id)
+	assert.Equal(t, "start", data.Configuration.Resources.Masters[0].Primitive.Operations[0].Name)
+	assert.Equal(t, 0, data.Configuration.Resources.Masters[0].Primitive.Operations[0].Interval)
+	assert.Equal(t, 3600, data.Configuration.Resources.Masters[0].Primitive.Operations[0].Timeout)
+	assert.Equal(t, "rsc_SAPHana_PRD_HDB00-stop-0", data.Configuration.Resources.Masters[0].Primitive.Operations[1].Id)
+	assert.Equal(t, "stop", data.Configuration.Resources.Masters[0].Primitive.Operations[1].Name)
+	assert.Equal(t, 0, data.Configuration.Resources.Masters[0].Primitive.Operations[1].Interval)
+	assert.Equal(t, 3600, data.Configuration.Resources.Masters[0].Primitive.Operations[1].Timeout)
+	assert.Equal(t, "rsc_SAPHana_PRD_HDB00-promote-0", data.Configuration.Resources.Masters[0].Primitive.Operations[2].Id)
+	assert.Equal(t, "promote", data.Configuration.Resources.Masters[0].Primitive.Operations[2].Name)
+	assert.Equal(t, 0, data.Configuration.Resources.Masters[0].Primitive.Operations[2].Interval)
+	assert.Equal(t, 3600, data.Configuration.Resources.Masters[0].Primitive.Operations[2].Timeout)
+	assert.Equal(t, "rsc_SAPHana_PRD_HDB00-monitor-60", data.Configuration.Resources.Masters[0].Primitive.Operations[3].Id)
+	assert.Equal(t, "monitor", data.Configuration.Resources.Masters[0].Primitive.Operations[3].Name)
+	assert.Equal(t, "Master", data.Configuration.Resources.Masters[0].Primitive.Operations[3].Role)
+	assert.Equal(t, 60, data.Configuration.Resources.Masters[0].Primitive.Operations[3].Interval)
+	assert.Equal(t, 700, data.Configuration.Resources.Masters[0].Primitive.Operations[3].Timeout)
+	assert.Equal(t, "rsc_SAPHana_PRD_HDB00-monitor-61", data.Configuration.Resources.Masters[0].Primitive.Operations[4].Id)
+	assert.Equal(t, "monitor", data.Configuration.Resources.Masters[0].Primitive.Operations[4].Name)
+	assert.Equal(t, "Slave", data.Configuration.Resources.Masters[0].Primitive.Operations[4].Role)
+	assert.Equal(t, 61, data.Configuration.Resources.Masters[0].Primitive.Operations[4].Interval)
+	assert.Equal(t, 700, data.Configuration.Resources.Masters[0].Primitive.Operations[4].Timeout)
+	assert.Equal(t, "test", data.Configuration.Resources.Primitives[2].Id)
+	assert.Equal(t, "ocf", data.Configuration.Resources.Primitives[2].Class)
+	assert.Equal(t, "heartbeat", data.Configuration.Resources.Primitives[2].Provider)
+	assert.Equal(t, "Dummy", data.Configuration.Resources.Primitives[2].Type)
+
+}
diff --git a/collector/pacemaker/crmmon/parser_test.go b/collector/pacemaker/crmmon/parser_test.go
new file mode 100644
index 0000000..4ebf043
--- /dev/null
+++ b/collector/pacemaker/crmmon/parser_test.go
@@ -0,0 +1,50 @@
+package crmmon
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConstructor(t *testing.T) {
+	p := NewCrmMonParser("foo")
+	assert.Equal(t, "foo", p.crmMonPath)
+}
+
+func TestParse(t *testing.T) {
+	p := NewCrmMonParser("../../../test/fake_crm_mon.sh")
+	data, err := p.Parse()
+	assert.NoError(t, err)
+	assert.Equal(t, "2.0.0", data.Version)
+	assert.Equal(t, 6, data.Summary.Resources.Number)
+	assert.Equal(t, 0, data.Summary.Resources.Disabled)
+	assert.Equal(t, 0, data.Summary.Resources.Blocked)
+	assert.Equal(t, "Fri Oct 18 11:48:22 2019", data.Summary.LastChange.Time)
+	assert.Equal(t, 2, data.Summary.Nodes.Number)
+	assert.Equal(t, "hana01", data.Nodes[0].Name)
+	assert.Equal(t, "1084783375", data.Nodes[0].ID)
+	assert.Equal(t, true, data.Nodes[0].Online)
+	assert.Equal(t, true, data.Nodes[0].ExpectedUp)
+	assert.Equal(t, true, data.Nodes[0].DC)
+	assert.Equal(t, false, data.Nodes[0].Unclean)
+	assert.Equal(t, false, data.Nodes[0].Shutdown)
+	assert.Equal(t, false, data.Nodes[0].StandbyOnFail)
+	assert.Equal(t, false, data.Nodes[0].Maintenance)
+	assert.Equal(t, false, data.Nodes[0].Pending)
+	assert.Equal(t, false, data.Nodes[0].Standby)
+	assert.Equal(t, "hana02", data.Nodes[1].Name)
+	assert.Equal(t, "1084783376", data.Nodes[1].ID)
+	assert.Equal(t, true, data.Nodes[1].Online)
+	assert.Equal(t, true, data.Nodes[1].ExpectedUp)
+	assert.Equal(t, false, data.Nodes[1].DC)
+	assert.Equal(t, false, data.Nodes[1].Unclean)
+	assert.Equal(t, false, data.Nodes[1].Shutdown)
+	assert.Equal(t, false, data.Nodes[1].StandbyOnFail)
+	assert.Equal(t, false, data.Nodes[1].Maintenance)
+	assert.Equal(t, false, data.Nodes[1].Pending)
+	assert.Equal(t, false, data.Nodes[1].Standby)
+	assert.Equal(t, "hana01", data.NodeHistory.Node[0].Name)
+	assert.Equal(t, 5000, data.NodeHistory.Node[0].ResourceHistory[0].MigrationThreshold)
+	assert.Equal(t, 2, data.NodeHistory.Node[0].ResourceHistory[1].FailCount)
+	assert.Equal(t, "rsc_SAPHana_PRD_HDB00", data.NodeHistory.Node[0].ResourceHistory[0].Name)
+}
diff --git a/collector/pacemaker/pacemaker_test.go b/collector/pacemaker/pacemaker_test.go
index 40ee226..2c53285 100644
--- a/collector/pacemaker/pacemaker_test.go
+++ b/collector/pacemaker/pacemaker_test.go
@@ -8,89 +8,6 @@ import (
 	assertcustom "github.com/ClusterLabs/ha_cluster_exporter/internal/assert"
 )
 
-func TestParsePacemakerXML(t *testing.T) {
-	// this is a full config file more or less , in other tests it is cutted
-	pacemakerxml := `<?xml version="1.0"?>
-	<crm_mon version="2.0.0">
-		<summary>
-			<stack type="corosync" />
-			<current_dc present="true" version="2.0.0+20181108.62ffcafbc-1.1-2.0.0+20181108.62ffcafbc" name="Hawk3-1" id="168430211" with_quorum="true" />
-			<last_update time="Tue Jan 15 22:20:05 2019" />
-			<last_change time="Tue Jan 15 22:19:59 2019" user="root" client="cibadmin" origin="Hawk3-2" />
-			<nodes_configured number="2" />
-			<resources_configured number="3" disabled="0" blocked="0" />
-			<cluster_options stonith-enabled="false" symmetric-cluster="true" no-quorum-policy="stop" maintenance-mode="false" />
-		</summary>
-		<nodes>
-			<node name="Hawk3-1" id="168430211" online="true" standby="false" standby_onfail="false" maintenance="false" pending="false" unclean="false" shutdown="false" expected_up="true" is_dc="true" resources_running="1" type="member" />
-			<node name="Hawk3-2" id="168430212" online="true" standby="false" standby_onfail="false" maintenance="false" pending="false" unclean="false" shutdown="false" expected_up="true" is_dc="false" resources_running="1" type="member" />
-		</nodes>
-		<resources>
-			<resource id="d1" resource_agent="ocf::heartbeat:Dummy" role="Started" active="true" orphaned="false" blocked="false" managed="true" failed="false" failure_ignored="false" nodes_running_on="1" >
-				<node name="Hawk3-1" id="168430211" cached="false"/>
-			</resource>
-			<resource id="vip1" resource_agent="ocf::heartbeat:IPaddr2" role="Started" active="true" orphaned="false" blocked="false" managed="true" failed="false" failure_ignored="false" nodes_running_on="1" >
-				<node name="Hawk3-2" id="168430212" cached="false"/>
-			</resource>
-		</resources>
-		<node_attributes>
-			<node name="Hawk3-1">
-			</node>
-			<node name="Hawk3-2">
-			</node>
-		</node_attributes>
-		<node_history>
-			<node name="Hawk3-2">
-				<resource_history id="vip1" orphan="false" migration-threshold="3">
-					<operation_history call="10" task="start" last-rc-change="Tue Jan 15 22:19:14 2019" last-run="Tue Jan 15 22:19:14 2019" exec-time="66ms" queue-time="0ms" rc="0" rc_text="ok" />
-					<operation_history call="11" task="monitor" interval="10000ms" last-rc-change="Tue Jan 15 22:19:15 2019" exec-time="34ms" queue-time="0ms" rc="0" rc_text="ok" />
-				</resource_history>
-				<resource_history id="ddd" orphan="false" migration-threshold="3" fail-count="1000000" last-failure="Tue Jan 15 22:20:00 2019">
-					<operation_history call="16" task="start" last-rc-change="Tue Jan 15 22:19:59 2019" last-run="Tue Jan 15 22:19:59 2019" exec-time="34ms" queue-time="0ms" rc="5" rc_text="not installed" />
-					<operation_history call="17" task="stop" last-rc-change="Tue Jan 15 22:19:59 2019" last-run="Tue Jan 15 22:19:59 2019" exec-time="36ms" queue-time="0ms" rc="0" rc_text="ok" />
-				</resource_history>
-			</node>
-			<node name="Hawk3-1">
-				<resource_history id="d1" orphan="false" migration-threshold="3" fail-count="300" last-failure="Tue Jan 15 22:20:00 2019">
-					<operation_history call="10" task="start" last-rc-change="Tue Jan 15 22:19:15 2019" last-run="Tue Jan 15 22:19:15 2019" exec-time="23ms" queue-time="0ms" rc="0" rc_text="ok" />
-				</resource_history>
-				<resource_history id="ddd" orphan="false" migration-threshold="3" fail-count="1000000" last-failure="Tue Jan 15 22:19:59 2019">
-					<operation_history call="15" task="start" last-rc-change="Tue Jan 15 22:19:59 2019" last-run="Tue Jan 15 22:19:59 2019" exec-time="45ms" queue-time="0ms" rc="5" rc_text="not installed" />
-					<operation_history call="16" task="stop" last-rc-change="Tue Jan 15 22:19:59 2019" last-run="Tue Jan 15 22:19:59 2019" exec-time="38ms" queue-time="0ms" rc="0" rc_text="ok" />
-				</resource_history>
-			</node>
-		</node_history>
-		<failures>
-			<failure op_key="ddd_start_0" node="Hawk3-2" exitstatus="not installed" exitreason="Setup problem: couldn&apos;t find command: /usr/bin/safe_mysqld" exitcode="5" call="16" status="complete" last-rc-change="Tue Jan 15 22:19:59 2019" queued="0" exec="34" interval="0" task="start" />
-			<failure op_key="ddd_start_0" node="Hawk3-1" exitstatus="not installed" exitreason="Setup problem: couldn&apos;t find command: /usr/bin/safe_mysqld" exitcode="5" call="15" status="complete" last-rc-change="Tue Jan 15 22:19:59 2019" queued="0" exec="45" interval="0" task="start" />
-		</failures>
-		<fence_history>
-		</fence_history>
-		<tickets>
-		</tickets>
-	</crm_mon>`
-
-	status, err := parsePacemakerStatus([]byte(pacemakerxml))
-
-	assert.Nil(t, err)
-	assert.Equal(t, "2.0.0", status.Version)
-	assert.Equal(t, 3, status.Summary.Resources.Number)
-	assert.Equal(t, 0, status.Summary.Resources.Disabled)
-	assert.Equal(t, 0, status.Summary.Resources.Blocked)
-	assert.Equal(t, "Tue Jan 15 22:19:59 2019", status.Summary.LastChange.Time)
-	assert.Equal(t, 2, status.Summary.Nodes.Number)
-	assert.Equal(t, "Hawk3-1", status.Nodes.Node[0].Name)
-	assert.Equal(t, "168430211", status.Nodes.Node[0].ID)
-	assert.Equal(t, true, status.Nodes.Node[0].Online)
-	assert.Equal(t, "Hawk3-2", status.Nodes.Node[1].Name)
-	assert.Equal(t, "168430212", status.Nodes.Node[1].ID)
-	assert.Equal(t, true, status.Nodes.Node[1].Online)
-	assert.Equal(t, "Hawk3-2", status.NodeHistory.Node[0].Name)
-	assert.Equal(t, 3, status.NodeHistory.Node[0].ResourceHistory[0].MigrationThreshold)
-	assert.Equal(t, 1000000, status.NodeHistory.Node[0].ResourceHistory[1].FailCount)
-	assert.Equal(t, "vip1", status.NodeHistory.Node[0].ResourceHistory[0].Name)
-}
-
 func TestNewPacemakerCollector(t *testing.T) {
 	_, err := NewCollector("../../test/fake_crm_mon.sh", "../../test/fake_cibadmin.sh")