From 8430abe841892908e0e5a1487304d0f46f78e3ed Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 13 Oct 2025 12:56:25 -0400 Subject: [PATCH 1/9] module reload improvements --- cli/app.go | 116 ++++++++++++++--- cli/module_build.go | 304 +++++++++++++++++++++++++++++--------------- 2 files changed, 296 insertions(+), 124 deletions(-) diff --git a/cli/app.go b/cli/app.go index ae14c9aa118..3773b45b936 100644 --- a/cli/app.go +++ b/cli/app.go @@ -2929,31 +2929,59 @@ This won't work unless you have an existing installation of our GitHub app on yo }, }, { - Name: "reload", + Name: "restart", + Usage: "restart a currently-running module", + UsageText: createUsageText("module restart", nil, true, false), + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: generalFlagPartID, + Usage: "part ID of machine. get from 'Live/Offline' dropdown in the web app", + DefaultText: "/etc/viam.json", + }, + &cli.StringFlag{ + Name: moduleFlagPath, + Usage: "path to a meta.json. used for module ID. can be overridden with --id or --name", + Value: "meta.json", + }, + &cli.StringFlag{ + Name: generalFlagName, + Usage: "name of module to restart. pass at most one of --name, --id", + }, + &cli.StringFlag{ + Name: generalFlagID, + Usage: "ID of module to restart, for example viam:wifi-sensor. pass at most one of --name, --id", + }, + &cli.PathFlag{ + Name: moduleBuildFlagCloudConfig, + Usage: "Provide the location of the viam.json file with robot ID to lookup the part-id. Use instead of --part-id option.", + Value: "/etc/viam.json", + }, + }, + Action: createCommandWithT[moduleRestartArgs](ModuleRestartAction), + }, + { + Name: "reload-local", Usage: "build a module locally and run it on a target device. rebuild & restart if already running", - UsageText: createUsageText("module reload", nil, true, false), + UsageText: createUsageText("module reload-local", nil, true, false), Description: `Example invocations: # A full reload command. This will build your module, send the tarball to the machine with given part ID, # and configure or restart it. - viam module reload --part-id UUID - - # Restart a module running on your local viam server, by name, without building or reconfiguring. - viam module reload --restart-only --id viam:python-example-module + viam module reload-local --part-id UUID - # Use cloudbuild to build the tar.gz that will be copied to the part for hot reloading. - viam module reload --cloud-build + # Reload from an already-built module, without performing a new local build. + viam module reload-local --no-build # Run viam module reload on a mac and use the downloaded viam.json file instead of --part-id - viam module reload --cloud-config ~/Downloads/viam-mac-main.json + viam module reload-local --cloud-config ~/Downloads/viam-mac-main.json # Specify a component/service model (and optionally a name) to add to the config along with # the module (the API is automatically looked up from meta.json) # By default, no resources are added when a module is reloaded - viam module reload --model-name acme:module-name:mybase --name my-resource + viam module reload-local --model-name acme:module-name:mybase --name my-resource - # Build and configure a module on your local machine without shipping a tarball. - viam module reload --local`, + # Build and configure a module running on your local machine without shipping a tarball. + viam module reload-local --local`, Flags: []cli.Flag{ &cli.StringFlag{ Name: generalFlagPartID, @@ -2973,10 +3001,6 @@ This won't work unless you have an existing installation of our GitHub app on yo Name: generalFlagID, Usage: "ID of module to restart, for example viam:wifi-sensor. pass at most one of --name, --id", }, - &cli.BoolFlag{ - Name: moduleBuildRestartOnly, - Usage: "just restart the module on the target system, don't do other reload steps", - }, &cli.BoolFlag{ Name: moduleBuildFlagNoBuild, Usage: "don't do build step", @@ -2994,9 +3018,65 @@ This won't work unless you have an existing installation of our GitHub app on yo Usage: "remote user's home directory. only necessary if you're targeting a remote machine where $HOME is not /root", Value: "~", }, + &cli.PathFlag{ + Name: moduleBuildFlagCloudConfig, + Usage: "Provide the location of the viam.json file with robot ID to lookup the part-id. Use instead of --part-id option.", + Value: "/etc/viam.json", + }, + &cli.StringFlag{ + Name: moduleFlagModelName, + Usage: "If passed, creates a resource in the part config with the given model triple", + DefaultText: "Don't create a new resource", + }, + &cli.StringFlag{ + Name: moduleBuildFlagWorkdir, + Usage: "use this to indicate that your meta.json is in a subdirectory of your repo. --module flag should be relative to this", + Value: ".", + }, + &cli.StringFlag{ + Name: dataFlagResourceName, + Usage: "Use with model-name to name the newly added resource", + DefaultText: "resource type with a unique numerical suffix", + }, + }, + Action: createCommandWithT[reloadModuleArgs](ReloadModuleLocalAction), + }, + { + Name: "reload", + Usage: "build a module in the cloud and run it on a target device. rebuild & restart if already running", + UsageText: createUsageText("module reload", nil, true, false), + Description: `Example invocations: + + # A full reload command. This will build your module, send the tarball to the machine with given part ID, + # and configure or restart it. + viam module reload --part-id UUID + + # Run viam module reload on a mac and use the downloaded viam.json file instead of --part-id + viam module reload --cloud-config ~/Downloads/viam-mac-main.json + + # Specify a component/service model (and optionally a name) to add to the config along with + # the module (the API is automatically looked up from meta.json) + # By default, no resources are added when a module is reloaded + viam module reload --model-name acme:module-name:mybase --name my-resource`, + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: generalFlagPartID, + Usage: "part ID of machine. get from 'Live/Offline' dropdown in the web app", + DefaultText: "/etc/viam.json", + }, + &cli.StringFlag{ + Name: moduleFlagPath, + Usage: "path to a meta.json. used for module ID. can be overridden with --id or --name", + Value: "meta.json", + }, &cli.BoolFlag{ - Name: moduleBuildFlagCloudBuild, - Usage: "Run the module's build script using cloud build instead of locally, downloading to the build.path field in meta.json", + Name: generalFlagNoProgress, + Usage: "hide progress of the file transfer", + }, + &cli.StringFlag{ + Name: moduleFlagHomeDir, + Usage: "remote user's home directory. only necessary if you're targeting a remote machine where $HOME is not /root", + Value: "~", }, &cli.PathFlag{ Name: moduleBuildFlagCloudConfig, diff --git a/cli/module_build.go b/cli/module_build.go index 3d631efb0f9..cb33b99d1e4 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -21,6 +21,7 @@ import ( "go.uber.org/multierr" "go.uber.org/zap" buildpb "go.viam.com/api/app/build/v1" + packagespb "go.viam.com/api/app/packages/v1" v1 "go.viam.com/api/app/packages/v1" apppb "go.viam.com/api/app/v1" "go.viam.com/utils/pexec" @@ -499,13 +500,12 @@ func jobStatusFromProto(s buildpb.JobStatus) jobStatus { } type reloadModuleArgs struct { - PartID string - Module string - RestartOnly bool - NoBuild bool - Local bool - NoProgress bool - CloudBuild bool + PartID string + Module string + NoBuild bool + Local bool + NoProgress bool + CloudBuild bool // CloudConfig is a path to the `viam.json`, or the config containing the robot ID. CloudConfig string @@ -726,74 +726,139 @@ func (c *viamClient) ensureModuleRegisteredInCloud(ctx *cli.Context, moduleID mo return nil } -// moduleCloudReload triggers a cloud build and then reloads the specified module with that build. -func (c *viamClient) moduleCloudReload(ctx *cli.Context, args reloadModuleArgs, platform string) (string, error) { - manifest, err := loadManifest(args.Module) +func (c *viamClient) inferOrgIDFromManifest(module string, manifest moduleManifest) (string, error) { + moduleID, err := parseModuleID(manifest.ModuleID) if err != nil { return "", err } - - // ensure that the module has been registered in the cloud - moduleID, err := parseModuleID(manifest.ModuleID) + org, err := getOrgByModuleIDPrefix(c, moduleID.prefix) if err != nil { return "", err } - err = c.ensureModuleRegisteredInCloud(ctx, moduleID, &manifest) + return org.GetId(), nil +} + +func (c *viamClient) triggerCloudReload( + ctx *cli.Context, + args reloadModuleArgs, + manifest moduleManifest, + archivePath string, +) (string, error) { + stream, err := c.buildClient.StartReloadBuild(ctx.Context) if err != nil { return "", err } - id := ctx.String(generalFlagID) - if id == "" { - id = manifest.ModuleID + file, err := os.Open(archivePath) + if err != nil { + return "", err } - archivePath, err := c.createGitArchive(args.Path) + orgID, err := c.inferOrgIDFromManifest(args.Module, manifest) if err != nil { return "", err } - org, err := getOrgByModuleIDPrefix(c, moduleID.prefix) + + part, err := c.getRobotPart(args.PartID) if err != nil { return "", err } - // Upload a package with the bundled local dev code. Note that "reload" is a sentinel - // value for hot reloading modules. App expects it; don't change without making a - // complimentary update to the app repo - resp, err := c.uploadPackage(org.GetId(), reloadVersion, reloadVersion, "module", archivePath, nil) + if part.Part == nil { + return "", fmt.Errorf("part with id=%s not found", args.PartID) + } + + if part.Part.UserSuppliedInfo == nil { + return "", errors.New("unable to determine platform for part") + } + + platform := part.Part.UserSuppliedInfo.Fields["platform"].GetStringValue() + req := &buildpb.StartReloadBuildRequest{ + CloudBuild: &buildpb.StartReloadBuildRequest_BuildInfo{ + BuildInfo: &buildpb.ReloadBuildInfo{ + Platform: platform, + Workdir: &args.Workdir, + ModuleId: manifest.ModuleID, + }, + }, + } + if err := stream.Send(req); err != nil { + return "", err + } + + pkgInfo := packagespb.PackageInfo{ + OrganizationId: orgID, + Name: reloadVersion, + Version: reloadVersion, + Type: v1.PackageType_PACKAGE_TYPE_MODULE, + } + reqInner := &packagespb.CreatePackageRequest{ + Package: &packagespb.CreatePackageRequest_Info{ + Info: &pkgInfo, + }, + } + req = &buildpb.StartReloadBuildRequest{ + CloudBuild: &buildpb.StartReloadBuildRequest_Package{ + Package: reqInner, + }, + } + + if err := stream.Send(req); err != nil { + return "", err + } + + var errs error + if err := sendUploadRequests( + ctx.Context, stream, file, c.c.App.Writer, getNextReloadBuildUploadRequest); err != nil && !errors.Is(err, io.EOF) { + errs = multierr.Combine(errs, errors.Wrapf(err, "could not upload %s", file.Name())) + } + + resp, closeErr := stream.CloseAndRecv() + if err != nil && !errors.Is(closeErr, io.EOF) { + errs = multierr.Combine(errs, closeErr) + } + return resp.GetBuildId(), errs +} + +func getNextReloadBuildUploadRequest(file *os.File) (*buildpb.StartReloadBuildRequest, int, error) { + packagesRequest, byteLen, err := getNextPackageUploadRequest(file) + if err != nil { + return nil, 0, err + } + + return &buildpb.StartReloadBuildRequest{ + CloudBuild: &buildpb.StartReloadBuildRequest_Package{ + Package: packagesRequest, + }, + }, byteLen, nil +} + +// moduleCloudReload triggers a cloud build and then reloads the specified module with that build. +func (c *viamClient) moduleCloudReload(ctx *cli.Context, args reloadModuleArgs, platform string, manifest moduleManifest) (string, error) { + // ensure that the module has been registered in the cloud + moduleID, err := parseModuleID(manifest.ModuleID) if err != nil { return "", err } - // get package URL for downloading purposes - packageURL, err := c.getPackageDownloadURL(org.GetId(), reloadVersion, reloadVersion, "module") + err = c.ensureModuleRegisteredInCloud(ctx, moduleID, &manifest) if err != nil { return "", err } - // object ID stringifies as `ObjectID("{actual_id}")` but we only want the actual ID when - // passing back to app for package lookup. - versionParts := strings.Split(resp.Version, "\"") - if len(versionParts) != 3 { - return "", errors.Errorf("malformed ID %s", versionParts) + id := ctx.String(generalFlagID) + if id == "" { + id = manifest.ModuleID } - resp.Version = versionParts[1] - // (TODO RSDK-11531) It'd be nice to add some streaming logs for this so we can see how the progress is going. create a new build - // TODO (RSDK-11692) - passing org ID in the ref field and `resp.Version` (which is actually an object ID) - // in the token field is pretty hacky, let's fix it up - infof(c.c.App.Writer, "Creating a new cloud build and swapping it onto the requested machine part. This may take a few minutes...") - buildArgs := moduleBuildStartArgs{ - Module: args.Module, - Version: reloadVersion, - Workdir: args.Workdir, - Ref: org.GetId(), - Platforms: []string{platform}, - Token: resp.Version, + archivePath, err := c.createGitArchive(args.Path) + if err != nil { + return "", err } - buildID, err := c.moduleBuildStartForRepo(ctx, buildArgs, &manifest, packageURL) + infof(c.c.App.Writer, "Creating a new cloud build and swapping it onto the requested machine part. This may take a few minutes...") + buildID, err := c.triggerCloudReload(ctx, args, manifest, archivePath) if err != nil { return "", err } @@ -822,14 +887,8 @@ func (c *viamClient) moduleCloudReload(ctx *cli.Context, args reloadModuleArgs, Platform: platform, } - // delete the package now that the build is complete - _, err = c.packageClient.DeletePackage( - ctx.Context, - &v1.DeletePackageRequest{Id: resp.GetId(), Version: reloadVersion, Type: v1.PackageType_PACKAGE_TYPE_MODULE}, - ) - if err != nil { - warningf(ctx.App.Writer, "failed to delete package: %s", err.Error()) - } + // CR erodkin: confirm that we don't actually want to delete the package like this anymore. + // good to talk to michael lee to confirm // delete the archive we created if err := os.Remove(archivePath); err != nil { @@ -839,8 +898,16 @@ func (c *viamClient) moduleCloudReload(ctx *cli.Context, args reloadModuleArgs, return c.downloadModuleAction(ctx, downloadArgs) } +func ReloadModuleLocalAction(c *cli.Context, args reloadModuleArgs) error { + return reloadModuleAction(c, args, false) +} + // ReloadModuleAction builds a module, configures it on a robot, and starts or restarts it. func ReloadModuleAction(c *cli.Context, args reloadModuleArgs) error { + return reloadModuleAction(c, args, true) +} + +func reloadModuleAction(c *cli.Context, args reloadModuleArgs, cloudBuild bool) error { vc, err := newViamClient(c) if err != nil { return err @@ -856,11 +923,11 @@ func ReloadModuleAction(c *cli.Context, args reloadModuleArgs) error { logger = logging.NewDebugLogger("cli") } - return reloadModuleAction(c, vc, args, logger) + return reloadModuleActionInner(c, vc, args, logger, cloudBuild) } // reloadModuleAction is the testable inner reload logic. -func reloadModuleAction(c *cli.Context, vc *viamClient, args reloadModuleArgs, logger logging.Logger) error { +func reloadModuleActionInner(c *cli.Context, vc *viamClient, args reloadModuleArgs, logger logging.Logger, cloudBuild bool) error { // TODO(RSDK-9727) it'd be nice for this to be a method on a viam client rather than taking one as an arg partID, err := resolvePartID(args.PartID, args.CloudConfig) if err != nil { @@ -909,68 +976,67 @@ func reloadModuleAction(c *cli.Context, vc *viamClient, args reloadModuleArgs, l // with this behavior, and the robot will eventually converge to what is in config. needsRestart := true var buildPath string - if !args.RestartOnly { - if !args.NoBuild { - if manifest == nil { - return fmt.Errorf(`manifest not found at "%s". manifest required for build`, moduleFlagPath) - } - if !args.CloudBuild { - err = moduleBuildLocalAction(c, manifest, environment) - buildPath = manifest.Build.Path - } else { - buildPath, err = vc.moduleCloudReload(c, args, platform) - } - if err != nil { - return err - } + if !args.NoBuild { + if manifest == nil { + return fmt.Errorf(`manifest not found at "%s". manifest required for build`, moduleFlagPath) } - if !args.Local { - if manifest == nil || manifest.Build == nil || buildPath == "" { - return errors.New( - "remote reloading requires a meta.json with the 'build.path' field set. " + - "try --local if you are testing on the same machine.", - ) - } - if err := validateReloadableArchive(c, manifest.Build); err != nil { - // if it is a cloud build then it makes sense that we might not have a reloadable - // archive locally, so we can safely ignore the error - if !args.CloudBuild { - return err - } - } - if err := addShellService(c, vc, part.Part, true); err != nil { - return err - } - infof(c.App.Writer, "Copying %s to part %s", buildPath, part.Part.Id) - globalArgs, err := getGlobalArgs(c) - if err != nil { + if !cloudBuild { + err = moduleBuildLocalAction(c, manifest, environment) + buildPath = manifest.Build.Path + } else { + buildPath, err = vc.moduleCloudReload(c, args, platform, *manifest) + } + if err != nil { + return err + } + } + if !args.Local { + if manifest == nil || manifest.Build == nil || buildPath == "" { + return errors.New( + "remote reloading requires a meta.json with the 'build.path' field set. " + + "try --local if you are testing on the same machine.", + ) + } + if err := validateReloadableArchive(c, manifest.Build); err != nil { + // if it is a cloud build then it makes sense that we might not have a reloadable + // archive locally, so we can safely ignore the error + if cloudBuild { return err } - dest := reloadingDestination(c, manifest) - err = vc.copyFilesToFqdn( - part.Part.Fqdn, globalArgs.Debug, false, false, []string{buildPath}, - dest, logging.NewLogger(reloadVersion), args.NoProgress) - if err != nil { - if s, ok := status.FromError(err); ok && s.Code() == codes.PermissionDenied { - warningf(c.App.ErrWriter, "RDK couldn't write to the default file copy destination. "+ - "If you're running as non-root, try adding --home $HOME or --home /user/username to your CLI command. "+ - "Alternatively, run the RDK as root.") - } - return fmt.Errorf("failed copying to part (%v): %w", dest, err) - } } - var newPart *apppb.RobotPart - newPart, needsRestart, err = configureModule(c, vc, manifest, part.Part, args.Local) - // if the module has been configured, the cached response we have may no longer accurately reflect - // the update, so we set the updated `part.Part` - if newPart != nil { - part.Part = newPart + if err := addShellService(c, vc, part.Part, true); err != nil { + return err } - + infof(c.App.Writer, "Copying %s to part %s", buildPath, part.Part.Id) + globalArgs, err := getGlobalArgs(c) if err != nil { return err } + dest := reloadingDestination(c, manifest) + err = vc.copyFilesToFqdn( + part.Part.Fqdn, globalArgs.Debug, false, false, []string{buildPath}, + dest, logging.NewLogger(reloadVersion), args.NoProgress) + if err != nil { + if s, ok := status.FromError(err); ok && s.Code() == codes.PermissionDenied { + warningf(c.App.ErrWriter, "RDK couldn't write to the default file copy destination. "+ + "If you're running as non-root, try adding --home $HOME or --home /user/username to your CLI command. "+ + "Alternatively, run the RDK as root.") + } + return fmt.Errorf("failed copying to part (%v): %w", dest, err) + } + } + var newPart *apppb.RobotPart + newPart, needsRestart, err = configureModule(c, vc, manifest, part.Part, args.Local) + // if the module has been configured, the cached response we have may no longer accurately reflect + // the update, so we set the updated `part.Part` + if newPart != nil { + part.Part = newPart + } + + if err != nil { + return err } + if needsRestart { if err = restartModule(c, vc, part.Part, manifest, logger); err != nil { return err @@ -1077,6 +1143,32 @@ func resolveTargetModule(c *cli.Context, manifest *moduleManifest) (*robot.Resta return request, nil } +type moduleRestartArgs struct { + PartID string + Module string + CloudConfig string +} + +func ModuleRestartAction(c *cli.Context, args moduleRestartArgs) error { + client, err := newViamClient(c) + if err != nil { + return err + } + + part, err := client.getRobotPart(args.PartID) + if err != nil { + return err + } + + manifest, err := loadManifestOrNil(args.Module) + if err != nil { + return err + } + logger := logging.FromZapCompatible(zap.NewNop().Sugar()) + + return restartModule(c, client, part.Part, manifest, logger) +} + // restartModule restarts a module on a robot. func restartModule( c *cli.Context, From 40be5d11d3376fd780d775bfd32e5f9e2103f110 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 13 Oct 2025 13:07:41 -0400 Subject: [PATCH 2/9] minor cleanup --- cli/module_build.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/module_build.go b/cli/module_build.go index cb33b99d1e4..06946ce72a5 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -773,6 +773,7 @@ func (c *viamClient) triggerCloudReload( return "", errors.New("unable to determine platform for part") } + // App expects `BuildInfo` as the first request platform := part.Part.UserSuppliedInfo.Fields["platform"].GetStringValue() req := &buildpb.StartReloadBuildRequest{ CloudBuild: &buildpb.StartReloadBuildRequest_BuildInfo{ @@ -887,9 +888,6 @@ func (c *viamClient) moduleCloudReload(ctx *cli.Context, args reloadModuleArgs, Platform: platform, } - // CR erodkin: confirm that we don't actually want to delete the package like this anymore. - // good to talk to michael lee to confirm - // delete the archive we created if err := os.Remove(archivePath); err != nil { warningf(ctx.App.Writer, "failed to delete archive at %s", archivePath) @@ -898,6 +896,7 @@ func (c *viamClient) moduleCloudReload(ctx *cli.Context, args reloadModuleArgs, return c.downloadModuleAction(ctx, downloadArgs) } +// ReloadModuleLocalAction builds a module locally, configures it on a robot, and starts or restarts it. func ReloadModuleLocalAction(c *cli.Context, args reloadModuleArgs) error { return reloadModuleAction(c, args, false) } @@ -1149,6 +1148,7 @@ type moduleRestartArgs struct { CloudConfig string } +// ModuleRestartAction triggers a restart of the requested module. func ModuleRestartAction(c *cli.Context, args moduleRestartArgs) error { client, err := newViamClient(c) if err != nil { From 81a533455920e9775e029b12097ab4f231b8950d Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 14 Oct 2025 11:06:54 -0400 Subject: [PATCH 3/9] update api repo --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index b05769c8f02..b3ca337a2dd 100644 --- a/go.mod +++ b/go.mod @@ -84,7 +84,7 @@ require ( go.uber.org/atomic v1.11.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 - go.viam.com/api v0.1.477 + go.viam.com/api v0.1.480 go.viam.com/test v1.2.4 go.viam.com/utils v0.1.172 goji.io v2.0.2+incompatible diff --git a/go.sum b/go.sum index a1d86bebb69..de7632d1842 100644 --- a/go.sum +++ b/go.sum @@ -1540,8 +1540,8 @@ 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.477 h1:huAOmn3iejrRapzlYSyB3R0S47itXTUkQ3+kt0Yx02I= -go.viam.com/api v0.1.477/go.mod h1:p/am76zx8SZ74V/F4rEAYQIpHaaLUwJgY2q3Uw3FIWk= +go.viam.com/api v0.1.480 h1:Q4IGvsl1spot79aKed6SmnJIzWzYrbjG32n4QPFbWkg= +go.viam.com/api v0.1.480/go.mod h1:p/am76zx8SZ74V/F4rEAYQIpHaaLUwJgY2q3Uw3FIWk= 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.172 h1:0sH8+LxKWrS/8o34EDfhoyYd+dhI4ws0C3CWqM8j8iI= From 875d8babcdc6375fb9820f4ef61e0cf1311cea91 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 14 Oct 2025 11:12:59 -0400 Subject: [PATCH 4/9] fix test --- cli/module_reload_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/module_reload_test.go b/cli/module_reload_test.go index ad9b396f311..16acba3853e 100644 --- a/cli/module_reload_test.go +++ b/cli/module_reload_test.go @@ -71,7 +71,7 @@ func TestFullReloadFlow(t *testing.T) { "token", ) test.That(t, vc.loginAction(cCtx), test.ShouldBeNil) - err = reloadModuleAction(cCtx, vc, parseStructFromCtx[reloadModuleArgs](cCtx), logger) + err = reloadModuleActionInner(cCtx, vc, parseStructFromCtx[reloadModuleArgs](cCtx), logger, false) test.That(t, err, test.ShouldBeNil) test.That(t, updateCount, test.ShouldEqual, 1) From ebe9522de8cd36e3d9b64754d4a1be6ddc999499 Mon Sep 17 00:00:00 2001 From: Ethan Date: Tue, 14 Oct 2025 11:15:47 -0400 Subject: [PATCH 5/9] tabs vs spaces --- cli/module_build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/module_build.go b/cli/module_build.go index 2036b77088f..6ad68e9f812 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -936,7 +936,7 @@ func reloadModuleActionInner( vc *viamClient, args reloadModuleArgs, logger logging.Logger, - cloudBuild bool, + cloudBuild bool, ) error { // TODO(RSDK-9727) it'd be nice for this to be a method on a viam client rather than taking one as an arg partID, err := resolvePartID(args.PartID, args.CloudConfig) From 9040fbc5d22a2edb60902b46e4b0e7b01771864a Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 14 Oct 2025 11:49:15 -0400 Subject: [PATCH 6/9] fix tests --- cli/module_reload_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/module_reload_test.go b/cli/module_reload_test.go index f588e52a46b..4ad94905e03 100644 --- a/cli/module_reload_test.go +++ b/cli/module_reload_test.go @@ -170,7 +170,7 @@ func TestFullReloadFlow(t *testing.T) { "token", ) - err = reloadModuleAction(cCtx, vc, parseStructFromCtx[reloadModuleArgs](cCtx), logger) + err = reloadModuleActionInner(cCtx, vc, parseStructFromCtx[reloadModuleArgs](cCtx), logger, false) test.That(t, err, test.ShouldNotBeNil) test.That(t, err.Error(), test.ShouldContainSubstring, "not supported for hot reloading") }) @@ -195,7 +195,7 @@ func TestFullReloadFlow(t *testing.T) { "token", ) - err = reloadModuleAction(cCtx, vc, parseStructFromCtx[reloadModuleArgs](cCtx), logger) + err = reloadModuleActionInner(cCtx, vc, parseStructFromCtx[reloadModuleArgs](cCtx), logger, false) test.That(t, err, test.ShouldBeNil) test.That(t, updateCount, test.ShouldEqual, 1) }) From 05a23cfab3a62bfa682d4a3b1972085defd39c92 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 14 Oct 2025 14:13:30 -0400 Subject: [PATCH 7/9] make lint --- cli/module_build.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/module_build.go b/cli/module_build.go index 6ad68e9f812..f4bd8d587ad 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -22,7 +22,6 @@ import ( "go.uber.org/multierr" "go.uber.org/zap" buildpb "go.viam.com/api/app/build/v1" - packagespb "go.viam.com/api/app/packages/v1" v1 "go.viam.com/api/app/packages/v1" apppb "go.viam.com/api/app/v1" "go.viam.com/utils/pexec" @@ -727,7 +726,7 @@ func (c *viamClient) ensureModuleRegisteredInCloud(ctx *cli.Context, moduleID mo return nil } -func (c *viamClient) inferOrgIDFromManifest(module string, manifest moduleManifest) (string, error) { +func (c *viamClient) inferOrgIDFromManifest(manifest moduleManifest) (string, error) { moduleID, err := parseModuleID(manifest.ModuleID) if err != nil { return "", err @@ -751,12 +750,13 @@ func (c *viamClient) triggerCloudReload( return "", err } + //nolint:gosec file, err := os.Open(archivePath) if err != nil { return "", err } - orgID, err := c.inferOrgIDFromManifest(args.Module, manifest) + orgID, err := c.inferOrgIDFromManifest(manifest) if err != nil { return "", err } @@ -789,14 +789,14 @@ func (c *viamClient) triggerCloudReload( return "", err } - pkgInfo := packagespb.PackageInfo{ + pkgInfo := v1.PackageInfo{ OrganizationId: orgID, Name: reloadVersion, Version: reloadVersion, Type: v1.PackageType_PACKAGE_TYPE_MODULE, } - reqInner := &packagespb.CreatePackageRequest{ - Package: &packagespb.CreatePackageRequest_Info{ + reqInner := &v1.CreatePackageRequest{ + Package: &v1.CreatePackageRequest_Info{ Info: &pkgInfo, }, } @@ -817,7 +817,7 @@ func (c *viamClient) triggerCloudReload( } resp, closeErr := stream.CloseAndRecv() - if err != nil && !errors.Is(closeErr, io.EOF) { + if closeErr != nil && !errors.Is(closeErr, io.EOF) { errs = multierr.Combine(errs, closeErr) } return resp.GetBuildId(), errs @@ -995,7 +995,7 @@ func reloadModuleActionInner( // CLI will see configuration changes before the robot, and skip to the needsRestart // case on the second call. Because these are triggered by user actions, we're okay // with this behavior, and the robot will eventually converge to what is in config. - needsRestart := true + var needsRestart bool var buildPath string if !args.NoBuild { if manifest == nil { From 60935baad3d55ec6311587347046493fee99e58a Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 16 Oct 2025 14:31:53 -0400 Subject: [PATCH 8/9] address pr comment, lingering merge conflict --- cli/module_build.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/module_build.go b/cli/module_build.go index 71156e8a6d7..80382bf4c48 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -739,7 +739,7 @@ func (c *viamClient) inferOrgIDFromManifest(manifest moduleManifest) (string, er return org.GetId(), nil } -func (c *viamClient) triggerCloudReload( +func (c *viamClient) triggerCloudReloadBuild( ctx *cli.Context, args reloadModuleArgs, manifest moduleManifest, @@ -871,7 +871,7 @@ func (c *viamClient) moduleCloudReload( } infof(c.c.App.Writer, "Creating a new cloud build and swapping it onto the requested machine part. This may take a few minutes...") - buildID, err := c.triggerCloudReload(ctx, args, manifest, archivePath, partID) + buildID, err := c.triggerCloudReloadBuild(ctx, args, manifest, archivePath, partID) if err != nil { return "", err } @@ -896,7 +896,7 @@ func (c *viamClient) moduleCloudReload( downloadArgs := downloadModuleFlags{ ID: id, - Version: reloadVersionFormatted, + Version: getReloadVersion(reloadVersionPrefix, partID), Platform: platform, } From 3df81256a9149d385b5a89917dedc04429800edb Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 16 Oct 2025 15:05:14 -0400 Subject: [PATCH 9/9] remove comment to self --- cli/module_build.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/module_build.go b/cli/module_build.go index 80382bf4c48..97879f44d28 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -799,7 +799,6 @@ func (c *viamClient) triggerCloudReloadBuild( Version: getReloadVersion(reloadSourceVersionPrefix, partID), Type: v1.PackageType_PACKAGE_TYPE_MODULE, } - // CR erodkin: the current logic has us using `reloadVersionPrefix` for module building and downloading but `source` prefix for the package. look into this. reqInner := &v1.CreatePackageRequest{ Package: &v1.CreatePackageRequest_Info{ Info: &pkgInfo,