diff --git a/pkg/api/types.go b/pkg/api/types.go index 33b37d07b..d78a4832c 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -261,6 +261,10 @@ type InstallResult struct { // Error describes last error encountered during install operation Error error + + // FailedSources is a list of sources that were attempted but failed + // when downloading this script + FailedSources []string } // SourceInfo stores information about the source code diff --git a/pkg/build/strategies/sti/sti.go b/pkg/build/strategies/sti/sti.go index 0c9705b11..77b1e6161 100644 --- a/pkg/build/strategies/sti/sti.go +++ b/pkg/build/strategies/sti/sti.go @@ -218,6 +218,24 @@ func (b *STI) Prepare(config *api.Config) error { } optional := b.installer.InstallOptional(b.optionalScripts, config.WorkingDir) + // If a ScriptsURL was specified, but no scripts were downloaded from it, throw an error + if len(config.ScriptsURL) > 0 { + failedCount := 0 + for _, result := range required { + if includes(result.FailedSources, scripts.ScriptURLHandler) { + failedCount++ + } + } + for _, result := range optional { + if includes(result.FailedSources, scripts.ScriptURLHandler) { + failedCount++ + } + } + if failedCount == len(required)+len(optional) { + return fmt.Errorf("Could not download any scripts from URL %v", config.ScriptsURL) + } + } + for _, r := range append(required, optional...) { if r.Error == nil { b.externalScripts[r.Script] = r.Downloaded @@ -594,3 +612,12 @@ func isMissingRequirements(text string) bool { sh, _ := regexp.MatchString(`.*/bin/sh.*no such file or directory`, text) return tar || sh } + +func includes(arr []string, str string) bool { + for _, s := range arr { + if s == str { + return true + } + } + return false +} diff --git a/pkg/scripts/install.go b/pkg/scripts/install.go index f104faf46..dc6cdee14 100644 --- a/pkg/scripts/install.go +++ b/pkg/scripts/install.go @@ -29,29 +29,41 @@ type ScriptHandler interface { String() string } -// UrlScriptHandler handles script download using URL. -type UrlScriptHandler struct { +// URLScriptHandler handles script download using URL. +type URLScriptHandler struct { URL string DestinationDir string download Downloader fs util.FileSystem + name string } -const sourcesRootAbbrev = "/" +const ( + sourcesRootAbbrev = "/" + + // ScriptURLHandler is the name of the script URL handler + ScriptURLHandler = "script URL handler" + + // ImageURLHandler is the name of the image URL handler + ImageURLHandler = "image URL handler" + + // SourceHandler is the name of the source script handler + SourceHandler = "source handler" +) // SetDestinationDir sets the destination where the scripts should be // downloaded. -func (s *UrlScriptHandler) SetDestinationDir(baseDir string) { +func (s *URLScriptHandler) SetDestinationDir(baseDir string) { s.DestinationDir = filepath.Join(baseDir, api.UploadScripts) } // String implements the String() function. -func (s *UrlScriptHandler) String() string { - return "url handler" +func (s *URLScriptHandler) String() string { + return s.name } // Get parses the provided URL and the script name. -func (s *UrlScriptHandler) Get(script string) *api.InstallResult { +func (s *URLScriptHandler) Get(script string) *api.InstallResult { if len(s.URL) == 0 { return nil } @@ -67,7 +79,7 @@ func (s *UrlScriptHandler) Get(script string) *api.InstallResult { } // Install downloads the script and fix its permissions. -func (s *UrlScriptHandler) Install(r *api.InstallResult) error { +func (s *URLScriptHandler) Install(r *api.InstallResult) error { downloadURL, err := url.Parse(r.URL) if err != nil { return err @@ -116,7 +128,7 @@ func (s *SourceScriptHandler) Get(script string) *api.InstallResult { // String implements the String() function. func (s *SourceScriptHandler) String() string { - return "source handler" + return SourceHandler } // Install copies the script into upload directory and fix its permissions. @@ -184,7 +196,7 @@ func NewInstaller(image string, scriptsURL string, proxyConfig *api.ProxyConfig, // Order is important here, first we try to get the scripts from provided URL, // then we look into sources and check for .s2i/bin scripts. if len(m.ScriptsURL) > 0 { - m.Add(&UrlScriptHandler{URL: m.ScriptsURL, download: m.download, fs: m.fs}) + m.Add(&URLScriptHandler{URL: m.ScriptsURL, download: m.download, fs: m.fs, name: ScriptURLHandler}) } m.Add(&SourceScriptHandler{fs: m.fs}) @@ -193,7 +205,7 @@ func NewInstaller(image string, scriptsURL string, proxyConfig *api.ProxyConfig, // docker image itself. defaultURL, err := m.docker.GetScriptsURL(m.Image) if err == nil && defaultURL != "" { - m.Add(&UrlScriptHandler{URL: defaultURL, download: m.download, fs: m.fs}) + m.Add(&URLScriptHandler{URL: defaultURL, download: m.download, fs: m.fs, name: ImageURLHandler}) } return &m } @@ -222,14 +234,17 @@ func (i *DefaultScriptSourceManager) InstallOptional(scripts []string, dstDir st result := []api.InstallResult{} for _, script := range scripts { installed := false + failedSources := []string{} for _, e := range i.sources { detected := false h := e.(ScriptHandler) h.SetDestinationDir(dstDir) if r := h.Get(script); r != nil { if err := h.Install(r); err != nil { + failedSources = append(failedSources, h.String()) glog.Errorf("script %q found by the %s, but failed to install: %v", script, h, err) } else { + r.FailedSources = failedSources result = append(result, *r) installed = true detected = true @@ -242,8 +257,9 @@ func (i *DefaultScriptSourceManager) InstallOptional(scripts []string, dstDir st } if !installed { result = append(result, api.InstallResult{ - Script: script, - Error: fmt.Errorf("script %q not installed", script), + FailedSources: failedSources, + Script: script, + Error: fmt.Errorf("script %q not installed", script), }) } } diff --git a/pkg/scripts/install_test.go b/pkg/scripts/install_test.go index 389051791..5f1951289 100644 --- a/pkg/scripts/install_test.go +++ b/pkg/scripts/install_test.go @@ -3,6 +3,7 @@ package scripts import ( "fmt" "path/filepath" + "reflect" "strings" "testing" @@ -37,11 +38,11 @@ func newFakeInstaller(config *fakeScriptManagerConfig) Installer { fs: config.fs, download: config.download, } - m.Add(&UrlScriptHandler{URL: m.ScriptsURL, download: m.download, fs: m.fs}) + m.Add(&URLScriptHandler{URL: m.ScriptsURL, download: m.download, fs: m.fs, name: ScriptURLHandler}) m.Add(&SourceScriptHandler{fs: m.fs}) defaultURL, err := m.docker.GetScriptsURL(m.Image) if err == nil && defaultURL != "" { - m.Add(&UrlScriptHandler{URL: defaultURL, download: m.download, fs: m.fs}) + m.Add(&URLScriptHandler{URL: defaultURL, download: m.download, fs: m.fs, name: ImageURLHandler}) } return &m } @@ -264,14 +265,14 @@ func TestNewInstaller(t *testing.T) { docker := &dockerpkg.FakeDocker{DefaultURLResult: "image://docker"} inst := NewInstaller("test-image", "http://foo.bar", nil, docker, dockerClient.AuthConfiguration{}) sources := inst.(*DefaultScriptSourceManager).sources - firstHandler, ok := sources[0].(*UrlScriptHandler) + firstHandler, ok := sources[0].(*URLScriptHandler) if !ok { t.Errorf("expected first handler to be script url handler, got %#v", inst.(*DefaultScriptSourceManager).sources) } if firstHandler.URL != "http://foo.bar" { t.Errorf("expected first handler to handle the script url, got %+v", firstHandler) } - lastHandler, ok := sources[len(sources)-1].(*UrlScriptHandler) + lastHandler, ok := sources[len(sources)-1].(*URLScriptHandler) if !ok { t.Errorf("expected last handler to be docker url handler, got %#v", inst.(*DefaultScriptSourceManager).sources) } @@ -279,3 +280,45 @@ func TestNewInstaller(t *testing.T) { t.Errorf("expected last handler to handle the docker default url, got %+v", lastHandler) } } + +type fakeSource struct { + name string + failOn map[string]struct{} +} + +func (f *fakeSource) Get(script string) *api.InstallResult { + return &api.InstallResult{Script: script} +} + +func (f *fakeSource) Install(r *api.InstallResult) error { + if _, fail := f.failOn[r.Script]; fail { + return fmt.Errorf("error") + } + return nil +} + +func (f *fakeSource) SetDestinationDir(string) {} + +func (f *fakeSource) String() string { + return f.name +} + +func TestInstallOptionalFailedSources(t *testing.T) { + + m := DefaultScriptSourceManager{} + m.Add(&fakeSource{name: "failing1", failOn: map[string]struct{}{"one": {}, "two": {}, "three": {}}}) + m.Add(&fakeSource{name: "failing2", failOn: map[string]struct{}{"one": {}, "two": {}, "three": {}}}) + m.Add(&fakeSource{name: "almostpassing", failOn: map[string]struct{}{"three": {}}}) + + expect := map[string][]string{ + "one": {"failing1", "failing2"}, + "two": {"failing1", "failing2"}, + "three": {"failing1", "failing2", "almostpassing"}, + } + results := m.InstallOptional([]string{"one", "two", "three"}, "foo") + for _, result := range results { + if !reflect.DeepEqual(result.FailedSources, expect[result.Script]) { + t.Errorf("Did not get expected failed sources: %#v", result) + } + } +} diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 51cb26845..0db7b5a29 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -58,7 +58,7 @@ const ( // 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/.s2i/bin" + FakeScriptsHttpURL = "http://127.0.0.1:23456/.s2i/bin" ) // TestInjectionBuild tests the build where we inject files to assemble script.