sql, cli: Add attrs and locality to the node command

This patch fixes a number of aspects about how we display node and store
attributes as well as adds node attributes and localities to the

* The String method on attributes proto was incorrectly using a `,` to join the
array. Attributes always use a `:` so that it can be parsed via the command
line and this fixes that.
* Also, the internal tables `crdb_interal.gossip_nodes`,
`crdb_internal.kv_node_status` and `crdb_internal.kv_store_status` had the
attributes listed as JSON. This is not useful as it won't match what's in the
command line. This converts it to a string.
* This also adds the node attributes and localities to the
`cockroach node status` command. As it was missing and can be helpful.

Release note (cli change): Node attributes and localities have been added to
the `cockroach node status` command.

pkg/cli/node.go

@@ -83,6 +83,8 @@ var baseNodeColumnHeaders = []string{
 	"updated_at",
 	"is_available",
 	"is_live",
+	"locality",
+	"attrs",
 }
 
 var statusNodesColumnHeadersForRanges = []string{
@@ -158,8 +160,11 @@ func runStatusNodeInner(showDecommissioned bool, args []string) ([]string, [][]s
                  THEN true
                  ELSE false
                  END AS is_available,
-            ifnull(is_live, false)
-     FROM crdb_internal.gossip_liveness LEFT JOIN crdb_internal.gossip_nodes USING (node_id)`,
+            ifnull(is_live, false),
+            locality,
+            attrs
+     FROM crdb_internal.gossip_liveness
+     LEFT JOIN crdb_internal.gossip_nodes USING (node_id)`,
 	)
 
 	const rangesQuery = `

pkg/roachpb/metadata.go

@@ -100,7 +100,7 @@ func (a Attributes) Equals(b Attributes) bool {
 
 // String implements the fmt.Stringer interface.
 func (a Attributes) String() string {
-	return strings.Join(a.Attrs, ",")
+	return strings.Join(a.Attrs, ":")
 }
 
 // RSpan returns the RangeDescriptor's resolved span.

pkg/sql/crdb_internal.go

@@ -1953,7 +1953,7 @@ CREATE TABLE crdb_internal.gossip_nodes (
   network         		STRING NOT NULL,
   address         		STRING NOT NULL,
   advertise_address   STRING NOT NULL,
-  attrs           		JSON NOT NULL,
+  attrs           		STRING NOT NULL,
   locality        		STRING NOT NULL,
   server_version  		STRING NOT NULL,
   build_tag       		STRING NOT NULL,
@@ -2033,11 +2033,6 @@ CREATE TABLE crdb_internal.gossip_nodes (
 		}
 
 		for _, d := range descriptors {
-			attrs := json.NewArrayBuilder(len(d.Attrs.Attrs))
-			for _, a := range d.Attrs.Attrs {
-				attrs.Add(json.FromString(a))
-			}
-
 			addr, err := g.GetNodeIDAddress(d.NodeID)
 			if err != nil {
 				return err
@@ -2048,7 +2043,7 @@ CREATE TABLE crdb_internal.gossip_nodes (
 				tree.NewDString(d.Address.NetworkField),
 				tree.NewDString(d.Address.AddressField),
 				tree.NewDString(addr.String()),
-				tree.NewDJSON(attrs.Build()),
+				tree.NewDString(d.Attrs.String()),
 				tree.NewDString(d.Locality.String()),
 				tree.NewDString(d.ServerVersion.String()),
 				tree.NewDString(d.BuildTag),
@@ -2316,7 +2311,7 @@ CREATE TABLE crdb_internal.kv_node_status (
   node_id        INT NOT NULL,
   network        STRING NOT NULL,
   address        STRING NOT NULL,
-  attrs          JSON NOT NULL,
+  attrs          STRING NOT NULL,
   locality       STRING NOT NULL,
   server_version STRING NOT NULL,
   go_version     STRING NOT NULL,
@@ -2347,11 +2342,6 @@ CREATE TABLE crdb_internal.kv_node_status (
 		}
 
 		for _, n := range response.Nodes {
-			attrs := json.NewArrayBuilder(len(n.Desc.Attrs.Attrs))
-			for _, a := range n.Desc.Attrs.Attrs {
-				attrs.Add(json.FromString(a))
-			}
-
 			var dependencies string
 			if n.BuildInfo.Dependencies == nil {
 				dependencies = ""
@@ -2391,7 +2381,7 @@ CREATE TABLE crdb_internal.kv_node_status (
 				tree.NewDInt(tree.DInt(n.Desc.NodeID)),
 				tree.NewDString(n.Desc.Address.NetworkField),
 				tree.NewDString(n.Desc.Address.AddressField),
-				tree.NewDJSON(attrs.Build()),
+				tree.NewDString(n.Desc.Attrs.String()),
 				tree.NewDString(n.Desc.Locality.String()),
 				tree.NewDString(n.Desc.ServerVersion.String()),
 				tree.NewDString(n.BuildInfo.GoVersion),
@@ -2426,7 +2416,7 @@ var crdbInternalKVStoreStatusTable = virtualSchemaTable{
 CREATE TABLE crdb_internal.kv_store_status (
   node_id            INT NOT NULL,
   store_id           INT NOT NULL,
-  attrs              JSON NOT NULL,
+  attrs              STRING NOT NULL,
   capacity           INT NOT NULL,
   available          INT NOT NULL,
   used               INT NOT NULL,
@@ -2451,11 +2441,6 @@ CREATE TABLE crdb_internal.kv_store_status (
 
 		for _, n := range response.Nodes {
 			for _, s := range n.StoreStatuses {
-				attrs := json.NewArrayBuilder(len(s.Desc.Attrs.Attrs))
-				for _, a := range s.Desc.Attrs.Attrs {
-					attrs.Add(json.FromString(a))
-				}
-
 				metrics := json.NewObjectBuilder(len(s.Metrics))
 				for k, v := range s.Metrics {
 					metric, err := json.FromFloat64(v)
@@ -2512,7 +2497,7 @@ CREATE TABLE crdb_internal.kv_store_status (
 				if err := addRow(
 					tree.NewDInt(tree.DInt(s.Desc.Node.NodeID)),
 					tree.NewDInt(tree.DInt(s.Desc.StoreID)),
-					tree.NewDJSON(attrs.Build()),
+					tree.NewDString(s.Desc.Attrs.String()),
 					tree.NewDInt(tree.DInt(s.Desc.Capacity.Capacity)),
 					tree.NewDInt(tree.DInt(s.Desc.Capacity.Available)),
 					tree.NewDInt(tree.DInt(s.Desc.Capacity.Used)),

pkg/sql/logictest/testdata/logic_test/crdb_internal

@@ -302,7 +302,7 @@ query ITTTTT colnames
 SELECT node_id, network, regexp_replace(address, '\d+$', '<port>') as address, attrs, locality, regexp_replace(server_version, '^\d+\.\d+(-\d+)?$', '<server_version>') as server_version FROM crdb_internal.gossip_nodes WHERE node_id = 1
 ----
 node_id  network  address           attrs  locality            server_version
-1        tcp      127.0.0.1:<port>  []     region=test,dc=dc1  <server_version>
+1        tcp      127.0.0.1:<port>  ·      region=test,dc=dc1  <server_version>
 
 query IITBB colnames
 SELECT node_id, epoch, regexp_replace(expiration, '^\d+\.\d+,\d+$', '<timestamp>') as expiration, draining, decommissioning FROM crdb_internal.gossip_liveness WHERE node_id = 1
@@ -315,14 +315,14 @@ SELECT node_id, network, regexp_replace(address, '\d+$', '<port>') as address, a
 FROM crdb_internal.kv_node_status WHERE node_id = 1
 ----
 node_id  network  address           attrs  locality            server_version    go_version
-1        tcp      127.0.0.1:<port>  []     region=test,dc=dc1  <server_version>  <go_version>
+1        tcp      127.0.0.1:<port>  ·      region=test,dc=dc1  <server_version>  <go_version>
 
 query IITI colnames
 SELECT node_id, store_id, attrs, used
 FROM crdb_internal.kv_store_status WHERE node_id = 1
 ----
 node_id  store_id  attrs  used
-1        1         []     0
+1        1         ·      0
 
 statement ok
 CREATE TABLE foo (a INT PRIMARY KEY); INSERT INTO foo VALUES(1)