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-5608 - CLI commands for managing cloud builds #3298

Merged
merged 27 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b5baca8
revertme: target local api
abe-winter Nov 9, 2023
4e169ce
stub to trigger StartBuild rpc
abe-winter Nov 9, 2023
01093ea
rm commented code
abe-winter Nov 9, 2023
5569f20
default build info / arch from manifest
abe-winter Nov 10, 2023
72a5209
include moduleId in 'build start' command
abe-winter Nov 13, 2023
57d4236
RSDK-5607 - Cli Viam module build local (#3231)
zaporter-work Nov 14, 2023
2b95437
lint
zaporter-work Nov 16, 2023
d9eef89
build list
zaporter-work Nov 15, 2023
296ad30
merge conflicts
zaporter-work Nov 16, 2023
47e7855
include repo + ref in startBuild
abe-winter Nov 16, 2023
a04319c
fix bugs in formatting
zaporter-work Nov 27, 2023
f9ee988
attempt simplification, removal of job id
zaporter-work Nov 28, 2023
1e988aa
compilation fixes
zaporter-work Nov 28, 2023
f96cac4
logs + list improvements
zaporter-work Nov 28, 2023
3c03147
added versions. viam module build list mostly works now
zaporter-work Nov 28, 2023
286370d
cleanup list
zaporter-work Nov 29, 2023
c9fdc43
Merge remote-tracking branch 'upstream/main' into cloud-build
zaporter-work Nov 30, 2023
7364fc2
bump api
zaporter-work Nov 30, 2023
a8309fc
lint + readd directive
zaporter-work Nov 30, 2023
9df086b
logs + list are fully functional but need tests
zaporter-work Dec 1, 2023
c03f258
polish
zaporter-work Dec 5, 2023
a49ca92
remove local replace
zaporter-work Dec 5, 2023
f1d120d
pr comments
zaporter-work Dec 6, 2023
4f215e7
revert to recv() after becoming confident that the ctx is respected
zaporter-work Dec 6, 2023
e56f148
remove unused import
abe-winter Dec 6, 2023
2316031
add some basic unit tests
zaporter-work Dec 8, 2023
4ee828d
forgot to add this file last night
zaporter-work Dec 8, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 67 additions & 2 deletions cli/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ const (
moduleFlagPlatform = "platform"
moduleFlagForce = "force"

moduleBuildFlagPath = "module"
moduleBuildFlagCount = "count"
moduleBuildFlagVersion = "version"
moduleBuildFlagBuildID = "id"
moduleBuildFlagPlatform = "platform"
moduleBuildFlagWait = "wait"

dataFlagDestination = "destination"
dataFlagDataType = "data-type"
dataFlagOrgIDs = "org-ids"
Expand Down Expand Up @@ -1084,8 +1091,8 @@ Uses the "build" section of your meta.json.
Example:
"build": {
"setup": "setup.sh", // optional - command to install your build dependencies
"build": "make module.tar.gz", // command that will build your module
"path" : "module.tar.gz", // optional - path to your built module
"build": "make module.tar.gz", // command that will build your module
"path" : "module.tar.gz", // optional - path to your built module
// (passed to the 'viam module upload' command)
"arch" : ["linux/amd64", "linux/arm64"] // architectures to build for
}`,
Expand All @@ -1103,6 +1110,64 @@ Example:
},
Action: ModuleBuildLocalAction,
},
{
Name: "start",
Description: "start a remote build",
Flags: []cli.Flag{
&cli.StringFlag{
Name: moduleBuildFlagPath,
Usage: "path to meta.json",
Value: "./meta.json",
TakesFile: true,
},
&cli.StringFlag{
Name: moduleBuildFlagVersion,
Usage: "version of the module to upload (semver2.0) ex: \"0.1.0\"",
Required: true,
},
},
Action: ModuleBuildStartAction,
},
{
Name: "list",
Usage: "check on the status of your cloud builds",
Flags: []cli.Flag{
&cli.StringFlag{
Name: moduleFlagPath,
Usage: "path to meta.json",
Value: "./meta.json",
TakesFile: true,
},
&cli.IntFlag{
Name: moduleBuildFlagCount,
Usage: "number of builds to list",
Aliases: []string{"c"},
DefaultText: "all",
Copy link
Member

Choose a reason for hiding this comment

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

I tried to look this up myself but couldn't figure it out. Should DefaultText always come with a Value? Is it that DefaultText is a suggested value where as Value is an actual default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, this surprising behavior was fixed for the other cli commands in #3162

My understanding is that:
DefaultText => shows in help msg, does not actually change the default value when you try to read the flag
Value => shows in help msg + actual default value

Copy link
Member

@michaellee1019 michaellee1019 Dec 6, 2023

Choose a reason for hiding this comment

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

Yup thanks for confirming what I found. I feel like DefaultText isn't appropriate then. The "default" is that you don't specify the flag at all, otherwise you specify it and it should be a int.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, as we discussed keep it as-is.

},
&cli.StringFlag{
Name: moduleBuildFlagBuildID,
Usage: "restrict output to just return builds that match this id",
},
},
Action: ModuleBuildListAction,
},
{
Name: "logs",
Usage: "get the logs from one of your cloud builds",
Flags: []cli.Flag{
&cli.StringFlag{
Name: moduleBuildFlagBuildID,
Usage: "build that you want to get the logs for",
Required: true,
},
&cli.StringFlag{
Name: moduleBuildFlagPlatform,
Usage: "build platform to get the logs for. Ex: linux/arm64",
Required: true,
},
},
Action: ModuleBuildLogsAction,
},
},
},
},
Expand Down
2 changes: 2 additions & 0 deletions cli/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/golang-jwt/jwt/v4"
"github.com/pkg/errors"
"github.com/urfave/cli/v2"
buildpb "go.viam.com/api/app/build/v1"
datapb "go.viam.com/api/app/data/v1"
datasetpb "go.viam.com/api/app/dataset/v1"
mltrainingpb "go.viam.com/api/app/mltraining/v1"
Expand Down Expand Up @@ -497,6 +498,7 @@ func (c *viamClient) ensureLoggedIn() error {
c.packageClient = packagepb.NewPackageServiceClient(conn)
c.datasetClient = datasetpb.NewDatasetServiceClient(conn)
c.mlTrainingClient = mltrainingpb.NewMLTrainingServiceClient(conn)
c.buildClient = buildpb.NewBuildServiceClient(conn)

return nil
}
Expand Down
20 changes: 10 additions & 10 deletions cli/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

func TestLoginAction(t *testing.T) {
cCtx, ac, out, errOut := setup(nil, nil, nil, "token")
cCtx, ac, out, errOut := setup(nil, nil, nil, nil, "token")

test.That(t, ac.loginAction(cCtx), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand All @@ -26,7 +26,7 @@ func TestLoginAction(t *testing.T) {
}

func TestAPIKeyAuth(t *testing.T) {
_, ac, _, errOut := setup(nil, nil, nil, "apiKey")
_, ac, _, errOut := setup(nil, nil, nil, nil, "apiKey")
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
APIKey, isAPIKey := ac.conf.Auth.(*apiKey)
test.That(t, isAPIKey, test.ShouldBeTrue)
Expand All @@ -36,7 +36,7 @@ func TestAPIKeyAuth(t *testing.T) {

func TestPrintAccessTokenAction(t *testing.T) {
// AppServiceClient needed for any Action that calls ensureLoggedIn.
cCtx, ac, out, errOut := setup(&inject.AppServiceClient{}, nil, nil, "token")
cCtx, ac, out, errOut := setup(&inject.AppServiceClient{}, nil, nil, nil, "token")

test.That(t, ac.printAccessTokenAction(cCtx), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand All @@ -53,7 +53,7 @@ func TestAPIKeyCreateAction(t *testing.T) {
asc := &inject.AppServiceClient{
CreateKeyFunc: createKeyFunc,
}
cCtx, ac, out, errOut := setup(asc, nil, nil, "token")
cCtx, ac, out, errOut := setup(asc, nil, nil, nil, "token")

test.That(t, ac.organizationsAPIKeyCreateAction(cCtx), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand All @@ -80,7 +80,7 @@ func TestRobotAPIKeyCreateAction(t *testing.T) {
flags[apiKeyCreateFlagOrgID] = fakeOrgID
flags[apiKeyFlagRobotID] = fakeRobotID
flags[apiKeyCreateFlagName] = "my-name"
cCtx, ac, out, errOut := setup(asc, nil, &flags, "token")
cCtx, ac, out, errOut := setup(asc, nil, nil, &flags, "token")

test.That(t, ac.robotAPIKeyCreateAction(cCtx), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand Down Expand Up @@ -135,7 +135,7 @@ func TestRobotAPIKeyCreateAction(t *testing.T) {
flags[apiKeyFlagRobotID] = fakeRobotID
flags[apiKeyCreateFlagOrgID] = ""
flags[apiKeyCreateFlagName] = "test-me"
cCtx, ac, out, _ = setup(asc, nil, &flags, "token")
cCtx, ac, out, _ = setup(asc, nil, nil, &flags, "token")
err = ac.robotAPIKeyCreateAction(cCtx)
test.That(t, err, test.ShouldNotBeNil)

Expand All @@ -162,7 +162,7 @@ func TestLocationAPIKeyCreateAction(t *testing.T) {
flags[apiKeyCreateFlagOrgID] = ""
flags[apiKeyCreateFlagName] = "" // testing no locationID

cCtx, ac, out, errOut := setup(asc, nil, &flags, "token")
cCtx, ac, out, errOut := setup(asc, nil, nil, &flags, "token")
err := ac.locationAPIKeyCreateAction(cCtx)
test.That(t, err, test.ShouldNotBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestLocationAPIKeyCreateAction(t *testing.T) {
flags[apiKeyCreateFlagOrgID] = ""
flags[apiKeyCreateFlagName] = "test-name"

cCtx, ac, _, _ = setup(asc, nil, &flags, "token")
cCtx, ac, _, _ = setup(asc, nil, nil, &flags, "token")

err = ac.locationAPIKeyCreateAction(cCtx)
test.That(t, err, test.ShouldNotBeNil)
Expand All @@ -212,7 +212,7 @@ func TestLocationAPIKeyCreateAction(t *testing.T) {
}

func TestLogoutAction(t *testing.T) {
cCtx, ac, out, errOut := setup(nil, nil, nil, "token")
cCtx, ac, out, errOut := setup(nil, nil, nil, nil, "token")

test.That(t, ac.logoutAction(cCtx), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand All @@ -222,7 +222,7 @@ func TestLogoutAction(t *testing.T) {
}

func TestWhoAmIAction(t *testing.T) {
cCtx, ac, out, errOut := setup(nil, nil, nil, "token")
cCtx, ac, out, errOut := setup(nil, nil, nil, nil, "token")

test.That(t, ac.whoAmIAction(cCtx), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand Down
4 changes: 2 additions & 2 deletions cli/boards_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func TestAppendAuthHeadersTokenAuth(t *testing.T) {
cCtx, ac, _, _ := setup(&inject.AppServiceClient{}, nil, nil, "token")
cCtx, ac, _, _ := setup(&inject.AppServiceClient{}, nil, nil, nil, "token")

testReq, err := http.NewRequestWithContext(cCtx.Context, http.MethodGet, "/test", nil)
test.That(t, err, test.ShouldBeNil)
Expand All @@ -21,7 +21,7 @@ func TestAppendAuthHeadersTokenAuth(t *testing.T) {
}

func TestAppendAuthHeadersAPIKeyAuth(t *testing.T) {
cCtx, ac, _, _ := setup(&inject.AppServiceClient{}, nil, nil, "apiKey")
cCtx, ac, _, _ := setup(&inject.AppServiceClient{}, nil, nil, nil, "apiKey")

testReq, err := http.NewRequestWithContext(cCtx.Context, http.MethodGet, "/test", nil)
test.That(t, err, test.ShouldBeNil)
Expand Down
2 changes: 2 additions & 0 deletions cli/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/pkg/errors"
"github.com/urfave/cli/v2"
"go.uber.org/zap"
buildpb "go.viam.com/api/app/build/v1"
datapb "go.viam.com/api/app/data/v1"
datasetpb "go.viam.com/api/app/dataset/v1"
mltrainingpb "go.viam.com/api/app/mltraining/v1"
Expand Down Expand Up @@ -48,6 +49,7 @@ type viamClient struct {
packageClient packagepb.PackageServiceClient
datasetClient datasetpb.DatasetServiceClient
mlTrainingClient mltrainingpb.MLTrainingServiceClient
buildClient buildpb.BuildServiceClient
baseURL *url.URL
rpcOpts []rpc.DialOption
authFlow *authFlow
Expand Down
22 changes: 14 additions & 8 deletions cli/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/urfave/cli/v2"
buildpb "go.viam.com/api/app/build/v1"
datapb "go.viam.com/api/app/data/v1"
apppb "go.viam.com/api/app/v1"
"go.viam.com/test"
Expand Down Expand Up @@ -39,8 +40,12 @@ func (tw *testWriter) Write(b []byte) (int, error) {
// setup creates a new cli.Context and viamClient with fake auth and the passed
// in AppServiceClient and DataServiceClient. It also returns testWriters that capture Stdout and
// Stdin.
func setup(asc apppb.AppServiceClient, dataClient datapb.DataServiceClient,
defaultFlags *map[string]string, authMethod string,
func setup(
asc apppb.AppServiceClient,
dataClient datapb.DataServiceClient,
buildClient buildpb.BuildServiceClient,
defaultFlags *map[string]string,
authMethod string,
) (*cli.Context, *viamClient, *testWriter, *testWriter) {
out := &testWriter{}
errOut := &testWriter{}
Expand Down Expand Up @@ -76,10 +81,11 @@ func setup(asc apppb.AppServiceClient, dataClient datapb.DataServiceClient,
}
}
ac := &viamClient{
client: asc,
conf: conf,
c: cCtx,
dataClient: dataClient,
client: asc,
conf: conf,
c: cCtx,
dataClient: dataClient,
buildClient: buildClient,
}
return cCtx, ac, out, errOut
}
Expand All @@ -94,7 +100,7 @@ func TestListOrganizationsAction(t *testing.T) {
asc := &inject.AppServiceClient{
ListOrganizationsFunc: listOrganizationsFunc,
}
cCtx, ac, out, errOut := setup(asc, nil, nil, "token")
cCtx, ac, out, errOut := setup(asc, nil, nil, nil, "token")

test.That(t, ac.listOrganizationsAction(cCtx), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand Down Expand Up @@ -129,7 +135,7 @@ func TestTabularDataByFilterAction(t *testing.T) {
TabularDataByFilterFunc: tabularDataByFilterFunc,
}

cCtx, ac, out, errOut := setup(&inject.AppServiceClient{}, dsc, nil, "token")
cCtx, ac, out, errOut := setup(&inject.AppServiceClient{}, dsc, nil, nil, "token")

test.That(t, ac.dataExportAction(cCtx), test.ShouldBeNil)
test.That(t, len(errOut.messages), test.ShouldEqual, 0)
Expand Down
Loading
Loading