Skip to content

Commit

Permalink
RSDK-8883: Have package downloading better interpret file does not ex…
Browse files Browse the repository at this point in the history
…ist errors. (#4402)

Those errors represent that we have a new package to download. We should instead
log that intention.
  • Loading branch information
dgottlieb authored Sep 30, 2024
1 parent 24b4ba4 commit d975cf2
Showing 1 changed file with 29 additions and 21 deletions.
50 changes: 29 additions & 21 deletions robot/packages/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"compress/gzip"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"io/fs"
Expand All @@ -13,7 +14,6 @@ import (
"strings"
"time"

"github.com/pkg/errors"
"go.uber.org/multierr"
"go.viam.com/utils"

Expand Down Expand Up @@ -66,7 +66,7 @@ func installPackage(
// unpack to temp directory to ensure we do an atomic rename once finished.
tmpDataPath, err := os.MkdirTemp(p.LocalDataParentDirectory(packagesDir), "*.tmp")
if err != nil {
return errors.Wrap(err, "failed to create temp data dir path")
return fmt.Errorf("failed to create temp data dir path %w", err)
}

defer func() {
Expand Down Expand Up @@ -155,7 +155,7 @@ func unpackFile(ctx context.Context, fromFile, toDir string) error {
}

if err != nil {
return errors.Wrap(err, "read tar")
return fmt.Errorf("read tar %w", err)
}

path := header.Name
Expand All @@ -173,7 +173,7 @@ func unpackFile(ctx context.Context, fromFile, toDir string) error {
switch header.Typeflag {
case tar.TypeDir:
if err := os.Mkdir(path, info.Mode()); err != nil {
return errors.Wrapf(err, "failed to create directory %s", path)
return fmt.Errorf("failed to create directory %q %w", path, err)
}

case tar.TypeReg:
Expand All @@ -182,18 +182,18 @@ func unpackFile(ctx context.Context, fromFile, toDir string) error {
// Ex: tar -czf package.tar.gz ./bin/module.exe
parent := filepath.Dir(path)
if err := os.MkdirAll(parent, 0o700); err != nil {
return errors.Wrapf(err, "failed to create directory %q", parent)
return fmt.Errorf("failed to create directory %q %w", parent, err)
}
//nolint:gosec // path sanitized with rutils.SafeJoin
outFile, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o600|info.Mode().Perm())
if err != nil {
return errors.Wrapf(err, "failed to create file %s", path)
return fmt.Errorf("failed to create file %q %w", path, err)
}
if _, err := io.CopyN(outFile, tarReader, maxPackageSize); err != nil && !errors.Is(err, io.EOF) {
return errors.Wrapf(err, "failed to copy file %s", path)
return fmt.Errorf("failed to copy file %q %w", path, err)
}
if err := outFile.Sync(); err != nil {
return errors.Wrapf(err, "failed to sync %s", path)
return fmt.Errorf("failed to sync %q %w", path, err)
}
utils.UncheckedError(outFile.Close())

Expand All @@ -219,7 +219,7 @@ func unpackFile(ctx context.Context, fromFile, toDir string) error {
return err
}
if err := linkFile(links[i].Name, links[i].Path); err != nil {
return errors.Wrapf(err, "failed to create link %s", links[i].Path)
return fmt.Errorf("failed to create link %q %w", links[i].Path, err)
}
}

Expand All @@ -228,7 +228,7 @@ func unpackFile(ctx context.Context, fromFile, toDir string) error {
return err
}
if err := linkFile(symlinks[i].Name, symlinks[i].Path); err != nil {
return errors.Wrapf(err, "failed to create link %s", links[i].Path)
return fmt.Errorf("failed to create link %q %w", links[i].Path, err)
}
}

Expand Down Expand Up @@ -308,16 +308,24 @@ var statusFileExt = ".status.json"

func packageIsSynced(pkg config.PackageConfig, packagesDir string, logger logging.Logger) bool {
syncFile, err := readStatusFile(pkg, packagesDir)
if err != nil {
utils.UncheckedError(err)
}
if syncFile.PackageID == pkg.Package && syncFile.Version == pkg.Version && syncFile.Status == syncStatusDone {
switch {
case errors.Is(err, fs.ErrNotExist):
logger.Infow("New package to download detected", "name", pkg.Name, "version", pkg.Version, "id", pkg.Package)
return false
case err != nil:
logger.Warnw("Error reading status file for package",
// We prefix log output with "package" to avoid ambiguity. E.g: `name` could mean
// filename given the log line context.
"packageName", pkg.Name, "packageVersion", pkg.Version, "packageId", pkg.Package, "packagesDir", packagesDir, "err", err)
return false
case syncFile.PackageID == pkg.Package && syncFile.Version == pkg.Version && syncFile.Status == syncStatusDone:
logger.Debugf("Package already downloaded at %s, skipping.", pkg.LocalDataDirectory(packagesDir))
return true
default:
logger.Infof("Package is not in sync for %s: status of '%s' (file) != '%s' (expected) and version of '%s' (file) != '%s' (expected)",
pkg.Package, syncFile.Status, syncStatusDone, syncFile.Version, pkg.Version)
return false
}
logger.Debugf("Package is not in sync for %s: status of '%s' (file) != '%s' (expected) and version of '%s' (file) != '%s' (expected)",
pkg.Package, syncFile.Status, syncStatusDone, syncFile.Version, pkg.Version)
return false
}

func packagesAreSynced(packages []config.PackageConfig, packagesDir string, logger logging.Logger) bool {
Expand All @@ -339,7 +347,7 @@ func readStatusFile(pkg config.PackageConfig, packagesDir string) (packageSyncFi
syncFileBytes, err := os.ReadFile(syncFileName)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
return packageSyncFile{}, errors.Wrapf(err, "cannot find %s", syncFileName)
return packageSyncFile{}, fmt.Errorf("cannot find %q %w", syncFileName, err)
}
return packageSyncFile{}, err
}
Expand All @@ -360,13 +368,13 @@ func writeStatusFile(pkg config.PackageConfig, statusFile packageSyncFile, packa
//nolint:gosec
syncFile, err := os.Create(syncFileName)
if err != nil {
return errors.Wrapf(err, "failed to create %s", syncFileName)
return fmt.Errorf("failed to create %q %w", syncFileName, err)
}
if _, err := syncFile.Write(statusFileBytes); err != nil {
return errors.Wrapf(err, "failed to write syncfile to %s", syncFileName)
return fmt.Errorf("failed to write syncfile to %q %w", syncFileName, err)
}
if err := syncFile.Sync(); err != nil {
return errors.Wrapf(err, "failed to sync %s", syncFileName)
return fmt.Errorf("failed to sync %q %w", syncFileName, err)
}

return nil
Expand Down

0 comments on commit d975cf2

Please sign in to comment.