diff --git a/pkg/build/strategies/layered/layered.go b/pkg/build/strategies/layered/layered.go index ac85d3fa5..b2527f269 100644 --- a/pkg/build/strategies/layered/layered.go +++ b/pkg/build/strategies/layered/layered.go @@ -125,17 +125,6 @@ func (builder *Layered) CreateDockerfile(config *api.Config) error { return nil } -// SourceTar returns a stream to the source tar file. -// TODO: this should stop generating a file, and instead stream the tar. -func (builder *Layered) SourceTar(config *api.Config) (io.ReadCloser, error) { - uploadDir := filepath.Join(config.WorkingDir, "upload") - tarFileName, err := builder.tar.CreateTarFile(builder.config.WorkingDir, uploadDir) - if err != nil { - return nil, err - } - return builder.fs.Open(tarFileName) -} - // Build handles the `docker build` equivalent execution, returning the // success/failure details. func (builder *Layered) Build(config *api.Config) (*api.Result, error) { @@ -157,11 +146,7 @@ func (builder *Layered) Build(config *api.Config) (*api.Result, error) { } glog.V(2).Info("Creating application source code image") - tarStream, err := builder.SourceTar(config) - if err != nil { - buildResult.BuildInfo.FailureReason = utilstatus.NewFailureReason(utilstatus.ReasonTarSourceFailed, utilstatus.ReasonMessageTarSourceFailed) - return buildResult, err - } + tarStream := builder.tar.CreateTarStreamReader(filepath.Join(config.WorkingDir, "upload"), false) defer tarStream.Close() newBuilderImage := fmt.Sprintf("s2i-layered-temp-image-%d", time.Now().UnixNano()) @@ -193,7 +178,7 @@ func (builder *Layered) Build(config *api.Config) (*api.Result, error) { }(outReader) glog.V(2).Infof("Building new image %s with scripts and sources already inside", newBuilderImage) - if err = builder.docker.BuildImage(opts); err != nil { + if err := builder.docker.BuildImage(opts); err != nil { buildResult.BuildInfo.FailureReason = utilstatus.NewFailureReason(utilstatus.ReasonDockerImageBuildFailed, utilstatus.ReasonMessageDockerImageBuildFailed) return buildResult, err } @@ -208,6 +193,7 @@ func (builder *Layered) Build(config *api.Config) (*api.Result, error) { if scriptsIncluded { builder.config.ScriptsURL = "image://" + path.Join(getDestination(config), "scripts") } else { + var err error builder.config.ScriptsURL, err = builder.docker.GetScriptsURL(newBuilderImage) if err != nil { buildResult.BuildInfo.FailureReason = utilstatus.NewFailureReason(utilstatus.ReasonGenericS2IBuildFailed, utilstatus.ReasonMessageGenericS2iBuildFailed) diff --git a/pkg/build/strategies/layered/layered_test.go b/pkg/build/strategies/layered/layered_test.go index 4d4f22075..a7a9e0ed8 100644 --- a/pkg/build/strategies/layered/layered_test.go +++ b/pkg/build/strategies/layered/layered_test.go @@ -30,7 +30,7 @@ func newFakeLayered() *Layered { } } -func newFakeLayeredWithScripts(assemble, workDir string) *Layered { +func newFakeLayeredWithScripts(workDir string) *Layered { return &Layered{ docker: &docker.FakeDocker{}, config: &api.Config{WorkingDir: workDir}, @@ -51,7 +51,7 @@ func TestBuildOK(t *testing.T) { } defer file.Close() defer os.RemoveAll(workDir) - l := newFakeLayeredWithScripts(assemble, workDir) + l := newFakeLayeredWithScripts(workDir) l.config.BuilderImage = "test/image" _, err = l.Build(l.config) if err != nil { @@ -84,7 +84,7 @@ func TestBuildOKWithImageRef(t *testing.T) { } defer file.Close() defer os.RemoveAll(workDir) - l := newFakeLayeredWithScripts(assemble, workDir) + l := newFakeLayeredWithScripts(workDir) l.config.BuilderImage = "docker.io/uptoknow/ruby-20-centos7@sha256:d6f5718b85126954d98931e654483ee794ac357e0a98f4a680c1e848d78863a1" _, err = l.Build(l.config) if err != nil { @@ -158,16 +158,6 @@ func TestBuildErrorCreateTarFile(t *testing.T) { } } -func TestBuildErrorOpenTarFile(t *testing.T) { - l := newFakeLayered() - l.config.BuilderImage = "test/image" - l.fs.(*test.FakeFileSystem).OpenError = errors.New("OpenTarError") - _, err := l.Build(l.config) - if err == nil || err.Error() != "OpenTarError" { - t.Errorf("An error was expected for OpenTarFile, but got different: %v", err) - } -} - func TestBuildErrorBuildImage(t *testing.T) { l := newFakeLayered() l.config.BuilderImage = "test/image" diff --git a/pkg/build/strategies/onbuild/onbuild.go b/pkg/build/strategies/onbuild/onbuild.go index a6d408d8b..31d263b1b 100644 --- a/pkg/build/strategies/onbuild/onbuild.go +++ b/pkg/build/strategies/onbuild/onbuild.go @@ -3,7 +3,6 @@ package onbuild import ( "bytes" "fmt" - "io" "os" "path/filepath" @@ -76,17 +75,6 @@ func New(config *api.Config, overrides build.Overrides) (*OnBuild, error) { return builder, nil } -// SourceTar produces a tar archive containing application source and streams -// it -func (builder *OnBuild) SourceTar(config *api.Config) (io.ReadCloser, error) { - uploadDir := filepath.Join(config.WorkingDir, "upload", "src") - tarFileName, err := builder.tar.CreateTarFile(config.WorkingDir, uploadDir) - if err != nil { - return nil, err - } - return builder.fs.Open(tarFileName) -} - // Build executes the ONBUILD kind of build func (builder *OnBuild) Build(config *api.Config) (*api.Result, error) { buildResult := &api.Result{} @@ -112,11 +100,7 @@ func (builder *OnBuild) Build(config *api.Config) (*api.Result, error) { } glog.V(2).Info("Creating application source code image") - tarStream, err := builder.SourceTar(config) - if err != nil { - buildResult.BuildInfo.FailureReason = utilstatus.NewFailureReason(utilstatus.ReasonTarSourceFailed, utilstatus.ReasonMessageTarSourceFailed) - return buildResult, err - } + tarStream := builder.tar.CreateTarStreamReader(filepath.Join(config.WorkingDir, "upload", "src"), false) defer tarStream.Close() opts := docker.BuildImageOptions{ @@ -127,7 +111,7 @@ func (builder *OnBuild) Build(config *api.Config) (*api.Result, error) { } glog.V(2).Info("Building the application source") - if err = builder.docker.BuildImage(opts); err != nil { + if err := builder.docker.BuildImage(opts); err != nil { buildResult.BuildInfo.FailureReason = utilstatus.NewFailureReason(utilstatus.ReasonDockerImageBuildFailed, utilstatus.ReasonMessageDockerImageBuildFailed) return buildResult, err } @@ -136,7 +120,7 @@ func (builder *OnBuild) Build(config *api.Config) (*api.Result, error) { builder.garbage.Cleanup(config) var imageID string - + var err error if len(opts.Name) > 0 { if imageID, err = builder.docker.GetImageID(opts.Name); err != nil { buildResult.BuildInfo.FailureReason = utilstatus.NewFailureReason(utilstatus.ReasonGenericS2IBuildFailed, utilstatus.ReasonMessageGenericS2iBuildFailed) diff --git a/pkg/build/strategies/sti/postexecutorstep.go b/pkg/build/strategies/sti/postexecutorstep.go index 2e2200152..4dd239552 100644 --- a/pkg/build/strategies/sti/postexecutorstep.go +++ b/pkg/build/strategies/sti/postexecutorstep.go @@ -1,19 +1,19 @@ package sti import ( + "archive/tar" "fmt" "io" "io/ioutil" "os" "path" "path/filepath" - "runtime" "strings" "github.com/openshift/source-to-image/pkg/api" dockerpkg "github.com/openshift/source-to-image/pkg/docker" "github.com/openshift/source-to-image/pkg/errors" - "github.com/openshift/source-to-image/pkg/tar" + s2itar "github.com/openshift/source-to-image/pkg/tar" "github.com/openshift/source-to-image/pkg/util" utilstatus "github.com/openshift/source-to-image/pkg/util/status" ) @@ -142,7 +142,7 @@ type downloadFilesFromBuilderImageStep struct { builder *STI docker dockerpkg.Docker fs util.FileSystem - tar tar.Tar + tar s2itar.Tar } func (step *downloadFilesFromBuilderImageStep) execute(ctx *postExecutorStepContext) error { @@ -295,37 +295,18 @@ func (step *startRuntimeImageAndUploadFilesStep) execute(ctx *postExecutorStepCo } opts.OnStart = func(containerID string) error { - setStandardPerms := func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - // chmod does nothing on windows anyway. - if runtime.GOOS == "windows" { - return nil - } - // Skip chmod for symlinks - if info.Mode()&os.ModeSymlink != 0 { - return nil - } - // file should be writable by owner (u=w) and readable by other users (a=r), - // executable bit should be left as is - mode := os.FileMode(0644) - // syscall.S_IEXEC == 0x40 but we can't reference the constant if we want - // to build releases for windows. - if info.IsDir() || info.Mode()&0x40 != 0 { - mode = 0755 - } - return step.fs.Chmod(path, mode) + setStandardPerms := func(writer io.Writer) s2itar.Writer { + return s2itar.ChmodAdapter{Writer: tar.NewWriter(writer), NewFileMode: 0644, NewExecFileMode: 0755, NewDirMode: 0755} } glog.V(5).Infof("Uploading directory %q -> %q", artifactsDir, workDir) - onStartErr := step.docker.UploadToContainerWithCallback(artifactsDir, workDir, containerID, setStandardPerms, true) + onStartErr := step.docker.UploadToContainerWithTarWriter(artifactsDir, workDir, containerID, setStandardPerms) if onStartErr != nil { return fmt.Errorf("Couldn't upload directory (%q -> %q) into container %s: %v", artifactsDir, workDir, containerID, err) } glog.V(5).Infof("Uploading file %q -> %q", lastFilePath, lastFileDstPath) - onStartErr = step.docker.UploadToContainerWithCallback(lastFilePath, lastFileDstPath, containerID, setStandardPerms, true) + onStartErr = step.docker.UploadToContainerWithTarWriter(lastFilePath, lastFileDstPath, containerID, setStandardPerms) if onStartErr != nil { return fmt.Errorf("Couldn't upload file (%q -> %q) into container %s: %v", lastFilePath, lastFileDstPath, containerID, err) } diff --git a/pkg/docker/docker.go b/pkg/docker/docker.go index 511603299..875da290d 100644 --- a/pkg/docker/docker.go +++ b/pkg/docker/docker.go @@ -1,6 +1,7 @@ package docker import ( + "archive/tar" "bytes" "encoding/base64" "encoding/json" @@ -28,7 +29,7 @@ import ( "github.com/openshift/source-to-image/pkg/api" "github.com/openshift/source-to-image/pkg/errors" - "github.com/openshift/source-to-image/pkg/tar" + s2itar "github.com/openshift/source-to-image/pkg/tar" "github.com/openshift/source-to-image/pkg/util" "github.com/openshift/source-to-image/pkg/util/interrupt" ) @@ -113,7 +114,7 @@ type Docker interface { GetImageEntrypoint(name string) ([]string, error) GetLabels(name string) (map[string]string, error) UploadToContainer(srcPath, destPath, container string) error - UploadToContainerWithCallback(srcPath, destPath, container string, walkFn filepath.WalkFunc, modifyInplace bool) error + UploadToContainerWithTarWriter(srcPath, destPath, container string, makeTarWriter func(io.Writer) s2itar.Writer) error DownloadFromContainer(containerPath string, w io.Writer, container string) error Version() (dockertypes.Version, error) } @@ -360,62 +361,40 @@ func (d *stiDocker) GetImageEntrypoint(name string) ([]string, error) { // UploadToContainer uploads artifacts to the container. func (d *stiDocker) UploadToContainer(src, dest, container string) error { - makeFileWorldWritable := func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - // Skip chmod if on windows OS and for symlinks - if runtime.GOOS == "windows" || info.Mode()&os.ModeSymlink != 0 { - return nil - } - mode := os.FileMode(0666) - if info.IsDir() { - mode = 0777 - } - return os.Chmod(path, mode) + makeWorldWritable := func(writer io.Writer) s2itar.Writer { + return s2itar.ChmodAdapter{Writer: tar.NewWriter(writer), NewFileMode: 0666, NewExecFileMode: 0666, NewDirMode: 0777} } - return d.UploadToContainerWithCallback(src, dest, container, makeFileWorldWritable, false) + + return d.UploadToContainerWithTarWriter(src, dest, container, makeWorldWritable) } -// UploadToContainerWithCallback uploads artifacts to the container. +// UploadToContainerWithTarWriter uploads artifacts to the container. // If the source is a directory, then all files and sub-folders are copied into // the destination (which has to be directory as well). // If the source is a single file, then the file copied into destination (which // has to be full path to a file inside the container). -// If the destination path is empty or set to ".", then we will try to figure -// out the WORKDIR of the image that the container was created from and use that -// as a destination. If the WORKDIR is not set, then we copy files into "/" -// folder (docker upload default). -func (d *stiDocker) UploadToContainerWithCallback(src, dest, container string, walkFn filepath.WalkFunc, modifyInplace bool) error { +func (d *stiDocker) UploadToContainerWithTarWriter(src, dest, container string, makeTarWriter func(io.Writer) s2itar.Writer) error { path := filepath.Dir(dest) - f, err := os.Open(src) - if err != nil { - return err - } - info, _ := f.Stat() - defer f.Close() - t := tar.New() r, w := io.Pipe() - if info.IsDir() { - path = dest - go func() { - defer w.Close() - if err := t.StreamDirAsTarWithCallback(src, w, walkFn, modifyInplace); err != nil { - glog.V(0).Infof("error: Uploading directory to container failed: %v", err) - } - }() - } else { - go func() { - defer w.Close() - if err := t.StreamFileAsTarWithCallback(src, filepath.Base(dest), w, walkFn, modifyInplace); err != nil { - glog.V(0).Infof("error: Uploading files to container failed: %v", err) - } - }() - } + go func() { + tarWriter := makeTarWriter(w) + tarWriter = s2itar.RenameAdapter{Writer: tarWriter, Old: filepath.Base(src), New: filepath.Base(dest)} + + err := s2itar.New().CreateTarStreamToTarWriter(src, true, tarWriter, nil) + if err == nil { + err = tarWriter.Close() + } + + w.CloseWithError(err) + }() glog.V(3).Infof("Uploading %q to %q ...", src, path) ctx, cancel := getDefaultContext() defer cancel() - return d.client.CopyToContainer(ctx, container, path, r, dockertypes.CopyToContainerOptions{}) + err := d.client.CopyToContainer(ctx, container, path, r, dockertypes.CopyToContainerOptions{}) + if err != nil { + glog.V(0).Infof("error: Uploading to container failed: %v", err) + } + return err } // DownloadFromContainer downloads file (or directory) from the container. diff --git a/pkg/docker/docker_test.go b/pkg/docker/docker_test.go index fb88f631d..fb7af4847 100644 --- a/pkg/docker/docker_test.go +++ b/pkg/docker/docker_test.go @@ -119,6 +119,7 @@ func TestCopyToContainer(t *testing.T) { }, "error": { containerID: "test-container-id", + src: "badsource", }, } diff --git a/pkg/docker/fake_docker.go b/pkg/docker/fake_docker.go index dde952e4d..a7addae0a 100644 --- a/pkg/docker/fake_docker.go +++ b/pkg/docker/fake_docker.go @@ -3,10 +3,11 @@ package docker import ( "errors" "io" - "path/filepath" + "io/ioutil" dockertypes "github.com/docker/engine-api/types" "github.com/openshift/source-to-image/pkg/api" + "github.com/openshift/source-to-image/pkg/tar" ) // FakeDocker provides a fake docker interface @@ -119,8 +120,8 @@ func (f *FakeDocker) UploadToContainer(srcPath, destPath, container string) erro return nil } -// UploadToContainerWithCallback uploads artifacts to the container. -func (f *FakeDocker) UploadToContainerWithCallback(srcPath, destPath, container string, walkFn filepath.WalkFunc, modifyInplace bool) error { +// UploadToContainerWithTarWriter uploads artifacts to the container. +func (f *FakeDocker) UploadToContainerWithTarWriter(srcPath, destPath, container string, makeTarWriter func(io.Writer) tar.Writer) error { return errors.New("not implemented") } @@ -182,6 +183,12 @@ func (f *FakeDocker) CheckAndPullImage(name string) (*api.Image, error) { // BuildImage builds image func (f *FakeDocker) BuildImage(opts BuildImageOptions) error { f.BuildImageOpts = opts + if opts.Stdin != nil { + _, err := io.Copy(ioutil.Discard, opts.Stdin) + if err != nil { + return err + } + } return f.BuildImageError } diff --git a/pkg/tar/tar.go b/pkg/tar/tar.go index 2f9518e7b..7becd7cf6 100644 --- a/pkg/tar/tar.go +++ b/pkg/tar/tar.go @@ -9,6 +9,7 @@ import ( "path/filepath" "regexp" "runtime" + "strings" "time" utilglog "github.com/openshift/source-to-image/pkg/util/glog" @@ -39,17 +40,21 @@ type Tar interface { // The name of the new tar file is returned if successful CreateTarFile(base, dir string) (string, error) - // CreateTarStreamWithLogging creates a tar from the given directory + // CreateTarStreamToTarWriter creates a tar from the given directory // and streams it to the given writer. // An error is returned if an error occurs during streaming. // Archived file names are written to the logger if provided - CreateTarStreamWithLogging(dir string, includeDirInPath bool, writer io.Writer, logger io.Writer) error + CreateTarStreamToTarWriter(dir string, includeDirInPath bool, writer Writer, logger io.Writer) error // CreateTarStream creates a tar from the given directory // and streams it to the given writer. // An error is returned if an error occurs during streaming. CreateTarStream(dir string, includeDirInPath bool, writer io.Writer) error + // CreateTarStreamReader returns an io.ReadCloser from which a tar stream can be + // read. The tar stream is created using CreateTarStream. + CreateTarStreamReader(dir string, includeDirInPath bool) io.ReadCloser + // ExtractTarStream extracts files from a given tar stream. // Times out if reading from the stream for any given file // exceeds the value of timeout. @@ -66,27 +71,6 @@ type Tar interface { // exceeds the value of timeout. // Extracted file names are written to the logger if provided. ExtractTarStreamFromTarReader(dir string, tarReader Reader, logger io.Writer) error - - // StreamFileAsTar streams a single file as a TAR archive into specified - // writer. The second argument is the file name in archive. - // The file permissions in tar archive will change to 0666. - StreamFileAsTar(string, string, io.Writer) error - - // StreamFileAsTarWithCallback streams a single file as a TAR archive into specified - // writer. By specifying walkFn you can modify file's permissions in arbitrary way. - // If modifyInplace is set to false, file will be copied into a temporary directory before changing its permissions. - StreamFileAsTarWithCallback(source, name string, writer io.Writer, walkFn filepath.WalkFunc, modifyInplace bool) error - - // StreamDirAsTar streams a directory as a TAR archive into specified writer. - // The second argument is the name of the folder in the archive. - // All files in the source folder will have permissions changed to 0666 in the - // tar archive. - StreamDirAsTar(string, string, io.Writer) error - - // StreamDirAsTarWithCallback streams a directory as a TAR archive into specified writer. - // By specifying walkFn you can modify files' permissions in arbitrary way. - // If modifyInplace is set to false, all the files will be copied into a temporary directory before changing their permissions. - StreamDirAsTarWithCallback(source string, writer io.Writer, walkFn filepath.WalkFunc, modifyInplace bool) error } // Reader is an interface which tar.Reader implements. @@ -95,6 +79,58 @@ type Reader interface { Next() (*tar.Header, error) } +// Writer is an interface which tar.Writer implements. +type Writer interface { + io.WriteCloser + Flush() error + WriteHeader(hdr *tar.Header) error +} + +// ChmodAdapter changes the mode of files and directories inline as a tarfile is +// being written +type ChmodAdapter struct { + Writer + NewFileMode int64 + NewExecFileMode int64 + NewDirMode int64 +} + +// WriteHeader changes the mode of files and directories inline as a tarfile is +// being written +func (a ChmodAdapter) WriteHeader(hdr *tar.Header) error { + if hdr.FileInfo().Mode()&os.ModeSymlink == 0 { + hdr.Mode &^= 0777 + if hdr.FileInfo().IsDir() { + hdr.Mode |= a.NewDirMode + } else if hdr.FileInfo().Mode()&0010 != 0 { // S_IXUSR + hdr.Mode |= a.NewExecFileMode + } else { + hdr.Mode |= a.NewFileMode + } + } + return a.Writer.WriteHeader(hdr) +} + +// RenameAdapter renames files and directories inline as a tarfile is being +// written +type RenameAdapter struct { + Writer + Old string + New string +} + +// WriteHeader renames files and directories inline as a tarfile is being +// written +func (a RenameAdapter) WriteHeader(hdr *tar.Header) error { + if hdr.Name == a.Old { + hdr.Name = a.New + } else if strings.HasPrefix(hdr.Name, a.Old+"/") { + hdr.Name = a.New + hdr.Name[len(a.Old):] + } + + return a.Writer.WriteHeader(hdr) +} + // New creates a new Tar func New() Tar { return &stiTar{ @@ -115,106 +151,6 @@ func (t *stiTar) SetExclusionPattern(p *regexp.Regexp) { t.exclude = p } -// StreamDirAsTar streams the source directory as a tar archive. -// The permissions of the file is changed to 0666. -func (t *stiTar) StreamDirAsTar(source, dest string, writer io.Writer) error { - makeFileWorldWritable := func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - // Skip chmod if on windows OS and for symlinks - if runtime.GOOS == "windows" || info.Mode()&os.ModeSymlink != 0 { - return nil - } - mode := os.FileMode(0666) - if info.IsDir() { - mode = 0777 - } - return os.Chmod(path, mode) - } - return t.StreamDirAsTarWithCallback(source, writer, makeFileWorldWritable, false) -} - -// StreamDirAsTarWithCallback streams the source directory as a tar archive. -func (t *stiTar) StreamDirAsTarWithCallback(source string, writer io.Writer, walkFn filepath.WalkFunc, modifyInplace bool) error { - f, err := os.Open(source) - if err != nil { - return err - } - defer f.Close() - info, err := f.Stat() - if err != nil { - return err - } - if !info.IsDir() { - return fmt.Errorf("the source %q has to be directory, not a file", source) - } - destDir := source - if !modifyInplace { - fs := util.NewFileSystem() - tmpDir, err := ioutil.TempDir("", "s2i-") - if err != nil { - return err - } - defer os.RemoveAll(tmpDir) - if err = fs.Copy(source, tmpDir); err != nil { - return err - } - destDir = tmpDir - } - if err := filepath.Walk(destDir, walkFn); err != nil { - return err - } - return t.CreateTarStream(destDir, false, writer) -} - -// StreamFileAsTar streams the source file as a tar archive. -// The permissions of all files in archive is changed to 0666. -func (t *stiTar) StreamFileAsTar(source, name string, writer io.Writer) error { - makeFileWorldWritable := func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - // Skip chmod if on windows OS and for symlinks - if runtime.GOOS == "windows" || info.Mode()&os.ModeSymlink != 0 { - return nil - } - return os.Chmod(path, 0666) - } - return t.StreamFileAsTarWithCallback(source, name, writer, makeFileWorldWritable, false) -} - -// StreamFileAsTarWithCallback streams the source file as a tar archive. -func (t *stiTar) StreamFileAsTarWithCallback(source, name string, writer io.Writer, walkFn filepath.WalkFunc, modifyInplace bool) error { - f, err := os.Open(source) - if err != nil { - return err - } - defer f.Close() - info, err := f.Stat() - if err != nil { - return err - } - if info.IsDir() { - return fmt.Errorf("the source %q has to be regular file, not directory", source) - } - fs := util.NewFileSystem() - tmpDir, err := ioutil.TempDir("", "s2i-") - if err != nil { - return err - } - defer os.RemoveAll(tmpDir) - dst := filepath.Join(tmpDir, name) - if err := fs.Copy(source, dst); err != nil { - return err - } - fileInfo, fileErr := os.Stat(dst) - if err := walkFn(dst, fileInfo, fileErr); err != nil { - return err - } - return t.CreateTarStream(tmpDir, false, writer) -} - // CreateTarFile creates a tar file from the given directory // while excluding files that match the given exclusion pattern // It returns the name of the created file @@ -234,19 +170,29 @@ func (t *stiTar) shouldExclude(path string) bool { return t.exclude != nil && t.exclude.String() != "" && t.exclude.MatchString(path) } -// CreateTarStream calls CreateTarStreamWithLogging with a nil logger +// CreateTarStream calls CreateTarStreamToTarWriter with a nil logger func (t *stiTar) CreateTarStream(dir string, includeDirInPath bool, writer io.Writer) error { - return t.CreateTarStreamWithLogging(dir, includeDirInPath, writer, nil) -} - -// CreateTarStreamWithLogging creates a tar stream on the given writer from -// the given directory while excluding files that match the given -// exclusion pattern. -// TODO: this should encapsulate the goroutine that generates the stream. -func (t *stiTar) CreateTarStreamWithLogging(dir string, includeDirInPath bool, writer io.Writer, logger io.Writer) error { - dir = filepath.Clean(dir) // remove relative paths and extraneous slashes tarWriter := tar.NewWriter(writer) defer tarWriter.Close() + + return t.CreateTarStreamToTarWriter(dir, includeDirInPath, tarWriter, nil) +} + +// CreateTarStreamReader returns an io.ReadCloser from which a tar stream can be +// read. The tar stream is created using CreateTarStream. +func (t *stiTar) CreateTarStreamReader(dir string, includeDirInPath bool) io.ReadCloser { + r, w := io.Pipe() + go func() { + w.CloseWithError(t.CreateTarStream(dir, includeDirInPath, w)) + }() + return r +} + +// CreateTarStreamToTarWriter creates a tar stream on the given writer from +// the given directory while excluding files that match the given +// exclusion pattern. +func (t *stiTar) CreateTarStreamToTarWriter(dir string, includeDirInPath bool, tarWriter Writer, logger io.Writer) error { + dir = filepath.Clean(dir) // remove relative paths and extraneous slashes glog.V(5).Infof("Adding %q to tar ...", dir) err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { if err != nil { @@ -292,7 +238,7 @@ func (t *stiTar) CreateTarStreamWithLogging(dir string, includeDirInPath bool, w } // writeTarHeader writes tar header for given file, returns error if operation fails -func (t *stiTar) writeTarHeader(tarWriter *tar.Writer, dir string, path string, info os.FileInfo, includeDirInPath bool, logger io.Writer) error { +func (t *stiTar) writeTarHeader(tarWriter Writer, dir string, path string, info os.FileInfo, includeDirInPath bool, logger io.Writer) error { var ( link string err error @@ -341,31 +287,26 @@ func (t *stiTar) ExtractTarStreamWithLogging(dir string, reader io.Reader, logge // Times out if reading from the stream for any given file // exceeds the value of timeout func (t *stiTar) ExtractTarStreamFromTarReader(dir string, tarReader Reader, logger io.Writer) error { - errorChannel := make(chan error, 1) - timeoutTimer := time.NewTimer(t.timeout) - go func() { + err := util.TimeoutAfter(t.timeout, "", func(timeoutTimer *time.Timer) error { for { header, err := tarReader.Next() if !timeoutTimer.Stop() { - break + return &util.TimeoutError{} } timeoutTimer.Reset(t.timeout) if err == io.EOF { - errorChannel <- nil - break + return nil } if err != nil { glog.Errorf("Error reading next tar header: %v", err) - errorChannel <- err - break + return err } if header.FileInfo().IsDir() { dirPath := filepath.Join(dir, header.Name) glog.V(3).Infof("Creating directory %s", dirPath) if err = os.MkdirAll(dirPath, 0700); err != nil { glog.Errorf("Error creating dir %q: %v", dirPath, err) - errorChannel <- err - break + return err } } else { fileDir := filepath.Dir(header.Name) @@ -373,40 +314,35 @@ func (t *stiTar) ExtractTarStreamFromTarReader(dir string, tarReader Reader, log glog.V(3).Infof("Creating directory %s", dirPath) if err = os.MkdirAll(dirPath, 0700); err != nil { glog.Errorf("Error creating dir %q: %v", dirPath, err) - errorChannel <- err - break + return err } if header.Typeflag == tar.TypeSymlink { if err := extractLink(dir, header, tarReader); err != nil { glog.Errorf("Error extracting link %q: %v", header.Name, err) - errorChannel <- err - break + return err } continue } logFile(logger, header.Name) if err := extractFile(dir, header, tarReader); err != nil { glog.Errorf("Error extracting file %q: %v", header.Name, err) - errorChannel <- err - break + return err } } } - }() + }) - for { - select { - case err := <-errorChannel: - if err != nil { - glog.Error("Error extracting tar stream") - } else { - glog.V(2).Info("Done extracting tar stream") - } - return err - case <-timeoutTimer.C: - return errors.NewTarTimeoutError() - } + if err != nil { + glog.Error("Error extracting tar stream") + } else { + glog.V(2).Info("Done extracting tar stream") } + + if util.IsTimeoutError(err) { + err = errors.NewTarTimeoutError() + } + + return err } func extractLink(dir string, header *tar.Header, tarReader io.Reader) error { diff --git a/pkg/test/tar.go b/pkg/test/tar.go index ab2b35a0e..d33052454 100644 --- a/pkg/test/tar.go +++ b/pkg/test/tar.go @@ -3,7 +3,6 @@ package test import ( "errors" "io" - "path/filepath" "regexp" "sync" @@ -73,29 +72,9 @@ func (f *FakeTar) ExtractTarStream(dir string, reader io.Reader) error { func (f *FakeTar) SetExclusionPattern(*regexp.Regexp) { } -// StreamFileAsTar streams a single file as a TAR archive into specified writer. -func (f *FakeTar) StreamFileAsTar(string, string, io.Writer) error { - return nil -} - -// StreamFileAsTarWithCallback streams a single file as a TAR archive into specified writer. -func (f *FakeTar) StreamFileAsTarWithCallback(source, name string, writer io.Writer, walkFn filepath.WalkFunc, modifyInplace bool) error { - return errors.New("not implemented") -} - -// StreamDirAsTar streams a directory as a TAR archive into specified writer. -func (f *FakeTar) StreamDirAsTar(string, string, io.Writer) error { - return nil -} - -// StreamDirAsTarWithCallback streams a directory as a TAR archive into specified writer. -func (f *FakeTar) StreamDirAsTarWithCallback(source string, writer io.Writer, walkFn filepath.WalkFunc, modifyInplace bool) error { - return errors.New("not implemented") -} - -// CreateTarStreamWithLogging creates a tar from the given directory and streams +// CreateTarStreamToTarWriter creates a tar from the given directory and streams // it to the given writer. -func (f *FakeTar) CreateTarStreamWithLogging(dir string, includeDirInPath bool, writer io.Writer, logger io.Writer) error { +func (f *FakeTar) CreateTarStreamToTarWriter(dir string, includeDirInPath bool, writer tar.Writer, logger io.Writer) error { f.lock.Lock() defer f.lock.Unlock() f.CreateTarDir = dir @@ -105,5 +84,14 @@ func (f *FakeTar) CreateTarStreamWithLogging(dir string, includeDirInPath bool, // CreateTarStream creates a tar from the given directory and streams it to the // given writer. func (f *FakeTar) CreateTarStream(dir string, includeDirInPath bool, writer io.Writer) error { - return f.CreateTarStreamWithLogging(dir, includeDirInPath, writer, nil) + return f.CreateTarStreamToTarWriter(dir, includeDirInPath, nil, nil) +} + +// CreateTarStreamReader returns an io.ReadCloser from which a tar stream can be +// read. The tar stream is created using CreateTarStream. +func (f *FakeTar) CreateTarStreamReader(dir string, includeDirInPath bool) io.ReadCloser { + f.CreateTarStreamToTarWriter(dir, includeDirInPath, nil, nil) + r, w := io.Pipe() + go w.CloseWithError(f.CreateTarError) + return r } diff --git a/pkg/util/status/build_status.go b/pkg/util/status/build_status.go index 74902e780..d6dabd69d 100644 --- a/pkg/util/status/build_status.go +++ b/pkg/util/status/build_status.go @@ -97,13 +97,6 @@ const ( // range of failures. ReasonMessageGenericS2iBuildFailed api.StepFailureMessage = "Generic S2I Build failure - check S2I logs for details" - // ReasonTarSourceFailed is the failure reason associated with a failure to - // tar the current source. - ReasonTarSourceFailed api.StepFailureReason = "TarSourceFailed" - // ReasonMessageTarSourceFailed is the message associated with a failure to - // tar the current source. - ReasonMessageTarSourceFailed api.StepFailureMessage = "Failed to tar source files" - // ReasonOnBuildForbidden is the failure reason associated with an image that // uses the ONBUILD instruction when it's not allowed. ReasonOnBuildForbidden api.StepFailureReason = "OnBuildForbidden"