Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RSDK-9620] implement discover service #4665

Merged
merged 14 commits into from
Jan 8, 2025
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -431,3 +431,5 @@ require (
github.com/ziutek/mymysql v1.5.4 // indirect
golang.org/x/exp v0.0.0-20240904232852-e7e105dedf7e
)

replace go.viam.com/api => github.com/johnn193/api v0.0.0-20241231164642-99f059defc82
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will revert once api is merged

4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,8 @@ github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHW
github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGwWFoC7ycTf1rcQZHOlsJ6N8=
github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U=
github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks=
github.com/johnn193/api v0.0.0-20241231164642-99f059defc82 h1:V2fQZbYvvjDK/oz7LxluzuKXdFGljuvbpJhs7cqT2jo=
github.com/johnn193/api v0.0.0-20241231164642-99f059defc82/go.mod h1:g5eipXHNm0rQmW7DWya6avKcmzoypLmxnMlAaIsE5Ls=
github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=
github.com/jonboulle/clockwork v0.3.0 h1:9BSCMi8C+0qdApAp4auwX0RkLGUjs956h0EkuQymUhg=
github.com/jonboulle/clockwork v0.3.0/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8=
Expand Down Expand Up @@ -1513,8 +1515,6 @@ go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI=
go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY=
go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8=
go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
go.viam.com/api v0.1.372 h1:Al9P7yojBDdNVAF7nrr5BAbzCvb+vrSp8N7BitbV0mQ=
go.viam.com/api v0.1.372/go.mod h1:g5eipXHNm0rQmW7DWya6avKcmzoypLmxnMlAaIsE5Ls=
go.viam.com/test v1.2.4 h1:JYgZhsuGAQ8sL9jWkziAXN9VJJiKbjoi9BsO33TW3ug=
go.viam.com/test v1.2.4/go.mod h1:zI2xzosHdqXAJ/kFqcN+OIF78kQuTV2nIhGZ8EzvaJI=
go.viam.com/utils v0.1.118 h1:Kp6ebrCBiYReeSC1XnWPTjtBJoTUsQ6YWAomQkQF/mE=
Expand Down
75 changes: 75 additions & 0 deletions services/discovery/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package discovery

import (
"context"

"go.opencensus.io/trace"
pb "go.viam.com/api/service/discovery/v1"
"go.viam.com/utils/protoutils"
"go.viam.com/utils/rpc"

"go.viam.com/rdk/config"
"go.viam.com/rdk/logging"
rprotoutils "go.viam.com/rdk/protoutils"
"go.viam.com/rdk/resource"
)

// client implements DiscoveryServiceClient.
type client struct {
resource.Named
resource.TriviallyReconfigurable
resource.TriviallyCloseable
name string
client pb.DiscoveryServiceClient
logger logging.Logger
}

// NewClientFromConn constructs a new Client from the connection passed in.
func NewClientFromConn(
ctx context.Context,
conn rpc.ClientConn,
remoteName string,
name resource.Name,
logger logging.Logger,
) (Service, error) {
grpcClient := pb.NewDiscoveryServiceClient(conn)
c := &client{
Named: name.PrependRemote(remoteName).AsNamed(),
name: name.ShortName(),
client: grpcClient,
logger: logger,
}
return c, nil
}

func (c *client) DiscoverResources(ctx context.Context, extra map[string]any) ([]*resource.Config, error) {
ctx, span := trace.StartSpan(ctx, "discovery::client::DoCommand")
defer span.End()
ext, err := protoutils.StructToStructPb(extra)
if err != nil {
return nil, err
}

req := &pb.DiscoverResourcesRequest{Name: c.name, Extra: ext}
resp, err := c.client.DiscoverResources(ctx, req)
if err != nil {
return nil, err
}
discoveredConfigs := []*resource.Config{}
protoConfigs := resp.GetDiscovery()
for _, proto := range protoConfigs {
config, err := config.ComponentConfigFromProto(proto)
if err != nil {
return nil, err
}
discoveredConfigs = append(discoveredConfigs, config)
}
return discoveredConfigs, nil
}

func (c *client) DoCommand(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) {
ctx, span := trace.StartSpan(ctx, "discovery::client::DoCommand")
defer span.End()

return rprotoutils.DoFromResourceClient(ctx, c.client, c.name, cmd)
}
50 changes: 50 additions & 0 deletions services/discovery/discovery.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Package discovery implements the discovery service, which lets users surface resource configs for their machines to use.
package discovery

