Add possibility for a client to cancel a build…

… by updating the status of a build.

It will mark the build as failed and clean any pods related to it.

Signed-off-by: Vincent Demeester <[email protected]>

pkg/apis/build/v1alpha1/build_types.go

@@ -98,8 +98,21 @@ type BuildSpec struct {
 	// If specified, the pod's scheduling constraints
 	// +optional
 	Affinity *corev1.Affinity `json:"affinity,omitempty"`
+
+	// Used for cancelling a job (and maybe more later on)
+	// +optional
+	Status BuildSpecStatus
 }
 
+// BuildSpecStatus defines the build spec status the user can provide
+type BuildSpecStatus string
+
+const (
+	// BuildSpecStatusCancelled indicates that the user wants to cancel the build,
+	// if not already cancelled or terminated
+	BuildSpecStatusCancelled = "BuildCancelled"
+)
+
 // TemplateKind defines the type of BuildTemplate used by the build.
 type TemplateKind string
 
@@ -281,6 +294,8 @@ type GoogleSpec struct {
 // will be False.
 const BuildSucceeded = duckv1alpha1.ConditionSucceeded
 
+const BuildCancelled duckv1alpha1.ConditionType = "Cancelled"
+
 var buildCondSet = duckv1alpha1.NewBatchConditionSet()
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/reconciler/build/build.go

@@ -153,6 +153,36 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		return nil
 	}
 
