From 95b345971651e76da1167c64253e7b45e1965faa Mon Sep 17 00:00:00 2001 From: Cesar Wong Date: Tue, 28 Nov 2017 14:04:52 -0500 Subject: [PATCH] build controller: fix start of serial builds on cancellation/deletion --- .../controller/build/build_controller.go | 30 ++++++++++- test/extended/builds/run_policy.go | 53 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/pkg/build/controller/build/build_controller.go b/pkg/build/controller/build/build_controller.go index 923aadad0a62..567bf156b5c4 100644 --- a/pkg/build/controller/build/build_controller.go +++ b/pkg/build/controller/build/build_controller.go @@ -213,6 +213,7 @@ func NewBuildController(params *BuildControllerParams) *BuildController { c.buildInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: c.buildAdded, UpdateFunc: c.buildUpdated, + DeleteFunc: c.buildDeleted, }) params.ImageStreamInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: c.imageStreamAdded, @@ -1067,10 +1068,14 @@ func (bc *BuildController) handleBuildConfig(bcNamespace string, bcName string) return err } glog.V(5).Infof("Build config %s/%s: has %d next builds, is running builds: %v", bcNamespace, bcName, len(nextBuilds), hasRunningBuilds) - if len(nextBuilds) == 0 && hasRunningBuilds { + if hasRunningBuilds { glog.V(4).Infof("Build config %s/%s has running builds, will retry", bcNamespace, bcName) return fmt.Errorf("build config %s/%s has running builds and cannot run more builds", bcNamespace, bcName) } + if len(nextBuilds) == 0 { + glog.V(4).Infof("Build config %s/%s has no builds to run next, will retry", bcNamespace, bcName) + return fmt.Errorf("build config %s/%s has no builds to run next", bcNamespace, bcName) + } // Enqueue any builds to build next for _, build := range nextBuilds { @@ -1174,6 +1179,29 @@ func (bc *BuildController) buildUpdated(old, cur interface{}) { bc.enqueueBuild(build) } +// buildDeleted is called by the build informer event handler whenever a build +// is deleted +func (bc *BuildController) buildDeleted(obj interface{}) { + build, ok := obj.(*buildapi.Build) + if !ok { + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + utilruntime.HandleError(fmt.Errorf("couldn't get object from tombstone: %+v", obj)) + return + } + build, ok = tombstone.Obj.(*buildapi.Build) + if !ok { + utilruntime.HandleError(fmt.Errorf("tombstone contained object that is not a pod: %+v", obj)) + return + } + } + // If the build was not in a complete state, poke the buildconfig to run the next build + if !buildutil.IsBuildComplete(build) { + bcName := buildutil.ConfigNameForBuild(build) + bc.enqueueBuildConfig(build.Namespace, bcName) + } +} + // enqueueBuild adds the given build to the buildQueue. func (bc *BuildController) enqueueBuild(build *buildapi.Build) { key := resourceName(build.Namespace, build.Name) diff --git a/test/extended/builds/run_policy.go b/test/extended/builds/run_policy.go index 6f7e0868c68b..8925188c4628 100644 --- a/test/extended/builds/run_policy.go +++ b/test/extended/builds/run_policy.go @@ -1,6 +1,7 @@ package builds import ( + "fmt" "strings" "time" @@ -310,6 +311,58 @@ var _ = g.Describe("[Feature:Builds][Slow] using build configuration runPolicy", }) }) + g.Describe("build configuration with Serial build run policy handling deletion", func() { + g.It("starts the next build immediately after running one is deleted", func() { + g.By("starting multiple builds") + + bcName := "sample-serial-build" + for i := 0; i < 3; i++ { + _, _, err := exutil.StartBuild(oc, bcName) + o.Expect(err).NotTo(o.HaveOccurred()) + } + + buildWatch, err := oc.BuildClient().Build().Builds(oc.Namespace()).Watch(metav1.ListOptions{ + LabelSelector: buildutil.BuildConfigSelector(bcName).String(), + }) + defer buildWatch.Stop() + o.Expect(err).NotTo(o.HaveOccurred()) + + var deleteTime, deleteTime2 time.Time + done := false + var timedOut error + for !done { + select { + case event := <-buildWatch.ResultChan(): + build := event.Object.(*buildapi.Build) + if build.Status.Phase == buildapi.BuildPhasePending { + if build.Name == "sample-serial-build-1" { + err := oc.Run("delete").Args("build", "sample-serial-build-1", "--ignore-not-found").Execute() + o.Expect(err).ToNot(o.HaveOccurred()) + deleteTime = time.Now() + } + if build.Name == "sample-serial-build-2" { + duration := time.Now().Sub(deleteTime) + o.Expect(duration).To(o.BeNumerically("<", 20*time.Second), "next build should have started less than 20s after deleted build") + err := oc.Run("delete").Args("build", "sample-serial-build-2", "--ignore-not-found").Execute() + o.Expect(err).ToNot(o.HaveOccurred()) + deleteTime2 = time.Now() + } + if build.Name == "sample-serial-build-3" { + duration := time.Now().Sub(deleteTime2) + o.Expect(duration).To(o.BeNumerically("<", 20*time.Second), "next build should have started less than 20s after deleted build") + done = true + } + } + case <-time.After(2 * time.Minute): + // Give up waiting and finish after 2 minutes + timedOut = fmt.Errorf("timed out waiting for pending build") + done = true + } + } + o.Expect(timedOut).NotTo(o.HaveOccurred()) + }) + }) + g.Describe("build configuration with SerialLatestOnly build run policy", func() { g.It("runs the builds in serial order but cancel previous builds", func() { g.By("starting multiple builds")