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

[teleport-update] Fix usage of trace #49388

Merged
merged 2 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions lib/autoupdate/agent/config.go
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
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
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
Loading