build controller: fix start of serial builds on cancellation/deletion

Author: csrwng

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)

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")