Repository: ClusterLabs/ha_cluster_exporter
Pull request #157 (2020-05-28 14:44:01)

make RingID parsing compatible with corosync-quorumtool on Corosync v2.3.6
We've been made aware of this incompatibility by MSFT who are running SLE12SP5, which runs an older version of Corosync we never tested against.

The format of the Ring ID changed in v2.4.0:
https://github.com/corosync/corosync/commit/434aa4fc6492b18d72975a8207e49d447d4ea7eb

We now parse the whole field as a string instead of trying to be too granular.


Patch:
diff --git a/collector/corosync/parser.go b/collector/corosync/parser.go
index 05df8f7..e0ccc7c 100644
--- a/collector/corosync/parser.go
+++ b/collector/corosync/parser.go
@@ -15,7 +15,6 @@ type Parser interface {
 type Status struct {
 	NodeId      string
 	RingId      string
-	Seq         uint64
 	Rings       []Ring
 	QuorumVotes QuorumVotes
 	Quorate     bool
@@ -57,7 +56,7 @@ func (p *defaultParser) Parse(cfgToolOutput []byte, quorumToolOutput []byte) (*S
 		return nil, errors.Wrap(err, "could not parse node id in corosync-quorumtool output")
 	}
 
-	status.RingId, status.Seq, err = parseRingIdAndSeq(quorumToolOutput)
+	status.RingId, err = parseRingId(quorumToolOutput)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not parse ring id and seq number in corosync-quorumtool output")
 	}
@@ -82,9 +81,9 @@ func (p *defaultParser) Parse(cfgToolOutput []byte, quorumToolOutput []byte) (*S
 	return status, nil
 }
 
-func parseNodeId(cfgToolOutput []byte) (string, error) {
+func parseNodeId(quorumToolOutput []byte) (string, error) {
 	nodeRe := regexp.MustCompile(`(?m)Node ID:\s+(\w+)`)
-	matches := nodeRe.FindSubmatch(cfgToolOutput)
+	matches := nodeRe.FindSubmatch(quorumToolOutput)
 	if matches == nil {
 		return "", errors.New("could not find Node ID line")
 	}
@@ -92,19 +91,36 @@ func parseNodeId(cfgToolOutput []byte) (string, error) {
 	return string(matches[1]), nil
 }
 
-func parseRingIdAndSeq(cfgToolOutput []byte) (string, uint64, error) {
-	nodeRe := regexp.MustCompile(`(?m)Ring ID:\s+(\w+)[\.|/](\d+)`)
-	matches := nodeRe.FindSubmatch(cfgToolOutput)
-	if matches == nil {
-		return "", 0, errors.New("could not find Ring ID line")
-	}
+func parseRingId(quorumToolOutput []byte) (string, error) {
+	// the following regex matches and capture the ring id from this kind of output from corosync-quorumtool
+	// the ring id is composed by the representative node id (not to be confused with the local node id) and the sequence number
+	/*
+		Quorum information
+		------------------
+		Date:             Sun Sep 29 16:10:37 2019
+		Quorum provider:  corosync_votequorum
+		Nodes:            2
+		Node ID:          1084780051
+		Ring ID:          1084780051.44
+		Quorate:          Yes
+	*/
+	// in corosync < v2.99 the line is slightly different:
+	/*
+		Ring ID:          1084780051/44
+	*/
+	// in corosync < v2.4 there is no representative node id:
+	/*
+		Ring ID:          1084780051
+	*/
 
-	seq, err := strconv.Atoi(string(matches[2]))
-	if err != nil {
-		return "", 0, errors.Wrap(err, "could not parse seq number to uint64")
+	// given the differences in format between corosync versions, we just parse it as a whole string
+	re := regexp.MustCompile(`(?m)Ring ID:\s+\b(.+)\b`)
+	matches := re.FindSubmatch(quorumToolOutput)
+	if matches == nil {
+		return "", errors.New("could not find Ring ID line")
 	}
 
-	return string(matches[1]), uint64(seq), nil
+	return string(matches[1]), nil
 }
 
 func parseQuorate(quorumToolOutput []byte) (bool, error) {


Test patch:
diff --git a/collector/corosync/parser_test.go b/collector/corosync/parser_test.go
index 0650de4..10bb665 100644
--- a/collector/corosync/parser_test.go
+++ b/collector/corosync/parser_test.go
@@ -57,8 +57,7 @@ Membership information
 
 	assert.True(t, status.Quorate)
 	assert.Equal(t, "1084780051", status.NodeId)
-	assert.Equal(t, "1084780051", status.RingId)
-	assert.EqualValues(t, 44, status.Seq)
+	assert.Equal(t, "1084780051.44", status.RingId)
 	assert.EqualValues(t, 232, status.QuorumVotes.ExpectedVotes)
 	assert.EqualValues(t, 22, status.QuorumVotes.HighestExpected)
 	assert.EqualValues(t, 21, status.QuorumVotes.TotalVotes)
@@ -76,18 +75,7 @@ Membership information
 	assert.EqualValues(t, 1, members[1].Votes)
 }
 
-func TestLegacyParse(t *testing.T) {
-	p := NewParser()
-
-	cfgToolOutput := []byte(`Printing ring status.
-Local node ID 1084780051
-RING ID 0
-		id      = 10.0.0.1
-		status  = ring 0 active with no faults
-RING ID 1
-		id      = 172.16.0.1
-		status  = ring 1 active with no faults`)
-
+func TestParseRingIdInCorosyncV2_4(t *testing.T) {
 	quoromToolOutput := []byte(`Quorum information
 ------------------
 Date:             Sun Sep 29 16:10:37 2019
@@ -111,38 +99,38 @@ Membership information
 1084780051          1 dma-dog-hana01 (local)
 1084780052          1 dma-dog-hana02`)
 
-	status, err := p.Parse(cfgToolOutput, quoromToolOutput)
+	ringId, err := parseRingId(quoromToolOutput)
 	assert.NoError(t, err)
 
-	rings := status.Rings
+	assert.Equal(t, "1084780051/44", ringId)
+}
 
-	assert.Len(t, rings, 2)
-	assert.Equal(t, "0", rings[0].Number)
-	assert.Equal(t, "10.0.0.1", rings[0].Address)
-	assert.False(t, rings[0].Faulty)
-	assert.Equal(t, "1", rings[1].Number)
-	assert.Equal(t, "172.16.0.1", rings[1].Address)
-	assert.False(t, rings[1].Faulty)
+func TestParseRingIdInCorosyncV2_3(t *testing.T) {
+	quoromToolOutput := []byte(`Quorum information
+------------------
+Date:             Wed May 27 14:16:10 2020
+Quorum provider:  corosync_votequorum
+Nodes:            2
+Node ID:          1
+Ring ID:          100
+Quorate:          Yes
+Votequorum information
+----------------------
+Expected votes:   2
+Highest expected: 2
+Total votes:      2
+Quorum:           1
+Flags:            2Node Quorate WaitForAll
+Membership information
+----------------------
+    Nodeid      Votes Name
+         1          1 10.1.2.4 (local)
+         2          1 10.1.2.5`)
 
-	assert.True(t, status.Quorate)
-	assert.Equal(t, "1084780051", status.NodeId)
-	assert.Equal(t, "1084780051", status.RingId)
-	assert.EqualValues(t, 44, status.Seq)
-	assert.EqualValues(t, 232, status.QuorumVotes.ExpectedVotes)
-	assert.EqualValues(t, 22, status.QuorumVotes.HighestExpected)
-	assert.EqualValues(t, 21, status.QuorumVotes.TotalVotes)
-	assert.EqualValues(t, 421, status.QuorumVotes.Quorum)
+	ringId, err := parseRingId(quoromToolOutput)
+	assert.NoError(t, err)
 
-	members := status.Members
-	assert.Len(t, members, 2)
-	assert.Exactly(t, "1084780051", members[0].Id)
-	assert.Exactly(t, "dma-dog-hana01", members[0].Name)
-	assert.True(t, members[0].Local)
-	assert.EqualValues(t, 1, members[0].Votes)
-	assert.Exactly(t, "1084780052", members[1].Id)
-	assert.Exactly(t, "dma-dog-hana02", members[1].Name)
-	assert.False(t, members[1].Local)
-	assert.EqualValues(t, 1, members[1].Votes)
+	assert.Equal(t, "100", ringId)
 }
 
 func TestParseFaultyRings(t *testing.T) {
@@ -162,7 +150,7 @@ func TestParseFaultyRings(t *testing.T) {
 	assert.False(t, rings[1].Faulty)
 }
 
-func TestLegacyParseFaultyRings(t *testing.T) {
+func TestParseFaultyRingsInCorosyncV2(t *testing.T) {
 	cfgToolOutput := []byte(`Printing ring status.
 	Local node ID 16777226
 	RING ID 0
@@ -211,28 +199,10 @@ func TestParseQuorumVotesEmptyError(t *testing.T) {
 func TestParseRingIdEmptyError(t *testing.T) {
 	quoromToolOutput := []byte(``)
 
-	_, _, err := parseRingIdAndSeq(quoromToolOutput)
+	_, err := parseRingId(quoromToolOutput)
 	assert.EqualError(t, err, "could not find Ring ID line")
 }
 
-func TestParseSeqUintError(t *testing.T) {
-	quoromToolOutput := []byte(`Ring ID:          1084780051.10000000000000000000000000000000000000000000000`)
-
-	_, _, err := parseRingIdAndSeq(quoromToolOutput)
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "could not parse seq number to uint64")
-	assert.Contains(t, err.Error(), "value out of range")
-}
-
-func TestParseLegacySeqUintError(t *testing.T) {
-	quoromToolOutput := []byte(`Ring ID:          1084780051/10000000000000000000000000000000000000000000000`)
-
-	_, _, err := parseRingIdAndSeq(quoromToolOutput)
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "could not parse seq number to uint64")
-	assert.Contains(t, err.Error(), "value out of range")
-}
-
 func TestParseQuorumVotesUintErrors(t *testing.T) {
 	quorumToolOutputs := [][]byte{
 		[]byte(`
diff --git a/test/corosync.metrics b/test/corosync.metrics
index a1fbbd7..4b2298e 100644
--- a/test/corosync.metrics
+++ b/test/corosync.metrics
@@ -12,8 +12,8 @@ ha_cluster_corosync_quorum_votes{type="total_votes"} 2
 ha_cluster_corosync_ring_errors 1
 # HELP ha_cluster_corosync_rings The status of each Corosync ring; 1 means healthy, 0 means faulty.
 # TYPE ha_cluster_corosync_rings gauge
-ha_cluster_corosync_rings{address="10.0.0.1",node_id="1084783375",number="0",ring_id="1084783375"} 0
-ha_cluster_corosync_rings{address="172.16.0.1",node_id="1084783375",number="1",ring_id="1084783375"} 1
+ha_cluster_corosync_rings{address="10.0.0.1",node_id="1084783375",number="0",ring_id="1084783375/40"} 0
+ha_cluster_corosync_rings{address="172.16.0.1",node_id="1084783375",number="1",ring_id="1084783375/40"} 1
 # HELP ha_cluster_corosync_member_votes How many votes each member node has contributed with to the current quorum
 # TYPE ha_cluster_corosync_member_votes gauge
 ha_cluster_corosync_member_votes{local="true",node="stefanotorresi-hana01",node_id="1084783375"} 1