The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/distrobuilder/pull/281
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) ===
From 5120ea1ae041659916746f9897085bda2f2cfd4e Mon Sep 17 00:00:00 2001 From: Thomas Hipp <thomas.h...@canonical.com> Date: Wed, 19 Feb 2020 11:46:35 +0100 Subject: [PATCH 1/3] *: Use errors.Wrap() when possible Signed-off-by: Thomas Hipp <thomas.h...@canonical.com> --- distrobuilder/chroot.go | 4 ++-- distrobuilder/main.go | 18 +++++++++--------- distrobuilder/main_lxc.go | 6 +++--- distrobuilder/main_lxd.go | 4 ++-- generators/cloud-init.go | 3 ++- generators/hostname.go | 6 +++--- generators/template.go | 3 ++- generators/upstart_tty.go | 6 +++--- image/lxc.go | 15 ++++++++------- managers/pacman.go | 6 +++--- shared/chroot.go | 16 +++++++++------- shared/definition.go | 4 ++-- sources/centos-http.go | 4 ++-- sources/opensuse-http.go | 12 ++++++------ sources/openwrt-http.go | 6 +++--- sources/oraclelinux-http.go | 5 +++-- sources/plamolinux-http.go | 5 +++-- sources/sabayon.go | 3 ++- sources/ubuntu-http.go | 8 ++++---- 19 files changed, 71 insertions(+), 63 deletions(-) diff --git a/distrobuilder/chroot.go b/distrobuilder/chroot.go index a38368b2..6c23f7f6 100644 --- a/distrobuilder/chroot.go +++ b/distrobuilder/chroot.go @@ -44,7 +44,7 @@ func manageRepositories(def *shared.Definition, manager *managers.Manager, image err = manager.RepoHandler(repo) if err != nil { - return fmt.Errorf("Error for repository %s: %s", repo.Name, err) + return errors.Wrapf(err, "Error for repository %s", repo.Name) } } @@ -69,7 +69,7 @@ func managePackages(def *shared.Definition, manager *managers.Manager, imageTarg for _, action := range def.GetRunnableActions("post-update", imageTarget) { err = shared.RunScript(action.Action) if err != nil { - return fmt.Errorf("Failed to run post-update: %s", err) + return errors.Wrap(err, "Failed to run post-update") } } } diff --git a/distrobuilder/main.go b/distrobuilder/main.go index ff8131ee..2d4c542d 100644 --- a/distrobuilder/main.go +++ b/distrobuilder/main.go @@ -54,7 +54,6 @@ import "C" import ( "bufio" "bytes" - "errors" "fmt" "io" "io/ioutil" @@ -64,6 +63,7 @@ import ( "strings" "time" + "github.com/pkg/errors" "github.com/spf13/cobra" "gopkg.in/yaml.v2" @@ -241,20 +241,20 @@ func (c *cmdGlobal) preRunBuild(cmd *cobra.Command, args []string) error { for i, key := range c.definition.Source.Keys { c.definition.Source.Keys[i], err = shared.RenderTemplate(key, c.definition) if err != nil { - return fmt.Errorf("Failed to render source keys: %s", err) + return errors.Wrap(err, "Failed to render source keys") } } // Download the root filesystem err = downloader.Run(*c.definition, c.sourceDir) if err != nil { - return fmt.Errorf("Error while downloading source: %s", err) + return errors.Wrap(err, "Error while downloading source") } // Setup the mounts and chroot into the rootfs exitChroot, err := shared.SetupChroot(c.sourceDir, c.definition.Environment, nil) if err != nil { - return fmt.Errorf("Failed to setup chroot: %s", err) + return errors.Wrap(err, "Failed to setup chroot") } // Unmount everything and exit the chroot defer exitChroot() @@ -293,28 +293,28 @@ func (c *cmdGlobal) preRunBuild(cmd *cobra.Command, args []string) error { err = manageRepositories(c.definition, manager, imageTargets) if err != nil { - return fmt.Errorf("Failed to manage repositories: %s", err) + return errors.Wrap(err, "Failed to manage repositories") } // Run post unpack hook for _, hook := range c.definition.GetRunnableActions("post-unpack", imageTargets) { err := shared.RunScript(hook.Action) if err != nil { - return fmt.Errorf("Failed to run post-unpack: %s", err) + return errors.Wrap(err, "Failed to run post-unpack") } } // Install/remove/update packages err = managePackages(c.definition, manager, imageTargets) if err != nil { - return fmt.Errorf("Failed to manage packages: %s", err) + return errors.Wrap(err, "Failed to manage packages") } // Run post packages hook for _, hook := range c.definition.GetRunnableActions("post-packages", imageTargets) { err := shared.RunScript(hook.Action) if err != nil { - return fmt.Errorf("Failed to run post-packages: %s", err) + return errors.Wrap(err, "Failed to run post-packages") } } @@ -397,7 +397,7 @@ func getDefinition(fname string, options []string) (*shared.Definition, error) { err := def.SetValue(parts[0], parts[1]) if err != nil { - return nil, fmt.Errorf("Failed to set option %s: %s", o, err) + return nil, errors.Wrapf(err, "Failed to set option %s", o) } } diff --git a/distrobuilder/main_lxc.go b/distrobuilder/main_lxc.go index 3f0204e9..766136c0 100644 --- a/distrobuilder/main_lxc.go +++ b/distrobuilder/main_lxc.go @@ -144,7 +144,7 @@ func (c *cmdLXC) run(cmd *cobra.Command, args []string, overlayDir string) error err := shared.RunScript(action.Action) if err != nil { exitChroot() - return fmt.Errorf("Failed to run post-files: %s", err) + return errors.Wrap(err, "Failed to run post-files") } } @@ -152,13 +152,13 @@ func (c *cmdLXC) run(cmd *cobra.Command, args []string, overlayDir string) error err = img.Build() if err != nil { - return fmt.Errorf("Failed to create LXC image: %s", err) + return errors.Wrap(err, "Failed to create LXC image") } // Clean up the chroot by restoring the orginal files. err = generators.RestoreFiles(c.global.flagCacheDir, overlayDir) if err != nil { - return fmt.Errorf("Failed to restore cached files: %s", err) + return errors.Wrap(err, "Failed to restore cached files") } return nil diff --git a/distrobuilder/main_lxd.go b/distrobuilder/main_lxd.go index 38efc123..119aa4e1 100644 --- a/distrobuilder/main_lxd.go +++ b/distrobuilder/main_lxd.go @@ -293,7 +293,7 @@ func (c *cmdLXD) run(cmd *cobra.Command, args []string, overlayDir string) error err := shared.RunScript(action.Action) if err != nil { exitChroot() - return fmt.Errorf("Failed to run post-files: %s", err) + return errors.Wrap(err, "Failed to run post-files") } } @@ -301,7 +301,7 @@ func (c *cmdLXD) run(cmd *cobra.Command, args []string, overlayDir string) error err = img.Build(c.flagType == "unified", c.flagCompression, c.flagVM) if err != nil { - return fmt.Errorf("Failed to create LXD image: %s", err) + return errors.Wrap(err, "Failed to create LXD image") } return nil diff --git a/generators/cloud-init.go b/generators/cloud-init.go index 5b2507b9..29a3f9b3 100644 --- a/generators/cloud-init.go +++ b/generators/cloud-init.go @@ -9,6 +9,7 @@ import ( lxd "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" + "github.com/pkg/errors" "github.com/lxc/distrobuilder/image" "github.com/lxc/distrobuilder/shared" @@ -161,7 +162,7 @@ config: _, err = file.WriteString(content) if err != nil { - return fmt.Errorf("Failed to write to content to %s template: %s", defFile.Name, err) + return errors.Wrapf(err, "Failed to write to content to %s template", defFile.Name) } if len(defFile.Template.Properties) > 0 { diff --git a/generators/hostname.go b/generators/hostname.go index 1539d25e..d8f8ccf6 100644 --- a/generators/hostname.go +++ b/generators/hostname.go @@ -1,12 +1,12 @@ package generators import ( - "fmt" "os" "path/filepath" lxd "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" + "github.com/pkg/errors" "github.com/lxc/distrobuilder/image" "github.com/lxc/distrobuilder/shared" @@ -40,7 +40,7 @@ func (g HostnameGenerator) RunLXC(cacheDir, sourceDir string, img *image.LXCImag // Write LXC specific string to the hostname file _, err = file.WriteString("LXC_NAME\n") if err != nil { - return fmt.Errorf("Failed to write to hostname file: %s", err) + return errors.Wrap(err, "Failed to write to hostname file") } // Add hostname path to LXC's templates file @@ -71,7 +71,7 @@ func (g HostnameGenerator) RunLXD(cacheDir, sourceDir string, img *image.LXDImag _, err = file.WriteString("{{ container.name }}\n") if err != nil { - return fmt.Errorf("Failed to write to hostname file: %s", err) + return errors.Wrap(err, "Failed to write to hostname file") } // Add to LXD templates diff --git a/generators/template.go b/generators/template.go index 4c1f5aa8..82a593a3 100644 --- a/generators/template.go +++ b/generators/template.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/lxc/lxd/shared/api" + "github.com/pkg/errors" "github.com/lxc/distrobuilder/image" "github.com/lxc/distrobuilder/shared" @@ -47,7 +48,7 @@ func (g TemplateGenerator) RunLXD(cacheDir, sourceDir string, img *image.LXDImag _, err = file.WriteString(defFile.Content) if err != nil { - return fmt.Errorf("Failed to write to content to %s template: %s", defFile.Name, err) + return errors.Wrapf(err, "Failed to write to content to %s template", defFile.Name) } // Add to LXD templates diff --git a/generators/upstart_tty.go b/generators/upstart_tty.go index a861ff81..afb2e32a 100644 --- a/generators/upstart_tty.go +++ b/generators/upstart_tty.go @@ -1,12 +1,12 @@ package generators import ( - "fmt" "os" "path/filepath" lxd "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" + "github.com/pkg/errors" "github.com/lxc/distrobuilder/image" "github.com/lxc/distrobuilder/shared" @@ -74,7 +74,7 @@ func (g UpstartTTYGenerator) RunLXC(cacheDir, sourceDir string, img *image.LXCIm // Write LXC specific string to the hostname file _, err = file.WriteString(upstartTTYJob) if err != nil { - return fmt.Errorf("Failed to write to upstart job file: %s", err) + return errors.Wrap(err, "Failed to write to upstart job file") } // Add hostname path to LXC's templates file @@ -105,7 +105,7 @@ func (g UpstartTTYGenerator) RunLXD(cacheDir, sourceDir string, img *image.LXDIm _, err = file.WriteString(upstartTTYJob) if err != nil { - return fmt.Errorf("Failed to write to upstart job file: %s", err) + return errors.Wrap(err, "Failed to write to upstart job file") } // Add to LXD templates diff --git a/image/lxc.go b/image/lxc.go index a3f0bb41..15198eb8 100644 --- a/image/lxc.go +++ b/image/lxc.go @@ -8,6 +8,7 @@ import ( "time" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "github.com/lxc/distrobuilder/shared" ) @@ -53,7 +54,7 @@ func (l *LXCImage) AddTemplate(path string) error { _, err = file.WriteString(fmt.Sprintf("%v\n", path)) if err != nil { - return fmt.Errorf("Failed to write to template file: %s", err) + return errors.Wrap(err, "Failed to write to template file") } return nil @@ -128,14 +129,14 @@ func (l *LXCImage) createMetadata() error { err := l.writeMetadata(filepath.Join(metaDir, "create-message"), l.definition.Targets.LXC.CreateMessage, false) if err != nil { - return fmt.Errorf("Error writing 'create-message': %s", err) + return errors.Wrap(err, "Error writing 'create-message'") } err = l.writeMetadata(filepath.Join(metaDir, "expiry"), fmt.Sprint(shared.GetExpiryDate(time.Now(), l.definition.Image.Expiry).Unix()), false) if err != nil { - return fmt.Errorf("Error writing 'expiry': %s", err) + return errors.Wrap(err, "Error writing 'expiry'") } var excludesUser string @@ -155,13 +156,13 @@ func (l *LXCImage) createMetadata() error { return nil }) if err != nil { - return fmt.Errorf("Error while walking /dev: %s", err) + return errors.Wrap(err, "Error while walking /dev") } } err = l.writeMetadata(filepath.Join(metaDir, "excludes-user"), excludesUser, false) if err != nil { - return fmt.Errorf("Error writing 'excludes-user': %s", err) + return errors.Wrap(err, "Error writing 'excludes-user'") } return nil @@ -187,7 +188,7 @@ func (l *LXCImage) packMetadata() error { err = shared.Pack(filepath.Join(l.targetDir, "meta.tar"), "xz", filepath.Join(l.cacheDir, "metadata"), files...) if err != nil { - return fmt.Errorf("Failed to create metadata: %s", err) + return errors.Wrap(err, "Failed to create metadata") } return nil @@ -236,7 +237,7 @@ func (l *LXCImage) writeConfig(compatLevel uint, filename, content string) error } err := l.writeMetadata(filename, content, true) if err != nil { - return fmt.Errorf("Error writing '%s': %s", filepath.Base(filename), err) + return errors.Wrapf(err, "Error writing '%s'", filepath.Base(filename)) } return nil diff --git a/managers/pacman.go b/managers/pacman.go index 62473865..d4e947b9 100644 --- a/managers/pacman.go +++ b/managers/pacman.go @@ -1,12 +1,12 @@ package managers import ( - "fmt" "os" "path/filepath" "runtime" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "github.com/lxc/distrobuilder/shared" ) @@ -71,7 +71,7 @@ func pacmanSetupTrustedKeys() error { err = shared.RunCommand("pacman-key", "--init") if err != nil { - return fmt.Errorf("Error initializing with pacman-key: %s", err) + return errors.Wrap(err, "Error initializing with pacman-key") } var keyring string @@ -84,7 +84,7 @@ func pacmanSetupTrustedKeys() error { err = shared.RunCommand("pacman-key", "--populate", keyring) if err != nil { - return fmt.Errorf("Error populating with pacman-key: %s", err) + return errors.Wrap(err, "Error populating with pacman-key") } return nil diff --git a/shared/chroot.go b/shared/chroot.go index 1c652b33..e7fc020e 100644 --- a/shared/chroot.go +++ b/shared/chroot.go @@ -10,6 +10,8 @@ import ( "syscall" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" + "golang.org/x/sys/unix" ) // ChrootMount defines mount args. @@ -52,7 +54,7 @@ func setupMounts(rootfs string, mounts []ChrootMount) error { // Mount to the temporary path err := syscall.Mount(mount.Source, tmpTarget, mount.FSType, mount.Flags, mount.Data) if err != nil { - return fmt.Errorf("Failed to mount '%s': %s", mount.Source, err) + return errors.Wrapf(err, "Failed to mount '%s'", mount.Source) } } @@ -109,7 +111,7 @@ func moveMounts(mounts []ChrootMount) error { // Move the mount to its destination err = syscall.Mount(tmpSource, target, "", syscall.MS_MOVE, "") if err != nil { - return fmt.Errorf("Failed to mount '%s': %s", mount.Source, err) + return errors.Wrapf(err, "Failed to mount '%s'", mount.Source) } } @@ -156,7 +158,7 @@ func SetupChroot(rootfs string, envs DefinitionEnv, m []ChrootMount) (func() err // Mount the rootfs err := syscall.Mount(rootfs, rootfs, "", syscall.MS_BIND, "") if err != nil { - return nil, fmt.Errorf("Failed to mount '%s': %s", rootfs, err) + return nil, errors.Wrapf(err, "Failed to mount '%s'", rootfs) } // Setup all other needed mounts @@ -187,17 +189,17 @@ func SetupChroot(rootfs string, envs DefinitionEnv, m []ChrootMount) (func() err err = setupMounts(rootfs, mounts) } if err != nil { - return nil, fmt.Errorf("Failed to mount filesystems: %v", err) + return nil, errors.Wrap(err, "Failed to mount filesystems") } // Chroot into the container's rootfs - err = syscall.Chroot(rootfs) + err = unix.Chroot(rootfs) if err != nil { root.Close() return nil, err } - err = syscall.Chdir("/") + err = unix.Chdir("/") if err != nil { return nil, err } @@ -299,7 +301,7 @@ exit 101 killChrootProcesses(rootfs) // And now unmount the entire tree - syscall.Unmount(rootfs, syscall.MNT_DETACH) + unix.Unmount(rootfs, syscall.MNT_DETACH) devPath := filepath.Join(rootfs, "dev") diff --git a/shared/definition.go b/shared/definition.go index 6f651e81..a61e4d3a 100644 --- a/shared/definition.go +++ b/shared/definition.go @@ -1,7 +1,6 @@ package shared import ( - "errors" "fmt" "reflect" "strconv" @@ -10,6 +9,7 @@ import ( "github.com/lxc/lxd/shared" lxdarch "github.com/lxc/lxd/shared/osarch" + "github.com/pkg/errors" ) // ImageTarget represents the image target. @@ -521,7 +521,7 @@ func (d *Definition) getMappedArchitecture() (string, error) { var err error arch, err = GetArch(d.Mappings.ArchitectureMap, d.Image.Architecture) if err != nil { - return "", fmt.Errorf("Failed to translate the architecture name: %s", err) + return "", errors.Wrap(err, "Failed to translate the architecture name") } } else if len(d.Mappings.Architectures) > 0 { // Translate the architecture using a user specified mapping diff --git a/sources/centos-http.go b/sources/centos-http.go index 98882170..a2cb3d20 100644 --- a/sources/centos-http.go +++ b/sources/centos-http.go @@ -3,7 +3,6 @@ package sources import ( "bytes" "crypto/sha256" - "errors" "fmt" "io/ioutil" "net/http" @@ -16,6 +15,7 @@ import ( "strings" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "golang.org/x/sys/unix" "github.com/lxc/distrobuilder/shared" @@ -111,7 +111,7 @@ func (s *CentOSHTTP) Run(definition shared.Definition, rootfsDir string) error { _, err = shared.DownloadHash(definition.Image, baseURL+s.fname, checksumFile, sha256.New()) if err != nil { - return fmt.Errorf("Error downloading CentOS image: %s", err) + return errors.Wrap(err, "Error downloading CentOS image") } if strings.HasSuffix(s.fname, ".raw.xz") || strings.HasSuffix(s.fname, ".raw") { diff --git a/sources/opensuse-http.go b/sources/opensuse-http.go index 9d7b224e..aacf6a3c 100644 --- a/sources/opensuse-http.go +++ b/sources/opensuse-http.go @@ -2,7 +2,6 @@ package sources import ( "crypto/sha256" - "errors" "fmt" "io" "net/http" @@ -15,6 +14,7 @@ import ( "strings" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "gopkg.in/antchfx/htmlquery.v1" "github.com/lxc/distrobuilder/shared" @@ -49,7 +49,7 @@ func (s *OpenSUSEHTTP) Run(definition shared.Definition, rootfsDir string) error resp, err := http.Head(tarballPath) if err != nil { - return fmt.Errorf("Couldn't resolve URL: %v", err) + return errors.Wrap(err, "Couldn't resolve URL") } baseURL, fname = path.Split(resp.Request.URL.String()) @@ -61,7 +61,7 @@ func (s *OpenSUSEHTTP) Run(definition shared.Definition, rootfsDir string) error fpath, err := shared.DownloadHash(definition.Image, url.String(), "", nil) if err != nil { - return fmt.Errorf("Error downloading openSUSE image: %s", err) + return errors.Wrap(err, "Error downloading openSUSE image") } if definition.Source.SkipVerification { @@ -86,21 +86,21 @@ func (s *OpenSUSEHTTP) Run(definition shared.Definition, rootfsDir string) error checksum, err := shared.GetSignedContent(filepath.Join(fpath, checksumFile), definition.Source.Keys, definition.Source.Keyserver) if err != nil { - return fmt.Errorf("Failed to read signed file: %v", err) + return errors.Wrap(err, "Failed to read signed file") } imagePath := filepath.Join(fpath, fname) image, err := os.Open(imagePath) if err != nil { - return fmt.Errorf("Failed to verify image: %v", err) + return errors.Wrap(err, "Failed to verify image") } hash := sha256.New() _, err = io.Copy(hash, image) if err != nil { image.Close() - return fmt.Errorf("Failed to verify image: %v", err) + return errors.Wrap(err, "Failed to verify image") } image.Close() diff --git a/sources/openwrt-http.go b/sources/openwrt-http.go index 3619863d..cd7d1482 100644 --- a/sources/openwrt-http.go +++ b/sources/openwrt-http.go @@ -3,7 +3,6 @@ package sources import ( "bufio" "crypto/sha256" - "errors" "fmt" "io/ioutil" "net/http" @@ -14,6 +13,7 @@ import ( "strings" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "github.com/lxc/distrobuilder/shared" ) @@ -134,12 +134,12 @@ func (s *OpenWrtHTTP) Run(definition shared.Definition, rootfsDir string) error err = lxd.Unpack(filepath.Join(fpath, "master.tar.gz"), filepath.Join(os.TempDir(), "distrobuilder", "fixes"), false, false, nil) if err != nil { - return fmt.Errorf("Failed to unpack scripts: %v", err) + return errors.Wrap(err, "Failed to unpack scripts") } err = lxd.Unpack(filepath.Join(fpath, sdk), tempSDKDir, false, false, nil) if err != nil { - return fmt.Errorf("Failed to unpack SDK: %v", err) + return errors.Wrap(err, "Failed to unpack SDK") } currentDir, err := os.Getwd() diff --git a/sources/oraclelinux-http.go b/sources/oraclelinux-http.go index 6d21904b..21f99a1e 100644 --- a/sources/oraclelinux-http.go +++ b/sources/oraclelinux-http.go @@ -10,6 +10,7 @@ import ( "strings" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "golang.org/x/sys/unix" "gopkg.in/antchfx/htmlquery.v1" @@ -42,7 +43,7 @@ func (s *OracleLinuxHTTP) Run(definition shared.Definition, rootfsDir string) er fpath, err := shared.DownloadHash(definition.Image, fmt.Sprintf("%s/%s/%s/%s", baseURL, latestUpdate, s.architecture, fname), "", nil) if err != nil { - return fmt.Errorf("Error downloading Oracle Linux image: %s", err) + return errors.Wrap(err, "Error downloading Oracle Linux image") } return s.unpackISO(latestUpdate[1:], filepath.Join(fpath, fname), rootfsDir) @@ -171,7 +172,7 @@ func (s *OracleLinuxHTTP) unpackISO(latestUpdate, filePath, rootfsDir string) er // Setup the mounts and chroot into the rootfs exitChroot, err := shared.SetupChroot(tempRootDir, shared.DefinitionEnv{}, nil) if err != nil { - return fmt.Errorf("Failed to setup chroot: %s", err) + return errors.Wrap(err, "Failed to setup chroot") } err = shared.RunScript(fmt.Sprintf(` diff --git a/sources/plamolinux-http.go b/sources/plamolinux-http.go index 3b72a191..183ffb0a 100644 --- a/sources/plamolinux-http.go +++ b/sources/plamolinux-http.go @@ -9,6 +9,7 @@ import ( "strings" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "gopkg.in/antchfx/htmlquery.v1" "github.com/lxc/distrobuilder/shared" @@ -29,7 +30,7 @@ func (s *PlamoLinuxHTTP) Run(definition shared.Definition, rootfsDir string) err release, err := strconv.Atoi(releaseStr) if err != nil { - return fmt.Errorf("Failed to determine release: %v", err) + return errors.Wrap(err, "Failed to determine release") } u, err := url.Parse(definition.Source.URL) @@ -55,7 +56,7 @@ func (s *PlamoLinuxHTTP) Run(definition shared.Definition, rootfsDir string) err pkgDir, err = s.downloadFiles(definition.Image, u.String(), ignoredPkgs) if err != nil { - return fmt.Errorf("Failed to download packages: %v", err) + return errors.Wrap(err, "Failed to download packages") } } diff --git a/sources/sabayon.go b/sources/sabayon.go index 5393163c..0e0cbd61 100644 --- a/sources/sabayon.go +++ b/sources/sabayon.go @@ -9,6 +9,7 @@ import ( "path/filepath" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "github.com/lxc/distrobuilder/shared" ) @@ -31,7 +32,7 @@ func (s *SabayonHTTP) Run(definition shared.Definition, rootfsDir string) error resp, err := http.Head(tarballPath) if err != nil { - return fmt.Errorf("Couldn't resolve URL: %v", err) + return errors.Wrap(err, "Couldn't resolve URL") } baseURL, fname = path.Split(resp.Request.URL.String()) diff --git a/sources/ubuntu-http.go b/sources/ubuntu-http.go index c7ca68c1..10243893 100644 --- a/sources/ubuntu-http.go +++ b/sources/ubuntu-http.go @@ -2,7 +2,6 @@ package sources import ( "crypto/sha256" - "errors" "fmt" "io" "io/ioutil" @@ -16,6 +15,7 @@ import ( "github.com/gobuffalo/packr/v2" lxd "github.com/lxc/lxd/shared" + "github.com/pkg/errors" "golang.org/x/sys/unix" "github.com/lxc/distrobuilder/shared" @@ -142,7 +142,7 @@ func (s *UbuntuHTTP) runCoreVariant(definition shared.Definition, rootfsDir stri _, err = shared.DownloadHash(definition.Image, coreImage, "", sha256.New()) if err != nil { - return fmt.Errorf("Error downloading base Ubuntu image: %s", err) + return errors.Wrap(err, "Error downloading base Ubuntu image") } err = s.unpack(filepath.Join(s.fpath, "rootfs.tar.xz"), baseImageDir) @@ -347,7 +347,7 @@ func (s *UbuntuHTTP) downloadImage(definition shared.Definition) error { s.fpath, err = shared.DownloadHash(definition.Image, baseURL+s.fname, checksumFile, sha256.New()) if err != nil { - return fmt.Errorf("Error downloading Ubuntu image: %s", err) + return errors.Wrap(err, "Error downloading Ubuntu image") } return nil @@ -359,7 +359,7 @@ func (s UbuntuHTTP) unpack(filePath, rootDir string) error { err := lxd.Unpack(filePath, rootDir, false, false, nil) if err != nil { - return fmt.Errorf("Failed to unpack tarball: %s", err) + return errors.Wrap(err, "Failed to unpack tarball") } return nil From e9c46bea6e121b9a7665571cc8fe5919a96f59bb Mon Sep 17 00:00:00 2001 From: Thomas Hipp <thomas.h...@canonical.com> Date: Wed, 19 Feb 2020 12:02:03 +0100 Subject: [PATCH 2/3] generators: Remove StoreFile and RestoreFiles This removes the functions StoreFile and RestoreFiles. They are not needed anymore since distrobuilder now uses overlayfs. Signed-off-by: Thomas Hipp <thomas.h...@canonical.com> --- distrobuilder/main_lxc.go | 6 --- generators/cloud-init.go | 16 ++---- generators/cloud-init_test.go | 21 -------- generators/generators.go | 92 ----------------------------------- generators/generators_test.go | 43 ---------------- generators/hostname.go | 6 --- generators/hostname_test.go | 6 --- generators/hosts.go | 6 --- generators/hosts_test.go | 8 --- generators/upstart_tty.go | 6 --- 10 files changed, 3 insertions(+), 207 deletions(-) diff --git a/distrobuilder/main_lxc.go b/distrobuilder/main_lxc.go index 766136c0..2ca3b98f 100644 --- a/distrobuilder/main_lxc.go +++ b/distrobuilder/main_lxc.go @@ -155,11 +155,5 @@ func (c *cmdLXC) run(cmd *cobra.Command, args []string, overlayDir string) error return errors.Wrap(err, "Failed to create LXC image") } - // Clean up the chroot by restoring the orginal files. - err = generators.RestoreFiles(c.global.flagCacheDir, overlayDir) - if err != nil { - return errors.Wrap(err, "Failed to restore cached files") - } - return nil } diff --git a/generators/cloud-init.go b/generators/cloud-init.go index 29a3f9b3..46c7468d 100644 --- a/generators/cloud-init.go +++ b/generators/cloud-init.go @@ -32,7 +32,7 @@ func (g CloudInitGenerator) RunLXC(cacheDir, sourceDir string, img *image.LXCIma } if lxd.StringInSlice(info.Name(), []string{"cloud-init-local", "cloud-config", "cloud-init", "cloud-final"}) { - err := StoreFile(cacheDir, sourceDir, strings.TrimPrefix(path, sourceDir)) + err := os.Remove(path) if err != nil { return err } @@ -62,7 +62,7 @@ func (g CloudInitGenerator) RunLXC(cacheDir, sourceDir string, img *image.LXCIma } if re.MatchString(info.Name()) { - err := StoreFile(cacheDir, sourceDir, strings.TrimPrefix(path, sourceDir)) + err := os.Remove(path) if err != nil { return err } @@ -74,20 +74,10 @@ func (g CloudInitGenerator) RunLXC(cacheDir, sourceDir string, img *image.LXCIma // With systemd: if !lxd.PathExists(filepath.Join(sourceDir, "/etc/cloud")) { - err := StoreFile(cacheDir, sourceDir, "/etc/cloud") + err := os.MkdirAll(filepath.Join(sourceDir, "/etc/cloud"), 0755) if err != nil { return err } - - err = os.MkdirAll(filepath.Join(sourceDir, "/etc/cloud"), 0755) - if err != nil { - return err - } - } - - err := StoreFile(cacheDir, sourceDir, "/etc/cloud/cloud-init.disabled") - if err != nil { - return err } // Create file /etc/cloud/cloud-init.disabled diff --git a/generators/cloud-init_test.go b/generators/cloud-init_test.go index bf1efca8..06ce41a6 100644 --- a/generators/cloud-init_test.go +++ b/generators/cloud-init_test.go @@ -71,27 +71,6 @@ func TestCloudInitGeneratorRunLXC(t *testing.T) { } require.FileExists(t, filepath.Join(rootfsDir, "etc", "cloud", "cloud-init.disabled")) - - err = RestoreFiles(cacheDir, rootfsDir) - require.NoError(t, err) - - // Check whether the files have been restored - for _, f := range []string{"cloud-init-local", "cloud-config", "cloud-init", "cloud-final"} { - fullPath := filepath.Join(rootfsDir, "etc", "runlevels", f) - require.FileExists(t, fullPath) - } - - for i := 0; i <= 6; i++ { - dir := filepath.Join(rootfsDir, "etc", "rc.d", fmt.Sprintf("rc%d.d", i)) - - for _, f := range []string{"cloud-init-local", "cloud-config", "cloud-init", "cloud-final"} { - fullPath := filepath.Join(dir, fmt.Sprintf("S99%s", f)) - require.FileExists(t, fullPath) - } - } - - fullPath := filepath.Join(rootfsDir, "etc", "cloud", "cloud-init.disabled") - require.Falsef(t, lxd.PathExists(fullPath), "File '%s' exists but shouldn't", fullPath) } func TestCloudInitGeneratorRunLXD(t *testing.T) { diff --git a/generators/generators.go b/generators/generators.go index 288afce1..db34e76c 100644 --- a/generators/generators.go +++ b/generators/generators.go @@ -2,12 +2,6 @@ package generators import ( "errors" - "os" - p "path" - "path/filepath" - "strings" - - lxd "github.com/lxc/lxd/shared" "github.com/lxc/distrobuilder/image" "github.com/lxc/distrobuilder/shared" @@ -48,89 +42,3 @@ func Get(generator string) Generator { return nil } - -var storedFiles = map[string]os.FileInfo{} - -// StoreFile caches a file which can be restored with the RestoreFiles function. -func StoreFile(cacheDir, sourceDir, path string) error { - fullPath := filepath.Join(sourceDir, path) - - _, ok := storedFiles[fullPath] - if ok { - // This file or directory has already been recorded - return nil - } - - // Record newly created files - if !lxd.PathExists(fullPath) { - storedFiles[fullPath] = nil - return nil - } - - // create temporary directory containing old files - err := os.MkdirAll(filepath.Join(cacheDir, "tmp", p.Dir(path)), 0755) - if err != nil { - return err - } - - info, err := os.Lstat(fullPath) - if err != nil { - return err - } - - storedFiles[fullPath] = info - - err = os.Rename(fullPath, filepath.Join(cacheDir, "tmp", path)) - if err == nil { - return nil - } - - // Try copying the file since renaming it failed - if info.IsDir() { - err = lxd.DirCopy(fullPath, filepath.Join(cacheDir, "tmp", path)) - } else { - err = lxd.FileCopy(fullPath, filepath.Join(cacheDir, "tmp", path)) - } - if err != nil { - return err - } - - return os.RemoveAll(fullPath) -} - -// RestoreFiles restores original files which were cached by StoreFile. -func RestoreFiles(cacheDir, sourceDir string) error { - var err error - - for origPath, fi := range storedFiles { - // Deal with newly created files - if fi == nil { - err := os.RemoveAll(origPath) - if err != nil { - return err - } - - continue - } - - err = os.Rename(filepath.Join(cacheDir, "tmp", strings.TrimPrefix(origPath, sourceDir)), origPath) - if err == nil { - continue - } - - // Try copying the file or directory since renaming it failed - if fi.IsDir() { - err = lxd.DirCopy(filepath.Join(cacheDir, "tmp", strings.TrimPrefix(origPath, sourceDir)), origPath) - } else { - err = lxd.FileCopy(filepath.Join(cacheDir, "tmp", strings.TrimPrefix(origPath, sourceDir)), origPath) - } - if err != nil { - return err - } - } - - // Reset the list of stored files - storedFiles = map[string]os.FileInfo{} - - return nil -} diff --git a/generators/generators_test.go b/generators/generators_test.go index c312fb92..3c39d7b9 100644 --- a/generators/generators_test.go +++ b/generators/generators_test.go @@ -31,49 +31,6 @@ func TestGet(t *testing.T) { require.Nil(t, generator) } -func TestRestoreFiles(t *testing.T) { - cacheDir := filepath.Join(os.TempDir(), "distrobuilder-test") - rootfsDir := filepath.Join(cacheDir, "rootfs") - - setup(t, cacheDir) - defer teardown(cacheDir) - - // Create test directory - err := os.MkdirAll(filepath.Join(cacheDir, "rootfs", "testdir1"), 0755) - require.NoError(t, err) - - // Create original test file - createTestFile(t, filepath.Join(cacheDir, "rootfs", "testdir1", "testfile1"), - "original file") - - // Chmod cache directory which should lead to StoreFile failing - err = os.Chmod(cacheDir, 0600) - require.NoError(t, err) - - err = StoreFile(cacheDir, rootfsDir, filepath.Join("/testdir1", "testfile1")) - require.Error(t, err) - - // Restore permissions - err = os.Chmod(cacheDir, 0755) - require.NoError(t, err) - - err = StoreFile(cacheDir, rootfsDir, filepath.Join("/testdir1", "testfile1")) - require.NoError(t, err) - - validateTestFile(t, filepath.Join(cacheDir, "tmp", "testdir1", "testfile1"), - "original file") - - // Change content of original file - createTestFile(t, filepath.Join(cacheDir, "rootfs", "testdir1", "testfile1"), - "modified file") - - err = RestoreFiles(cacheDir, rootfsDir) - require.NoError(t, err) - - validateTestFile(t, filepath.Join(cacheDir, "rootfs", "testdir1", "testfile1"), - "original file") -} - func createTestFile(t *testing.T, path, content string) { file, err := os.Create(path) require.NoError(t, err) diff --git a/generators/hostname.go b/generators/hostname.go index d8f8ccf6..136609c6 100644 --- a/generators/hostname.go +++ b/generators/hostname.go @@ -24,12 +24,6 @@ func (g HostnameGenerator) RunLXC(cacheDir, sourceDir string, img *image.LXCImag return nil } - // Store original file - err := StoreFile(cacheDir, sourceDir, defFile.Path) - if err != nil { - return err - } - // Create new hostname file file, err := os.Create(filepath.Join(sourceDir, defFile.Path)) if err != nil { diff --git a/generators/hostname_test.go b/generators/hostname_test.go index ebc00e58..cb598843 100644 --- a/generators/hostname_test.go +++ b/generators/hostname_test.go @@ -39,13 +39,7 @@ func TestHostnameGeneratorRunLXC(t *testing.T) { shared.DefinitionFile{Path: "/etc/hostname"}) require.NoError(t, err) - validateTestFile(t, filepath.Join(cacheDir, "tmp", "etc", "hostname"), "hostname") validateTestFile(t, filepath.Join(cacheDir, "rootfs", "etc", "hostname"), "LXC_NAME\n") - - err = RestoreFiles(cacheDir, rootfsDir) - require.NoError(t, err) - - validateTestFile(t, filepath.Join(cacheDir, "rootfs", "etc", "hostname"), "hostname") } func TestHostnameGeneratorRunLXD(t *testing.T) { diff --git a/generators/hosts.go b/generators/hosts.go index 8465d96f..b4ac000d 100644 --- a/generators/hosts.go +++ b/generators/hosts.go @@ -30,12 +30,6 @@ func (g HostsGenerator) RunLXC(cacheDir, sourceDir string, img *image.LXCImage, return err } - // Store original file - err = StoreFile(cacheDir, sourceDir, defFile.Path) - if err != nil { - return err - } - // Replace hostname with placeholder content = []byte(strings.Replace(string(content), "distrobuilder", "LXC_NAME", -1)) diff --git a/generators/hosts_test.go b/generators/hosts_test.go index e0409b70..cabb1ca2 100644 --- a/generators/hosts_test.go +++ b/generators/hosts_test.go @@ -40,16 +40,8 @@ func TestHostsGeneratorRunLXC(t *testing.T) { shared.DefinitionFile{Path: "/etc/hosts"}) require.NoError(t, err) - validateTestFile(t, filepath.Join(cacheDir, "tmp", "etc", "hosts"), - "127.0.0.1\tlocalhost\n127.0.0.1\tdistrobuilder\n") validateTestFile(t, filepath.Join(cacheDir, "rootfs", "etc", "hosts"), "127.0.0.1\tlocalhost\n127.0.0.1\tLXC_NAME\n") - - err = RestoreFiles(cacheDir, rootfsDir) - require.NoError(t, err) - - validateTestFile(t, filepath.Join(cacheDir, "rootfs", "etc", "hosts"), - "127.0.0.1\tlocalhost\n127.0.0.1\tdistrobuilder\n") } func TestHostsGeneratorRunLXD(t *testing.T) { diff --git a/generators/upstart_tty.go b/generators/upstart_tty.go index afb2e32a..ea04e55e 100644 --- a/generators/upstart_tty.go +++ b/generators/upstart_tty.go @@ -58,12 +58,6 @@ func (g UpstartTTYGenerator) RunLXC(cacheDir, sourceDir string, img *image.LXCIm return nil } - // Store original file - err := StoreFile(cacheDir, sourceDir, defFile.Path) - if err != nil { - return err - } - // Create new hostname file file, err := os.Create(filepath.Join(sourceDir, defFile.Path)) if err != nil { From c3fd936a74860dd174362476d0d25ecc75aa2049 Mon Sep 17 00:00:00 2001 From: Thomas Hipp <thomas.h...@canonical.com> Date: Wed, 19 Feb 2020 12:04:36 +0100 Subject: [PATCH 3/3] *: Replace syscall package with unix Signed-off-by: Thomas Hipp <thomas.h...@canonical.com> --- image/lxc_test.go | 6 +++--- shared/chroot.go | 19 +++++++++---------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/image/lxc_test.go b/image/lxc_test.go index ad8885cd..e544c0d7 100644 --- a/image/lxc_test.go +++ b/image/lxc_test.go @@ -8,10 +8,10 @@ import ( "os" "path/filepath" "strings" - "syscall" "testing" "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" "github.com/lxc/distrobuilder/shared" ) @@ -196,8 +196,8 @@ func TestLXCCreateMetadataBasic(t *testing.T) { func(l LXCImage) *LXCImage { // Create /dev and device file. os.MkdirAll(filepath.Join(lxcCacheDir(), "rootfs", "dev"), 0755) - syscall.Mknod(filepath.Join(lxcCacheDir(), "rootfs", "dev", "null"), - syscall.S_IFCHR, 0) + unix.Mknod(filepath.Join(lxcCacheDir(), "rootfs", "dev", "null"), + unix.S_IFCHR, 0) return &l }, }, diff --git a/shared/chroot.go b/shared/chroot.go index e7fc020e..c176f51b 100644 --- a/shared/chroot.go +++ b/shared/chroot.go @@ -7,7 +7,6 @@ import ( "path/filepath" "regexp" "strconv" - "syscall" lxd "github.com/lxc/lxd/shared" "github.com/pkg/errors" @@ -52,7 +51,7 @@ func setupMounts(rootfs string, mounts []ChrootMount) error { } // Mount to the temporary path - err := syscall.Mount(mount.Source, tmpTarget, mount.FSType, mount.Flags, mount.Data) + err := unix.Mount(mount.Source, tmpTarget, mount.FSType, mount.Flags, mount.Data) if err != nil { return errors.Wrapf(err, "Failed to mount '%s'", mount.Source) } @@ -109,7 +108,7 @@ func moveMounts(mounts []ChrootMount) error { } // Move the mount to its destination - err = syscall.Mount(tmpSource, target, "", syscall.MS_MOVE, "") + err = unix.Mount(tmpSource, target, "", unix.MS_MOVE, "") if err != nil { return errors.Wrapf(err, "Failed to mount '%s'", mount.Source) } @@ -145,7 +144,7 @@ func killChrootProcesses(rootfs string) error { link, _ := os.Readlink(filepath.Join(rootfs, "proc", dir, "root")) if link == rootfs { pid, _ := strconv.Atoi(dir) - syscall.Kill(pid, syscall.SIGKILL) + unix.Kill(pid, unix.SIGKILL) } } } @@ -156,7 +155,7 @@ func killChrootProcesses(rootfs string) error { // SetupChroot sets up mount and files, a reverter and then chroots for you func SetupChroot(rootfs string, envs DefinitionEnv, m []ChrootMount) (func() error, error) { // Mount the rootfs - err := syscall.Mount(rootfs, rootfs, "", syscall.MS_BIND, "") + err := unix.Mount(rootfs, rootfs, "", unix.MS_BIND, "") if err != nil { return nil, errors.Wrapf(err, "Failed to mount '%s'", rootfs) } @@ -165,10 +164,10 @@ func SetupChroot(rootfs string, envs DefinitionEnv, m []ChrootMount) (func() err mounts := []ChrootMount{ {"none", "/proc", "proc", 0, "", true}, {"none", "/sys", "sysfs", 0, "", true}, - {"/dev", "/dev", "", syscall.MS_BIND, "", true}, + {"/dev", "/dev", "", unix.MS_BIND, "", true}, {"none", "/run", "tmpfs", 0, "", true}, {"none", "/tmp", "tmpfs", 0, "", true}, - {"/etc/resolv.conf", "/etc/resolv.conf", "", syscall.MS_BIND, "", false}, + {"/etc/resolv.conf", "/etc/resolv.conf", "", unix.MS_BIND, "", false}, } // Keep a reference to the host rootfs and cwd @@ -286,12 +285,12 @@ exit 101 return err } - err = syscall.Chroot(".") + err = unix.Chroot(".") if err != nil { return err } - err = syscall.Chdir(cwd) + err = unix.Chdir(cwd) if err != nil { return err } @@ -301,7 +300,7 @@ exit 101 killChrootProcesses(rootfs) // And now unmount the entire tree - unix.Unmount(rootfs, syscall.MNT_DETACH) + unix.Unmount(rootfs, unix.MNT_DETACH) devPath := filepath.Join(rootfs, "dev")
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel