diff --git a/cli/app.go b/cli/app.go index e042d42f314..603d30c06f1 100644 --- a/cli/app.go +++ b/cli/app.go @@ -1416,7 +1416,6 @@ var app = &cli.App{ Usage: "removes binary data with file IDs in a single org and location from dataset", UsageText: createUsageText("dataset data remove", []string{datasetFlagDatasetID, generalFlagOrgID, dataFlagLocationID, dataFlagFileIDs}, false), - // TODO(RSDK-9286) do we need to ask for og and location here? Flags: []cli.Flag{ &cli.StringFlag{ Name: datasetFlagDatasetID, @@ -1741,8 +1740,14 @@ var app = &cli.App{ Name: "status", Usage: "display machine status", UsageText: createUsageText("machines status", []string{generalFlagMachine}, true), - // TODO(RSDK-9286) - do we need to ask for all three of these? Flags: []cli.Flag{ + &AliasStringFlag{ + cli.StringFlag{ + Name: generalFlagMachine, + Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, + Required: true, + }, + }, &cli.StringFlag{ Name: generalFlagOrganization, Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, @@ -1753,13 +1758,6 @@ var app = &cli.App{ Aliases: []string{generalFlagLocationID, generalFlagAliasLocationName}, DefaultText: "first location alphabetically", }, - &AliasStringFlag{ - cli.StringFlag{ - Name: generalFlagMachine, - Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, - Required: true, - }, - }, }, Action: createCommandWithT[robotsStatusArgs](RobotsStatusAction), }, @@ -1768,8 +1766,14 @@ var app = &cli.App{ Aliases: []string{"log"}, Usage: "display machine logs", UsageText: createUsageText("machines logs", []string{generalFlagMachine}, true), - // TODO(RSDK-9286) do we need to ask for og and location and machine here? Flags: []cli.Flag{ + &AliasStringFlag{ + cli.StringFlag{ + Name: generalFlagMachine, + Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, + Required: true, + }, + }, &cli.StringFlag{ Name: generalFlagOrganization, Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, @@ -1780,13 +1784,6 @@ var app = &cli.App{ Aliases: []string{generalFlagLocationID, generalFlagAliasLocationName}, DefaultText: "first location alphabetically", }, - &AliasStringFlag{ - cli.StringFlag{ - Name: generalFlagMachine, - Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, - Required: true, - }, - }, &cli.StringFlag{ Name: logsFlagOutputFile, Usage: "path to output file", @@ -1827,9 +1824,15 @@ var app = &cli.App{ { Name: "status", Usage: "display part status", - UsageText: createUsageText("machines part status", []string{generalFlagMachine, generalFlagPart}, true), - // TODO(RSDK-9286) do we need to ask for og and location and machine and part here? + UsageText: createUsageText("machines part status", []string{generalFlagPart}, true), Flags: []cli.Flag{ + &AliasStringFlag{ + cli.StringFlag{ + Name: generalFlagPart, + Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, + Required: true, + }, + }, &cli.StringFlag{ Name: generalFlagOrganization, Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, @@ -1842,16 +1845,8 @@ var app = &cli.App{ }, &AliasStringFlag{ cli.StringFlag{ - Name: generalFlagMachine, - Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, - Required: true, - }, - }, - &AliasStringFlag{ - cli.StringFlag{ - Name: generalFlagPart, - Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, - Required: true, + Name: generalFlagMachine, + Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, }, }, }, @@ -1861,8 +1856,15 @@ var app = &cli.App{ Name: "logs", Aliases: []string{"log"}, Usage: "display part logs", - UsageText: createUsageText("machines part logs", []string{generalFlagMachine, generalFlagPart}, true), + UsageText: createUsageText("machines part logs", []string{generalFlagPart}, true), Flags: []cli.Flag{ + &AliasStringFlag{ + cli.StringFlag{ + Name: generalFlagPart, + Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, + Required: true, + }, + }, &cli.StringFlag{ Name: generalFlagOrganization, Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, @@ -1875,16 +1877,8 @@ var app = &cli.App{ }, &AliasStringFlag{ cli.StringFlag{ - Name: generalFlagMachine, - Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, - Required: true, - }, - }, - &AliasStringFlag{ - cli.StringFlag{ - Name: generalFlagPart, - Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, - Required: true, + Name: generalFlagMachine, + Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, }, }, &cli.BoolFlag{ @@ -1908,9 +1902,15 @@ var app = &cli.App{ Name: "restart", Aliases: []string{}, Usage: "request part restart", - UsageText: createUsageText("machines part restart", []string{generalFlagMachine, generalFlagPart}, true), - // TODO(RSDK-9286) revisit flags + UsageText: createUsageText("machines part restart", []string{generalFlagPart}, true), Flags: []cli.Flag{ + &AliasStringFlag{ + cli.StringFlag{ + Name: generalFlagPart, + Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, + Required: true, + }, + }, &cli.StringFlag{ Name: generalFlagOrganization, Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, @@ -1923,54 +1923,41 @@ var app = &cli.App{ }, &AliasStringFlag{ cli.StringFlag{ - Name: generalFlagMachine, - Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, - Required: true, - }, - }, - &AliasStringFlag{ - cli.StringFlag{ - Name: generalFlagPart, - Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, - Required: true, + Name: generalFlagMachine, + Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, }, }, }, Action: createCommandWithT[robotsPartRestartArgs](RobotsPartRestartAction), }, { - Name: "run", - Usage: "run a command on a machine part", - UsageText: createUsageText("machines part run", []string{ - generalFlagOrganization, generalFlagLocation, generalFlagMachine, generalFlagPart, - }, true, ""), + Name: "run", + Usage: "run a command on a machine part", + UsageText: createUsageText("machines part run", []string{generalFlagPart}, true, ""), Flags: []cli.Flag{ &AliasStringFlag{ cli.StringFlag{ - Name: generalFlagOrganization, - Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, + Name: generalFlagPart, + Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, Required: true, }, }, &AliasStringFlag{ cli.StringFlag{ - Name: generalFlagLocation, - Aliases: []string{generalFlagLocationID, generalFlagAliasLocationName}, - Required: true, + Name: generalFlagOrganization, + Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, }, }, &AliasStringFlag{ cli.StringFlag{ - Name: generalFlagMachine, - Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, - Required: true, + Name: generalFlagLocation, + Aliases: []string{generalFlagLocationID, generalFlagAliasLocationName}, }, }, &AliasStringFlag{ cli.StringFlag{ - Name: generalFlagPart, - Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, - Required: true, + Name: generalFlagMachine, + Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, }, }, &cli.StringFlag{ @@ -1989,10 +1976,14 @@ var app = &cli.App{ Usage: "start a shell on a machine part", Description: `In order to use the shell command, the machine must have a valid shell type service.`, UsageText: createUsageText( - "machines part shell", []string{generalFlagOrganization, generalFlagLocation, generalFlagMachine, generalFlagPart}, false, + "machines part shell", []string{generalFlagPart}, false, ), - // TODO(RSDK-9286) do we need to ask for og and location and machine and part here? Flags: []cli.Flag{ + &cli.StringFlag{ + Name: generalFlagPart, + Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, + Required: true, + }, &cli.StringFlag{ Name: generalFlagOrganization, Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, @@ -2007,10 +1998,6 @@ var app = &cli.App{ Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, }, }, - &cli.StringFlag{ - Name: generalFlagPart, - Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, - }, }, Action: createCommandWithT[robotsPartShellArgs](RobotsPartShellAction), }, @@ -2045,10 +2032,17 @@ Copy multiple files from the machine to a local destination with recursion and k `, UsageText: createUsageText( "machines part cp", - []string{generalFlagOrganization, generalFlagLocation, generalFlagMachine, generalFlagPart}, + []string{generalFlagPart}, true, "[-p] [-r] source ([machine:]files) ... target ([machine:]files"), Flags: []cli.Flag{ + &AliasStringFlag{ + cli.StringFlag{ + Name: generalFlagPart, + Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, + Required: true, + }, + }, &cli.StringFlag{ Name: generalFlagOrganization, Aliases: []string{generalFlagAliasOrg, generalFlagOrgID, generalFlagAliasOrgName}, @@ -2059,16 +2053,8 @@ Copy multiple files from the machine to a local destination with recursion and k }, &AliasStringFlag{ cli.StringFlag{ - Name: generalFlagMachine, - Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, - Required: true, - }, - }, - &AliasStringFlag{ - cli.StringFlag{ - Name: generalFlagPart, - Aliases: []string{generalFlagPartID, generalFlagAliasPartName}, - Required: true, + Name: generalFlagMachine, + Aliases: []string{generalFlagAliasRobot, generalFlagMachineID, generalFlagAliasMachineName}, }, }, &cli.BoolFlag{ diff --git a/cli/client.go b/cli/client.go index 99bcf669209..de70afcb944 100644 --- a/cli/client.go +++ b/cli/client.go @@ -25,6 +25,7 @@ import ( "github.com/nathan-fiscaletti/consolesize-go" "github.com/pkg/errors" "github.com/urfave/cli/v2" + "go.uber.org/multierr" "go.uber.org/zap" buildpb "go.viam.com/api/app/build/v1" datapb "go.viam.com/api/app/data/v1" @@ -672,6 +673,26 @@ type robotsStatusArgs struct { Machine string } +func (c *viamClient) getOrgAndLocationNamesForRobot(ctx context.Context, robot *apppb.Robot) (string, string, error) { + orgs, err := c.client.GetOrganizationsWithAccessToLocation( + ctx, &apppb.GetOrganizationsWithAccessToLocationRequest{LocationId: robot.Location}, + ) + if err != nil { + return "", "", err + } + if len(orgs.OrganizationIdentities) == 0 { + return "", "", errors.Errorf("no parent org found for robot: %s", robot.Id) + } + org := orgs.OrganizationIdentities[0] + + location, err := c.client.GetLocation(ctx, &apppb.GetLocationRequest{LocationId: robot.Location}) + if err != nil { + return "", "", err + } + + return org.Name, location.Location.Name, nil +} + // RobotsStatusAction is the corresponding Action for 'machines status'. func RobotsStatusAction(c *cli.Context, args robotsStatusArgs) error { client, err := newViamClient(c) @@ -691,7 +712,11 @@ func RobotsStatusAction(c *cli.Context, args robotsStatusArgs) error { } if orgStr == "" || locStr == "" { - printf(c.App.Writer, "%s -> %s", client.selectedOrg.Name, client.selectedLoc.Name) + orgName, locName, err := client.getOrgAndLocationNamesForRobot(c.Context, robot) + if err != nil { + return err + } + printf(c.App.Writer, "%s -> %s", orgName, locName) } printf( @@ -781,6 +806,9 @@ func RobotsLogsAction(c *cli.Context, args robotsLogsArgs) error { return errors.Wrap(err, "could not get machine") } + // TODO(RSDK-9727) - this is a little inefficient insofar as a `robot` is created immediately + // above and then also again within this `robotParts` call. Might be nice to have a helper + // API for getting parts when we already have a `Robot` parts, err := client.robotParts(orgStr, locStr, robotStr) if err != nil { return errors.Wrap(err, "could not get machine parts") @@ -965,18 +993,21 @@ func RobotsPartStatusAction(c *cli.Context, args robotsPartStatusArgs) error { orgStr := args.Organization locStr := args.Location robotStr := args.Machine - robot, err := client.robot(orgStr, locStr, robotStr) - if err != nil { - return errors.Wrap(err, "could not get machine") - } - part, err := client.robotPart(orgStr, locStr, robotStr, args.Part) if err != nil { return errors.Wrap(err, "could not get machine part") } if orgStr == "" || locStr == "" || robotStr == "" { - printf(c.App.Writer, "%s -> %s -> %s", client.selectedOrg.Name, client.selectedLoc.Name, robot.Name) + robot, err := client.robot(orgStr, locStr, part.Robot) + if err != nil { + return err + } + orgName, locName, err := client.getOrgAndLocationNamesForRobot(c.Context, robot) + if err != nil { + return err + } + printf(c.App.Writer, "%s -> %s -> %s", orgName, locName, robot.Name) } name := part.Name @@ -1019,18 +1050,30 @@ func (c *viamClient) robotsPartLogsAction(cCtx *cli.Context, args robotsPartLogs orgStr := args.Organization locStr := args.Location robotStr := args.Machine - robot, err := c.robot(orgStr, locStr, robotStr) - if err != nil { - return errors.Wrap(err, "could not get machine") - } + partStr := args.Part var header string if orgStr == "" || locStr == "" || robotStr == "" { - header = fmt.Sprintf("%s -> %s -> %s", c.selectedOrg.Name, c.selectedLoc.Name, robot.Name) + // TODO(RSDK-9727) - this is a little inefficient insofar as a `part` is created immediately + // here then also again within this `{tail|print}RobotPartLogs` call. Might be nice to have a + // helper API for getting logs from an already-existing `part` + part, err := c.robotPart(orgStr, locStr, robotStr, partStr) + if err != nil { + return err + } + robot, err := c.robot(orgStr, locStr, part.Robot) + if err != nil { + return err + } + orgName, locName, err := c.getOrgAndLocationNamesForRobot(cCtx.Context, robot) + if err != nil { + return err + } + header = fmt.Sprintf("%s -> %s -> %s", orgName, locName, robot.Name) } if args.Tail { return c.tailRobotPartLogs( - orgStr, locStr, robotStr, args.Part, + orgStr, locStr, robotStr, partStr, args.Errors, "", header, @@ -1041,7 +1084,7 @@ func (c *viamClient) robotsPartLogsAction(cCtx *cli.Context, args robotsPartLogs return err } return c.printRobotPartLogs( - orgStr, locStr, robotStr, args.Part, + orgStr, locStr, robotStr, partStr, args.Errors, "", header, @@ -1809,6 +1852,22 @@ func (c *viamClient) robot(orgStr, locStr, robotStr string) (*apppb.Robot, error } func (c *viamClient) robotPart(orgStr, locStr, robotStr, partStr string) (*apppb.RobotPart, error) { + part, err := c.robotPartInner(orgStr, locStr, robotStr, partStr) + if err == nil { + return part, nil + } + + // if we still haven't found the part, it's possible no robotStr was passed. That's okay + // so long as the partStr was passed as an ID, so let's try to get the part with just that. + resp, err2 := c.getRobotPart(partStr) + if err2 == nil { + return resp.Part, nil + } + + return nil, multierr.Combine(err, err2) +} + +func (c *viamClient) robotPartInner(orgStr, locStr, robotStr, partStr string) (*apppb.RobotPart, error) { if err := c.ensureLoggedIn(); err != nil { return nil, err } diff --git a/cli/client_test.go b/cli/client_test.go index 055b619894f..d6fcdbf710f 100644 --- a/cli/client_test.go +++ b/cli/client_test.go @@ -697,17 +697,29 @@ func TestGetRobotPartLogs(t *testing.T) { return resp, nil } + loc := apppb.Location{Name: "naboo"} + listOrganizationsFunc := func(ctx context.Context, in *apppb.ListOrganizationsRequest, opts ...grpc.CallOption, ) (*apppb.ListOrganizationsResponse, error) { orgs := []*apppb.Organization{{Name: "jedi", Id: "123"}} return &apppb.ListOrganizationsResponse{Organizations: orgs}, nil } + getOrganizationsWithAccessToLocationFunc := func(ctx context.Context, in *apppb.GetOrganizationsWithAccessToLocationRequest, + opts ...grpc.CallOption, + ) (*apppb.GetOrganizationsWithAccessToLocationResponse, error) { + orgIdentities := []*apppb.OrganizationIdentity{{Name: "jedi", Id: "123"}} + return &apppb.GetOrganizationsWithAccessToLocationResponse{OrganizationIdentities: orgIdentities}, nil + } + getLocationFunc := func(ctx context.Context, in *apppb.GetLocationRequest, + opts ...grpc.CallOption, + ) (*apppb.GetLocationResponse, error) { + return &apppb.GetLocationResponse{Location: &loc}, nil + } listLocationsFunc := func(ctx context.Context, in *apppb.ListLocationsRequest, opts ...grpc.CallOption, ) (*apppb.ListLocationsResponse, error) { - locs := []*apppb.Location{{Name: "naboo"}} - return &apppb.ListLocationsResponse{Locations: locs}, nil + return &apppb.ListLocationsResponse{Locations: []*apppb.Location{&loc}}, nil } listRobotsFunc := func(ctx context.Context, in *apppb.ListRobotsRequest, opts ...grpc.CallOption, @@ -726,10 +738,12 @@ func TestGetRobotPartLogs(t *testing.T) { GetRobotPartLogsFunc: getRobotPartLogsFunc, // Supply some injected functions to avoid a panic when loading // organizations, locations, robots and parts. - ListOrganizationsFunc: listOrganizationsFunc, - ListLocationsFunc: listLocationsFunc, - ListRobotsFunc: listRobotsFunc, - GetRobotPartsFunc: getRobotPartsFunc, + ListOrganizationsFunc: listOrganizationsFunc, + ListLocationsFunc: listLocationsFunc, + ListRobotsFunc: listRobotsFunc, + GetRobotPartsFunc: getRobotPartsFunc, + GetLocationFunc: getLocationFunc, + GetOrganizationsWithAccessToLocationFunc: getOrganizationsWithAccessToLocationFunc, } t.Run("no count", func(t *testing.T) {