Remove broken host map subset assertions from nodetool

Fixes: #331

pkg/cassandra/nodetool/nodetool.go

@@ -23,7 +23,7 @@ const (
 	NodeStateMoving  NodeState = "Moving"
 )
 
-// NodeState represents the state of reachability of of a C* node from the
+// NodeState represents the reachability of a C* node from the
 // perspective of the node answering the query.
 type NodeStatus string
 
@@ -83,16 +83,36 @@ func setsIntersect(setsToCheck ...sets.String) bool {
 	return all.Len() != totalLength
 }
 
+// Status generates a summary of the status of every C* node in the cluster.
+// From the perspective of the local node.
+//
+// It is intended to produce identical information to the `nodetool status` utility.
+// But it only a reports a subset of the `nodetool status` information, for now.
+// Enough to allow Navigator to determine whether a node is Up and Normal (healthy).
+// This function returns structured information about the Cassandra cluster health,
+// which avoids having to parse the unstructured, human readable output of `nodetool status`.
+// Here is an example of the parsing that we are seeking to avoid:
+// https://github.com/kubernetes/examples/blob/b86c9d50be45eaf5ce74dee7159ce38b0e149d38/cassandra/image/files/ready-probe.sh
+// And here is the source code for the `nodetool status`:
+// https://github.com/apache/cassandra/blob/cassandra-3.11.2/src/java/org/apache/cassandra/tools/nodetool/Status.java
+//
+// # Algorithm
+//
+// For every C* node that has reported its `host_id` (i.e. present in HostIdMap):
+// * Determine the status of the node (one of live, unreachable, unknown)
+// * Determine the state of the node (one of leaving, joining, moving, normal)
+//
+// We perform additional assertions to check that a node is only present in one status and one state.
+// If these assertions fail, we return an error so as to avoid reporting false positive status.
+// Note: `nodetool status` does not perform these assertions.
 func (t *tool) Status() (NodeMap, error) {
 	ssInfo, err := t.client.StorageService()
 	if err != nil {
 		return nil, err
 	}
 
 	nodes := NodeMap{}
-	mappedNodes := sets.NewString()
 	for host, id := range ssInfo.HostIdMap {
-		mappedNodes.Insert(host)
 		nodes[host] = &Node{
 			Host:   host,
 			ID:     id,
@@ -103,49 +123,30 @@ func (t *tool) Status() (NodeMap, error) {
 
 	liveNodes := sets.NewString(ssInfo.LiveNodes...)
 	unreachableNodes := sets.NewString(ssInfo.UnreachableNodes...)
+	// Assert that a nodes are only in one state.
 	if setsIntersect(liveNodes, unreachableNodes) {
 		return nil, fmt.Errorf(
-			"the sets of live and unreachable nodes should not intersect. "+
+			"unexpected state: some nodes were reported in more than one status. "+
 				"Live: %v, "+
 				"Unreachable: %v",
 			liveNodes, unreachableNodes,
 		)
 	}
-	if !mappedNodes.IsSuperset(liveNodes.Union(unreachableNodes)) {
-		return nil, fmt.Errorf(
-			"mapped nodes must be a superset of Live and Unreachable nodes. "+
-				"Live: %v, "+
-				"Unreachable: %v, "+
-				"Mapped: %v",
-			liveNodes, unreachableNodes, mappedNodes,
-		)
-	}
 
 	leavingNodes := sets.NewString(ssInfo.LeavingNodes...)
 	joiningNodes := sets.NewString(ssInfo.JoiningNodes...)
 	movingNodes := sets.NewString(ssInfo.MovingNodes...)
 
 	if setsIntersect(leavingNodes, joiningNodes, movingNodes) {
 		return nil, fmt.Errorf(
-			"the sets of leaving, joining and moving nodes should not intersect. "+
+			"unexpected state: some nodes were reported in more than one state. "+
 				"Leaving: %v, "+
 				"Joining: %v, "+
 				"Moving: %v",
 			leavingNodes, joiningNodes, movingNodes,
 		)
 	}
 
-	if !mappedNodes.IsSuperset(leavingNodes.Union(joiningNodes).Union(movingNodes)) {
-		return nil, fmt.Errorf(
-			"mapped nodes must be a superset of leaving, joining and moving nodes. "+
-				"Leaving: %v, "+
-				"Joining: %v, "+
-				"Moving: %v, "+
-				"Mapped: %v",
-			leavingNodes, joiningNodes, movingNodes, mappedNodes,
-		)
-	}
-
 	for host, node := range nodes {
 		switch {
 		case liveNodes.Has(host):

pkg/cassandra/nodetool/nodetool_test.go

@@ -231,18 +231,6 @@ func TestNodeToolStatus(t *testing.T) {
 				},
 			},
 		},
-		{
-			title: "Live node not in HostIdMap",
-			handler: func(t *testing.T, w http.ResponseWriter, r *http.Request) {
-				_, err := w.Write([]byte(
-					`{"value": {"HostIdMap": {}, "LiveNodes": ["192.0.2.254"]}}`,
-				))
-				if err != nil {
-					t.Fatal(err)
-				}
-			},
-			expectedError: true,
-		},
 		{
 			title: "Live intersects with unreachable",
 			handler: func(t *testing.T, w http.ResponseWriter, r *http.Request) {
@@ -258,18 +246,6 @@ func TestNodeToolStatus(t *testing.T) {
 			},
 			expectedError: true,
 		},
-		{
-			title: "Leaving node not in HostIdMap",
-			handler: func(t *testing.T, w http.ResponseWriter, r *http.Request) {
-				_, err := w.Write([]byte(
-					`{"value": {"HostIdMap": {}, "LeavingNodes": ["192.0.2.254"]}}`,
-				))
-				if err != nil {
-					t.Fatal(err)
-				}
-			},
-			expectedError: true,
-		},
 		{
 			title: "Leaving intersects with joining",
 			handler: func(t *testing.T, w http.ResponseWriter, r *http.Request) {