+	// If the build's status is cancelled, kill resources and update status
+	if isCancelled(build.Spec) {
+		logger.Warnf("Build has been cancelled: %v", build.Name)
+		build.Status.SetCondition(&duckv1alpha1.Condition{
+			Type:    v1alpha1.BuildSucceeded,
+			Status:  corev1.ConditionFalse,
+			Reason:  "BuildCancelled",
+			Message: fmt.Sprintf("Build %q was cancelled", build.Name),
+		})
+		if err := c.updateStatus(build); err != nil {
+			return err
+		}
+		if build.Status.Cluster == nil {
+			logger.Warnf("build %q has no pod running yet", build.Name)
+			return nil
+		}
+		p, err := c.kubeclientset.CoreV1().Pods(build.Namespace).Get(build.Status.Cluster.PodName, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		if p == nil {
+			logger.Warnf("build %q has no pod running", build.Name)
+			return nil
+		}
+		if err := c.kubeclientset.CoreV1().Pods(build.Namespace).Delete(p.Name, &metav1.DeleteOptions{}); err != nil {
+			return err
+		}
+		return nil
+	}
+
 	// If the build hasn't started yet, validate it and create a Pod for it
 	// and record that pod's name in the build status.
 	var p *corev1.Pod
@@ -257,12 +287,17 @@ func (c *Reconciler) startPodForBuild(build *v1alpha1.Build) (*corev1.Pod, error
 	return c.kubeclientset.CoreV1().Pods(p.Namespace).Create(p)
 }
 
-// IsDone returns true if the build's status indicates the build is done.
+// isDone returns true if the build's status indicates the build is done.
 func isDone(status *v1alpha1.BuildStatus) bool {
 	cond := status.GetCondition(v1alpha1.BuildSucceeded)
 	return cond != nil && cond.Status != corev1.ConditionUnknown
 }
 
+// isCancelled returns true if the build's spec indicates the build is cancelled.
+func isCancelled(buildSpec v1alpha1.BuildSpec) bool {
+	return buildSpec.Status == v1alpha1.BuildSpecStatusCancelled
+}
+
 func (c *Reconciler) checkTimeout(build *v1alpha1.Build) error {
 	// If build has not started timeout, startTime should be zero.
 	if build.Status.StartTime.IsZero() {

pkg/reconciler/build/build_test.go

@@ -561,3 +561,62 @@ func TestTimeoutFlow(t *testing.T) {
 		t.Errorf("Unexpected build status %s", d)
 	}
 }
+
+func TestCancelledFlow(t *testing.T) {
+	b := newBuild("cancelled")
+
+	f := &fixture{
+		t:          t,
+		objects:    []runtime.Object{b},
+		client:     fake.NewSimpleClientset(b),
+		kubeclient: k8sfake.NewSimpleClientset(),
+	}
+	f.createServiceAccount()
+
+	r, i, k8sI := f.newReconciler()
+	f.updateIndex(i, b)
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+	i.Start(stopCh)
+	k8sI.Start(stopCh)
+
+	ctx := context.Background()
+	if err := r.Reconcile(ctx, getKey(b, t)); err != nil {
+		t.Errorf("Not Expect error when syncing build")
+	}
+
+	buildClient := f.client.BuildV1alpha1().Builds(b.Namespace)
+	b, err := buildClient.Get(b.Name, metav1.GetOptions{})
+	if err != nil {
+		t.Errorf("error fetching build: %v", err)
+	}
+
+	// Get pod info
+	b, err = buildClient.Get(b.Name, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("error getting build: %v", err)
+	}
+	if b.Status.Cluster == nil || b.Status.Cluster.PodName == "" {
+		t.Fatalf("build status did not specify podName: %v", b.Status.Cluster)
+	}
+	b.Spec.Status = v1alpha1.BuildSpecStatusCancelled
+
+	f.updateIndex(i, b)
+	if err := r.Reconcile(ctx, getKey(b, t)); err != nil {
+		t.Errorf("error syncing build: %v", err)
+	}
+
+	// Check that the build has the expected timeout status.
+	b, err = buildClient.Get(b.Name, metav1.GetOptions{})
+	if err != nil {
+		t.Errorf("error fetching build: %v", err)
+	}
+	if d := cmp.Diff(b.Status.GetCondition(duckv1alpha1.ConditionSucceeded), &duckv1alpha1.Condition{
+		Type:    duckv1alpha1.ConditionSucceeded,
+		Status:  corev1.ConditionFalse,
+		Reason:  "BuildCancelled",
+		Message: fmt.Sprintf("Build %q was cancelled", b.Name),
+	}, ignoreVolatileTime); d != "" {
+		t.Errorf("Unexpected build status %s", d)
+	}
+}

test/e2e/simple_test.go

@@ -605,3 +605,47 @@ func TestSimpleBuildWithHybridSources(t *testing.T) {
 		t.Fatalf("Error watching build: %v", err)
 	}
 }
+
+// TestCancelledBuild tests that a build with a long running step
+// so that we have time to cancel it.
+func TestCancelledBuild(t *testing.T) {
+	logger := logging.GetContextLogger("TestCancelledBuild")
+
+	clients := buildClients(logger)
+	buildName := "cancelled-build"
+
+	test.CleanupOnInterrupt(func() { teardownBuild(clients, logger, buildName) }, logger)
+	defer teardownBuild(clients, logger, buildName)
+
+	if _, err := clients.buildClient.builds.Create(&v1alpha1.Build{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: buildTestNamespace,
+			Name:      buildName,
+		},
+		Spec: v1alpha1.BuildSpec{
+			Steps: []corev1.Container{{
+				Image: "busybox",
+				Args:  []string{"sleep", "5000"}, // fails.
+			}},
+		},
+	}); err != nil {
+		t.Fatalf("Error creating build: %v", err)
+	}
+
+	// Wait for a little while
+	// FIXME(vdemeester) I would prefer something less flaky-prone
+	time.Sleep(20 * time.Second)
+
+	b, err := clients.buildClient.builds.Get(buildName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Error getting build %s: %v", buildName, err)
+	}
+	b.Spec.Status = v1alpha1.BuildSpecStatusCancelled
+	if _, err := clients.buildClient.builds.Update(b); err != nil {
+		t.Fatalf("Error update build status for %s: %v", buildName, err)
+	}
+
+	if _, err := clients.buildClient.watchBuild(buildName); err == nil {
+		t.Fatalf("watchBuild did not return expected `cancelled` error")
+	}
+}