diff --git a/cmd/sti/main.go b/cmd/sti/main.go index c9eac9b88..fb44e975e 100644 --- a/cmd/sti/main.go +++ b/cmd/sti/main.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/pflag" "github.com/openshift/source-to-image/pkg/sti" + "github.com/openshift/source-to-image/pkg/sti/api" "github.com/openshift/source-to-image/pkg/sti/errors" "github.com/openshift/source-to-image/pkg/sti/version" ) @@ -52,7 +53,7 @@ func newCmdVersion() *cobra.Command { } } -func newCmdBuild(req *sti.Request) *cobra.Command { +func newCmdBuild(req *api.Request) *cobra.Command { buildCmd := &cobra.Command{ Use: "build ", Short: "Build a new image", @@ -85,12 +86,13 @@ func newCmdBuild(req *sti.Request) *cobra.Command { buildCmd.Flags().StringVarP(&(req.Ref), "ref", "r", "", "Specify a ref to check-out") buildCmd.Flags().StringVar(&(req.CallbackURL), "callbackURL", "", "Specify a URL to invoke via HTTP POST upon build completion") buildCmd.Flags().StringVarP(&(req.ScriptsURL), "scripts", "s", "", "Specify a URL for the assemble and run scripts") + buildCmd.Flags().StringVarP(&(req.Location), "location", "l", "", "Specify a destination location for untar operation") buildCmd.Flags().BoolVar(&(req.ForcePull), "forcePull", true, "Always pull the builder image even if it is present locally") buildCmd.Flags().BoolVar(&(req.PreserveWorkingDir), "saveTempDir", false, "Save the temporary directory used by STI instead of deleting it") return buildCmd } -func newCmdUsage(req *sti.Request) *cobra.Command { +func newCmdUsage(req *api.Request) *cobra.Command { usageCmd := &cobra.Command{ Use: "usage ", Short: "Print usage of the assemble script associated with the image", @@ -116,6 +118,7 @@ func newCmdUsage(req *sti.Request) *cobra.Command { usageCmd.Flags().StringVarP(&(req.ScriptsURL), "scripts", "s", "", "Specify a URL for the assemble and run scripts") usageCmd.Flags().BoolVar(&(req.ForcePull), "forcePull", true, "Always pull the builder image even if it is present locally") usageCmd.Flags().BoolVar(&(req.PreserveWorkingDir), "saveTempDir", false, "Save the temporary directory used by STI instead of deleting it") + usageCmd.Flags().StringVarP(&(req.Location), "location", "l", "", "Specify a destination location for untar operation") return usageCmd } @@ -151,7 +154,7 @@ func checkErr(err error) { } func main() { - req := &sti.Request{} + req := &api.Request{} stiCmd := &cobra.Command{ Use: "sti", Long: "Source-to-image (STI) is a tool for building repeatable docker images.\n\n" + diff --git a/hack/build-test-images.sh b/hack/build-test-images.sh index 7f9bce1be..98f9a61f9 100755 --- a/hack/build-test-images.sh +++ b/hack/build-test-images.sh @@ -25,3 +25,4 @@ buildimage sti_test/sti-fake test/integration/images/sti-fake buildimage sti_test/sti-fake-user test/integration/images/sti-fake-user buildimage sti_test/sti-fake-scripts test/integration/images/sti-fake-scripts buildimage sti_test/sti-fake-scripts-no-save-artifacts test/integration/images/sti-fake-scripts-no-save-artifacts +buildimage sti_test/sti-fake-no-tar test/integration/images/sti-fake-no-tar diff --git a/hack/test-integration.sh b/hack/test-integration.sh index 24a4d8fe3..ac8ac7ac7 100755 --- a/hack/test-integration.sh +++ b/hack/test-integration.sh @@ -17,7 +17,7 @@ set +e img_count=$(docker images | grep -c sti_test/sti-fake) set -e -if [ "${img_count}" != "4" ]; then +if [ "${img_count}" != "5" ]; then echo "You do not have necessary test images, be sure to run 'hack/build-test-images.sh' beforehand." exit 1 fi diff --git a/pkg/sti/api/doc.go b/pkg/sti/api/doc.go new file mode 100644 index 000000000..2bca55413 --- /dev/null +++ b/pkg/sti/api/doc.go @@ -0,0 +1,2 @@ +// Provides types used for processing sti requests. +package api diff --git a/pkg/sti/api/script.go b/pkg/sti/api/script.go new file mode 100644 index 000000000..3c49e037d --- /dev/null +++ b/pkg/sti/api/script.go @@ -0,0 +1,20 @@ +package api + +// Script defines script names used by STI process. +type Script string + +const ( + // Assemble is the name of the script responsible for build process of the resulting image. + Assemble Script = "assemble" + // Run is the name of the script responsible for running the final application. + Run Script = "run" + // SaveArtifacts is the name of the script responsible for storing dependencies etc. between builds. + SaveArtifacts Script = "save-artifacts" + // Usage i the name of the script responsible for printing the builder image's short info. + Usage Script = "usage" +) + +// String returns name of the script. +func (s Script) String() string { + return string(s) +} diff --git a/pkg/sti/types.go b/pkg/sti/api/types.go similarity index 75% rename from pkg/sti/types.go rename to pkg/sti/api/types.go index 66661ddd0..924f150d2 100644 --- a/pkg/sti/types.go +++ b/pkg/sti/api/types.go @@ -1,4 +1,4 @@ -package sti +package api // Request contains essential fields for any request. type Request struct { @@ -37,20 +37,26 @@ type Request struct { // ScriptsURL is a URL describing the localization of STI scripts used during build process. ScriptsURL string + // Location specifies a location where the untar operation will place its artifacts. + Location string + // ForcePull describes if the builder should pull the images from registry prior to building. ForcePull bool - // incremental describes incremental status of current build - incremental bool + // Incremental describes incremental status of current build + Incremental bool + + // WorkingDir describes temporary directory used for downloading sources, scripts and tar operations. + WorkingDir string - // workingDir describes temporary directory used for downloading sources, scripts and tar operations. - workingDir string + // ExternalRequiredScripts describes if required scripts are from external URL. + ExternalRequiredScripts bool - // externalRequiredScripts describes if required scripts are from external URL. - externalRequiredScripts bool + // ExternalOptionalScripts describes if optional scripts are from external URL. + ExternalOptionalScripts bool - // externalOptionalScripts describes if optional scripts are from external URL. - externalOptionalScripts bool + // LayeredBuild describes if this is build which layered scripts and sources on top of BaseImage. + LayeredBuild bool } // Result structure contains information from build process. diff --git a/pkg/sti/build.go b/pkg/sti/build.go index f5fd8b311..beda4021e 100644 --- a/pkg/sti/build.go +++ b/pkg/sti/build.go @@ -3,9 +3,11 @@ package sti import ( "io" "path/filepath" + "regexp" "github.com/golang/glog" + "github.com/openshift/source-to-image/pkg/sti/api" "github.com/openshift/source-to-image/pkg/sti/docker" "github.com/openshift/source-to-image/pkg/sti/errors" "github.com/openshift/source-to-image/pkg/sti/util" @@ -18,13 +20,15 @@ type Builder struct { type buildHandlerInterface interface { cleanup() - setup(requiredScripts, optionalScripts []string) error + setup(required []api.Script, optional []api.Script) error determineIncremental() error - Request() *Request - Result() *Result + Request() *api.Request + Result() *api.Result saveArtifacts() error fetchSource() error - execute(command string) error + execute(command api.Script) error + wasExpectedError(text string) bool + build() error } type buildHandler struct { @@ -33,7 +37,7 @@ type buildHandler struct { } // NewBuilder returns a new Builder -func NewBuilder(req *Request) (*Builder, error) { +func NewBuilder(req *api.Request) (*Builder, error) { handler, err := newBuildHandler(req) if err != nil { return nil, err @@ -43,7 +47,7 @@ func NewBuilder(req *Request) (*Builder, error) { }, nil } -func newBuildHandler(req *Request) (*buildHandler, error) { +func newBuildHandler(req *api.Request) (*buildHandler, error) { rh, err := newRequestHandler(req) if err != nil { return nil, err @@ -53,18 +57,19 @@ func newBuildHandler(req *Request) (*buildHandler, error) { callbackInvoker: util.NewCallbackInvoker(), } rh.postExecutor = bh + rh.errorChecker = bh return bh, nil } -// Build processes a Request and returns a *Result and an error. +// Build processes a Request and returns a *api.Result and an error. // An error represents a failure performing the build rather than a failure // of the build itself. Callers should check the Success field of the result // to determine whether a build succeeded or not. -func (b *Builder) Build() (*Result, error) { +func (b *Builder) Build() (*api.Result, error) { bh := b.handler defer bh.cleanup() - err := bh.setup([]string{"assemble", "run"}, []string{"save-artifacts"}) + err := bh.setup([]api.Script{api.Assemble, api.Run}, []api.Script{api.SaveArtifacts}) if err != nil { return nil, err } @@ -73,40 +78,61 @@ func (b *Builder) Build() (*Result, error) { if err != nil { return nil, err } - if bh.Request().incremental { + if bh.Request().Incremental { glog.V(1).Infof("Existing image for tag %s detected for incremental build.", bh.Request().Tag) } else { glog.V(1).Infof("Clean build will be performed") } glog.V(2).Infof("Performing source build from %s", bh.Request().Source) - if bh.Request().incremental { + if bh.Request().Incremental { if err = bh.saveArtifacts(); err != nil { glog.Warning("Error saving previous build artifacts: %v", err) glog.Warning("Clean build will be performed!") } } - if err = bh.execute("assemble"); err != nil { + glog.V(1).Infof("Building %s", bh.Request().Tag) + err = bh.execute(api.Assemble) + if e, ok := err.(errors.ContainerError); ok && bh.wasExpectedError(e.ExpectedError) { + glog.Warningf("Image %s does not have tar! Performing additional build to add the scripts and sources.", + bh.Request().BaseImage) + if err := bh.build(); err != nil { + return nil, err + } + glog.V(2).Infof("Building %s using sti-enabled image", bh.Request().Tag) + if err := bh.execute(api.Assemble); err != nil { + return nil, err + } + } else if err != nil { return nil, err } return bh.Result(), nil } -func (h *buildHandler) PostExecute(containerID string, cmd []string) error { +// wasExpectedError is used for determining whether the error that appeared +// authorizes us to do the additional build injecting the scripts and sources. +func (h *buildHandler) wasExpectedError(text string) bool { + tar, _ := regexp.MatchString(`.*tar.*not found`, text) + sh, _ := regexp.MatchString(`.*/bin/sh.*no such file or directory`, text) + return tar || sh +} + +func (h *buildHandler) PostExecute(containerID string, location string) error { var ( err error previousImageID string ) - if h.request.incremental && h.request.RemovePreviousImage { + if h.request.Incremental && h.request.RemovePreviousImage { if previousImageID, err = h.docker.GetImageID(h.request.Tag); err != nil { glog.Errorf("Error retrieving previous image's metadata: %v", err) } } + cmd := []string{} opts := docker.CommitContainerOptions{ - Command: append(cmd, "run"), + Command: append(cmd, filepath.Join(location, string(api.Run))), Env: h.generateConfigEnv(), ContainerID: containerID, Repository: h.request.Tag, @@ -119,7 +145,7 @@ func (h *buildHandler) PostExecute(containerID string, cmd []string) error { h.result.ImageID = imageID glog.V(1).Infof("Tagged %s as %s", imageID, h.request.Tag) - if h.request.incremental && h.request.RemovePreviousImage && previousImageID != "" { + if h.request.Incremental && h.request.RemovePreviousImage && previousImageID != "" { glog.V(1).Infof("Removing previously-tagged image %s", previousImageID) if err = h.docker.RemoveImage(previousImageID); err != nil { glog.Errorf("Unable to remove previous image: %v", err) @@ -136,7 +162,7 @@ func (h *buildHandler) PostExecute(containerID string, cmd []string) error { } func (h *buildHandler) determineIncremental() (err error) { - h.request.incremental = false + h.request.Incremental = false if h.request.Clean { return } @@ -150,14 +176,14 @@ func (h *buildHandler) determineIncremental() (err error) { // we're assuming save-artifacts to exists for embedded scripts (if not we'll // warn a user upon container failure and proceed with clean build) // for external save-artifacts - check its existence - saveArtifactsExists := !h.request.externalOptionalScripts || - h.fs.Exists(filepath.Join(h.request.workingDir, "upload", "scripts", "save-artifacts")) - h.request.incremental = previousImageExists && saveArtifactsExists + saveArtifactsExists := !h.request.ExternalOptionalScripts || + h.fs.Exists(filepath.Join(h.request.WorkingDir, "upload", "scripts", string(api.SaveArtifacts))) + h.request.Incremental = previousImageExists && saveArtifactsExists return nil } func (h *buildHandler) saveArtifacts() (err error) { - artifactTmpDir := filepath.Join(h.request.workingDir, "upload", "artifacts") + artifactTmpDir := filepath.Join(h.request.WorkingDir, "upload", "artifacts") if err = h.fs.Mkdir(artifactTmpDir); err != nil { return err } @@ -171,11 +197,13 @@ func (h *buildHandler) saveArtifacts() (err error) { } opts := docker.RunContainerOptions{ - Image: image, - OverwriteCmd: true, - Command: "save-artifacts", - Stdout: writer, - OnStart: extractFunc, + Image: image, + ExternalScripts: h.request.ExternalRequiredScripts, + ScriptsURL: h.request.ScriptsURL, + Location: h.request.Location, + Command: api.SaveArtifacts, + Stdout: writer, + OnStart: extractFunc, } err = h.docker.RunContainer(opts) writer.Close() @@ -185,10 +213,10 @@ func (h *buildHandler) saveArtifacts() (err error) { return err } -func (h *buildHandler) Request() *Request { +func (h *buildHandler) Request() *api.Request { return h.request } -func (h *buildHandler) Result() *Result { +func (h *buildHandler) Result() *api.Result { return h.result } diff --git a/pkg/sti/build_test.go b/pkg/sti/build_test.go index 6d37739e5..d3ed69895 100644 --- a/pkg/sti/build_test.go +++ b/pkg/sti/build_test.go @@ -1,36 +1,41 @@ package sti import ( + "errors" "fmt" "reflect" "testing" - "github.com/openshift/source-to-image/pkg/sti/errors" + "github.com/openshift/source-to-image/pkg/sti/api" + stierr "github.com/openshift/source-to-image/pkg/sti/errors" "github.com/openshift/source-to-image/pkg/sti/test" ) type FakeBuildHandler struct { CleanupCalled bool - SetupRequired []string - SetupOptional []string + SetupRequired []api.Script + SetupOptional []api.Script SetupError error DetermineIncrementalCalled bool DetermineIncrementalError error - BuildRequest *Request - BuildResult *Result + BuildRequest *api.Request + BuildResult *api.Result SaveArtifactsCalled bool SaveArtifactsError error FetchSourceCalled bool FetchSourceError error - ExecuteCommand string + ExecuteCommand api.Script ExecuteError error + ExpectedError bool + LayeredBuildCalled bool + LayeredBuildError error } func (f *FakeBuildHandler) cleanup() { f.CleanupCalled = true } -func (f *FakeBuildHandler) setup(required []string, optional []string) error { +func (f *FakeBuildHandler) setup(required []api.Script, optional []api.Script) error { f.SetupRequired = required f.SetupOptional = optional return f.SetupError @@ -41,11 +46,11 @@ func (f *FakeBuildHandler) determineIncremental() error { return f.DetermineIncrementalError } -func (f *FakeBuildHandler) Request() *Request { +func (f *FakeBuildHandler) Request() *api.Request { return f.BuildRequest } -func (f *FakeBuildHandler) Result() *Result { +func (f *FakeBuildHandler) Result() *api.Result { return f.BuildResult } @@ -58,18 +63,27 @@ func (f *FakeBuildHandler) fetchSource() error { return f.FetchSourceError } -func (f *FakeBuildHandler) execute(command string) error { +func (f *FakeBuildHandler) execute(command api.Script) error { f.ExecuteCommand = command return f.ExecuteError } +func (f *FakeBuildHandler) wasExpectedError(text string) bool { + return f.ExpectedError +} + +func (f *FakeBuildHandler) build() error { + f.LayeredBuildCalled = true + return f.LayeredBuildError +} + func TestBuild(t *testing.T) { incrementalTest := []bool{false, true} for _, incremental := range incrementalTest { fh := &FakeBuildHandler{ - BuildRequest: &Request{incremental: incremental}, - BuildResult: &Result{}, + BuildRequest: &api.Request{Incremental: incremental}, + BuildResult: &api.Result{}, } builder := Builder{ handler: fh, @@ -77,10 +91,10 @@ func TestBuild(t *testing.T) { builder.Build() // Verify the right scripts were requested - if !reflect.DeepEqual(fh.SetupRequired, []string{"assemble", "run"}) { + if !reflect.DeepEqual(fh.SetupRequired, []api.Script{api.Assemble, api.Run}) { t.Errorf("Unexpected required scripts requested: %#v", fh.SetupRequired) } - if !reflect.DeepEqual(fh.SetupOptional, []string{"save-artifacts"}) { + if !reflect.DeepEqual(fh.SetupOptional, []api.Script{api.SaveArtifacts}) { t.Errorf("Unexpected optional scripts requested: %#v", fh.SetupOptional) } @@ -95,12 +109,83 @@ func TestBuild(t *testing.T) { } // Verify that execute was called with the right script - if fh.ExecuteCommand != "assemble" { + if fh.ExecuteCommand != api.Assemble { t.Errorf("Unexpected execute command: %s", fh.ExecuteCommand) } } } +func TestLayeredBuild(t *testing.T) { + fh := &FakeBuildHandler{ + BuildRequest: &api.Request{ + BaseImage: "testimage", + }, + BuildResult: &api.Result{}, + ExecuteError: stierr.NewContainerError("", 1, `/bin/sh: tar: not found`), + ExpectedError: true, + } + builder := Builder{ + handler: fh, + } + builder.Build() + // Verify layered build + if !fh.LayeredBuildCalled { + t.Errorf("Layered build was not called.") + } +} + +func TestBuildErrorExecute(t *testing.T) { + fh := &FakeBuildHandler{ + BuildRequest: &api.Request{ + BaseImage: "testimage", + }, + BuildResult: &api.Result{}, + ExecuteError: errors.New("ExecuteError"), + ExpectedError: false, + } + builder := Builder{ + handler: fh, + } + _, err := builder.Build() + if err == nil || err.Error() != "ExecuteError" { + t.Errorf("An error was expected, but got different %v", err) + } +} + +func TestWasExpectedError(t *testing.T) { + type expErr struct { + text string + expected bool + } + + tests := []expErr{ + { // 0 - tar error + text: `/bin/sh: tar: not found`, + expected: true, + }, + { // 1 - tar error + text: `/bin/sh: tar: command not found`, + expected: true, + }, + { // 2 - /bin/sh error + text: `exec: "/bin/sh": stat /bin/sh: no such file or directory`, + expected: true, + }, + { // 3 - non container error + text: "other error", + expected: false, + }, + } + + for i, ti := range tests { + bh := &buildHandler{} + result := bh.wasExpectedError(ti.text) + if result != ti.expected { + t.Errorf("(%d) Unexpected result: %v. Expected: %v", i, result, ti.expected) + } + } +} + func testBuildHandler() *buildHandler { requestHandler := &requestHandler{ docker: &test.FakeDocker{}, @@ -108,8 +193,9 @@ func testBuildHandler() *buildHandler { git: &test.FakeGit{}, fs: &test.FakeFileSystem{}, tar: &test.FakeTar{}, - request: &Request{}, - result: &Result{}, + + request: &api.Request{}, + result: &api.Result{}, } buildHandler := &buildHandler{ requestHandler: requestHandler, @@ -129,35 +215,30 @@ func TestPostExecute(t *testing.T) { bh.request.CallbackURL = "https://my.callback.org/test" bh.request.Tag = "test/tag" dh := bh.docker.(*test.FakeDocker) - bh.request.incremental = incremental + bh.request.Incremental = incremental if previousImageID != "" { bh.request.RemovePreviousImage = true bh.docker.(*test.FakeDocker).GetImageIDResult = previousImageID } - err := bh.PostExecute("test-container-id", []string{"cmd1", "arg1"}) + err := bh.PostExecute("test-container-id", "cmd1") if err != nil { t.Errorf("Unexpected errror from postExecute: %v", err) } // Ensure CommitContainer was called with the right parameters - if !reflect.DeepEqual(dh.CommitContainerOpts.Command, - []string{"cmd1", "arg1", "run"}) { - t.Errorf("Unexpected commit container command: %#v", - dh.CommitContainerOpts.Command) + if !reflect.DeepEqual(dh.CommitContainerOpts.Command, []string{"cmd1/run"}) { + t.Errorf("Unexpected commit container command: %#v", dh.CommitContainerOpts.Command) } if dh.CommitContainerOpts.Repository != bh.request.Tag { - t.Errorf("Unexpected tag commited: %s", - dh.CommitContainerOpts.Repository) + t.Errorf("Unexpected tag commited: %s", dh.CommitContainerOpts.Repository) } if incremental && previousImageID != "" { if dh.RemoveImageName != "test-image" { - t.Errorf("Previous image was not removed: %s", - dh.RemoveImageName) + t.Errorf("Previous image was not removed: %s", dh.RemoveImageName) } } else { if dh.RemoveImageName != "" { - t.Errorf("Unexpected image removed: %s", - dh.RemoveImageName) + t.Errorf("Unexpected image removed: %s", dh.RemoveImageName) } } @@ -213,9 +294,9 @@ func TestDetermineIncremental(t *testing.T) { for i, ti := range tests { bh := testBuildHandler() - bh.request.workingDir = "/working-dir" + bh.request.WorkingDir = "/working-dir" bh.request.Clean = ti.clean - bh.request.externalOptionalScripts = ti.scriptDownload + bh.request.ExternalOptionalScripts = ti.scriptDownload bh.docker.(*test.FakeDocker).LocalRegistryResult = ti.previousImage if ti.scriptExists { bh.fs.(*test.FakeFileSystem).ExistsResult = map[string]bool{ @@ -223,9 +304,9 @@ func TestDetermineIncremental(t *testing.T) { } } bh.determineIncremental() - if bh.request.incremental != ti.expected { + if bh.request.Incremental != ti.expected { t.Errorf("(%d) Unexpected incremental result: %v. Expected: %v", - i, bh.request.incremental, ti.expected) + i, bh.request.Incremental, ti.expected) } if !ti.clean && ti.previousImage && ti.scriptDownload { scriptChecked := bh.fs.(*test.FakeFileSystem).ExistsFile[0] @@ -240,7 +321,7 @@ func TestDetermineIncremental(t *testing.T) { func TestSaveArtifacts(t *testing.T) { bh := testBuildHandler() - bh.request.workingDir = "/working-dir" + bh.request.WorkingDir = "/working-dir" bh.request.Tag = "image/tag" fs := bh.fs.(*test.FakeFileSystem) fd := bh.docker.(*test.FakeDocker) @@ -266,11 +347,11 @@ func TestSaveArtifacts(t *testing.T) { func TestSaveArtifactsRunError(t *testing.T) { tests := []error{ fmt.Errorf("Run error"), - errors.NewContainerError("", -1, nil), + stierr.NewContainerError("", -1, ""), } expected := []error{ tests[0], - errors.NewSaveArtifactsError("", tests[1]), + stierr.NewSaveArtifactsError("", tests[1]), } // test with tar extract error or not tarError := []bool{true, false} @@ -354,7 +435,7 @@ func TestFetchSource(t *testing.T) { gh := bh.git.(*test.FakeGit) fh := bh.fs.(*test.FakeFileSystem) - bh.request.workingDir = "/working-dir" + bh.request.WorkingDir = "/working-dir" gh.ValidCloneSpecResult = ft.validCloneSpec if ft.refSpecified { bh.request.Ref = "a-branch" diff --git a/pkg/sti/docker/docker.go b/pkg/sti/docker/docker.go index 4acac6261..b9847f2bf 100644 --- a/pkg/sti/docker/docker.go +++ b/pkg/sti/docker/docker.go @@ -1,29 +1,38 @@ package docker import ( + "fmt" "io" + "path/filepath" "strings" "sync" "github.com/fsouza/go-dockerclient" "github.com/golang/glog" + "github.com/openshift/source-to-image/pkg/sti/api" "github.com/openshift/source-to-image/pkg/sti/errors" ) +const ( + ScriptsURL = "STI_SCRIPTS_URL" + Location = "STI_LOCATION" +) + // Docker is the interface between STI and the Docker client // It contains higher level operations called from the STI // build or usage commands type Docker interface { IsImageInLocalRegistry(name string) (bool, error) RemoveContainer(id string) error - GetDefaultScriptsURL(name string) (string, error) + GetScriptsURL(name string) (string, error) RunContainer(opts RunContainerOptions) error GetImageID(name string) (string, error) CommitContainer(opts CommitContainerOptions) (string, error) RemoveImage(name string) error PullImage(name string) error CheckAndPull(name string) (*docker.Image, error) + BuildImage(opts BuildImageOptions) error } // Client contains all methods called on the go Docker @@ -39,6 +48,7 @@ type Client interface { RemoveContainer(opts docker.RemoveContainerOptions) error CommitContainer(opts docker.CommitContainerOptions) (*docker.Image, error) CopyFromContainer(opts docker.CopyFromContainerOptions) error + BuildImage(opts docker.BuildImageOptions) error } type stiDocker struct { @@ -46,21 +56,23 @@ type stiDocker struct { } type postExecutor interface { - PostExecute(containerID string, cmd []string) error + PostExecute(containerID string, location string) error } // RunContainerOptions are options passed in to the RunContainer method type RunContainerOptions struct { - Image string - PullImage bool - OverwriteCmd bool - Command string - Env []string - Stdin io.Reader - Stdout io.Writer - Stderr io.Writer - OnStart func() error - PostExec postExecutor + Image string + PullImage bool + ExternalScripts bool + ScriptsURL string + Location string + Command api.Script + Env []string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer + OnStart func() error + PostExec postExecutor } // CommitContainerOptions are options passed in to the CommitContainer method @@ -71,6 +83,13 @@ type CommitContainerOptions struct { Env []string } +// BuildImageOptions are options passed in to the BuildImage method +type BuildImageOptions struct { + Name string + Stdin io.Reader + Stdout io.Writer +} + // NewDocker creates a new implementation of the STI Docker interface func NewDocker(endpoint string) (Docker, error) { client, err := docker.NewClient(endpoint) @@ -129,26 +148,34 @@ func (d *stiDocker) RemoveContainer(id string) error { return d.client.RemoveContainer(docker.RemoveContainerOptions{id, true, true}) } -// GetDefaultUrl finds a script URL in the given image's metadata -func (d *stiDocker) GetDefaultScriptsURL(name string) (string, error) { - imageMetadata, err := d.CheckAndPull(name) - if err != nil { - return "", err - } - defaultScriptsURL := "" - env := append(imageMetadata.ContainerConfig.Env, imageMetadata.Config.Env...) +// getVariable gets environment variable's value from the image metadata +func (d *stiDocker) getVariable(image *docker.Image, name string) string { + envName := name + "=" + env := append(image.ContainerConfig.Env, image.Config.Env...) for _, v := range env { - if strings.HasPrefix(v, "STI_SCRIPTS_URL=") { - defaultScriptsURL = strings.TrimSpace((v[len("STI_SCRIPTS_URL="):])) - break + if strings.HasPrefix(v, envName) { + return strings.TrimSpace((v[len(envName):])) } } - if len(defaultScriptsURL) == 0 { - glog.Warningf("Image does not contain default script URL") + + return "" +} + +// GetScriptsURL finds a STI_SCRIPTS_URL in the given image's metadata +func (d *stiDocker) GetScriptsURL(image string) (string, error) { + imageMetadata, err := d.CheckAndPull(image) + if err != nil { + return "", err + } + + scriptsURL := d.getVariable(imageMetadata, ScriptsURL) + if len(scriptsURL) == 0 { + glog.Warningf("Image does not contain a value for the STI_SCRIPTS_URL environment variable") } else { - glog.V(2).Infof("Image contains default script URL '%s'", defaultScriptsURL) + glog.V(2).Infof("Image contains STI_SCRIPTS_URL set to '%s'", scriptsURL) } - return defaultScriptsURL, nil + + return scriptsURL, nil } // RunContainer creates and starts a container using the image specified in the options with the ability @@ -166,11 +193,40 @@ func (d *stiDocker) RunContainer(opts RunContainerOptions) (err error) { return err } - cmd := imageMetadata.Config.Cmd - if opts.OverwriteCmd { - cmd[len(cmd)-1] = opts.Command + // base directory for all STI commands + var commandBaseDir string + // untar operation destination directory + tarDestination := opts.Location + if len(tarDestination) == 0 { + if val := d.getVariable(imageMetadata, Location); len(val) != 0 { + tarDestination = val + } else { + // default directory if none is specified + tarDestination = "/tmp" + } + } + if opts.ExternalScripts { + // for external scripts we must always append 'scripts' because this is + // the default subdirectory inside tar for them + commandBaseDir = filepath.Join(tarDestination, "scripts") + glog.V(2).Infof("Both scripts and untarred source will be placed in '%s'", tarDestination) } else { - cmd = append(cmd, opts.Command) + // for internal scripts we can have separate path for scripts and untar operation destination + scriptsURL := opts.ScriptsURL + if len(scriptsURL) == 0 { + scriptsURL = d.getVariable(imageMetadata, ScriptsURL) + } + commandBaseDir = strings.TrimPrefix(scriptsURL, "image://") + glog.V(2).Infof("Base directory for STI scripts is '%s'. Untarring destination is '%s'.", + commandBaseDir, tarDestination) + } + + cmd := []string{filepath.Join(commandBaseDir, string(opts.Command))} + // when calling assemble script with Stdin parameter set (the tar file) + // we need to first untar the whole archive and only then call the assemble script + if opts.Stdin != nil && (opts.Command == api.Assemble || opts.Command == api.Usage) { + cmd = []string{"/bin/sh", "-c", fmt.Sprintf("tar -C %s -xf - && %s", + tarDestination, filepath.Join(commandBaseDir, string(opts.Command)))} } config := docker.Config{ Image: opts.Image, @@ -218,8 +274,10 @@ func (d *stiDocker) RunContainer(opts RunContainerOptions) (err error) { }() attached <- <-attached - // If attaching both stdin and stdout, attach stdout in + // If attaching both stdin and stdout or stderr, attach stdout and stderr in // a second goroutine + // TODO remove this goroutine when docker 1.4 will be in broad usage, + // see: https://github.com/docker/docker/commit/f936a10d8048f471d115978472006e1b58a7c67d if opts.Stdin != nil && opts.Stdout != nil { attached2 := make(chan struct{}) attachOpts2 := docker.AttachToContainerOptions{ @@ -260,19 +318,18 @@ func (d *stiDocker) RunContainer(opts RunContainerOptions) (err error) { glog.V(2).Infof("Container exited") if exitCode != 0 { - return errors.NewContainerError(container.Name, exitCode, nil) + return errors.NewContainerError(container.Name, exitCode, "") } - if opts.PostExec != nil { glog.V(2).Infof("Invoking postExecution function") - if err = opts.PostExec.PostExecute(container.ID, imageMetadata.Config.Cmd); err != nil { + if err = opts.PostExec.PostExecute(container.ID, commandBaseDir); err != nil { return err } } return nil } -// GetImageID retrives the ID of the image identified by name +// GetImageID retrieves the ID of the image identified by name func (d *stiDocker) GetImageID(name string) (string, error) { image, err := d.client.InspectImage(name) if err != nil { @@ -284,7 +341,6 @@ func (d *stiDocker) GetImageID(name string) (string, error) { // CommitContainer commits a container to an image with a specific tag. // The new image ID is returned func (d *stiDocker) CommitContainer(opts CommitContainerOptions) (string, error) { - repository, tag := docker.ParseRepositoryTag(opts.Repository) dockerOpts := docker.CommitContainerOptions{ Container: opts.ContainerID, @@ -297,7 +353,7 @@ func (d *stiDocker) CommitContainer(opts CommitContainerOptions) (string, error) Env: opts.Env, } dockerOpts.Run = &config - glog.V(2).Infof("Commiting container with config: %+v", config) + glog.V(2).Infof("Committing container with config: %+v", config) } image, err := d.client.CommitContainer(dockerOpts) @@ -311,3 +367,17 @@ func (d *stiDocker) CommitContainer(opts CommitContainerOptions) (string, error) func (d *stiDocker) RemoveImage(imageID string) error { return d.client.RemoveImage(imageID) } + +// BuildImage builds the image according to specified options +func (d *stiDocker) BuildImage(opts BuildImageOptions) error { + dockerOpts := docker.BuildImageOptions{ + Name: opts.Name, + NoCache: true, + SuppressOutput: true, + RmTmpContainer: true, + ForceRmTmpContainer: true, + InputStream: opts.Stdin, + OutputStream: opts.Stdout, + } + return d.client.BuildImage(dockerOpts) +} diff --git a/pkg/sti/docker/docker_test.go b/pkg/sti/docker/docker_test.go index 6bba894c5..6c26c358a 100644 --- a/pkg/sti/docker/docker_test.go +++ b/pkg/sti/docker/docker_test.go @@ -8,6 +8,8 @@ import ( "time" "github.com/fsouza/go-dockerclient" + + "github.com/openshift/source-to-image/pkg/sti/api" "github.com/openshift/source-to-image/pkg/sti/docker/test" "github.com/openshift/source-to-image/pkg/sti/errors" ) @@ -187,7 +189,7 @@ func TestCommitContainerError(t *testing.T) { } } -func TestGetDefaultUrl(t *testing.T) { +func TestGetScriptsURL(t *testing.T) { type urltest struct { image docker.Image result string @@ -210,7 +212,7 @@ func TestGetDefaultUrl(t *testing.T) { "in containerConfig": { image: docker.Image{ ContainerConfig: docker.Config{ - Env: []string{"Env1=value1", "STI_SCRIPTS_URL=test_url_value"}, + Env: []string{"Env1=value1", ScriptsURL + "=test_url_value"}, }, Config: &docker.Config{}, }, @@ -223,7 +225,7 @@ func TestGetDefaultUrl(t *testing.T) { Config: &docker.Config{ Env: []string{ "Env1=value1", - "STI_SCRIPTS_URL=test_url_value_2", + ScriptsURL + "=test_url_value_2", "Env2=value2", }, }, @@ -234,7 +236,7 @@ func TestGetDefaultUrl(t *testing.T) { "contains =": { image: docker.Image{ ContainerConfig: docker.Config{ - Env: []string{"STI_SCRIPTS_URL=http://my.test.url/test?param=one"}, + Env: []string{ScriptsURL + "=http://my.test.url/test?param=one"}, }, Config: &docker.Config{}, }, @@ -255,7 +257,7 @@ func TestGetDefaultUrl(t *testing.T) { fakeDocker.InspectImageErr = []error{tst.inspectErr} } dh := getDocker(fakeDocker) - url, err := dh.GetDefaultScriptsURL("test/image") + url, err := dh.GetScriptsURL("test/image") if err != nil && !tst.errExpected { t.Errorf("%s: Unexpected error returned: %v", desc, err) } else if err == nil && tst.errExpected { @@ -269,64 +271,157 @@ func TestGetDefaultUrl(t *testing.T) { } func TestRunContainer(t *testing.T) { - fakeDocker := &test.FakeDockerClient{ - InspectImageResult: []*docker.Image{ - { - Config: &docker.Config{ - Cmd: []string{"test", "command"}, + type runtest struct { + image docker.Image + cmd api.Script + externalScripts bool + paramScriptsURL string + paramLocation string + cmdExpected []string + } + + tests := map[string]runtest{ + "default": { + image: docker.Image{ + ContainerConfig: docker.Config{}, + Config: &docker.Config{}, + }, + cmd: api.Assemble, + externalScripts: true, + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /tmp -xf - && /tmp/scripts/%s", api.Assemble)}, + }, + "paramLocation": { + image: docker.Image{ + ContainerConfig: docker.Config{}, + Config: &docker.Config{}, + }, + cmd: api.Assemble, + externalScripts: true, + paramLocation: "/opt/test", + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /opt/test -xf - && /opt/test/scripts/%s", api.Assemble)}, + }, + "paramLocation¶mScripts": { + image: docker.Image{ + ContainerConfig: docker.Config{}, + Config: &docker.Config{}, + }, + cmd: api.Assemble, + externalScripts: true, + paramLocation: "/opt/test", + paramScriptsURL: "http://my.test.url/test?param=one", + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /opt/test -xf - && /opt/test/scripts/%s", api.Assemble)}, + }, + "scriptsInsideImage": { + image: docker.Image{ + ContainerConfig: docker.Config{ + Env: []string{ScriptsURL + "=image:///opt/bin/"}, }, + Config: &docker.Config{}, }, + cmd: api.Assemble, + externalScripts: false, + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /tmp -xf - && /opt/bin/%s", api.Assemble)}, }, - Container: &docker.Container{ - ID: "12345-test", + "scriptsInsideImageWithParamLocation": { + image: docker.Image{ + ContainerConfig: docker.Config{ + Env: []string{ScriptsURL + "=image:///opt/bin"}, + }, + Config: &docker.Config{}, + }, + cmd: api.Assemble, + externalScripts: false, + paramLocation: "/opt/sti", + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /opt/sti -xf - && /opt/bin/%s", api.Assemble)}, + }, + "paramLocationFromImageVar": { + image: docker.Image{ + ContainerConfig: docker.Config{ + Env: []string{Location + "=/opt", ScriptsURL + "=http://my.test.url/test?param=one"}, + }, + Config: &docker.Config{}, + }, + cmd: api.Assemble, + externalScripts: true, + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /opt -xf - && /opt/scripts/%s", api.Assemble)}, + }, + "usageCommand": { + image: docker.Image{ + ContainerConfig: docker.Config{}, + Config: &docker.Config{}, + }, + cmd: api.Usage, + externalScripts: true, + cmdExpected: []string{"/bin/sh", "-c", fmt.Sprintf("tar -C /tmp -xf - && /tmp/scripts/%s", api.Usage)}, + }, + "otherCommand": { + image: docker.Image{ + ContainerConfig: docker.Config{}, + Config: &docker.Config{}, + }, + cmd: api.Run, + externalScripts: true, + cmdExpected: []string{fmt.Sprintf("/tmp/scripts/%s", api.Run)}, }, - AttachToContainerSleep: 200 * time.Millisecond, - } - dh := getDocker(fakeDocker) - err := dh.RunContainer(RunContainerOptions{ - Image: "test/image", - PullImage: true, - Command: "assemble", - Env: []string{"Key1=Value1", "Key2=Value2"}, - Stdin: os.Stdin, - Stdout: os.Stdout, - Stderr: os.Stdout, - }) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - // Validate the CreateContainer parameters - createConfig := fakeDocker.CreateContainerOpts.Config - if createConfig.Image != "test/image" { - t.Errorf("Unexpected create config image: %s", createConfig.Image) - } - if !reflect.DeepEqual(createConfig.Cmd, []string{"test", "command", "assemble"}) { - t.Errorf("Unexpected create config command: %#v", createConfig.Cmd) - } - if !reflect.DeepEqual(createConfig.Env, []string{"Key1=Value1", "Key2=Value2"}) { - t.Errorf("Unexpected create config env: %#v", createConfig.Env) - } - if !createConfig.OpenStdin || !createConfig.StdinOnce { - t.Errorf("Unexpected stdin flags for createConfig: OpenStdin - %v"+ - " StdinOnce - %v", createConfig.OpenStdin, createConfig.StdinOnce) } - // Verify that remove container was called - if fakeDocker.RemoveContainerOpts.ID != "12345-test" { - t.Errorf("RemoveContainer was not called with the expected container ID") - } + for desc, tst := range tests { + fakeDocker := &test.FakeDockerClient{ + InspectImageResult: []*docker.Image{&tst.image}, + Container: &docker.Container{ + ID: "12345-test", + }, + AttachToContainerSleep: 200 * time.Millisecond, + } + dh := getDocker(fakeDocker) + err := dh.RunContainer(RunContainerOptions{ + Image: "test/image", + PullImage: true, + ExternalScripts: tst.externalScripts, + ScriptsURL: tst.paramScriptsURL, + Location: tst.paramLocation, + Command: tst.cmd, + Env: []string{"Key1=Value1", "Key2=Value2"}, + Stdin: os.Stdin, + Stdout: os.Stdout, + Stderr: os.Stdout, + }) + if err != nil { + t.Errorf("%s: Unexpected error: %v", desc, err) + } + // Validate the CreateContainer parameters + createConfig := fakeDocker.CreateContainerOpts.Config + if createConfig.Image != "test/image" { + t.Errorf("%s: Unexpected create config image: %s", desc, createConfig.Image) + } + if !reflect.DeepEqual(createConfig.Cmd, tst.cmdExpected) { + t.Errorf("%s: Unexpected create config command: %#v", desc, createConfig.Cmd) + } + if !reflect.DeepEqual(createConfig.Env, []string{"Key1=Value1", "Key2=Value2"}) { + t.Errorf("%s: Unexpected create config env: %#v", desc, createConfig.Env) + } + if !createConfig.OpenStdin || !createConfig.StdinOnce { + t.Errorf("%s: Unexpected stdin flags for createConfig: OpenStdin - %v"+ + " StdinOnce - %v", desc, createConfig.OpenStdin, createConfig.StdinOnce) + } + + // Verify that remove container was called + if fakeDocker.RemoveContainerOpts.ID != "12345-test" { + t.Errorf("%s: RemoveContainer was not called with the expected container ID", desc) + } - // Verify that AttachToContainer was called twice (Stdin/Stdout) - if len(fakeDocker.AttachToContainerOpts) != 2 { - t.Errorf("AttachToContainer was not called the expected number of times.") - } - // Make sure AttachToContainer was not called with both Stdin & Stdout - for _, opt := range fakeDocker.AttachToContainerOpts { - if opt.InputStream != nil && (opt.OutputStream != nil || opt.ErrorStream != nil) { - t.Errorf("AttachToContainer was called with both Stdin and Stdout: %#v", opt) + // Verify that AttachToContainer was called twice (Stdin/Stdout) + if len(fakeDocker.AttachToContainerOpts) != 2 { + t.Errorf("%s: AttachToContainer was not called the expected number of times.", desc) } - if opt.Stdin && (opt.Stdout || opt.Stderr) { - t.Errorf("AttachToContainer was called with both Stdin and Stdout flags: %#v", opt) + // Make sure AttachToContainer was not called with both Stdin & Stdout + for _, opt := range fakeDocker.AttachToContainerOpts { + if opt.InputStream != nil && (opt.OutputStream != nil || opt.ErrorStream != nil) { + t.Errorf("%s: AttachToContainer was called with both Stdin and Stdout: %#v", desc, opt) + } + if opt.Stdin && (opt.Stdout || opt.Stderr) { + t.Errorf("%s: AttachToContainer was called with both Stdin and Stdout flags: %#v", desc, opt) + } } } } diff --git a/pkg/sti/docker/test/client.go b/pkg/sti/docker/test/client.go index a98e474e3..955a65710 100644 --- a/pkg/sti/docker/test/client.go +++ b/pkg/sti/docker/test/client.go @@ -23,6 +23,7 @@ type FakeDockerClient struct { RemoveContainerErr error CommitContainerErr error CopyFromContainerErr error + BuildImageErr error RemoveImageName string InspectImageName []string @@ -37,6 +38,7 @@ type FakeDockerClient struct { RemoveContainerOpts docker.RemoveContainerOptions CommitContainerOpts docker.CommitContainerOptions CopyFromContainerOpts docker.CopyFromContainerOptions + BuildImageOpts docker.BuildImageOptions mutex sync.Mutex } @@ -124,3 +126,9 @@ func (d *FakeDockerClient) CopyFromContainer(opts docker.CopyFromContainerOption d.CopyFromContainerOpts = opts return d.CopyFromContainerErr } + +// BuildImage builds image +func (d *FakeDockerClient) BuildImage(opts docker.BuildImageOptions) error { + d.BuildImageOpts = opts + return d.BuildImageErr +} diff --git a/pkg/sti/errors/errors.go b/pkg/sti/errors/errors.go index 9db687708..1a0f7a679 100644 --- a/pkg/sti/errors/errors.go +++ b/pkg/sti/errors/errors.go @@ -2,6 +2,8 @@ package errors import ( "fmt" + + "github.com/openshift/source-to-image/pkg/sti/api" ) // Common STI errors @@ -11,7 +13,7 @@ const ( ErrScriptDownload ErrSaveArtifacts ErrBuild - ErrStiContainer + ErrSTIContainer ErrTarTimeout ErrWorkDir ErrDownload @@ -30,11 +32,11 @@ type Error struct { // ContainerError is an error returned when a container exits with a non-zero code. // ExitCode contains the exit code from the container type ContainerError struct { - Message string - Details error - ErrorCode int - Suggestion string - ExitCode int + Message string + ExpectedError string + ErrorCode int + Suggestion string + ExitCode int } // Error returns a string for a given error @@ -71,7 +73,7 @@ func NewPullImageError(name string, err error) error { // NewScriptDownloadError returns a new error which indicates there was a problem // downloading a script -func NewScriptDownloadError(name string, err error) error { +func NewScriptDownloadError(name api.Script, err error) error { return Error{ Message: fmt.Sprintf("%s script download failed", name), Details: err, @@ -159,12 +161,12 @@ func NewDefaultScriptsURLError(err error) error { // NewContainerError return a new error which indicates there was a problem // invoking command inside container -func NewContainerError(name string, code int, err error) error { +func NewContainerError(name string, code int, expected string) error { return ContainerError{ - Message: fmt.Sprintf("non-zero (%d) exit code from %s", code, name), - Details: err, - ErrorCode: ErrStiContainer, - Suggestion: "check the container logs for more information on the failure", - ExitCode: code, + Message: fmt.Sprintf("non-zero (%d) exit code from %s", code, name), + ExpectedError: expected, + ErrorCode: ErrSTIContainer, + Suggestion: "check the container logs for more information on the failure", + ExitCode: code, } } diff --git a/pkg/sti/executor.go b/pkg/sti/executor.go index 0691bfe2c..ae258225f 100644 --- a/pkg/sti/executor.go +++ b/pkg/sti/executor.go @@ -1,23 +1,37 @@ package sti import ( - "os" + "bufio" + "bytes" + "fmt" + "io" "path/filepath" + "time" "github.com/golang/glog" + "github.com/openshift/source-to-image/pkg/sti/api" "github.com/openshift/source-to-image/pkg/sti/docker" + "github.com/openshift/source-to-image/pkg/sti/errors" "github.com/openshift/source-to-image/pkg/sti/git" "github.com/openshift/source-to-image/pkg/sti/script" "github.com/openshift/source-to-image/pkg/sti/tar" "github.com/openshift/source-to-image/pkg/sti/util" ) +const ( + // maxErrorOutput is the maximum length of the error output saved for processing + maxErrorOutput = 1024 + // defaultLocation is the default location of the scripts and sources in image + defaultLocation = "/tmp" +) + // requestHandler encapsulates dependencies needed to fulfill requests. type requestHandler struct { - request *Request - result *Result + request *api.Request + result *api.Result postExecutor postExecutor + errorChecker errorChecker installer script.Installer git git.Git fs util.FileSystem @@ -26,11 +40,15 @@ type requestHandler struct { } type postExecutor interface { - PostExecute(containerID string, cmd []string) error + PostExecute(containerID string, location string) error +} + +type errorChecker interface { + wasExpectedError(text string) bool } // newRequestHandler returns a new handler for a given request. -func newRequestHandler(req *Request) (*requestHandler, error) { +func newRequestHandler(req *api.Request) (*requestHandler, error) { glog.V(2).Infof("Using docker socket: %s", req.DockerSocket) docker, err := docker.NewDocker(req.DockerSocket) @@ -48,14 +66,14 @@ func newRequestHandler(req *Request) (*requestHandler, error) { }, nil } -func (h *requestHandler) setup(requiredScripts, optionalScripts []string) (err error) { - if h.request.workingDir, err = h.fs.CreateWorkingDirectory(); err != nil { +func (h *requestHandler) setup(requiredScripts, optionalScripts []api.Script) (err error) { + if h.request.WorkingDir, err = h.fs.CreateWorkingDirectory(); err != nil { return err } - h.result = &Result{ + h.result = &api.Result{ Success: false, - WorkingDir: h.request.workingDir, + WorkingDir: h.request.WorkingDir, } // immediately pull the image if forcepull is true, that way later code that @@ -78,18 +96,18 @@ func (h *requestHandler) setup(requiredScripts, optionalScripts []string) (err e dirs := []string{"upload/scripts", "downloads/scripts", "downloads/defaultScripts"} for _, v := range dirs { - if err = h.fs.MkdirAll(filepath.Join(h.request.workingDir, v)); err != nil { + if err = h.fs.MkdirAll(filepath.Join(h.request.WorkingDir, v)); err != nil { return err } } - if h.request.externalRequiredScripts, err = h.installer.DownloadAndInstall( - requiredScripts, h.request.workingDir, true); err != nil { + if h.request.ExternalRequiredScripts, err = h.installer.DownloadAndInstall( + requiredScripts, h.request.WorkingDir, true); err != nil { return err } - if h.request.externalOptionalScripts, err = h.installer.DownloadAndInstall( - optionalScripts, h.request.workingDir, false); err != nil { - return err + if h.request.ExternalOptionalScripts, err = h.installer.DownloadAndInstall( + optionalScripts, h.request.WorkingDir, false); err != nil { + glog.Warningf("Failed downloading optional scripts: %v", err) } return nil @@ -104,11 +122,11 @@ func (h *requestHandler) generateConfigEnv() (configEnv []string) { return } -func (h *requestHandler) execute(command string) error { +func (h *requestHandler) execute(command api.Script) error { glog.V(2).Infof("Using image name %s", h.request.BaseImage) - uploadDir := filepath.Join(h.request.workingDir, "upload") - tarFileName, err := h.tar.CreateTarFile(h.request.workingDir, uploadDir) + uploadDir := filepath.Join(h.request.WorkingDir, "upload") + tarFileName, err := h.tar.CreateTarFile(h.request.WorkingDir, uploadDir) if err != nil { return err } @@ -119,23 +137,122 @@ func (h *requestHandler) execute(command string) error { } defer tarFile.Close() + expectedError := "" + outReader, outWriter := io.Pipe() + errReader, errWriter := io.Pipe() + defer outReader.Close() + defer outWriter.Close() + defer errReader.Close() + defer errWriter.Close() opts := docker.RunContainerOptions{ - Image: h.request.BaseImage, - Stdin: tarFile, - Stdout: os.Stdout, - Stderr: os.Stderr, - PullImage: true, - Command: command, - Env: h.generateConfigEnv(), - PostExec: h, - } - return h.docker.RunContainer(opts) + Image: h.request.BaseImage, + Stdout: outWriter, + Stderr: errWriter, + PullImage: h.request.ForcePull, + ExternalScripts: h.request.ExternalRequiredScripts, + ScriptsURL: h.request.ScriptsURL, + Location: h.request.Location, + Command: command, + Env: h.generateConfigEnv(), + PostExec: h, + } + if !h.request.LayeredBuild { + opts.Stdin = tarFile + } + // goroutine to stream container's output + go func(reader io.Reader) { + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + if glog.V(2) || command == api.Usage { + glog.Info(scanner.Text()) + } + } + }(outReader) + // goroutine to stream container's error + go func(reader io.Reader) { + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + text := scanner.Text() + if glog.V(1) { + glog.Errorf(text) + } + if h.errorChecker != nil && h.errorChecker.wasExpectedError(text) && + len(expectedError) < maxErrorOutput { + expectedError += text + "; " + } + } + }(errReader) + + err = h.docker.RunContainer(opts) + if e, ok := err.(errors.ContainerError); ok { + return errors.NewContainerError(h.request.BaseImage, e.ErrorCode, expectedError) + } + return err } -func (h *requestHandler) PostExecute(containerID string, cmd []string) (err error) { +func (h *requestHandler) build() error { + // create Dockerfile + buffer := bytes.Buffer{} + location := h.request.Location + if len(location) == 0 { + location = defaultLocation + } + buffer.WriteString(fmt.Sprintf("FROM %s\n", h.request.BaseImage)) + buffer.WriteString(fmt.Sprintf("ADD scripts %s\n", filepath.Join(location, "scripts"))) + buffer.WriteString(fmt.Sprintf("ADD src %s\n", filepath.Join(location, "src"))) + uploadDir := filepath.Join(h.request.WorkingDir, "upload") + if err := h.fs.WriteFile(filepath.Join(uploadDir, "Dockerfile"), buffer.Bytes()); err != nil { + return err + } + glog.V(2).Infof("Writing custom Dockerfile to %s", uploadDir) + + tarFileName, err := h.tar.CreateTarFile(h.request.WorkingDir, uploadDir) + if err != nil { + return err + } + tarFile, err := h.fs.Open(tarFileName) + if err != nil { + return err + } + defer tarFile.Close() + + newBaseImage := fmt.Sprintf("%s-%d", h.request.BaseImage, time.Now().UnixNano()) + outReader, outWriter := io.Pipe() + defer outReader.Close() + defer outWriter.Close() + opts := docker.BuildImageOptions{ + Name: newBaseImage, + Stdin: tarFile, + Stdout: outWriter, + } + // goroutine to stream container's output + go func(reader io.Reader) { + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + glog.V(2).Info(scanner.Text()) + } + }(outReader) + glog.V(2).Infof("Building new image %s with scripts and sources already inside", newBaseImage) + if err = h.docker.BuildImage(opts); err != nil { + return err + } + + // upon successful build we need to modify current request + h.request.LayeredBuild = true + // new image name + h.request.BaseImage = newBaseImage + // the scripts are inside the image + h.request.ExternalRequiredScripts = false + h.request.ScriptsURL = "image://" + filepath.Join(location, "scripts") + // the source is also inside the image + h.request.Location = filepath.Join(location, "src") + return nil +} + +func (h *requestHandler) PostExecute(containerID string, location string) (err error) { h.result.Success = true if h.postExecutor != nil { - err = h.postExecutor.PostExecute(containerID, cmd) + err = h.postExecutor.PostExecute(containerID, location) if err != nil { glog.Errorf("An error occurred in post executor: %v", err) } @@ -145,14 +262,19 @@ func (h *requestHandler) PostExecute(containerID string, cmd []string) (err erro func (h *requestHandler) cleanup() { if h.request.PreserveWorkingDir { - glog.Infof("Temporary directory '%s' will be saved, not deleted", h.request.workingDir) + glog.Infof("Temporary directory '%s' will be saved, not deleted", h.request.WorkingDir) } else { - h.fs.RemoveDirectory(h.request.workingDir) + glog.V(2).Infof("Removing temporary directory %s", h.request.WorkingDir) + h.fs.RemoveDirectory(h.request.WorkingDir) + } + if h.request.LayeredBuild { + glog.V(2).Infof("Removing temporary image %s", h.request.BaseImage) + h.docker.RemoveImage(h.request.BaseImage) } } func (h *requestHandler) fetchSource() error { - targetSourceDir := filepath.Join(h.request.workingDir, "upload", "src") + targetSourceDir := filepath.Join(h.request.WorkingDir, "upload", "src") glog.V(1).Infof("Downloading %s to directory %s", h.request.Source, targetSourceDir) if h.git.ValidCloneSpec(h.request.Source) { if err := h.git.Clone(h.request.Source, targetSourceDir); err != nil { diff --git a/pkg/sti/executor_test.go b/pkg/sti/executor_test.go index a2b5eb964..9fa11e919 100644 --- a/pkg/sti/executor_test.go +++ b/pkg/sti/executor_test.go @@ -1,9 +1,13 @@ package sti import ( + "errors" + "fmt" "reflect" + "regexp" "testing" + "github.com/openshift/source-to-image/pkg/sti/api" "github.com/openshift/source-to-image/pkg/sti/test" ) @@ -14,15 +18,16 @@ func testRequestHandler() *requestHandler { git: &test.FakeGit{}, fs: &test.FakeFileSystem{}, tar: &test.FakeTar{}, - request: &Request{}, - result: &Result{}, + + request: &api.Request{}, + result: &api.Result{}, } } -func TestSetup(t *testing.T) { +func TestSetupOK(t *testing.T) { rh := testRequestHandler() rh.fs.(*test.FakeFileSystem).WorkingDirResult = "/working-dir" - err := rh.setup([]string{"required1", "required2"}, []string{"optional1", "optional2"}) + err := rh.setup([]api.Script{api.Assemble, api.Run}, []api.Script{api.SaveArtifacts}) if err != nil { t.Errorf("An error occurred setting up the request handler: %v", err) } @@ -42,14 +47,50 @@ func TestSetup(t *testing.T) { t.Errorf("Unexpected set of required flags: %#v", requiredFlags) } scripts := rh.installer.(*test.FakeInstaller).Scripts - if !reflect.DeepEqual(scripts[0], []string{"required1", "required2"}) { + if !reflect.DeepEqual(scripts[0], []api.Script{api.Assemble, api.Run}) { t.Errorf("Unexpected set of required scripts: %#v", scripts[0]) } - if !reflect.DeepEqual(scripts[1], []string{"optional1", "optional2"}) { + if !reflect.DeepEqual(scripts[1], []api.Script{api.SaveArtifacts}) { t.Errorf("Unexpected set of optional scripts: %#v", scripts[1]) } } +func TestSetupErrorCreatingWorkingDir(t *testing.T) { + rh := testRequestHandler() + rh.fs.(*test.FakeFileSystem).WorkingDirError = errors.New("WorkingDirError") + err := rh.setup([]api.Script{api.Assemble, api.Run}, []api.Script{api.SaveArtifacts}) + if err == nil || err.Error() != "WorkingDirError" { + t.Errorf("An error was expected for WorkingDir, but got different: %v", err) + } +} + +func TestSetupErrorMkdirAll(t *testing.T) { + rh := testRequestHandler() + rh.fs.(*test.FakeFileSystem).MkdirAllError = errors.New("MkdirAllError") + err := rh.setup([]api.Script{api.Assemble, api.Run}, []api.Script{api.SaveArtifacts}) + if err == nil || err.Error() != "MkdirAllError" { + t.Errorf("An error was expected for MkdirAll, but got different: %v", err) + } +} + +func TestSetupErrorRequiredDownloadAndInstall(t *testing.T) { + rh := testRequestHandler() + rh.installer.(*test.FakeInstaller).ErrScript = api.Assemble + err := rh.setup([]api.Script{api.Assemble, api.Run}, []api.Script{api.SaveArtifacts}) + if err == nil || err.Error() != string(api.Assemble) { + t.Errorf("An error was expected for required DownloadAndInstall, but got different: %v", err) + } +} + +func TestSetupErrorOptionalDownloadAndInstall(t *testing.T) { + rh := testRequestHandler() + rh.installer.(*test.FakeInstaller).ErrScript = api.SaveArtifacts + err := rh.setup([]api.Script{api.Assemble, api.Run}, []api.Script{api.SaveArtifacts}) + if err != nil { + t.Errorf("Unexpected error when downloading optional scripts: %v", err) + } +} + func equalArrayContents(a []string, b []string) bool { if len(a) != len(b) { return false @@ -86,21 +127,24 @@ func TestGenerateConfigEnv(t *testing.T) { type FakePostExecutor struct { containerID string - cmd []string + location string + err error } -func (f *FakePostExecutor) PostExecute(id string, cmd []string) error { +func (f *FakePostExecutor) PostExecute(id string, location string) error { + fmt.Errorf("Post execute called!!!!") f.containerID = id - f.cmd = cmd - return nil + f.location = location + return f.err } -func TestExecute(t *testing.T) { +func TestExecuteOK(t *testing.T) { rh := testRequestHandler() pe := &FakePostExecutor{} rh.postExecutor = pe - rh.request.workingDir = "/working-dir" + rh.request.WorkingDir = "/working-dir" rh.request.BaseImage = "test/image" + rh.request.ForcePull = true th := rh.tar.(*test.FakeTar) th.CreateTarResult = "/working-dir/test.tar" fd := rh.docker.(*test.FakeDocker) @@ -146,14 +190,92 @@ func TestExecute(t *testing.T) { t.Errorf("PostExecutor not called with expected ID: %s", pe.containerID) } - if !reflect.DeepEqual(pe.cmd, []string{"one", "two", "test-command"}) { - t.Errorf("PostExecutor not called with expected command: %#v", pe.cmd) + if !reflect.DeepEqual(pe.location, "test-command") { + t.Errorf("PostExecutor not called with expected command: %s", pe.location) + } +} + +func TestExecuteErrorCreateTarFile(t *testing.T) { + rh := testRequestHandler() + rh.tar.(*test.FakeTar).CreateTarError = errors.New("CreateTarError") + err := rh.execute("test-command") + if err == nil || err.Error() != "CreateTarError" { + t.Errorf("An error was expected for CreateTarFile, but got different: %v", err) + } +} + +func TestExecuteErrorOpenTarFile(t *testing.T) { + rh := testRequestHandler() + rh.fs.(*test.FakeFileSystem).OpenError = errors.New("OpenTarError") + err := rh.execute("test-command") + if err == nil || err.Error() != "OpenTarError" { + t.Errorf("An error was expected for OpenTarFile, but got different: %v", err) + } +} + +func TestBuildOK(t *testing.T) { + rh := testRequestHandler() + rh.request.BaseImage = "test/image" + err := rh.build() + if err != nil { + t.Errorf("Unexpected error returned: %v", err) + } + if !rh.request.LayeredBuild { + t.Errorf("Expected LayeredBuild to be true!") + } + if m, _ := regexp.MatchString(`test/image-\d+`, rh.request.BaseImage); !m { + t.Errorf("Expected BaseImage test/image-withnumbers, but got %s", rh.request.BaseImage) + } + if rh.request.ExternalRequiredScripts { + t.Errorf("Expected ExternalRequiredScripts to be false!") + } + if rh.request.ScriptsURL != "image:///tmp/scripts" { + t.Error("Expected ScriptsURL image:///tmp/scripts, but got %s", rh.request.ScriptsURL) + } + if rh.request.Location != "/tmp/src" { + t.Errorf("Expected Location /tmp/src, but got %s", rh.request.Location) + } +} + +func TestBuildErrorWriteDockerfile(t *testing.T) { + rh := testRequestHandler() + rh.fs.(*test.FakeFileSystem).WriteFileError = errors.New("WriteDockerfileError") + err := rh.build() + if err == nil || err.Error() != "WriteDockerfileError" { + t.Error("An error was expected for WriteDockerfile, but got different: %v", err) + } +} + +func TestBuildErrorCreateTarFile(t *testing.T) { + rh := testRequestHandler() + rh.tar.(*test.FakeTar).CreateTarError = errors.New("CreateTarError") + err := rh.build() + if err == nil || err.Error() != "CreateTarError" { + t.Error("An error was expected for CreateTar, but got different: %v", err) + } +} + +func TestBuildErrorOpenTarFile(t *testing.T) { + rh := testRequestHandler() + rh.fs.(*test.FakeFileSystem).OpenError = errors.New("OpenTarError") + err := rh.build() + if err == nil || err.Error() != "OpenTarError" { + t.Errorf("An error was expected for OpenTarFile, but got different: %v", err) + } +} + +func TestBuildErrorBuildImage(t *testing.T) { + rh := testRequestHandler() + rh.docker.(*test.FakeDocker).BuildImageError = errors.New("BuildImageError") + err := rh.build() + if err == nil || err.Error() != "BuildImageError" { + t.Errorf("An error was expected for BuildImage, but got different: %v", err) } } func TestCleanup(t *testing.T) { rh := testRequestHandler() - rh.request.workingDir = "/working-dir" + rh.request.WorkingDir = "/working-dir" preserve := []bool{false, true} for _, p := range preserve { rh.request.PreserveWorkingDir = p diff --git a/pkg/sti/script/installer.go b/pkg/sti/script/installer.go index f0f8658d3..4e7511bc7 100644 --- a/pkg/sti/script/installer.go +++ b/pkg/sti/script/installer.go @@ -7,6 +7,7 @@ import ( "github.com/golang/glog" + "github.com/openshift/source-to-image/pkg/sti/api" "github.com/openshift/source-to-image/pkg/sti/docker" "github.com/openshift/source-to-image/pkg/sti/errors" "github.com/openshift/source-to-image/pkg/sti/util" @@ -14,7 +15,7 @@ import ( // Installer interface is responsible for installing scripts needed to run the build type Installer interface { - DownloadAndInstall(scripts []string, workingDir string, required bool) (bool, error) + DownloadAndInstall(scripts []api.Script, workingDir string, required bool) (bool, error) } // NewInstaller returns a new instance of the default Installer implementation @@ -42,21 +43,21 @@ type handler struct { } type scriptHandler interface { - download(scripts []string, workingDir string) (bool, error) - getPath(script string, workingDir string) string + download(scripts []api.Script, workingDir string) (bool, error) + getPath(script api.Script, workingDir string) string install(scriptPath string, workingDir string) error } type scriptInfo struct { url *url.URL - name string + name api.Script } // DownloadAndInstall downloads and installs a set of scripts using the specified // working directory. If the required flag is specified and a particular script // cannot be found, an error is returned, additionally the method returns information // whether the download actually happened. -func (i *installer) DownloadAndInstall(scripts []string, workingDir string, required bool) (bool, error) { +func (i *installer) DownloadAndInstall(scripts []api.Script, workingDir string, required bool) (bool, error) { download, err := i.handler.download(scripts, workingDir) if err != nil { return false, err @@ -77,21 +78,21 @@ func (i *installer) DownloadAndInstall(scripts []string, workingDir string, requ return true, nil } -func (s *handler) download(scripts []string, workingDir string) (bool, error) { +func (s *handler) download(scripts []api.Script, workingDir string) (bool, error) { if len(scripts) == 0 { return false, nil } wg := sync.WaitGroup{} - errs := make(map[string]chan error) - downloads := make(map[string]chan bool) + errs := make(map[api.Script]chan error) + downloads := make(map[api.Script]chan bool) for _, s := range scripts { errs[s] = make(chan error, 2) downloads[s] = make(chan bool, 2) } - downloadAsync := func(script string, scriptUrl *url.URL, targetFile string) { + downloadAsync := func(script api.Script, scriptUrl *url.URL, targetFile string) { defer wg.Done() download, err := s.downloader.DownloadFile(scriptUrl, targetFile) if err != nil { @@ -115,7 +116,7 @@ func (s *handler) download(scripts []string, workingDir string) (bool, error) { } } - defaultURL, err := s.docker.GetDefaultScriptsURL(s.image) + defaultURL, err := s.docker.GetScriptsURL(s.image) if err != nil { return false, errors.NewDefaultScriptsURLError(err) } @@ -148,7 +149,7 @@ func (s *handler) download(scripts []string, workingDir string) (bool, error) { return true, nil } -func (s *handler) getPath(script string, workingDir string) string { +func (s *handler) getPath(script api.Script, workingDir string) string { locations := []string{ "downloads/scripts", "upload/src/.sti/bin", @@ -161,7 +162,7 @@ func (s *handler) getPath(script string, workingDir string) string { } for i, location := range locations { - path := filepath.Join(workingDir, location, script) + path := filepath.Join(workingDir, location, string(script)) glog.V(2).Infof("Looking for %s script at %s", script, path) if s.fs.Exists(path) { glog.V(2).Infof("Found %s script from %s.", script, descriptions[i]) @@ -178,17 +179,17 @@ func (s *handler) install(path string, workingDir string) error { } // prepareScriptDownload turns the script name into proper URL -func (s *handler) prepareDownload(scripts []string, targetDir, baseURL string) map[string]scriptInfo { +func (s *handler) prepareDownload(scripts []api.Script, targetDir, baseURL string) map[string]scriptInfo { s.fs.MkdirAll(targetDir) info := make(map[string]scriptInfo) for _, script := range scripts { - url, err := url.Parse(baseURL + "/" + script) + url, err := url.Parse(baseURL + "/" + string(script)) if err != nil { glog.Warningf("Unable to parse script URL: %s/%s", baseURL, script) continue } - info[targetDir+"/"+script] = scriptInfo{url, script} + info[targetDir+"/"+string(script)] = scriptInfo{url, script} } return info diff --git a/pkg/sti/script/installer_test.go b/pkg/sti/script/installer_test.go index 457afb60f..c16667862 100644 --- a/pkg/sti/script/installer_test.go +++ b/pkg/sti/script/installer_test.go @@ -5,29 +5,30 @@ import ( "reflect" "testing" + "github.com/openshift/source-to-image/pkg/sti/api" "github.com/openshift/source-to-image/pkg/sti/test" ) type FakeScriptHandler struct { - DownloadScripts []string + DownloadScripts []api.Script DownloadWorkingDir string DownloadResult bool DownloadError error - DetermineScript string + DetermineScript api.Script DetermineWorkingDir string - DetermineResult map[string]string + DetermineResult map[api.Script]string InstallScriptPath string InstallWorkingDir string InstallError error } -func (f *FakeScriptHandler) download(scripts []string, workingDir string) (bool, error) { +func (f *FakeScriptHandler) download(scripts []api.Script, workingDir string) (bool, error) { f.DownloadScripts = scripts f.DownloadWorkingDir = workingDir return f.DownloadResult, f.DownloadError } -func (f *FakeScriptHandler) getPath(script string, workingDir string) string { +func (f *FakeScriptHandler) getPath(script api.Script, workingDir string) string { f.DetermineScript = script f.DetermineWorkingDir = workingDir return f.DetermineResult[script] @@ -50,10 +51,10 @@ func TestDownloadAndInstallScripts(t *testing.T) { "successRequired": { handler: &FakeScriptHandler{ DownloadResult: true, - DetermineResult: map[string]string{ - "one": "one", - "two": "two", - "three": "three", + DetermineResult: map[api.Script]string{ + api.Assemble: "one", + api.Run: "two", + api.SaveArtifacts: "three", }, }, required: true, @@ -62,10 +63,10 @@ func TestDownloadAndInstallScripts(t *testing.T) { "successOptional": { handler: &FakeScriptHandler{ DownloadResult: true, - DetermineResult: map[string]string{ - "one": "one", - "two": "", - "three": "three", + DetermineResult: map[api.Script]string{ + api.Assemble: "one", + api.Run: "", + api.SaveArtifacts: "three", }, }, required: false, @@ -82,10 +83,10 @@ func TestDownloadAndInstallScripts(t *testing.T) { "errorRequired": { handler: &FakeScriptHandler{ DownloadResult: true, - DetermineResult: map[string]string{ - "one": "one", - "two": "two", - "three": "", + DetermineResult: map[api.Script]string{ + api.Assemble: "one", + api.Run: "two", + api.SaveArtifacts: "", }, }, required: true, @@ -94,10 +95,10 @@ func TestDownloadAndInstallScripts(t *testing.T) { "installError": { handler: &FakeScriptHandler{ DownloadResult: true, - DetermineResult: map[string]string{ - "one": "one", - "two": "two", - "three": "three", + DetermineResult: map[api.Script]string{ + api.Assemble: "one", + api.Run: "two", + api.SaveArtifacts: "three", }, InstallError: err, }, @@ -107,10 +108,10 @@ func TestDownloadAndInstallScripts(t *testing.T) { "noDownload": { handler: &FakeScriptHandler{ DownloadResult: false, - DetermineResult: map[string]string{ - "one": "one", - "two": "two", - "three": "three", + DetermineResult: map[api.Script]string{ + api.Assemble: "one", + api.Run: "two", + api.SaveArtifacts: "three", }, }, required: false, @@ -122,15 +123,15 @@ func TestDownloadAndInstallScripts(t *testing.T) { sh := &installer{ handler: test.handler, } - _, err := sh.DownloadAndInstall([]string{"one", "two", "three"}, "/test-working-dir", test.required) + _, err := sh.DownloadAndInstall([]api.Script{api.Assemble, api.Run, api.SaveArtifacts}, "/test-working-dir", test.required) if !test.errExpected && err != nil { t.Errorf("%s: Unexpected error: %v", desc, err) } else if test.errExpected && err == nil { t.Errorf("%s: Error expected. Got nil.", desc) } if !reflect.DeepEqual(sh.handler.(*FakeScriptHandler).DownloadScripts, - []string{"one", "two", "three"}) { - t.Errorf("%s: Unexpected downwload scripts: %#v", + []api.Script{api.Assemble, api.Run, api.SaveArtifacts}) { + t.Errorf("%s: Unexpected downwload scripts: %#v", desc, sh.handler.(*FakeScriptHandler).DownloadScripts) } } @@ -168,7 +169,7 @@ func TestDownload(t *testing.T) { sh := getScriptHandler() dl := sh.downloader.(*test.FakeDownloader) sh.docker.(*test.FakeDocker).DefaultURLResult = "http://image.url/scripts" - _, err := sh.download([]string{"one", "two", "three"}, "/working-dir") + _, err := sh.download([]api.Script{api.Assemble, api.Run, api.SaveArtifacts}, "/working-dir") if err != nil { t.Errorf("Got unexpected error: %v", err) } @@ -177,12 +178,12 @@ func TestDownload(t *testing.T) { len(dl.URL)) } expectedUrls := []string{ - "http://the.scripts.url/scripts/one", - "http://the.scripts.url/scripts/two", - "http://the.scripts.url/scripts/three", - "http://image.url/scripts/one", - "http://image.url/scripts/two", - "http://image.url/scripts/three", + fmt.Sprintf("http://the.scripts.url/scripts/%s", api.Assemble), + fmt.Sprintf("http://the.scripts.url/scripts/%s", api.Run), + fmt.Sprintf("http://the.scripts.url/scripts/%s", api.SaveArtifacts), + fmt.Sprintf("http://image.url/scripts/%s", api.Assemble), + fmt.Sprintf("http://image.url/scripts/%s", api.Run), + fmt.Sprintf("http://image.url/scripts/%s", api.SaveArtifacts), } actualUrls := []string{} for _, u := range dl.URL { @@ -193,12 +194,12 @@ func TestDownload(t *testing.T) { } expectedFiles := []string{ - "/working-dir/downloads/scripts/one", - "/working-dir/downloads/scripts/two", - "/working-dir/downloads/scripts/three", - "/working-dir/downloads/defaultScripts/one", - "/working-dir/downloads/defaultScripts/two", - "/working-dir/downloads/defaultScripts/three", + fmt.Sprintf("/working-dir/downloads/scripts/%s", api.Assemble), + fmt.Sprintf("/working-dir/downloads/scripts/%s", api.Run), + fmt.Sprintf("/working-dir/downloads/scripts/%s", api.SaveArtifacts), + fmt.Sprintf("/working-dir/downloads/defaultScripts/%s", api.Assemble), + fmt.Sprintf("/working-dir/downloads/defaultScripts/%s", api.Run), + fmt.Sprintf("/working-dir/downloads/defaultScripts/%s", api.SaveArtifacts), } if !equalArrayContents(dl.File, expectedFiles) { @@ -219,7 +220,7 @@ func TestDownloadErrors1(t *testing.T) { "http://image.url/scripts/two": dlErr, "http://image.url/scripts/three": nil, } - _, err := sh.download([]string{"one", "two", "three"}, "/working-dir") + _, err := sh.download([]api.Script{api.Assemble, api.Run, api.SaveArtifacts}, "/working-dir") if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -231,14 +232,14 @@ func TestDownloadErrors2(t *testing.T) { sh.docker.(*test.FakeDocker).DefaultURLResult = "http://image.url/scripts" dlErr := fmt.Errorf("Download Error") dl.Err = map[string]error{ - "http://the.scripts.url/scripts/one": dlErr, - "http://the.scripts.url/scripts/two": nil, - "http://the.scripts.url/scripts/three": nil, - "http://image.url/scripts/one": dlErr, - "http://image.url/scripts/two": dlErr, - "http://image.url/scripts/three": nil, + fmt.Sprintf("http://the.scripts.url/scripts/%s", api.Assemble): dlErr, + fmt.Sprintf("http://the.scripts.url/scripts/%s", api.Run): nil, + fmt.Sprintf("http://the.scripts.url/scripts/%s", api.SaveArtifacts): nil, + fmt.Sprintf("http://image.url/scripts/%s", api.Assemble): dlErr, + fmt.Sprintf("http://image.url/scripts/%s", api.Run): dlErr, + fmt.Sprintf("http://image.url/scripts/%s", api.SaveArtifacts): nil, } - _, err := sh.download([]string{"one", "two", "three"}, "/working-dir") + _, err := sh.download([]api.Script{api.Assemble, api.Run, api.SaveArtifacts}, "/working-dir") if err == nil { t.Errorf("Expected an error because script could not be downloaded") } @@ -249,14 +250,14 @@ func TestDownloadChmodError(t *testing.T) { fsErr := fmt.Errorf("Chmod Error") sh.docker.(*test.FakeDocker).DefaultURLResult = "http://image.url/scripts" sh.fs.(*test.FakeFileSystem).ChmodError = map[string]error{ - "/working-dir/downloads/scripts/one": nil, - "/working-dir/downloads/scripts/two": nil, - "/working-dir/downloads/scripts/three": fsErr, - "/working-dir/downloads/defaultScripts/one": nil, - "/working-dir/downloads/defaultScripts/two": nil, - "/working-dir/downloads/defaultScripts/three": nil, + fmt.Sprintf("/working-dir/downloads/scripts/%s", api.Assemble): nil, + fmt.Sprintf("/working-dir/downloads/scripts/%s", api.Run): nil, + fmt.Sprintf("/working-dir/downloads/scripts/%s", api.SaveArtifacts): fsErr, + fmt.Sprintf("/working-dir/downloads/defaultScripts/%s", api.Assemble): nil, + fmt.Sprintf("/working-dir/downloads/defaultScripts/%s", api.Run): nil, + fmt.Sprintf("/working-dir/downloads/defaultScripts/%s", api.SaveArtifacts): nil, } - _, err := sh.download([]string{"one", "two", "three"}, "/working-dir") + _, err := sh.download([]api.Script{api.Assemble, api.Run, api.SaveArtifacts}, "/working-dir") if err == nil { t.Errorf("Expected an error because chmod returned an error.") } @@ -295,7 +296,7 @@ func TestInstall(t *testing.T) { func TestPrepareDownload(t *testing.T) { sh := getScriptHandler() result := sh.prepareDownload( - []string{"test1", "test2"}, + []api.Script{api.Assemble, api.Run}, "/working-dir/upload", "http://my.url/base") diff --git a/pkg/sti/tar/tar.go b/pkg/sti/tar/tar.go index dec9e70c9..80d7cb010 100644 --- a/pkg/sti/tar/tar.go +++ b/pkg/sti/tar/tar.go @@ -175,7 +175,7 @@ func (t *stiTar) ExtractTarStream(dir string, reader io.Reader) error { break } path := filepath.Join(dir, header.Name) - glog.V(2).Infof("Creating %s", path) + glog.V(3).Infof("Creating %s", path) success := false // The file times need to be modified after it's been closed // thus this function is deferred before the file close @@ -212,7 +212,7 @@ func (t *stiTar) ExtractTarStream(dir string, reader io.Reader) error { errorChannel <- err break } - glog.V(2).Infof("Done with %s", path) + glog.V(3).Infof("Done with %s", path) success = true } } diff --git a/pkg/sti/test/docker.go b/pkg/sti/test/docker.go index b6f4ac850..a98b25714 100644 --- a/pkg/sti/test/docker.go +++ b/pkg/sti/test/docker.go @@ -31,6 +31,8 @@ type FakeDocker struct { CommitContainerError error RemoveImageName string RemoveImageError error + BuildImageOpts docker.BuildImageOptions + BuildImageError error mutex sync.Mutex } @@ -47,8 +49,8 @@ func (f *FakeDocker) RemoveContainer(id string) error { return f.RemoveContainerError } -// GetDefaultScriptsURL returns a default STI scripts URL -func (f *FakeDocker) GetDefaultScriptsURL(image string) (string, error) { +// GetScriptsURL returns a default STI scripts URL +func (f *FakeDocker) GetScriptsURL(image string) (string, error) { f.DefaultURLImage = image return f.DefaultURLResult, f.DefaultURLError } @@ -65,7 +67,7 @@ func (f *FakeDocker) RunContainer(opts docker.RunContainerOptions) error { } } if opts.PostExec != nil { - opts.PostExec.PostExecute(f.RunContainerContainerID, append(f.RunContainerCmd, opts.Command)) + opts.PostExec.PostExecute(f.RunContainerContainerID, string(opts.Command)) } return f.RunContainerError } @@ -97,3 +99,9 @@ func (f *FakeDocker) PullImage(imageName string) error { func (f *FakeDocker) CheckAndPull(name string) (*dockerclient.Image, error) { return nil, nil } + +// BuildImage builds image +func (f *FakeDocker) BuildImage(opts docker.BuildImageOptions) error { + f.BuildImageOpts = opts + return f.BuildImageError +} diff --git a/pkg/sti/test/fs.go b/pkg/sti/test/fs.go index b7d8dd8c4..ff185abf7 100644 --- a/pkg/sti/test/fs.go +++ b/pkg/sti/test/fs.go @@ -43,6 +43,10 @@ type FakeFileSystem struct { OpenError error OpenCloseError error + WriteFileName string + WriteFileError error + WriteFileContent string + mutex sync.Mutex } @@ -123,3 +127,10 @@ func (f *FakeFileSystem) Open(file string) (io.ReadCloser, error) { } return f.OpenFileResult, f.OpenError } + +// Write writes a file +func (f *FakeFileSystem) WriteFile(file string, data []byte) error { + f.WriteFileName = file + f.WriteFileContent = string(data) + return f.WriteFileError +} diff --git a/pkg/sti/test/installer.go b/pkg/sti/test/installer.go index fd6264386..b165890cf 100644 --- a/pkg/sti/test/installer.go +++ b/pkg/sti/test/installer.go @@ -1,19 +1,29 @@ package test +import ( + "errors" + "github.com/openshift/source-to-image/pkg/sti/api" +) + // FakeInstaller provides a fake installer type FakeInstaller struct { - Scripts [][]string + Scripts [][]api.Script WorkingDir []string Required []bool - Download bool - Err error + Download bool + ErrScript api.Script } // DownloadAndInstall downloads and install the fake STI scripts -func (f *FakeInstaller) DownloadAndInstall(scripts []string, workingDir string, required bool) (bool, error) { +func (f *FakeInstaller) DownloadAndInstall(scripts []api.Script, workingDir string, required bool) (bool, error) { f.Scripts = append(f.Scripts, scripts) f.WorkingDir = append(f.WorkingDir, workingDir) f.Required = append(f.Required, required) - return f.Download, f.Err + for _, s := range scripts { + if f.ErrScript == s { + return f.Download, errors.New(string(f.ErrScript)) + } + } + return f.Download, nil } diff --git a/pkg/sti/usage.go b/pkg/sti/usage.go index 1987350d0..a5fbbb28c 100644 --- a/pkg/sti/usage.go +++ b/pkg/sti/usage.go @@ -1,10 +1,14 @@ package sti +import ( + "github.com/openshift/source-to-image/pkg/sti/api" +) + // UsageHandler handles a request to display usage type usageHandler interface { cleanup() - setup(requiredScripts, optionalScripts []string) error - execute(command string) error + setup(required []api.Script, optional []api.Script) error + execute(command api.Script) error } // Usage display usage information about a particular build image @@ -13,7 +17,7 @@ type Usage struct { } // NewUsage creates a new instance of the default Usage implementation -func NewUsage(req *Request) (*Usage, error) { +func NewUsage(req *api.Request) (*Usage, error) { h, err := newRequestHandler(req) if err != nil { return nil, err @@ -27,9 +31,9 @@ func (u *Usage) Show() error { h := u.handler defer h.cleanup() - if err := h.setup([]string{"usage"}, []string{}); err != nil { + if err := h.setup([]api.Script{api.Usage}, []api.Script{}); err != nil { return err } - return h.execute("usage") + return h.execute(api.Usage) } diff --git a/pkg/sti/usage_test.go b/pkg/sti/usage_test.go index 087c05d01..95caefa2c 100644 --- a/pkg/sti/usage_test.go +++ b/pkg/sti/usage_test.go @@ -4,14 +4,16 @@ import ( "fmt" "reflect" "testing" + + "github.com/openshift/source-to-image/pkg/sti/api" ) type FakeUsageHandler struct { cleanupCalled bool - setupRequired []string - setupOptional []string + setupRequired []api.Script + setupOptional []api.Script setupError error - executeCommand string + executeCommand api.Script executeError error } @@ -19,13 +21,13 @@ func (f *FakeUsageHandler) cleanup() { f.cleanupCalled = true } -func (f *FakeUsageHandler) setup(required []string, optional []string) error { +func (f *FakeUsageHandler) setup(required []api.Script, optional []api.Script) error { f.setupRequired = required f.setupOptional = optional return f.setupError } -func (f *FakeUsageHandler) execute(command string) error { +func (f *FakeUsageHandler) execute(command api.Script) error { f.executeCommand = command return f.executeError } @@ -43,10 +45,10 @@ func TestUsage(t *testing.T) { if err != nil { t.Errorf("Unexpected error returned from Usage: %v", err) } - if !reflect.DeepEqual(fh.setupOptional, []string{}) { + if !reflect.DeepEqual(fh.setupOptional, []api.Script{}) { t.Errorf("setup called with unexpected optional scripts: %#v", fh.setupOptional) } - if !reflect.DeepEqual(fh.setupRequired, []string{"usage"}) { + if !reflect.DeepEqual(fh.setupRequired, []api.Script{api.Usage}) { t.Errorf("setup called with unexpected required scripts: %#v", fh.setupRequired) } if fh.executeCommand != "usage" { diff --git a/pkg/sti/util/fs.go b/pkg/sti/util/fs.go index a1498d39a..a47310a26 100644 --- a/pkg/sti/util/fs.go +++ b/pkg/sti/util/fs.go @@ -23,6 +23,7 @@ type FileSystem interface { RemoveDirectory(dir string) error CreateWorkingDirectory() (string, error) Open(file string) (io.ReadCloser, error) + WriteFile(file string, data []byte) error } // NewFileSystem creates a new instance of the default FileSystem @@ -106,3 +107,8 @@ func (h *fs) CreateWorkingDirectory() (directory string, err error) { func (h *fs) Open(filename string) (io.ReadCloser, error) { return os.Open(filename) } + +// Write opens a file and writes data to it, returning error if such occured +func (h *fs) WriteFile(filename string, data []byte) error { + return ioutil.WriteFile(filename, data, 0700) +} diff --git a/test/integration/images/sti-fake-no-tar/Dockerfile b/test/integration/images/sti-fake-no-tar/Dockerfile new file mode 100644 index 000000000..b074ab40e --- /dev/null +++ b/test/integration/images/sti-fake-no-tar/Dockerfile @@ -0,0 +1,13 @@ +# +# This is basic fake image used for testing STI. +# +FROM busybox + +RUN mkdir -p /sti-fake/src && \ + rm /bin/tar && \ +# FIXME: The 'default' user has conflicting UID (1000). This should be fixed in +# how source-to-image handles UID's during isolation. +# + deluser default + +WORKDIR / diff --git a/test/integration/images/sti-fake-scripts-no-save-artifacts/Dockerfile b/test/integration/images/sti-fake-scripts-no-save-artifacts/Dockerfile index f4100f954..4d9259711 100644 --- a/test/integration/images/sti-fake-scripts-no-save-artifacts/Dockerfile +++ b/test/integration/images/sti-fake-scripts-no-save-artifacts/Dockerfile @@ -12,11 +12,8 @@ RUN mkdir -p /sti-fake/src && \ WORKDIR / -ADD scripts/sti-helper /tmp/sti-helper ADD scripts/.sti/bin/run /tmp/scripts/ ADD scripts/.sti/bin/assemble /tmp/scripts/ # Scripts are already in the image and this is their location -ENV STI_SCRIPTS_URL image:///tmp/scripts/.sti/bin - -CMD ["/tmp/sti-helper"] +ENV STI_SCRIPTS_URL image:///tmp/scripts/ diff --git a/test/integration/images/sti-fake-scripts/Dockerfile b/test/integration/images/sti-fake-scripts/Dockerfile index f9af9e7f7..bf5e47c12 100644 --- a/test/integration/images/sti-fake-scripts/Dockerfile +++ b/test/integration/images/sti-fake-scripts/Dockerfile @@ -12,10 +12,7 @@ RUN mkdir -p /sti-fake/src && \ WORKDIR / -ADD scripts/sti-helper /tmp/sti-helper ADD scripts/.sti/bin/ /tmp/scripts/ # Scripts are already in the image and this is their location -ENV STI_SCRIPTS_URL image:///tmp/scripts/.sti/bin - -CMD ["/tmp/sti-helper"] +ENV STI_SCRIPTS_URL image:///tmp/scripts/ diff --git a/test/integration/images/sti-fake/Dockerfile b/test/integration/images/sti-fake/Dockerfile index de9030ce9..f20bbcffc 100644 --- a/test/integration/images/sti-fake/Dockerfile +++ b/test/integration/images/sti-fake/Dockerfile @@ -11,13 +11,9 @@ RUN mkdir -p /sti-fake/src && \ WORKDIR / -ADD scripts/sti-helper /tmp/sti-helper - # Need to serve the scripts from localhost so any potential changes to the # scripts are made available for integration testing. # # Port 23456 must match the port used in the http server in STI's # integration_test.go ENV STI_SCRIPTS_URL http://127.0.0.1:23456/.sti/bin - -CMD ["/tmp/sti-helper"] diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index adc0dcc4d..9c045f1e4 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -9,13 +9,15 @@ import ( "net/http" "net/http/httptest" "os" - "path" + "path/filepath" "runtime" "testing" "time" "github.com/fsouza/go-dockerclient" + "github.com/openshift/source-to-image/pkg/sti" + "github.com/openshift/source-to-image/pkg/sti/api" ) const ( @@ -26,6 +28,7 @@ const ( FakeUserImage = "sti_test/sti-fake-user" FakeImageScripts = "sti_test/sti-fake-scripts" FakeImageScriptsNoSaveArtifacts = "sti_test/sti-fake-scripts-no-save-artifacts" + FakeImageNoTar = "sti_test/sti-fake-no-tar" TagCleanBuild = "test/sti-fake-app" TagCleanBuildUser = "test/sti-fake-app-user" @@ -34,12 +37,13 @@ const ( TagCleanBuildScripts = "test/sti-fake-app-scripts" TagIncrementalBuildScripts = "test/sti-incremental-app-scripts" TagIncrementalBuildScriptsNoSaveArtifacts = "test/sti-incremental-app-scripts-no-save-artifacts" + TagCleanLayeredBuildNoTar = "test/sti-fake-no-tar" // Need to serve the scripts from local host so any potential changes to the // scripts are made available for integration testing. // // Port 23456 must match the port used in the fake image Dockerfiles - FakeScriptsHttpUrl = "http://127.0.0.1:23456/sti-fake/.sti/bin" + FakeScriptsHttpURL = "http://127.0.0.1:23456/sti-fake/.sti/bin" ) type integrationTest struct { @@ -66,8 +70,8 @@ func (i *integrationTest) setup() { // get the full path to this .go file so we can construct the file url // using this file's dirname _, filename, _, _ := runtime.Caller(0) - testImagesDir := path.Join(path.Dir(filename), "scripts") - FakeScriptsFileURL = "file://" + path.Join(testImagesDir, ".sti", "bin") + testImagesDir := filepath.Join(filepath.Dir(filename), "scripts") + FakeScriptsFileURL = "file://" + filepath.Join(testImagesDir, ".sti", "bin") for _, image := range []string{TagCleanBuild, TagCleanBuildUser, TagIncrementalBuild, TagIncrementalBuildUser} { i.dockerClient.RemoveImage(image) @@ -120,13 +124,17 @@ func TestCleanBuildFileScriptsURL(t *testing.T) { } func TestCleanBuildHttpScriptsURL(t *testing.T) { - integration(t).exerciseCleanBuild(TagCleanBuild, false, FakeBaseImage, FakeScriptsHttpUrl) + integration(t).exerciseCleanBuild(TagCleanBuild, false, FakeBaseImage, FakeScriptsHttpURL) } func TestCleanBuildScripts(t *testing.T) { integration(t).exerciseCleanBuild(TagCleanBuildScripts, false, FakeImageScripts, "") } +func TestLayeredBuildNoTar(t *testing.T) { + integration(t).exerciseCleanBuild(TagCleanLayeredBuildNoTar, false, FakeImageNoTar, FakeScriptsFileURL) +} + // Test that a build request with a callbackURL will invoke HTTP endpoint func TestCleanBuildCallbackInvoked(t *testing.T) { integration(t).exerciseCleanBuild(TagCleanBuild, true, FakeBaseImage, "") @@ -162,7 +170,7 @@ func (i *integrationTest) exerciseCleanBuild(tag string, verifyCallback bool, im callbackURL = ts.URL } - req := &sti.Request{ + req := &api.Request{ DockerSocket: dockerSocket(), BaseImage: imageName, Source: TestSource, @@ -217,7 +225,7 @@ func TestIncrementalBuildScriptsNoSaveArtifacts(t *testing.T) { func (i *integrationTest) exerciseIncrementalBuild(tag, imageName string, removePreviousImage bool, expectClean bool) { t := i.t - req := &sti.Request{ + req := &api.Request{ DockerSocket: dockerSocket(), BaseImage: imageName, Source: TestSource, diff --git a/test/integration/scripts/sti-helper b/test/integration/scripts/sti-helper deleted file mode 100755 index 2c9fc6704..000000000 --- a/test/integration/scripts/sti-helper +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/sh -e - -if [ -z "$1" ]; then - echo "No command specified, exiting. For details see https://github.com/openshift/source-to-image." - exit 1 -fi - -if [ "$1" != "run" -a "$1" != "save-artifacts" ]; then - tar -C /tmp -xf - -fi - -exec /tmp/scripts/$1