diff --git a/pkg/build/generator/generator.go b/pkg/build/generator/generator.go index 31879f3847d3..cdd886909911 100644 --- a/pkg/build/generator/generator.go +++ b/pkg/build/generator/generator.go @@ -57,6 +57,7 @@ type GeneratorClient interface { UpdateBuildConfig(ctx kapi.Context, buildConfig *buildapi.BuildConfig) error GetBuild(ctx kapi.Context, name string) (*buildapi.Build, error) CreateBuild(ctx kapi.Context, build *buildapi.Build) error + UpdateBuild(ctx kapi.Context, build *buildapi.Build) error GetImageStream(ctx kapi.Context, name string) (*imageapi.ImageStream, error) GetImageStreamImage(ctx kapi.Context, name string) (*imageapi.ImageStreamImage, error) GetImageStreamTag(ctx kapi.Context, name string) (*imageapi.ImageStreamTag, error) @@ -68,6 +69,7 @@ type Client struct { UpdateBuildConfigFunc func(ctx kapi.Context, buildConfig *buildapi.BuildConfig) error GetBuildFunc func(ctx kapi.Context, name string) (*buildapi.Build, error) CreateBuildFunc func(ctx kapi.Context, build *buildapi.Build) error + UpdateBuildFunc func(ctx kapi.Context, build *buildapi.Build) error GetImageStreamFunc func(ctx kapi.Context, name string) (*imageapi.ImageStream, error) GetImageStreamImageFunc func(ctx kapi.Context, name string) (*imageapi.ImageStreamImage, error) GetImageStreamTagFunc func(ctx kapi.Context, name string) (*imageapi.ImageStreamTag, error) @@ -93,6 +95,11 @@ func (c Client) CreateBuild(ctx kapi.Context, build *buildapi.Build) error { return c.CreateBuildFunc(ctx, build) } +// UpdateBuild updates a build +func (c Client) UpdateBuild(ctx kapi.Context, build *buildapi.Build) error { + return c.UpdateBuildFunc(ctx, build) +} + // GetImageStream retrieves a named image stream func (c Client) GetImageStream(ctx kapi.Context, name string) (*imageapi.ImageStream, error) { return c.GetImageStreamFunc(ctx, name) diff --git a/pkg/build/registry/buildconfiginstantiate/rest.go b/pkg/build/registry/buildconfiginstantiate/rest.go index e7e4e98df72e..be631b6f5524 100644 --- a/pkg/build/registry/buildconfiginstantiate/rest.go +++ b/pkg/build/registry/buildconfiginstantiate/rest.go @@ -27,6 +27,11 @@ import ( buildutil "github.com/openshift/origin/pkg/build/util" ) +var ( + cancelPollInterval = 500 * time.Millisecond + cancelPollDuration = 30 * time.Second +) + // NewStorage creates a new storage object for build generation func NewStorage(generator *generator.BuildGenerator) *InstantiateREST { return &InstantiateREST{generator: generator} @@ -178,24 +183,39 @@ func (h *binaryInstantiateHandler) handle(r io.Reader) (runtime.Object, error) { } remaining := h.r.Timeout - time.Now().Sub(start) - latest, ok, err := registry.WaitForRunningBuild(h.r.Watcher, h.ctx, build, remaining) - if err != nil { - switch { - case latest.Status.Phase == buildapi.BuildPhaseError: - return nil, errors.NewBadRequest(fmt.Sprintf("build %s encountered an error: %s", build.Name, buildutil.NoBuildLogsMessage)) - case latest.Status.Phase == buildapi.BuildPhaseCancelled: - return nil, errors.NewBadRequest(fmt.Sprintf("build %s was cancelled: %s", build.Name, buildutil.NoBuildLogsMessage)) - case err == registry.ErrBuildDeleted: - return nil, errors.NewBadRequest(fmt.Sprintf("build %s was deleted before it started: %s", build.Name, buildutil.NoBuildLogsMessage)) - default: - return nil, errors.NewBadRequest(fmt.Sprintf("unable to wait for build %s to run: %v", build.Name, err)) + // Attempt to cancel the build if it did not start running + // before we gave up. + cancel := true + defer func() { + if !cancel { + return } - } - if !ok { - return nil, errors.NewTimeoutError(fmt.Sprintf("timed out waiting for build %s to start after %s", build.Name, h.r.Timeout), 0) - } - if latest.Status.Phase != buildapi.BuildPhaseRunning { + h.cancelBuild(build) + }() + + latest, ok, err := registry.WaitForRunningBuild(h.r.Watcher, h.ctx, build, remaining) + + switch { + case latest.Status.Phase == buildapi.BuildPhaseError: + // don't cancel the build if it reached a terminal state on its own + cancel = false + return nil, errors.NewBadRequest(fmt.Sprintf("build %s encountered an error: %s", build.Name, buildutil.NoBuildLogsMessage)) + case latest.Status.Phase == buildapi.BuildPhaseFailed: + // don't cancel the build if it reached a terminal state on its own + cancel = false + return nil, errors.NewBadRequest(fmt.Sprintf("build %s failed: %s: %s", build.Name, build.Status.Reason, build.Status.Message)) + case latest.Status.Phase == buildapi.BuildPhaseCancelled: + // don't cancel the build if it reached a terminal state on its own + cancel = false + return nil, errors.NewBadRequest(fmt.Sprintf("build %s was cancelled: %s", build.Name, buildutil.NoBuildLogsMessage)) + case latest.Status.Phase != buildapi.BuildPhaseRunning: return nil, errors.NewBadRequest(fmt.Sprintf("cannot upload file to build %s with status %s", build.Name, latest.Status.Phase)) + case err == registry.ErrBuildDeleted: + return nil, errors.NewBadRequest(fmt.Sprintf("build %s was deleted before it started: %s", build.Name, buildutil.NoBuildLogsMessage)) + case err != nil: + return nil, errors.NewBadRequest(fmt.Sprintf("unable to wait for build %s to run: %v", build.Name, err)) + case !ok: + return nil, errors.NewTimeoutError(fmt.Sprintf("timed out waiting for build %s to start after %s", build.Name, h.r.Timeout), 0) } // The container should be the default build container, so setting it to blank @@ -226,9 +246,28 @@ func (h *binaryInstantiateHandler) handle(r io.Reader) (runtime.Object, error) { if err := exec.Stream(streamOptions); err != nil { return nil, errors.NewInternalError(err) } + cancel = false return latest, nil } +// cancelBuild will mark a build for cancellation unless +// cancel is false in which case it is a no-op. +func (h *binaryInstantiateHandler) cancelBuild(build *buildapi.Build) { + build.Status.Cancelled = true + h.r.Generator.Client.UpdateBuild(h.ctx, build) + wait.Poll(cancelPollInterval, cancelPollDuration, func() (bool, error) { + build.Status.Cancelled = true + err := h.r.Generator.Client.UpdateBuild(h.ctx, build) + switch { + case err != nil && errors.IsConflict(err): + build, err = h.r.Generator.Client.GetBuild(h.ctx, build.Name) + return false, err + default: + return true, err + } + }) +} + type podGetter struct { podsNamespacer kcoreclient.PodsGetter } diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index 1d001181d4cc..4f0e0097ab9f 100644 --- a/pkg/cmd/server/origin/master.go +++ b/pkg/cmd/server/origin/master.go @@ -619,6 +619,7 @@ func (c *MasterConfig) GetRestStorage() map[string]rest.Storage { UpdateBuildConfigFunc: buildConfigRegistry.UpdateBuildConfig, GetBuildFunc: buildRegistry.GetBuild, CreateBuildFunc: buildRegistry.CreateBuild, + UpdateBuildFunc: buildRegistry.UpdateBuild, GetImageStreamFunc: imageStreamRegistry.GetImageStream, GetImageStreamImageFunc: imageStreamImageRegistry.GetImageStreamImage, GetImageStreamTagFunc: imageStreamTagRegistry.GetImageStreamTag, diff --git a/test/extended/builds/dockerfile.go b/test/extended/builds/dockerfile.go index a916254ace76..ab956099332f 100644 --- a/test/extended/builds/dockerfile.go +++ b/test/extended/builds/dockerfile.go @@ -46,7 +46,7 @@ USER 1001 buildName := "jenkins-1" g.By("expecting the Dockerfile build is in Complete phase") - err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), buildName, exutil.CheckBuildSuccessFn, exutil.CheckBuildFailedFn) + err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), buildName, nil, nil, nil) //debug for failures on jenkins if err != nil { exutil.DumpBuildLogs("jenkins", oc) @@ -75,7 +75,7 @@ USER 1001 buildName := "centos-1" g.By("expecting the Dockerfile build is in Complete phase") - err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), buildName, exutil.CheckBuildSuccessFn, exutil.CheckBuildFailedFn) + err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), buildName, nil, nil, nil) //debug for failures on jenkins if err != nil { exutil.DumpBuildLogs("centos", oc) diff --git a/test/extended/builds/gitauth.go b/test/extended/builds/gitauth.go index fb534edd3a3b..247238efdaf8 100644 --- a/test/extended/builds/gitauth.go +++ b/test/extended/builds/gitauth.go @@ -86,7 +86,7 @@ var _ = g.Describe("[builds][Slow] can use private repositories as build input", o.Expect(err).NotTo(o.HaveOccurred()) g.By(fmt.Sprintf("expecting build %s to complete successfully", buildName)) - err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), buildName, exutil.CheckBuildSuccessFn, exutil.CheckBuildFailedFn) + err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), buildName, nil, nil, nil) if err != nil { exutil.DumpBuildLogs(buildConfigName, oc) } diff --git a/test/extended/builds/start.go b/test/extended/builds/start.go index 857be780ec19..af7d8fd7d99c 100644 --- a/test/extended/builds/start.go +++ b/test/extended/builds/start.go @@ -12,6 +12,7 @@ import ( g "github.com/onsi/ginkgo" o "github.com/onsi/gomega" + buildapi "github.com/openshift/origin/pkg/build/api" exutil "github.com/openshift/origin/test/extended/util" ) @@ -103,7 +104,6 @@ var _ = g.Describe("[builds][Slow] starting a build using CLI", func() { o.Expect(br.StartBuildStdErr).To(o.ContainSubstring("Uploading file")) o.Expect(br.StartBuildStdErr).To(o.ContainSubstring("as binary input for the build ...")) o.Expect(buildLog).To(o.ContainSubstring("Your bundle is complete")) - }) g.It("should accept --from-dir as input", func() { @@ -194,6 +194,23 @@ var _ = g.Describe("[builds][Slow] starting a build using CLI", func() { }) }) + g.Describe("cancel a binary build that doesn't start running in 5 minutes", func() { + g.It("should start a build and wait for the build to be cancelled", func() { + g.By("starting a build with a nodeselector that can't be matched") + go func() { + exutil.StartBuild(oc, "sample-build-binary-invalidnodeselector", fmt.Sprintf("--from-file=%s", exampleGemfile)) + }() + build := &buildapi.Build{} + cancelFn := func(b *buildapi.Build) bool { + build = b + return exutil.CheckBuildCancelledFn(b) + } + err := exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), "sample-build-binary-invalidnodeselector-1", nil, nil, cancelFn) + o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(build.Status.Phase).To(o.Equal(buildapi.BuildPhaseCancelled)) + }) + }) + g.Describe("cancelling build started by oc start-build --wait", func() { g.It("should start a build and wait for the build to cancel", func() { g.By("starting the build with --wait flag") diff --git a/test/extended/image_ecosystem/s2i_perl.go b/test/extended/image_ecosystem/s2i_perl.go index b831283730cb..71c29dc09fdd 100644 --- a/test/extended/image_ecosystem/s2i_perl.go +++ b/test/extended/image_ecosystem/s2i_perl.go @@ -33,7 +33,7 @@ var _ = g.Describe("[image_ecosystem][perl][Slow] hot deploy for openshift perl o.Expect(err).NotTo(o.HaveOccurred()) g.By("waiting for build to finish") - err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), "dancer-mysql-example-1", exutil.CheckBuildSuccessFn, exutil.CheckBuildFailedFn) + err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), "dancer-mysql-example-1", nil, nil, nil) if err != nil { exutil.DumpBuildLogs("dancer-mysql-example", oc) } diff --git a/test/extended/image_ecosystem/s2i_php.go b/test/extended/image_ecosystem/s2i_php.go index d21ccffacd57..8d5ccfc3feab 100644 --- a/test/extended/image_ecosystem/s2i_php.go +++ b/test/extended/image_ecosystem/s2i_php.go @@ -31,7 +31,7 @@ var _ = g.Describe("[image_ecosystem][php][Slow] hot deploy for openshift php im o.Expect(err).NotTo(o.HaveOccurred()) g.By("waiting for build to finish") - err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), dcName, exutil.CheckBuildSuccessFn, exutil.CheckBuildFailedFn) + err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), dcName, nil, nil, nil) if err != nil { exutil.DumpBuildLogs("cakephp-mysql-example", oc) } diff --git a/test/extended/image_ecosystem/s2i_python.go b/test/extended/image_ecosystem/s2i_python.go index bb3fcc3f2999..c9aa9e4a315b 100644 --- a/test/extended/image_ecosystem/s2i_python.go +++ b/test/extended/image_ecosystem/s2i_python.go @@ -34,7 +34,7 @@ var _ = g.Describe("[image_ecosystem][python][Slow] hot deploy for openshift pyt o.Expect(err).NotTo(o.HaveOccurred()) g.By("waiting for build to finish") - err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), "django-ex-1", exutil.CheckBuildSuccessFn, exutil.CheckBuildFailedFn) + err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), "django-ex-1", nil, nil, nil) if err != nil { exutil.DumpBuildLogs("django-ex", oc) } diff --git a/test/extended/image_ecosystem/s2i_ruby.go b/test/extended/image_ecosystem/s2i_ruby.go index 0c2bd4a9b60c..973bc41293ff 100644 --- a/test/extended/image_ecosystem/s2i_ruby.go +++ b/test/extended/image_ecosystem/s2i_ruby.go @@ -32,7 +32,7 @@ var _ = g.Describe("[image_ecosystem][ruby][Slow] hot deploy for openshift ruby o.Expect(err).NotTo(o.HaveOccurred()) g.By("waiting for build to finish") - err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), dcName, exutil.CheckBuildSuccessFn, exutil.CheckBuildFailedFn) + err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), dcName, nil, nil, nil) if err != nil { exutil.DumpBuildLogs("rails-postgresql-example", oc) } diff --git a/test/extended/image_ecosystem/sample_repos.go b/test/extended/image_ecosystem/sample_repos.go index 75ef52e9c410..fadc1f11a344 100644 --- a/test/extended/image_ecosystem/sample_repos.go +++ b/test/extended/image_ecosystem/sample_repos.go @@ -50,7 +50,7 @@ func NewSampleRepoTest(c SampleRepoConfig) func() { buildName := c.buildConfigName + "-1" g.By("expecting the build is in the Complete phase") - err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), buildName, exutil.CheckBuildSuccessFn, exutil.CheckBuildFailedFn) + err = exutil.WaitForABuild(oc.Client().Builds(oc.Namespace()), buildName, nil, nil, nil) if err != nil { exutil.DumpBuildLogs(c.buildConfigName, oc) } diff --git a/test/extended/testdata/test-build.json b/test/extended/testdata/test-build.json index 3768833ceb22..3b8d5c92e3f8 100644 --- a/test/extended/testdata/test-build.json +++ b/test/extended/testdata/test-build.json @@ -171,6 +171,45 @@ "status": { "lastVersion": 0 } + }, + { + "kind": "BuildConfig", + "apiVersion": "v1", + "metadata": { + "name": "sample-build-binary-invalidnodeselector", + "creationTimestamp": null + }, + "spec": { + "triggers": [ + { + "type": "imageChange", + "imageChange": {} + } + ], + "source": { + "type": "Binary", + "binary": {} + }, + "strategy": { + "type": "Docker", + "dockerStrategy": { + "env": [ + { "name": "FOO", "value": "test" }, + { "name": "BAR", "value": "test" }, + { "name": "BUILD_LOGLEVEL", "value": "5" } + ], + "from": { + "kind": "DockerImage", + "name": "centos/ruby-22-centos7" + } + } + }, + "resources": {}, + "nodeSelector": {"nodelabelkey":"nodelabelvalue"} + }, + "status": { + "lastVersion": 0 + } } ] } diff --git a/test/extended/util/framework.go b/test/extended/util/framework.go index ff1382a13603..a08aa5740051 100644 --- a/test/extended/util/framework.go +++ b/test/extended/util/framework.go @@ -302,6 +302,8 @@ type BuildResult struct { BuildSuccess bool // BuildFailure is true if the build was finished with an error. BuildFailure bool + // BuildCancelled is true if the build was canceled. + BuildCancelled bool // BuildTimeout is true if there was a timeout waiting for the build to finish. BuildTimeout bool // The openshift client which created this build. @@ -400,6 +402,7 @@ func StartBuildAndWait(oc *CLI, args ...string) (result *BuildResult, err error) BuildAttempt: false, BuildSuccess: false, BuildFailure: false, + BuildCancelled: false, BuildTimeout: false, oc: oc, } @@ -428,6 +431,11 @@ func StartBuildAndWait(oc *CLI, args ...string) (result *BuildResult, err error) result.BuildFailure = CheckBuildFailedFn(b) return result.BuildFailure }, + func(b *buildapi.Build) bool { + result.Build = b + result.BuildCancelled = CheckBuildCancelledFn(b) + return result.BuildCancelled + }, ) if result.Build == nil { @@ -436,14 +444,24 @@ func StartBuildAndWait(oc *CLI, args ...string) (result *BuildResult, err error) } result.BuildAttempt = true - result.BuildTimeout = !(result.BuildFailure || result.BuildSuccess) + result.BuildTimeout = !(result.BuildFailure || result.BuildSuccess || result.BuildCancelled) fmt.Fprintf(g.GinkgoWriter, "Done waiting for %s: %#v\n", buildPath, *result) return result, nil } // WaitForABuild waits for a Build object to match either isOK or isFailed conditions. -func WaitForABuild(c client.BuildInterface, name string, isOK, isFailed func(*buildapi.Build) bool) error { +func WaitForABuild(c client.BuildInterface, name string, isOK, isFailed, isCanceled func(*buildapi.Build) bool) error { + if isOK == nil { + isOK = CheckBuildSuccessFn + } + if isFailed == nil { + isFailed = CheckBuildFailedFn + } + if isCanceled == nil { + isCanceled = CheckBuildCancelledFn + } + // wait 2 minutes for build to exist err := wait.Poll(1*time.Second, 2*time.Minute, func() (bool, error) { if _, err := c.Get(name); err != nil { @@ -464,7 +482,7 @@ func WaitForABuild(c client.BuildInterface, name string, isOK, isFailed func(*bu return false, err } for i := range list.Items { - if name == list.Items[i].Name && isOK(&list.Items[i]) { + if name == list.Items[i].Name && (isOK(&list.Items[i]) || isCanceled(&list.Items[i])) { return true, nil } if name != list.Items[i].Name || isFailed(&list.Items[i]) { @@ -489,6 +507,11 @@ var CheckBuildFailedFn = func(b *buildapi.Build) bool { return b.Status.Phase == buildapi.BuildPhaseFailed || b.Status.Phase == buildapi.BuildPhaseError } +// CheckBuildCancelledFn return true if the build was canceled +var CheckBuildCancelledFn = func(b *buildapi.Build) bool { + return b.Status.Phase == buildapi.BuildPhaseCancelled +} + // WaitForBuilderAccount waits until the builder service account gets fully // provisioned func WaitForBuilderAccount(c kcoreclient.ServiceAccountInterface) error {