import (
"context"

pb "go.viam.com/api/service/discovery/v1"

"go.viam.com/rdk/resource"
"go.viam.com/rdk/robot"
)

func init() {
resource.RegisterAPI(API, resource.APIRegistration[Service]{
RPCServiceServerConstructor: NewRPCServiceServer,
RPCServiceHandler: pb.RegisterDiscoveryServiceHandlerFromEndpoint,
RPCServiceDesc: &pb.DiscoveryService_ServiceDesc,
RPCClient: NewClientFromConn,
})
}

// SubtypeName is the name of the type of service.
const (
SubtypeName = "discovery"
)

// API is a variable that identifies the slam resource API.
var API = resource.APINamespaceRDK.WithServiceType(SubtypeName)

// Named is a helper for getting the named service's typed resource name.
func Named(name string) resource.Name {
return resource.NewName(API, name)
}

// FromRobot is a helper for getting the named discovery service from the given Robot.
func FromRobot(r robot.Robot, name string) (Service, error) {
return robot.ResourceFromRobot[Service](r, Named(name))
}

// FromDependencies is a helper for getting the named discovery service from a collection of
// dependencies.
func FromDependencies(deps resource.Dependencies, name string) (Service, error) {
return resource.FromDependencies[Service](deps, Named(name))
}

// Service describes the functions that are available to the service.
type Service interface {
resource.Resource
DiscoverResources(ctx context.Context, extra map[string]any) ([]*resource.Config, error)
}
68 changes: 68 additions & 0 deletions services/discovery/server.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package discovery

import (
"context"

"go.opencensus.io/trace"
apppb "go.viam.com/api/app/v1"
commonpb "go.viam.com/api/common/v1"
pb "go.viam.com/api/service/discovery/v1"

"go.viam.com/rdk/config"
"go.viam.com/rdk/protoutils"
"go.viam.com/rdk/resource"
)

// serviceServer implements the DiscoveryService from the discovery proto.
type serviceServer struct {
pb.UnimplementedDiscoveryServiceServer
coll resource.APIResourceCollection[Service]
}

// NewRPCServiceServer constructs a the discovery gRPC service server.
// It is intentionally untyped to prevent use outside of tests.
func NewRPCServiceServer(coll resource.APIResourceCollection[Service]) interface{} {
return &serviceServer{coll: coll}
}

// DiscoverResources returns a list of components discovered by a discovery service.
func (server *serviceServer) DiscoverResources(ctx context.Context, req *pb.DiscoverResourcesRequest) (
*pb.DiscoverResourcesResponse, error,
) {
ctx, span := trace.StartSpan(ctx, "discovery::server::DiscoverResources")
defer span.End()

svc, err := server.coll.Resource(req.Name)
if err != nil {
return nil, err
}

configs, err := svc.DiscoverResources(ctx, req.GetExtra().AsMap())
if err != nil {
return nil, err
}
protoConfigs := []*apppb.ComponentConfig{}
for _, cfg := range configs {
proto, err := config.ComponentConfigToProto(cfg)
if err != nil {
return nil, err
}
protoConfigs = append(protoConfigs, proto)
}

return &pb.DiscoverResourcesResponse{Discovery: protoConfigs}, nil
}

// DoCommand receives arbitrary commands.
func (server *serviceServer) DoCommand(ctx context.Context,
req *commonpb.DoCommandRequest,
) (*commonpb.DoCommandResponse, error) {
ctx, span := trace.StartSpan(ctx, "discovery::server::DoCommand")
defer span.End()

svc, err := server.coll.Resource(req.Name)
if err != nil {
return nil, err
}
return protoutils.DoFromResourceServer(ctx, svc, req)
}
73 changes: 73 additions & 0 deletions services/discovery/server_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package discovery_test

import (
"context"
"errors"
"testing"

commonpb "go.viam.com/api/common/v1"
pb "go.viam.com/api/service/discovery/v1"
"go.viam.com/test"
"go.viam.com/utils/protoutils"

"go.viam.com/rdk/resource"
"go.viam.com/rdk/services/discovery"
"go.viam.com/rdk/testutils"
"go.viam.com/rdk/testutils/inject"
)

var errDoFailed = errors.New("do failed")

