RSDK-8765 - Add CloudMetadata to MachineStatus (viamrobotics#4637)

Co-authored-by: Kschappacher <[email protected]>
Co-authored-by: Benjamin Rewis <[email protected]>

protoutils/messages.go

@@ -8,6 +8,7 @@ import (
 	"github.com/golang/geo/r3"
 	protov1 "github.com/golang/protobuf/proto" //nolint:staticcheck
 	commonpb "go.viam.com/api/common/v1"
+	robotpb "go.viam.com/api/robot/v1"
 	"go.viam.com/utils/protoutils"
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/proto"
@@ -16,10 +17,36 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 
+	"go.viam.com/rdk/cloud"
 	"go.viam.com/rdk/resource"
 	"go.viam.com/rdk/spatialmath"
 )
 
+// MetadataFromProto converts a proto GetCloudMetadataResponse to Metadata.
+func MetadataFromProto(pbMetadata *robotpb.GetCloudMetadataResponse) cloud.Metadata {
+	if pbMetadata == nil {
+		return cloud.Metadata{}
+	}
+	return cloud.Metadata{
+		MachinePartID: pbMetadata.MachinePartId,
+		MachineID:     pbMetadata.MachineId,
+		PrimaryOrgID:  pbMetadata.PrimaryOrgId,
+		LocationID:    pbMetadata.LocationId,
+	}
+}
+
+// MetadataToProto converts a Metadata its proto counterpart.
+func MetadataToProto(metadata cloud.Metadata) *robotpb.GetCloudMetadataResponse {
+	return &robotpb.GetCloudMetadataResponse{
+		// TODO: RSDK-7181 remove RobotPartId
+		RobotPartId:   metadata.MachinePartID, // Deprecated: Duplicates MachinePartId,
+		MachinePartId: metadata.MachinePartID,
+		MachineId:     metadata.MachineID,
+		PrimaryOrgId:  metadata.PrimaryOrgID,
+		LocationId:    metadata.LocationID,
+	}
+}
+
 // ResourceNameToProto converts a resource.Name to its proto counterpart.
 func ResourceNameToProto(name resource.Name) *commonpb.ResourceName {
 	return &commonpb.ResourceName{

protoutils/messages_test.go

@@ -5,10 +5,74 @@ import (
 	"strconv"
 	"testing"
 
+	robotpb "go.viam.com/api/robot/v1"
 	"go.viam.com/test"
 	"google.golang.org/protobuf/types/known/wrapperspb"
+
+	"go.viam.com/rdk/cloud"
 )
 
+func TestMetadataFromProto(t *testing.T) {
+	expected := cloud.Metadata{}
+	observed := MetadataFromProto(nil)
+	test.That(t, observed, test.ShouldResemble, expected)
+
+	partID := "123"
+	machineID := "abc"
+	orgID := "456"
+	locID := "def"
+
+	samePartID := &robotpb.GetCloudMetadataResponse{
+		RobotPartId:   partID,
+		MachinePartId: partID,
+		MachineId:     machineID,
+		PrimaryOrgId:  orgID,
+		LocationId:    locID,
+	}
+	expected = cloud.Metadata{
+		MachinePartID: partID,
+		MachineID:     machineID,
+		PrimaryOrgID:  orgID,
+		LocationID:    locID,
+	}
+	observed = MetadataFromProto(samePartID)
+	test.That(t, observed, test.ShouldResemble, expected)
+
+	robotPartID := "789"
+	diffPartID := &robotpb.GetCloudMetadataResponse{
+		RobotPartId:   robotPartID,
+		MachinePartId: partID,
+		MachineId:     machineID,
+		PrimaryOrgId:  orgID,
+		LocationId:    locID,
+	}
+	observed = MetadataFromProto(diffPartID)
+	test.That(t, observed, test.ShouldResemble, expected)
+}
+
+func TestMetadataToProto(t *testing.T) {
+	partID := "123"
+	machineID := "abc"
+	orgID := "456"
+	locID := "def"
+
+	metadata := cloud.Metadata{
+		MachinePartID: partID,
+		MachineID:     machineID,
+		PrimaryOrgID:  orgID,
+		LocationID:    locID,
+	}
+	expected := &robotpb.GetCloudMetadataResponse{
+		RobotPartId:   partID,
+		MachinePartId: partID,
+		MachineId:     machineID,
+		PrimaryOrgId:  orgID,
+		LocationId:    locID,
+	}
+	observed := MetadataToProto(metadata)
+	test.That(t, observed, test.ShouldResembleProto, expected)
+}
+
 func TestStringToAnyPB(t *testing.T) {
 	anyVal, err := ConvertStringToAnyPB("12")
 	test.That(t, err, test.ShouldBeNil)

resource/graph_node.go

@@ -555,37 +555,30 @@ func (w *GraphNode) transitionTo(state NodeState) {
 	w.transitionedAt = time.Now()
 }
 
-// ResourceStatus returns the current [Status].
-func (w *GraphNode) ResourceStatus() Status {
+// Status returns the current [NodeStatus].
+func (w *GraphNode) Status() NodeStatus {
 	w.mu.RLock()
 	defer w.mu.RUnlock()
 
-	return w.resourceStatus()
+	return w.status()
 }
 
-func (w *GraphNode) resourceStatus() Status {
-	var resName Name
-	if w.current == nil {
-		resName = w.config.ResourceName()
-	} else {
-		resName = w.current.Name()
-	}
-
+func (w *GraphNode) status() NodeStatus {
 	err := w.lastErr
 	logger := w.Logger()
 
 	// check invariants between state and error
 	switch {
 	case w.state == NodeStateUnhealthy && w.lastErr == nil:
-		logger.Warnw("an unhealthy node doesn't have an error", "resource", resName)
+		logger.Warnw("an unhealthy node doesn't have an error")
 	case w.state == NodeStateReady && w.lastErr != nil:
-		logger.Warnw("a ready node still has an error", "resource", resName, "error", err)
+		logger.Warnw("a ready node still has an error", "error", err)
 		// do not return leftover error in status if the node is ready
 		err = nil
 	}
 
-	return Status{
-		Name:        resName,
+	// TODO (RSDK-9550): Node should have the correct notion of its name
+	return NodeStatus{
 		State:       w.state,
 		LastUpdated: w.transitionedAt,
 		Revision:    w.revision,
@@ -623,8 +616,8 @@ func (w *GraphNode) Stats() any {
 	return ret
 }
 
-// Status encapsulates a resource name along with state transition metadata.
-type Status struct {
+// NodeStatus encapsulates a resource name along with state transition metadata.
+type NodeStatus struct {
 	Name        Name
 	State       NodeState
 	LastUpdated time.Time

resource/graph_node_test.go

@@ -40,7 +40,7 @@ func TestUninitializedLifecycle(t *testing.T) {
 	expectedState := resource.NodeStateUnconfigured
 	test.That(t, node.State(), test.ShouldEqual, expectedState)
 
-	status := node.ResourceStatus()
+	status := node.Status()
 	test.That(t, status.State, test.ShouldResemble, expectedState)
 
 	lifecycleTest(t, node, []string(nil))
@@ -71,8 +71,8 @@ func TestUnconfiguredLifecycle(t *testing.T) {
 	expectedState := resource.NodeStateConfiguring
 	test.That(t, node.State(), test.ShouldEqual, expectedState)
 
-	status := node.ResourceStatus()
-	test.That(t, status.Name.Name, test.ShouldEqual, "foo")
+	status := node.Status()
+	test.That(t, status.Name.Name, test.ShouldEqual, "")
 	test.That(t, status.State, test.ShouldResemble, expectedState)
 
 	lifecycleTest(t, node, initialDeps)
@@ -102,8 +102,8 @@ func TestConfiguredLifecycle(t *testing.T) {
 	expectedState := resource.NodeStateReady
 	test.That(t, node.State(), test.ShouldEqual, expectedState)
 
-	status := node.ResourceStatus()
-	test.That(t, status.Name, test.ShouldResemble, resName)
+	status := node.Status()
+	test.That(t, status.Name, test.ShouldResemble, resource.Name{})
 	test.That(t, status.State, test.ShouldResemble, resource.NodeStateReady)
 
 	lifecycleTest(t, node, []string(nil))
@@ -149,7 +149,7 @@ func lifecycleTest(t *testing.T, node *resource.GraphNode, initialDeps []string)
 	// Attempt to change status to [NodeStateUnhealthy]
 	ourErr = errors.New("whoops")
 	node.LogAndSetLastError(ourErr)
-	status := node.ResourceStatus()
+	status := node.Status()
 	// Ensure that error is set and node stays in [NodeStateUnhealthy]
 	// since state transition [NodeStateUnhealthy] -> [NodeStateRemoving] is blocked
 	test.That(t, status.Error.Error(), test.ShouldContainSubstring, "whoops")

resource/resource.go

@@ -25,6 +25,7 @@ import (
 	"github.com/jhump/protoreflect/desc"
 	"github.com/pkg/errors"
 
+	"go.viam.com/rdk/cloud"
 	"go.viam.com/rdk/spatialmath"
 	"go.viam.com/rdk/utils"
 )
@@ -304,3 +305,9 @@ func NewCloseOnlyResource(name Name, closeFunc func(ctx context.Context) error)
 func (r *closeOnlyResource) Close(ctx context.Context) error {
 	return r.closeFunc(ctx)
 }
+
+// Status is a combination of a resources node status and the cloudMetadata associated with that resource.
+type Status struct {
+	NodeStatus
+	CloudMetadata cloud.Metadata
+}

resource/resource_graph.go

@@ -695,13 +695,17 @@ func (g *Graph) MarkReachability(node Name, reachable bool) error {
 }
 
 // Status returns a slice of all graph node statuses.
-func (g *Graph) Status() []Status {
+func (g *Graph) Status() []NodeStatus {
 	g.mu.Lock()
 	defer g.mu.Unlock()
 
-	var result []Status
-	for _, node := range g.nodes {
-		result = append(result, node.ResourceStatus())
+	var result []NodeStatus
+	for name, node := range g.nodes {
+		// TODO (RSDK-9550): Node should have the correct notion of its name
+		// but they don't, so fill it in here
+		status := node.Status()
+		status.Name = name
+		result = append(result, status)
 	}
 
 	return result

robot/client/client.go

@@ -1014,17 +1014,12 @@ func (rc *RobotClient) Log(ctx context.Context, log zapcore.Entry, fields []zap.
 //
 //	metadata, err := machine.CloudMetadata(ctx.Background())
 func (rc *RobotClient) CloudMetadata(ctx context.Context) (cloud.Metadata, error) {
-	cloudMD := cloud.Metadata{}
 	req := &pb.GetCloudMetadataRequest{}
 	resp, err := rc.client.GetCloudMetadata(ctx, req)
 	if err != nil {
-		return cloudMD, err
+		return cloud.Metadata{}, err
 	}
-	cloudMD.PrimaryOrgID = resp.PrimaryOrgId
-	cloudMD.LocationID = resp.LocationId
-	cloudMD.MachineID = resp.MachineId
-	cloudMD.MachinePartID = resp.MachinePartId
-	return cloudMD, nil
+	return rprotoutils.MetadataFromProto(resp), nil
 }
 
 // RestartModule restarts a running module by name or ID.
@@ -1090,9 +1085,12 @@ func (rc *RobotClient) MachineStatus(ctx context.Context) (robot.MachineStatus,
 	mStatus.Resources = make([]resource.Status, 0, len(resp.Resources))
 	for _, pbResStatus := range resp.Resources {
 		resStatus := resource.Status{
-			Name:        rprotoutils.ResourceNameFromProto(pbResStatus.Name),
-			LastUpdated: pbResStatus.LastUpdated.AsTime(),
-			Revision:    pbResStatus.Revision,
+			NodeStatus: resource.NodeStatus{
+				Name:        rprotoutils.ResourceNameFromProto(pbResStatus.Name),
+				LastUpdated: pbResStatus.LastUpdated.AsTime(),
+				Revision:    pbResStatus.Revision,
+			},
+			CloudMetadata: rprotoutils.MetadataFromProto(pbResStatus.CloudMetadata),
 		}
 
 		switch pbResStatus.State {

robot/client/client_test.go

@@ -1984,8 +1984,10 @@ func TestMachineStatus(t *testing.T) {
 				Config: config.Revision{Revision: "rev1"},
 				Resources: []resource.Status{
 					{
-						Name:     arm.Named("badArm"),
-						Revision: "rev0",
+						NodeStatus: resource.NodeStatus{
+							Name:     arm.Named("badArm"),
+							Revision: "rev0",
+						},
 					},
 				},
 			},
@@ -1997,9 +1999,11 @@ func TestMachineStatus(t *testing.T) {
 				Config: config.Revision{Revision: "rev1"},
 				Resources: []resource.Status{
 					{
-						Name:     arm.Named("goodArm"),
-						State:    resource.NodeStateConfiguring,
-						Revision: "rev1",
+						NodeStatus: resource.NodeStatus{
+							Name:     arm.Named("goodArm"),
+							State:    resource.NodeStateConfiguring,
+							Revision: "rev1",
+						},
 					},
 				},
 			},
@@ -2011,17 +2015,23 @@ func TestMachineStatus(t *testing.T) {
 				Config: config.Revision{Revision: "rev1"},
 				Resources: []resource.Status{
 					{
-						Name:     arm.Named("goodArm"),
-						State:    resource.NodeStateConfiguring,
-						Revision: "rev1",
+						NodeStatus: resource.NodeStatus{
+							Name:     arm.Named("goodArm"),
+							State:    resource.NodeStateConfiguring,
+							Revision: "rev1",
+						},
 					},
 					{
-						Name:     arm.Named("badArm"),
-						Revision: "rev0",
+						NodeStatus: resource.NodeStatus{
+							Name:     arm.Named("badArm"),
+							Revision: "rev0",
+						},
 					},
 					{
-						Name:     arm.Named("anotherBadArm"),
-						Revision: "rev-1",
+						NodeStatus: resource.NodeStatus{
+							Name:     arm.Named("anotherBadArm"),
+							Revision: "rev-1",
+						},
 					},
 				},
 			},
@@ -2033,10 +2043,34 @@ func TestMachineStatus(t *testing.T) {
 				Config: config.Revision{Revision: "rev1"},
 				Resources: []resource.Status{
 					{
-						Name:     arm.Named("brokenArm"),
-						State:    resource.NodeStateUnhealthy,
-						Error:    errors.New("bad configuration"),
-						Revision: "rev1",
+						NodeStatus: resource.NodeStatus{
+							Name:     arm.Named("brokenArm"),
+							State:    resource.NodeStateUnhealthy,
+							Error:    errors.New("bad configuration"),
+							Revision: "rev1",
+						},
+					},
+				},
+			},
+			0,
+		},
+		{
+			"cloud metadata",
+			robot.MachineStatus{
+				Config: config.Revision{Revision: "rev1"},
+				Resources: []resource.Status{
+					{
+						NodeStatus: resource.NodeStatus{
+							Name:     arm.Named("arm1"),
+							State:    resource.NodeStateReady,
+							Revision: "rev1",
+						},
+						CloudMetadata: cloud.Metadata{
+							MachinePartID: "123",
+							MachineID:     "456",
+							PrimaryOrgID:  "789",
+							LocationID:    "abc",
+						},
 					},
 				},
 			},