Skip to content

Commit

Permalink
[teleport-update] Fix usage of trace (#49388)
Browse files Browse the repository at this point in the history 
* fix trace

* rebase
  • Loading branch information
@sclevine
sclevine authored Dec 4, 2024
1 parent bbfeabf commit 7eeed25
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 77 deletions.
4 changes: 2 additions & 2 deletions lib/autoupdate/agent/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ func readConfig(path string) (*UpdateConfig, error) {
}, nil
}
if err != nil {
return nil, trace.Errorf("failed to open: %w", err)
return nil, trace.Wrap(err, "failed to open")
}
defer f.Close()
var cfg UpdateConfig
if err := yaml.NewDecoder(f).Decode(&cfg); err != nil {
return nil, trace.Errorf("failed to parse: %w", err)
return nil, trace.Wrap(err, "failed to parse")
}
if k := cfg.Kind; k != updateConfigKind {
return nil, trace.Errorf("invalid kind %q", k)
Expand Down
48 changes: 24 additions & 24 deletions lib/autoupdate/agent/installer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ func (li *LocalInstaller) Remove(ctx context.Context, version string) error {

linked, err := li.isLinked(filepath.Join(versionDir, "bin"))
if err != nil && !errors.Is(err, os.ErrNotExist) {
return trace.Errorf("failed to determine if linked: %w", err)
return trace.Wrap(err, "failed to determine if linked")
}
if linked {
return trace.Errorf("refusing to remove: %w", ErrLinked)
return trace.Wrap(ErrLinked, "refusing to remove")
}

// invalidate checksum first, to protect against partially-removed
Expand Down Expand Up @@ -142,7 +142,7 @@ func (li *LocalInstaller) Install(ctx context.Context, version, template string,
checksumURI := uri + "." + checksumType
newSum, err := li.getChecksum(ctx, checksumURI)
if err != nil {
return trace.Errorf("failed to download checksum from %s: %w", checksumURI, err)
return trace.Wrap(err, "failed to download checksum from %s", checksumURI)
}
oldSum, err := readChecksum(sumPath)
if err == nil {
Expand All @@ -164,11 +164,11 @@ func (li *LocalInstaller) Install(ctx context.Context, version, template string,
// Verify that we have enough free temp space, then download tgz
freeTmp, err := utils.FreeDiskWithReserve(os.TempDir(), li.ReservedFreeTmpDisk)
if err != nil {
return trace.Errorf("failed to calculate free disk: %w", err)
return trace.Wrap(err, "failed to calculate free disk")
}
f, err := os.CreateTemp("", "teleport-update-")
if err != nil {
return trace.Errorf("failed to create temporary file: %w", err)
return trace.Wrap(err, "failed to create temporary file")
}
defer func() {
_ = f.Close() // data never read after close
Expand All @@ -178,11 +178,11 @@ func (li *LocalInstaller) Install(ctx context.Context, version, template string,
}()
pathSum, err := li.download(ctx, f, int64(freeTmp), uri)
if err != nil {
return trace.Errorf("failed to download teleport: %w", err)
return trace.Wrap(err, "failed to download teleport")
}
// Seek to the start of the tgz file after writing
if _, err := f.Seek(0, io.SeekStart); err != nil {
return trace.Errorf("failed seek to start of download: %w", err)
return trace.Wrap(err, "failed seek to start of download")
}

// If interrupted, close the file immediately to stop extracting.
Expand All @@ -198,11 +198,11 @@ func (li *LocalInstaller) Install(ctx context.Context, version, template string,
// Get uncompressed size of the tgz
n, err := uncompressedSize(f)
if err != nil {
return trace.Errorf("failed to determine uncompressed size: %w", err)
return trace.Wrap(err, "failed to determine uncompressed size")
}
// Seek to start of tgz after reading size
if _, err := f.Seek(0, io.SeekStart); err != nil {
return trace.Errorf("failed seek to start: %w", err)
return trace.Wrap(err, "failed seek to start")
}

// If there's an error after we start extracting, delete the version dir.
Expand All @@ -216,12 +216,12 @@ func (li *LocalInstaller) Install(ctx context.Context, version, template string,

// Extract tgz into version directory.
if err := li.extract(ctx, versionDir, f, n, flags); err != nil {
return trace.Errorf("failed to extract teleport: %w", err)
return trace.Wrap(err, "failed to extract teleport")
}
// Write the checksum last. This marks the version directory as valid.
err = renameio.WriteFile(sumPath, []byte(hex.EncodeToString(newSum)), configFileMode)
if err != nil {
return trace.Errorf("failed to write checksum: %w", err)
return trace.Wrap(err, "failed to write checksum")
}
return nil
}
Expand Down Expand Up @@ -346,15 +346,15 @@ func (li *LocalInstaller) extract(ctx context.Context, dstDir string, src io.Rea
}
free, err := utils.FreeDiskWithReserve(dstDir, li.ReservedFreeInstallDisk)
if err != nil {
return trace.Errorf("failed to calculate free disk in %q: %w", dstDir, err)
return trace.Wrap(err, "failed to calculate free disk in %q", dstDir)
}
// Bail if there's not enough free disk space at the target
if d := int64(free) - max; d < 0 {
return trace.Errorf("%q needs %d additional bytes of disk space for decompression", dstDir, -d)
}
zr, err := gzip.NewReader(src)
if err != nil {
return trace.Errorf("requires gzip-compressed body: %w", err)
return trace.Wrap(err, "requires gzip-compressed body")
}
li.Log.InfoContext(ctx, "Extracting Teleport tarball.", "path", dstDir, "size", max)

Expand Down Expand Up @@ -551,7 +551,7 @@ func (li *LocalInstaller) forceLinks(ctx context.Context, binDir, svcPath string

entries, err := os.ReadDir(binDir)
if err != nil {
return revert, trace.Errorf("failed to find Teleport binary directory: %w", err)
return revert, trace.Wrap(err, "failed to find Teleport binary directory")
}
var linked int
for _, entry := range entries {
Expand All @@ -562,7 +562,7 @@ func (li *LocalInstaller) forceLinks(ctx context.Context, binDir, svcPath string
newname := filepath.Join(li.LinkBinDir, entry.Name())
orig, err := forceLink(oldname, newname)
if err != nil && !errors.Is(err, os.ErrExist) {
return revert, trace.Errorf("failed to create symlink for %s: %w", filepath.Base(oldname), err)
return revert, trace.Wrap(err, "failed to create symlink for %s", filepath.Base(oldname))
}
if orig != "" {
revertLinks = append(revertLinks, symlink{
Expand All @@ -580,7 +580,7 @@ func (li *LocalInstaller) forceLinks(ctx context.Context, binDir, svcPath string

orig, err := li.forceCopyService(li.CopyServiceFile, svcPath, maxServiceFileSize)
if err != nil && !errors.Is(err, os.ErrExist) {
return revert, trace.Errorf("failed to copy service: %w", err)
return revert, trace.Wrap(err, "failed to copy service")
}
if orig != nil {
revertFiles = append(revertFiles, *orig)
Expand Down Expand Up @@ -688,7 +688,7 @@ func (li *LocalInstaller) removeLinks(ctx context.Context, binDir, svcPath strin
removeService := false
entries, err := os.ReadDir(binDir)
if err != nil {
return trace.Errorf("failed to find Teleport binary directory: %w", err)
return trace.Wrap(err, "failed to find Teleport binary directory")
}
for _, entry := range entries {
if entry.IsDir() {
Expand All @@ -704,7 +704,7 @@ func (li *LocalInstaller) removeLinks(ctx context.Context, binDir, svcPath strin
continue
}
if err != nil {
return trace.Errorf("error reading link for %s: %w", filepath.Base(newname), err)
return trace.Wrap(err, "error reading link for %s", filepath.Base(newname))
}
if v != oldname {
li.Log.DebugContext(ctx, "Skipping link to different binary.", "oldname", oldname, "newname", newname)
Expand Down Expand Up @@ -740,7 +740,7 @@ func (li *LocalInstaller) removeLinks(ctx context.Context, binDir, svcPath strin
return nil
}
if err := os.Remove(li.CopyServiceFile); err != nil {
return trace.Errorf("error removing copy of %s: %w", filepath.Base(li.CopyServiceFile), err)
return trace.Wrap(err, "error removing copy of %s", filepath.Base(li.CopyServiceFile))
}
return nil
}
Expand All @@ -766,7 +766,7 @@ func (li *LocalInstaller) tryLinks(ctx context.Context, binDir, svcPath string)
var linked int
entries, err := os.ReadDir(binDir)
if err != nil {
return trace.Errorf("failed to find Teleport binary directory: %w", err)
return trace.Wrap(err, "failed to find Teleport binary directory")
}
for _, entry := range entries {
if entry.IsDir() {
Expand All @@ -776,7 +776,7 @@ func (li *LocalInstaller) tryLinks(ctx context.Context, binDir, svcPath string)
newname := filepath.Join(li.LinkBinDir, entry.Name())
ok, err := needsLink(oldname, newname)
if err != nil {
return trace.Errorf("error evaluating link for %s: %w", filepath.Base(oldname), err)
return trace.Wrap(err, "error evaluating link for %s", filepath.Base(oldname))
}
if ok {
links = append(links, symlink{oldname, newname})
Expand All @@ -791,14 +791,14 @@ func (li *LocalInstaller) tryLinks(ctx context.Context, binDir, svcPath string)
// link binaries that are missing links
for _, link := range links {
if err := os.Symlink(link.oldname, link.newname); err != nil {
return trace.Errorf("failed to create symlink for %s: %w", filepath.Base(link.oldname), err)
return trace.Wrap(err, "failed to create symlink for %s", filepath.Base(link.oldname))
}
}

// if any binaries are linked from binDir, always link the service from svcDir
_, err = li.forceCopyService(li.CopyServiceFile, svcPath, maxServiceFileSize)
if err != nil && !errors.Is(err, os.ErrExist) {
return trace.Errorf("failed to copy service: %w", err)
return trace.Wrap(err, "failed to copy service")
}

return nil
Expand Down Expand Up @@ -828,7 +828,7 @@ func needsLink(oldname, newname string) (ok bool, err error) {
return false, trace.Wrap(err)
}
if orig != oldname {
return false, trace.Errorf("refusing to replace link at %s: %w", newname, ErrLinked)
return false, trace.Wrap(ErrLinked, "refusing to replace link at %s", newname)
}
return false, nil
}
Expand Down
16 changes: 8 additions & 8 deletions lib/autoupdate/agent/setup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,17 @@ func (ns *Namespace) Init() (lockFile string, err error) {
func (ns *Namespace) Setup(ctx context.Context) error {
err := ns.writeConfigFiles()
if err != nil {
return trace.Errorf("failed to write teleport-update systemd config files: %w", err)
return trace.Wrap(err, "failed to write teleport-update systemd config files")
}
svc := &SystemdService{
ServiceName: filepath.Base(ns.updaterTimerFile),
Log: ns.log,
}
if err := svc.Sync(ctx); err != nil {
return trace.Errorf("failed to sync systemd config: %w", err)
return trace.Wrap(err, "failed to sync systemd config")
}
if err := svc.Enable(ctx, true); err != nil {
return trace.Errorf("failed to enable teleport-update systemd timer: %w", err)
return trace.Wrap(err, "failed to enable teleport-update systemd timer")
}
return nil
}
Expand All @@ -202,19 +202,19 @@ func (ns *Namespace) Teardown(ctx context.Context) error {
Log: ns.log,
}
if err := svc.Disable(ctx); err != nil {
return trace.Errorf("failed to disable teleport-update systemd timer: %w", err)
return trace.Wrap(err, "failed to disable teleport-update systemd timer")
}
if err := os.Remove(ns.updaterServiceFile); err != nil && !errors.Is(err, fs.ErrNotExist) {
return trace.Errorf("failed to remove teleport-update systemd service: %w", err)
return trace.Wrap(err, "failed to remove teleport-update systemd service")
}
if err := os.Remove(ns.updaterTimerFile); err != nil && !errors.Is(err, fs.ErrNotExist) {
return trace.Errorf("failed to remove teleport-update systemd timer: %w", err)
return trace.Wrap(err, "failed to remove teleport-update systemd timer")
}
if err := svc.Sync(ctx); err != nil {
return trace.Errorf("failed to sync systemd config: %w", err)
return trace.Wrap(err, "failed to sync systemd config")
}
if err := os.RemoveAll(ns.versionsDir); err != nil {
return trace.Errorf("failed to remove versions directory: %w", err)
return trace.Wrap(err, "failed to remove versions directory")
}
return nil
}
Expand Down
Loading

0 comments on commit 7eeed25

Please sign in to comment.