func newServer() (pb.DiscoveryServiceServer, *inject.DiscoveryService, *inject.DiscoveryService, error) {
injectDiscovery1 := &inject.DiscoveryService{}
injectDiscovery2 := &inject.DiscoveryService{}
resourceMap := map[resource.Name]resource.Resource{
discovery.Named(testDiscoveryName): injectDiscovery,
discovery.Named(failDiscoveryName): injectDiscovery2,
}
injectSvc, err := resource.NewAPIResourceCollection(discovery.API, resourceMap)
if err != nil {
return nil, nil, nil, err
}
return discovery.NewRPCServiceServer(injectSvc).(pb.DiscoveryServiceServer), injectDiscovery, injectDiscovery2, nil
}

func TestDiscoveryDo(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is literally copy/paste from the generic service I should probably remove this test, but I'm also fine with leaving it in case we are worried this will somehow break

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is good to test all APIs of a service to ensure that there is adequate coverage. I can't imagine it breaking - but say someone with no context joins viam in the future and removes the DoCommands accidentally - good to have all these tests fail :) . It does not have to be separate from your other test, you can add an injected DoCommand to the injected service in your test.

Good test to have with DoComamnd are: 1. test that it returns errors if no DoCommand is implemented with resource.Named, and 2. responds with the expected responses when a DoCommand is implemented.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I just copied the testing structure that other services had, which is why it was separate. Moved all tests into the same test function based on feedback.

discoveryServer, workingDiscovery, failingDiscovery, err := newServer()
test.That(t, err, test.ShouldBeNil)

workingDiscovery.DoFunc = func(
ctx context.Context,
cmd map[string]interface{},
) (
map[string]interface{},
error,
) {
return cmd, nil
}
failingDiscovery.DoFunc = func(
ctx context.Context,
cmd map[string]interface{},
) (
map[string]interface{},
error,
) {
return nil, errDoFailed
}

commandStruct, err := protoutils.StructToStructPb(testutils.TestCommand)
test.That(t, err, test.ShouldBeNil)

req := commonpb.DoCommandRequest{Name: testDiscoveryName, Command: commandStruct}
resp, err := discoveryServer.DoCommand(context.Background(), &req)
test.That(t, err, test.ShouldBeNil)
test.That(t, resp, test.ShouldNotBeNil)
test.That(t, resp.Result.AsMap()["cmd"], test.ShouldEqual, testutils.TestCommand["cmd"])
test.That(t, resp.Result.AsMap()["data"], test.ShouldEqual, testutils.TestCommand["data"])

req = commonpb.DoCommandRequest{Name: failDiscoveryName, Command: commandStruct}
resp, err = discoveryServer.DoCommand(context.Background(), &req)
test.That(t, err, test.ShouldNotBeNil)
test.That(t, err.Error(), test.ShouldContainSubstring, errDoFailed.Error())
test.That(t, resp, test.ShouldBeNil)
}
42 changes: 42 additions & 0 deletions testutils/inject/discovery_service.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package inject

import (
"context"

"go.viam.com/rdk/resource"
"go.viam.com/rdk/services/discovery"
)

// GenericService is an injectable discovery service.
type DiscoveryService struct {
discovery.Service
name resource.Name
DiscoverResourcesFunc func(ctx context.Context, extra map[string]any) ([]*resource.Config, error)
DoFunc func(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error)
}

// NewGenericService returns a new injected generic service.
func NewDiscoveryService(name string) *DiscoveryService {
return &DiscoveryService{name: discovery.Named(name)}
}

// Name returns the name of the resource.
func (disSvc *DiscoveryService) Name() resource.Name {
return disSvc.name
}

// Position calls the injected PositionFunc or the real version.
func (disSvc *DiscoveryService) DiscoverResources(ctx context.Context, extra map[string]any) ([]*resource.Config, error) {
if disSvc.DiscoverResourcesFunc == nil {
return disSvc.Service.DiscoverResources(ctx, extra)
}
return disSvc.DiscoverResourcesFunc(ctx, extra)
}

// DoCommand calls the injected DoCommand or the real version.
func (disSvc *DiscoveryService) DoCommand(ctx context.Context, cmd map[string]interface{}) (map[string]interface{}, error) {
if disSvc.DoFunc == nil {
return disSvc.Service.DoCommand(ctx, cmd)
}
return disSvc.DoFunc(ctx, cmd)
}
Loading