Properly parse Gravity 5.0.36 status (#262)

* Add status tests for gravity 5.0.36.

These two tests ensure that Robotest continues to work correctly when
interacting with old versions of gravity (5.0.x).

Contributes to #247.

* Fix parsing of `gravity status` from gravity 5.0.x.

For backwards compatibility the `system_status` json field needs to be
able to accept `running` or `degraded` in addition to the integer
values.

Fixes #247.

Author: wadells

infra/gravity/gravity_test.go

@@ -19,9 +19,11 @@ package gravity
 import (
 	"bufio"
 	"bytes"
+	"encoding/json"
 	"os"
 	"testing"
 
+	"github.com/gravitational/robotest/lib/constants"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -109,3 +111,84 @@ func Test1641StatusValidation(t *testing.T) {
 	err = checkNotDegraded(status)
 	assert.Error(t, err)
 }
+
+// TestGravity5036ActiveStatusValidation ensures Robotest can correctly parse
+// an Active status from Gravity 5.0.36.
+//
+// This is important for testing --upgrade-via 5.0.x to 5.2.x to 5.5.x as well
+// as upgrade testing from stolon-app 1.10.x.
+//
+// See https://github.com/gravitational/robotest/issues/247 for more info.
+func TestGravity5036ActiveStatusValidation(t *testing.T) {
+	f, err := os.Open("testdata/status-active-5.0.36.json")
+	assert.NoError(t, err)
+	defer f.Close()
+
+	var status GravityStatus
+	err = parseStatus(&status)(bufio.NewReader(f))
+	assert.NoError(t, err)
+
+	err = checkNotDegraded(status)
+	assert.NoError(t, err)
+}
+
+// TestGravity5036DegradedStatusValidation ensures Robotest can correctly parse
+// a Degraded status from Gravity 5.0.36.
+//
+// This is important for testing --upgrade-via 5.0.x to 5.2.x to 5.5.x as well
+// as upgrade testing from stolon-app 1.10.x.
+//
+// See https://github.com/gravitational/robotest/issues/247 for more info.
+func TestGravity5036DegradedStatusValidation(t *testing.T) {
+	f, err := os.Open("testdata/status-degraded-5.0.36.json")
+	assert.NoError(t, err)
+	defer f.Close()
+
+	var status GravityStatus
+	err = parseStatus(&status)(bufio.NewReader(f))
+	assert.NoError(t, err)
+
+	err = checkNotDegraded(status)
+	assert.Error(t, err)
+}
+
+func TestSystemStatusStringDegradedUnmarshal(t *testing.T) {
+	input := json.RawMessage(`"degraded"`)
+	expected := constants.SystemStatus_Degraded
+	var status int
+	err := unmarshalSystemStatus(input, &status)
+	assert.NoError(t, err)
+	assert.Equal(t, expected, status)
+}
+
+func TestSystemStatusStringRunningUnmarshal(t *testing.T) {
+	input := json.RawMessage(`"running"`)
+	expected := constants.SystemStatus_Running
+	var status int
+	err := unmarshalSystemStatus(input, &status)
+	assert.NoError(t, err)
+	assert.Equal(t, expected, status)
+}
+
+func TestSystemStatusIntUnmarshal(t *testing.T) {
+	input := json.RawMessage(`2`)
+	expected := constants.SystemStatus_Degraded
+	var status int
+	err := unmarshalSystemStatus(input, &status)
+	assert.NoError(t, err)
+	assert.Equal(t, expected, status)
+}
+
+func TestSystemStatusInvalidFloatUnmarshal(t *testing.T) {
+	input := json.RawMessage(`3.14159`)
+	var status int
+	err := unmarshalSystemStatus(input, &status)
+	assert.Error(t, err)
+}
+
+func TestSystemStatusInvalidStringUnmarshal(t *testing.T) {
+	input := json.RawMessage(`"working perfectly"`)
+	var status int
+	err := unmarshalSystemStatus(input, &status)
+	assert.Error(t, err)
+}

infra/gravity/node_commands.go

@@ -155,14 +155,81 @@ type ClusterStatus struct {
 	Cluster string `json:"domain"`
 	// State is the cluster state
 	State string `json:"state"`
-	// SystemStatus is the cluster status, see https://github.com/gravitational/satellite/blob/7.1.0/agent/proto/agentpb/agent.proto#L50-L54
-	SystemStatus int `json:"system_status"`
+	// SystemStatus is the cluster status, custom unmarshalling logic needed to accommodate Gravity differences
+	SystemStatus int `json:"-"`
 	// Token is secure token which prevents rogue nodes from joining the cluster during installation
 	Token Token `json:"token"`
 	// Nodes describes the nodes in the cluster
 	Nodes []NodeStatus `json:"nodes"`
 }
 
+// unmarshalSystemStatus is a helper type to parse string or integer system statuses.
+//
+// For the original definitions and mappings see:
+//   https://github.com/gravitational/satellite/blob/5.0.2/agent/proto/agentpb/agent.proto#L36-L40
+//   https://github.com/gravitational/satellite/blob/5.0.2/agent/proto/agentpb/agent.pb.go#L48-L63
+func unmarshalSystemStatus(val json.RawMessage, status *int) error {
+	err := json.Unmarshal(val, &status)
+	if err == nil {
+		return nil
+	}
+	// parsing as int failed, try parsing as a string and converting
+	var str string
+	if err2 := json.Unmarshal(val, &str); err2 != nil {
+		return trace.BadParameter("unable to parse system_status %q as int or string: %v, %v", val, err, err2)
+	}
+	switch str {
+	case "running":
+		*status = constants.SystemStatus_Running
+	case "degraded":
+		*status = constants.SystemStatus_Degraded
+	default:
+		return trace.BadParameter("unknown system_status: %q", str)
+	}
+	return nil
+}
+
+// UnmarshalJSON fufills the Unmarshaler interface, allowing ClusterStatus
+// to perform custom JSON unmarshalling for certain fields.
+//
+// This is needed for the system_status field which Gravity 5.2.x+ marshals as
+// an integer and Gravity 5.0.36- marshals as a string. When Gravity 5.0.x
+// support is no longer needed, this function and unmarshalSystemStatus can be
+// removed and replaced by the following field tag in the Cluster status type
+// definition:
+//
+//   SystemStatus int `json:"system_status"`
+//
+// See https://github.com/gravitational/robotest/issues/247 for more info.
+func (cs *ClusterStatus) UnmarshalJSON(data []byte) error {
+	// First, populate all fields that don't require special handling,
+	type recursionBreaker ClusterStatus
+	var temp recursionBreaker
+	if err := json.Unmarshal(data, &temp); err != nil {
+		return trace.Wrap(err)
+	}
+	*cs = ClusterStatus(temp)
+
+	// handle system_status, which may be a string or an int
+	var jsonMap map[string]*json.RawMessage
+	if err := json.Unmarshal(data, &jsonMap); err != nil {
+		return trace.Wrap(err)
+	}
+
+	val, ok := jsonMap["system_status"]
+	if !ok {
+		return trace.BadParameter("system_status field is required: %v", data)
+	}
+
+	var status int
+	if err := unmarshalSystemStatus(*val, &status); err != nil {
+		return trace.Wrap(err)
+	}
+
+	cs.SystemStatus = status
+	return nil
+}
+
 // Application defines the cluster application
 type Application struct {
 	// Name is the name of the cluster application

infra/gravity/testdata/status-active-5.0.36.json

@@ -0,0 +1,41 @@
+{
+  "cluster": {
+    "application": {
+      "repository": "gravitational.io",
+      "name": "telekube",
+      "version": "5.0.36"
+    },
+    "state": "active",
+    "domain": "robotest-unit-test",
+    "token": {
+      "token": "ROBOTEST",
+      "expires": "0001-01-01T00:00:00Z",
+      "type": "expand",
+      "account_id": "00000000-0000-0000-0000-000000000001",
+      "site_domain": "robotest-unit-test",
+      "operation_id": "",
+      "user_email": "agent@robotest-unit-test"
+    },
+    "operation": {
+      "type": "operation_install",
+      "id": "6eb72cb8-56e6-4df4-a3f3-e971a4c64b80",
+      "state": "completed",
+      "created": "2020-08-18T00:20:18.854957796Z",
+      "progress": {
+        "message": "Operation has completed",
+        "completion": 100,
+        "created": "2020-08-18T00:20:27.428919029Z"
+      }
+    },
+    "system_status": "running",
+    "nodes": [
+      {
+        "hostname": "robotest-unit-test-node-0",
+        "advertise_ip": "10.138.0.27",
+        "role": "master",
+        "status": "healthy"
+      }
+    ]
+  }
+}
+

infra/gravity/testdata/status-degraded-5.0.36.json

@@ -0,0 +1,16 @@
+{
+  "cluster": {
+    "system_status": "degraded",
+    "nodes": [
+      {
+        "hostname": "",
+        "advertise_ip": "10.138.0.112",
+        "role": "master",
+        "status": "degraded",
+        "failed_probes": [
+          "etcd-healthz failed"
+        ]
+      }
+    ]
+  }
+}