From 1f710a7b378daa07339ae59147baa58981f1d6a4 Mon Sep 17 00:00:00 2001 From: Benny Zlotnik Date: Thu, 16 Apr 2026 13:34:46 +0300 Subject: [PATCH] introduce caib cancel Allow cancellation of builds Signed-off-by: Benny Zlotnik Assisted-by: claude-opus-4.6 --- api/v1alpha1/imagebuild_types.go | 21 +- cmd/caib/buildcmd/build.go | 32 ++- cmd/caib/buildcmd/logs.go | 7 +- cmd/caib/image/image.go | 29 +++ cmd/caib/runtime_wiring.go | 1 + ...tive.sdv.cloud.redhat.com_imagebuilds.yaml | 3 +- internal/buildapi/client/client.go | 14 +- internal/buildapi/container_builds.go | 4 +- internal/buildapi/server.go | 111 ++++++++- internal/buildapi/server_test.go | 217 ++++++++++++++++++ internal/controller/imagebuild/controller.go | 42 +++- 11 files changed, 444 insertions(+), 37 deletions(-) diff --git a/api/v1alpha1/imagebuild_types.go b/api/v1alpha1/imagebuild_types.go index 73748b5e..b077b051 100644 --- a/api/v1alpha1/imagebuild_types.go +++ b/api/v1alpha1/imagebuild_types.go @@ -20,6 +20,23 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// ImageBuild phase constants for Status.Phase. +const ( + ImageBuildPhasePending = "Pending" + ImageBuildPhaseUploading = "Uploading" + ImageBuildPhaseBuilding = "Building" + ImageBuildPhasePushing = "Pushing" + ImageBuildPhaseFlashing = "Flashing" + ImageBuildPhaseCompleted = "Completed" + ImageBuildPhaseFailed = "Failed" + ImageBuildPhaseCancelled = "Cancelled" +) + +// IsTerminalBuildPhase reports whether phase is a final build state. +func IsTerminalBuildPhase(phase string) bool { + return phase == ImageBuildPhaseCompleted || phase == ImageBuildPhaseFailed || phase == ImageBuildPhaseCancelled +} + // ImageBuildSpec defines the desired state of ImageBuild // +kubebuilder:printcolumn:name="StorageClass",type=string,JSONPath=`.spec.storageClass` type ImageBuildSpec struct { @@ -196,8 +213,8 @@ type ImageBuildStatus struct { // +optional ObservedGeneration int64 `json:"observedGeneration,omitempty"` - // Phase represents the current phase of the build (Building, Completed, Failed) - // +kubebuilder:validation:Enum=Pending;Uploading;Building;Pushing;Flashing;Completed;Failed + // Phase represents the current phase of the build (Building, Completed, Failed, Cancelled) + // +kubebuilder:validation:Enum=Pending;Uploading;Building;Pushing;Flashing;Completed;Failed;Cancelled Phase string `json:"phase,omitempty"` // StartTime is when the build started diff --git a/cmd/caib/buildcmd/build.go b/cmd/caib/buildcmd/build.go index 4e0de156..819e59dc 100644 --- a/cmd/caib/buildcmd/build.go +++ b/cmd/caib/buildcmd/build.go @@ -10,6 +10,7 @@ import ( "strings" "time" + automotivev1alpha1 "github.com/centos-automotive-suite/automotive-dev-operator/api/v1alpha1" common "github.com/centos-automotive-suite/automotive-dev-operator/cmd/caib/common" "github.com/centos-automotive-suite/automotive-dev-operator/cmd/caib/registryauth" buildapitypes "github.com/centos-automotive-suite/automotive-dev-operator/internal/buildapi" @@ -19,16 +20,19 @@ import ( ) const ( - phaseCompleted = "Completed" - phaseFailed = "Failed" - phaseFlashing = "Flashing" - phasePending = "Pending" + phaseCancelled = automotivev1alpha1.ImageBuildPhaseCancelled + phaseCompleted = automotivev1alpha1.ImageBuildPhaseCompleted + phaseFailed = automotivev1alpha1.ImageBuildPhaseFailed + phaseFlashing = automotivev1alpha1.ImageBuildPhaseFlashing + phasePending = automotivev1alpha1.ImageBuildPhasePending + phaseUploading = automotivev1alpha1.ImageBuildPhaseUploading phaseRunning = "Running" - phaseUploading = "Uploading" errPrefixFlash = "flash" ) +var isTerminalPhase = automotivev1alpha1.IsTerminalBuildPhase + // Options wires build handlers to caller-owned state and helper functions. type Options struct { ServerURL *string @@ -818,9 +822,19 @@ func (h *Handler) handleFileUploads( // RunDelete handles `caib image delete`. func (h *Handler) RunDelete(_ *cobra.Command, args []string) { - ctx := context.Background() - buildName := args[0] + h.runBuildAction(args[0], "deleted", func(ctx context.Context, api *buildapiclient.Client, name string) error { + return api.DeleteBuild(ctx, name) + }) +} + +// RunCancel handles `caib image cancel`. +func (h *Handler) RunCancel(_ *cobra.Command, args []string) { + h.runBuildAction(args[0], "cancelled", func(ctx context.Context, api *buildapiclient.Client, name string) error { + return api.CancelBuild(ctx, name) + }) +} +func (h *Handler) runBuildAction(buildName, verb string, action func(context.Context, *buildapiclient.Client, string) error) { if strings.TrimSpace(*h.opts.ServerURL) == "" { h.handleError(fmt.Errorf("server URL required (use --server, CAIB_SERVER, run 'caib login ' or 'jmp login ')")) return @@ -832,10 +846,10 @@ func (h *Handler) RunDelete(_ *cobra.Command, args []string) { return } - if err := api.DeleteBuild(ctx, buildName); err != nil { + if err := action(context.Background(), api, buildName); err != nil { h.handleError(err) return } - fmt.Printf("Build %q deleted\n", buildName) + fmt.Printf("Build %q %s\n", buildName, verb) } diff --git a/cmd/caib/buildcmd/logs.go b/cmd/caib/buildcmd/logs.go index 0de3272f..d79b1405 100644 --- a/cmd/caib/buildcmd/logs.go +++ b/cmd/caib/buildcmd/logs.go @@ -148,6 +148,11 @@ func (h *Handler) waitForBuildCompletion(ctx context.Context, api *buildapiclien } return nil } + if st.Phase == phaseCancelled { + pb.Clear() + fmt.Println("Build was cancelled.") + return fmt.Errorf("build cancelled") + } if st.Phase == phaseFailed { pb.Clear() isFlashFailure := strings.Contains(strings.ToLower(st.Message), errPrefixFlash) || @@ -273,7 +278,7 @@ func (h *Handler) RunLogs(_ *cobra.Command, args []string) { } fmt.Printf("Build %s: %s - %s\n", name, st.Phase, st.Message) - if st.Phase == phaseCompleted || st.Phase == phaseFailed { + if isTerminalPhase(st.Phase) { logTransport := &http.Transport{ ResponseHeaderTimeout: 30 * time.Second, } diff --git a/cmd/caib/image/image.go b/cmd/caib/image/image.go index 32b04aad..5b456dc1 100644 --- a/cmd/caib/image/image.go +++ b/cmd/caib/image/image.go @@ -26,6 +26,7 @@ type Options struct { RunInjectSigned func(*cobra.Command, []string) RunToken func(*cobra.Command, []string) RunDelete func(*cobra.Command, []string) + RunCancel func(*cobra.Command, []string) GetDefaultArch func() string @@ -107,6 +108,7 @@ func NewImageCmd(opts Options) *cobra.Command { tokenCmd := newTokenCmd(opts) deleteCmd := newDeleteCmd(opts) + cancelCmd := newCancelCmd(opts) prepareResealCmd := newPrepareResealCmd(opts) resealCmd := newResealCmd(opts) @@ -282,6 +284,10 @@ func NewImageCmd(opts Options) *cobra.Command { deleteCmd.Flags().StringVar(opts.ServerURL, "server", defaultServer, "REST API server base URL") deleteCmd.Flags().StringVar(opts.AuthToken, "token", os.Getenv("CAIB_TOKEN"), "Bearer token for authentication") + // cancel command flags + cancelCmd.Flags().StringVar(opts.ServerURL, "server", defaultServer, "REST API server base URL") + cancelCmd.Flags().StringVar(opts.AuthToken, "token", os.Getenv("CAIB_TOKEN"), "Bearer token for authentication") + // flash command flags flashCmd.Flags().StringVar(opts.ServerURL, "server", defaultServer, "REST API server base URL") flashCmd.Flags().StringVar(opts.AuthToken, "token", os.Getenv("CAIB_TOKEN"), "Bearer token for authentication") @@ -317,6 +323,7 @@ func NewImageCmd(opts Options) *cobra.Command { logsCmd, tokenCmd, deleteCmd, + cancelCmd, flashCmd, prepareResealCmd, resealCmd, @@ -523,6 +530,28 @@ Examples: } } +func newCancelCmd(opts Options) *cobra.Command { + return &cobra.Command{ + Use: "cancel ", + Short: "Cancel an in-progress build", + Long: `Cancel stops an in-progress build by cancelling its Tekton PipelineRun. +The ImageBuild resource is preserved so you can inspect its logs and status. + +Only builds in Pending, Uploading, or Building phase can be cancelled. +You can only cancel builds that you created. + +Examples: + # Cancel a running build + caib image cancel my-build + + # List builds first, then cancel one + caib image list + caib image cancel `, + Args: cobra.ExactArgs(1), + Run: opts.RunCancel, + } +} + func newPrepareResealCmd(opts Options) *cobra.Command { return &cobra.Command{ Use: "prepare-reseal [source-container] [output-container]", diff --git a/cmd/caib/runtime_wiring.go b/cmd/caib/runtime_wiring.go index 40735540..c4312298 100644 --- a/cmd/caib/runtime_wiring.go +++ b/cmd/caib/runtime_wiring.go @@ -259,6 +259,7 @@ func (s runtimeState) imageOptions(h handlerSet) image.Options { RunInjectSigned: h.sealed.RunInjectSigned, RunToken: h.token.RunToken, RunDelete: h.build.RunDelete, + RunCancel: h.build.RunCancel, GetDefaultArch: getDefaultArch, ServerURL: s.ServerURL, diff --git a/config/crd/bases/automotive.sdv.cloud.redhat.com_imagebuilds.yaml b/config/crd/bases/automotive.sdv.cloud.redhat.com_imagebuilds.yaml index 1bb631d3..36506711 100644 --- a/config/crd/bases/automotive.sdv.cloud.redhat.com_imagebuilds.yaml +++ b/config/crd/bases/automotive.sdv.cloud.redhat.com_imagebuilds.yaml @@ -313,7 +313,7 @@ spec: type: integer phase: description: Phase represents the current phase of the build (Building, - Completed, Failed) + Completed, Failed, Cancelled) enum: - Pending - Uploading @@ -322,6 +322,7 @@ spec: - Flashing - Completed - Failed + - Cancelled type: string pipelineRunName: description: PipelineRunName is the name of the active PipelineRun diff --git a/internal/buildapi/client/client.go b/internal/buildapi/client/client.go index 944a5ac2..40b11d25 100644 --- a/internal/buildapi/client/client.go +++ b/internal/buildapi/client/client.go @@ -170,8 +170,16 @@ func (c *Client) GetBuild(ctx context.Context, name string) (*buildapi.BuildResp // DeleteBuild deletes an image build by name. func (c *Client) DeleteBuild(ctx context.Context, name string) error { - endpoint := c.resolve(path.Join("/v1/builds", url.PathEscape(name))) - req, err := http.NewRequestWithContext(ctx, http.MethodDelete, endpoint, nil) + return c.doBuildAction(ctx, http.MethodDelete, path.Join("/v1/builds", url.PathEscape(name)), "delete build") +} + +// CancelBuild cancels an in-progress image build by name. +func (c *Client) CancelBuild(ctx context.Context, name string) error { + return c.doBuildAction(ctx, http.MethodPost, path.Join("/v1/builds", url.PathEscape(name), "cancel"), "cancel build") +} + +func (c *Client) doBuildAction(ctx context.Context, method, endpoint, action string) error { + req, err := http.NewRequestWithContext(ctx, method, c.resolve(endpoint), nil) if err != nil { return err } @@ -185,7 +193,7 @@ func (c *Client) DeleteBuild(ctx context.Context, name string) error { defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { b, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) - return fmt.Errorf("delete build failed: %s: %s", resp.Status, string(b)) + return fmt.Errorf("%s failed: %s: %s", action, resp.Status, string(b)) } return nil } diff --git a/internal/buildapi/container_builds.go b/internal/buildapi/container_builds.go index a9f8d656..2ceb107d 100644 --- a/internal/buildapi/container_builds.go +++ b/internal/buildapi/container_builds.go @@ -166,7 +166,7 @@ func (a *APIServer) streamContainerBuildLogs(c *gin.Context, name string) { // Check if build is complete AND all pod logs have been streamed if allPodsComplete { if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, cb); err == nil { - if cb.Status.Phase == phaseCompleted || cb.Status.Phase == phaseFailed { + if isTerminalPhase(cb.Status.Phase) { break } } @@ -459,7 +459,7 @@ func (a *APIServer) getContainerBuild(c *gin.Context, name string) { buildOwner := cb.Annotations["automotive.sdv.cloud.redhat.com/requested-by"] if requester == buildOwner && cb.Spec.UseServiceAccountAuth && - (cb.Status.Phase == phaseCompleted || cb.Status.Phase == phaseFailed) { + isTerminalPhase(cb.Status.Phase) { token, _, tokenErr := a.mintRegistryToken(ctx, c, namespace) if tokenErr != nil { a.log.Error(tokenErr, "failed to mint registry token for container build", "build", name) diff --git a/internal/buildapi/server.go b/internal/buildapi/server.go index df6e488b..a407846e 100644 --- a/internal/buildapi/server.go +++ b/internal/buildapi/server.go @@ -51,13 +51,16 @@ import ( ) const ( - // Build phase constants - phaseCompleted = "Completed" - phaseFailed = "Failed" - phasePending = "Pending" + // Build phase constants — aliases for readability; canonical values in api/v1alpha1 + phaseCancelled = automotivev1alpha1.ImageBuildPhaseCancelled + phaseCompleted = automotivev1alpha1.ImageBuildPhaseCompleted + phaseFailed = automotivev1alpha1.ImageBuildPhaseFailed + phasePending = automotivev1alpha1.ImageBuildPhasePending + phaseUploading = automotivev1alpha1.ImageBuildPhaseUploading + phaseBuilding = automotivev1alpha1.ImageBuildPhaseBuilding + phasePushing = automotivev1alpha1.ImageBuildPhasePushing + phaseFlashing = automotivev1alpha1.ImageBuildPhaseFlashing phaseRunning = "Running" - phaseUploading = "Uploading" - phaseBuilding = "Building" // Image format and compression constants formatImage = "image" @@ -571,6 +574,7 @@ func (a *APIServer) createRouter() *gin.Engine { buildsGroup.GET("/:name/template", a.handleGetBuildTemplate) buildsGroup.POST("/:name/uploads", a.handleUploadFiles) buildsGroup.POST("/:name/token", a.handleCreateBuildToken) + buildsGroup.POST("/:name/cancel", a.handleCancelBuild) buildsGroup.DELETE("/:name", a.handleDeleteBuild) } @@ -809,7 +813,6 @@ func (a *APIServer) deleteBuild(c *gin.Context, name string) { return } - // Verify the requesting user owns this build requester := a.resolveRequester(c) owner := build.Annotations["automotive.sdv.cloud.redhat.com/requested-by"] if owner != requester { @@ -817,7 +820,7 @@ func (a *APIServer) deleteBuild(c *gin.Context, name string) { return } - // If the build used the internal registry, clean up its ImageStream tags. + // Clean up ImageStream tags created by this build before deleting // Only delete the specific tags this build created; if the stream becomes // empty afterwards, delete the whole ImageStream. if build.Spec.GetUseServiceAccountAuth() { @@ -856,6 +859,90 @@ func (a *APIServer) deleteBuild(c *gin.Context, name string) { c.JSON(http.StatusOK, gin.H{"message": fmt.Sprintf("build %q deleted", name)}) } +func (a *APIServer) handleCancelBuild(c *gin.Context) { + name := c.Param("name") + a.log.Info("cancel build", "name", name, "reqID", c.GetString("reqID")) + a.cancelBuild(c, name) +} + +func (a *APIServer) cancelBuild(c *gin.Context, name string) { + k8sClient, err := getClientFromRequestFn(c) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create kubernetes client"}) + return + } + + namespace := resolveNamespace() + ctx := c.Request.Context() + + build := &automotivev1alpha1.ImageBuild{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, build); err != nil { + if k8serrors.IsNotFound(err) { + c.JSON(http.StatusNotFound, gin.H{"error": "build not found"}) + return + } + c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("error fetching build: %v", err)}) + return + } + + requester := a.resolveRequester(c) + owner := build.Annotations["automotive.sdv.cloud.redhat.com/requested-by"] + if owner != requester { + c.JSON(http.StatusForbidden, gin.H{"error": "you can only cancel your own builds"}) + return + } + + switch build.Status.Phase { + case "", phasePending, phaseUploading, phaseBuilding, phasePushing, phaseFlashing: + // cancellable + default: + c.JSON(http.StatusConflict, gin.H{ + "error": fmt.Sprintf("build is in %q phase and cannot be cancelled", build.Status.Phase), + }) + return + } + + if build.Status.PipelineRunName != "" { + pipelineRun := &tektonv1.PipelineRun{} + prKey := types.NamespacedName{Name: build.Status.PipelineRunName, Namespace: namespace} + if err := k8sClient.Get(ctx, prKey, pipelineRun); err != nil { + if !k8serrors.IsNotFound(err) { + c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("error fetching PipelineRun: %v", err)}) + return + } + } else if pipelineRun.Status.CompletionTime != nil { + c.JSON(http.StatusConflict, gin.H{ + "error": "build has already completed; refresh and retry", + }) + return + } else { + pipelineRun.Spec.Status = tektonv1.PipelineRunSpecStatusCancelled + if err := k8sClient.Update(ctx, pipelineRun); err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to cancel PipelineRun: %v", err)}) + return + } + } + } + + build.Status.Phase = phaseCancelled + build.Status.Message = "Build cancelled by user" + now := metav1.Now() + if build.Status.CompletionTime == nil { + build.Status.CompletionTime = &now + } + if err := k8sClient.Status().Update(ctx, build); err != nil { + // Controller may have already set phase to Cancelled after seeing the PipelineRun cancel + if k8serrors.IsConflict(err) { + c.JSON(http.StatusOK, gin.H{"message": fmt.Sprintf("build %q cancelled", name)}) + return + } + c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to update build status: %v", err)}) + return + } + + c.JSON(http.StatusOK, gin.H{"message": fmt.Sprintf("build %q cancelled", name)}) +} + // resolveImageStreamRefs extracts the ImageStream name and the set of tags // this build pushed to from its internal registry URLs. // URL pattern: image-registry.openshift-image-registry.svc:5000/{namespace}/{stream}:{tag} @@ -1232,6 +1319,8 @@ func (a *APIServer) streamLogs(c *gin.Context, name string) { writeLogStreamFooter(c, hadStream) } +var isTerminalPhase = automotivev1alpha1.IsTerminalBuildPhase + // shouldExitLogStream checks if the log streaming loop should exit func shouldExitLogStream( ctx context.Context, @@ -1241,7 +1330,7 @@ func shouldExitLogStream( allPodsComplete bool, ) bool { if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, ib); err == nil { - if (ib.Status.Phase == phaseCompleted || ib.Status.Phase == phaseFailed) && allPodsComplete { + if isTerminalPhase(ib.Status.Phase) && allPodsComplete { return true } } @@ -2252,7 +2341,7 @@ func (a *APIServer) getBuild(c *gin.Context, name string) { // For terminal builds, include Jumpstarter mapping so the CLI can show // manual flash guidance after successful or failed flash attempts. var jumpstarterInfo *JumpstarterInfo - if build.Status.Phase == phaseCompleted || build.Status.Phase == phaseFailed { + if isTerminalPhase(build.Status.Phase) { operatorConfig := &automotivev1alpha1.OperatorConfig{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: "config", Namespace: namespace}, operatorConfig); err == nil { if operatorConfig.Status.JumpstarterAvailable { @@ -2293,7 +2382,7 @@ func (a *APIServer) getBuild(c *gin.Context, name string) { buildOwner := build.Annotations["automotive.sdv.cloud.redhat.com/requested-by"] if requester == buildOwner && build.Spec.GetUseServiceAccountAuth() && - (build.Status.Phase == phaseCompleted || build.Status.Phase == phaseFailed) { + isTerminalPhase(build.Status.Phase) { var tokenErr error registryToken, _, tokenErr = a.mintRegistryToken(ctx, c, namespace) if tokenErr != nil { diff --git a/internal/buildapi/server_test.go b/internal/buildapi/server_test.go index d2ce12a3..c759e8c2 100644 --- a/internal/buildapi/server_test.go +++ b/internal/buildapi/server_test.go @@ -9,7 +9,9 @@ import ( "os" "time" + tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -88,6 +90,7 @@ var _ = Describe("APIServer", func() { {"GET", "/v1/builds/test-build/logs"}, {"GET", "/v1/builds/test-build/template"}, {"POST", "/v1/builds/test-build/uploads"}, + {"POST", "/v1/builds/test-build/cancel"}, {"DELETE", "/v1/builds/test-build"}, } @@ -293,6 +296,220 @@ var _ = Describe("APIServer", func() { }) }) + Context("Cancel Build", func() { + var ( + originalGetClientFromRequestFn func(*gin.Context) (ctrlclient.Client, error) + originalNamespace string + hasOriginalNamespace bool + ) + + BeforeEach(func() { + originalGetClientFromRequestFn = getClientFromRequestFn + originalNamespace, hasOriginalNamespace = os.LookupEnv("BUILD_API_NAMESPACE") + Expect(os.Setenv("BUILD_API_NAMESPACE", "test-ns")).To(Succeed()) + }) + + AfterEach(func() { + getClientFromRequestFn = originalGetClientFromRequestFn + if hasOriginalNamespace { + Expect(os.Setenv("BUILD_API_NAMESPACE", originalNamespace)).To(Succeed()) + } else { + Expect(os.Unsetenv("BUILD_API_NAMESPACE")).To(Succeed()) + } + }) + + newCancelTestBuild := func(phase, pipelineRunName string) *automotivev1alpha1.ImageBuild { + build := &automotivev1alpha1.ImageBuild{} + build.Name = "my-build" + build.Namespace = testNamespace + build.Annotations = map[string]string{ + "automotive.sdv.cloud.redhat.com/requested-by": "alice", + } + build.Status.Phase = phase + build.Status.PipelineRunName = pipelineRunName + return build + } + + newCancelFakeClient := func(objs ...ctrlclient.Object) ctrlclient.Client { + scheme := runtime.NewScheme() + Expect(automotivev1alpha1.AddToScheme(scheme)).To(Succeed()) + Expect(tektonv1.AddToScheme(scheme)).To(Succeed()) + return fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + WithStatusSubresource(&automotivev1alpha1.ImageBuild{}). + Build() + } + + It("should return 404 when build does not exist", func() { + fakeClient := newCancelFakeClient() + getClientFromRequestFn = func(_ *gin.Context) (ctrlclient.Client, error) { + return fakeClient, nil + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest(http.MethodPost, "/v1/builds/nonexistent/cancel", nil) + c.Set("requester", "alice") + + server.cancelBuild(c, "nonexistent") + + Expect(w.Code).To(Equal(http.StatusNotFound)) + Expect(w.Body.String()).To(ContainSubstring("build not found")) + }) + + It("should return 403 when user does not own the build", func() { + build := newCancelTestBuild("Building", "") + fakeClient := newCancelFakeClient(build) + getClientFromRequestFn = func(_ *gin.Context) (ctrlclient.Client, error) { + return fakeClient, nil + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest(http.MethodPost, "/v1/builds/my-build/cancel", nil) + c.Set("requester", "bob") + + server.cancelBuild(c, "my-build") + + Expect(w.Code).To(Equal(http.StatusForbidden)) + Expect(w.Body.String()).To(ContainSubstring("you can only cancel your own builds")) + }) + + It("should return 409 when build is already completed", func() { + build := newCancelTestBuild("Completed", "") + fakeClient := newCancelFakeClient(build) + getClientFromRequestFn = func(_ *gin.Context) (ctrlclient.Client, error) { + return fakeClient, nil + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest(http.MethodPost, "/v1/builds/my-build/cancel", nil) + c.Set("requester", "alice") + + server.cancelBuild(c, "my-build") + + Expect(w.Code).To(Equal(http.StatusConflict)) + Expect(w.Body.String()).To(ContainSubstring("cannot be cancelled")) + }) + + It("should return 409 when build has already failed", func() { + build := newCancelTestBuild("Failed", "") + fakeClient := newCancelFakeClient(build) + getClientFromRequestFn = func(_ *gin.Context) (ctrlclient.Client, error) { + return fakeClient, nil + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest(http.MethodPost, "/v1/builds/my-build/cancel", nil) + c.Set("requester", "alice") + + server.cancelBuild(c, "my-build") + + Expect(w.Code).To(Equal(http.StatusConflict)) + Expect(w.Body.String()).To(ContainSubstring("cannot be cancelled")) + }) + + It("should cancel a pending build without a PipelineRun", func() { + build := newCancelTestBuild("Pending", "") + fakeClient := newCancelFakeClient(build) + getClientFromRequestFn = func(_ *gin.Context) (ctrlclient.Client, error) { + return fakeClient, nil + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest(http.MethodPost, "/v1/builds/my-build/cancel", nil) + c.Set("requester", "alice") + + server.cancelBuild(c, "my-build") + + Expect(w.Code).To(Equal(http.StatusOK)) + var resp map[string]string + Expect(json.Unmarshal(w.Body.Bytes(), &resp)).To(Succeed()) + Expect(resp["message"]).To(ContainSubstring("cancelled")) + + // Verify ImageBuild status was updated + updated := &automotivev1alpha1.ImageBuild{} + Expect(fakeClient.Get(context.Background(), types.NamespacedName{ + Name: "my-build", Namespace: testNamespace, + }, updated)).To(Succeed()) + Expect(updated.Status.Phase).To(Equal("Cancelled")) + Expect(updated.Status.Message).To(Equal("Build cancelled by user")) + Expect(updated.Status.CompletionTime).NotTo(BeNil()) + }) + + It("should return 409 when PipelineRun already completed", func() { + build := newCancelTestBuild("Building", "my-build-pr") + completionTime := metav1.Now() + pipelineRun := &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-build-pr", + Namespace: testNamespace, + }, + Status: tektonv1.PipelineRunStatus{ + PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{ + CompletionTime: &completionTime, + }, + }, + } + fakeClient := newCancelFakeClient(build, pipelineRun) + getClientFromRequestFn = func(_ *gin.Context) (ctrlclient.Client, error) { + return fakeClient, nil + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest(http.MethodPost, "/v1/builds/my-build/cancel", nil) + c.Set("requester", "alice") + + server.cancelBuild(c, "my-build") + + Expect(w.Code).To(Equal(http.StatusConflict)) + Expect(w.Body.String()).To(ContainSubstring("already completed")) + }) + + It("should cancel a building build and patch its PipelineRun", func() { + build := newCancelTestBuild("Building", "my-build-pr") + pipelineRun := &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-build-pr", + Namespace: testNamespace, + }, + } + fakeClient := newCancelFakeClient(build, pipelineRun) + getClientFromRequestFn = func(_ *gin.Context) (ctrlclient.Client, error) { + return fakeClient, nil + } + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest(http.MethodPost, "/v1/builds/my-build/cancel", nil) + c.Set("requester", "alice") + + server.cancelBuild(c, "my-build") + + Expect(w.Code).To(Equal(http.StatusOK)) + + // Verify PipelineRun was patched with Cancelled status + updatedPR := &tektonv1.PipelineRun{} + Expect(fakeClient.Get(context.Background(), types.NamespacedName{ + Name: "my-build-pr", Namespace: testNamespace, + }, updatedPR)).To(Succeed()) + Expect(string(updatedPR.Spec.Status)).To(Equal("Cancelled")) + + // Verify ImageBuild status was updated + updated := &automotivev1alpha1.ImageBuild{} + Expect(fakeClient.Get(context.Background(), types.NamespacedName{ + Name: "my-build", Namespace: testNamespace, + }, updated)).To(Succeed()) + Expect(updated.Status.Phase).To(Equal("Cancelled")) + Expect(updated.Status.CompletionTime).NotTo(BeNil()) + }) + }) + Context("OperatorConfig Endpoint", func() { var ( originalGetClientFromRequestFn func(*gin.Context) (ctrlclient.Client, error) diff --git a/internal/controller/imagebuild/controller.go b/internal/controller/imagebuild/controller.go index 99dc0960..184d95b8 100644 --- a/internal/controller/imagebuild/controller.go +++ b/internal/controller/imagebuild/controller.go @@ -41,10 +41,11 @@ const ( // OperatorNamespace is the namespace where the operator is deployed. OperatorNamespace = "automotive-dev-operator-system" - // Phase constants for ImageBuild status - phaseBuilding = "Building" - phaseCompleted = "Completed" - phaseFailed = "Failed" + // Phase constants — aliases for readability; canonical values in api/v1alpha1 + phaseBuilding = automotivev1alpha1.ImageBuildPhaseBuilding + phaseCancelled = automotivev1alpha1.ImageBuildPhaseCancelled + phaseCompleted = automotivev1alpha1.ImageBuildPhaseCompleted + phaseFailed = automotivev1alpha1.ImageBuildPhaseFailed // Tekton condition type for completion status conditionSucceeded = "Succeeded" @@ -70,6 +71,8 @@ const ( eventReasonDiskBuildDone = "DiskBuildCompleted" ) +var isTerminalPhase = automotivev1alpha1.IsTerminalBuildPhase + // safeDerivedName generates a Kubernetes-safe derived resource name by truncating // the base name and appending a hash to preserve uniqueness. The final name will // never exceed maxK8sNameLength (63 chars for DNS label names) characters. @@ -153,8 +156,10 @@ func (r *ImageBuildReconciler) Reconcile(ctx context.Context, req ctrl.Request) return r.handleFlashingState(ctx, imageBuild) case phaseCompleted: return r.handleCompletedState(ctx, imageBuild) - case phaseFailed: - // Retry cleanup of any transient secrets that failed to delete. + case phaseCancelled, phaseFailed: + if err := r.shutdownUploadPod(ctx, imageBuild); err != nil { + return ctrl.Result{RequeueAfter: secretCleanupRequeue}, err + } if err := r.cleanupTransientSecrets(ctx, imageBuild, r.Log); err != nil { return ctrl.Result{RequeueAfter: secretCleanupRequeue}, nil } @@ -488,9 +493,26 @@ func (r *ImageBuildReconciler) checkBuildProgress( return ctrl.Result{}, nil } - // Build failed - cleanup transient secrets cleanupErr := r.cleanupTransientSecrets(ctx, imageBuild, r.Log) + if pipelineRun.Spec.Status == tektonv1.PipelineRunSpecStatusCancelled { + if imageBuild.Status.Phase == phaseCancelled { + if cleanupErr != nil { + return ctrl.Result{RequeueAfter: secretCleanupRequeue}, nil + } + return ctrl.Result{}, nil + } + if err := r.updateStatus(ctx, imageBuild, phaseCancelled, "Build cancelled by user"); err != nil { + log.Error(err, "Failed to update status to Cancelled") + return ctrl.Result{}, err + } + recordBuildMetrics(imageBuild, pipelineRun, buildStatusFailure) + if cleanupErr != nil { + return ctrl.Result{RequeueAfter: secretCleanupRequeue}, nil + } + return ctrl.Result{}, nil + } + if err := r.updateStatus(ctx, imageBuild, phaseFailed, r.pipelineRunFailureDetail(ctx, pipelineRun)); err != nil { log.Error(err, "Failed to update status to Failed") return ctrl.Result{}, err @@ -2076,6 +2098,10 @@ func (r *ImageBuildReconciler) updateStatus( return err } + if fresh.Status.Phase == phaseCancelled && phase != phaseCancelled { + return nil + } + patch := client.MergeFrom(fresh.DeepCopy()) oldPhase := fresh.Status.Phase oldMessage := fresh.Status.Message @@ -2086,7 +2112,7 @@ func (r *ImageBuildReconciler) updateStatus( if phase == phaseBuilding && fresh.Status.StartTime == nil { now := metav1.Now() fresh.Status.StartTime = &now - } else if (phase == phaseCompleted || phase == phaseFailed) && fresh.Status.CompletionTime == nil { + } else if isTerminalPhase(phase) && fresh.Status.CompletionTime == nil { now := metav1.Now() fresh.Status.CompletionTime = &now }