diff --git a/cmd/caib/buildcmd/logs.go b/cmd/caib/buildcmd/logs.go index 5c0132ba8..e34b52109 100644 --- a/cmd/caib/buildcmd/logs.go +++ b/cmd/caib/buildcmd/logs.go @@ -165,9 +165,8 @@ func (h *Handler) waitForBuildCompletion(ctx context.Context, api *buildapiclien lastPhase == phaseFlashing handleErr := fmt.Errorf("%s", st.Message) - // Only show push/flash results when an image was actually produced. hasImage := st.DiskImage != "" || st.ContainerImage != "" - if hasImage && (isFlashFailure || *h.opts.FlashAfterBuild) { + if hasImage && isFlashFailure { h.displayBuildResults(ctx, api, name) h.handleFlashError(handleErr, st) } else { diff --git a/cmd/caib/buildcmd/logs_test.go b/cmd/caib/buildcmd/logs_test.go new file mode 100644 index 000000000..22e4c332e --- /dev/null +++ b/cmd/caib/buildcmd/logs_test.go @@ -0,0 +1,280 @@ +package buildcmd + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + buildapitypes "github.com/centos-automotive-suite/automotive-dev-operator/internal/buildapi" + buildapiclient "github.com/centos-automotive-suite/automotive-dev-operator/internal/buildapi/client" +) + +func newTestOpts() Options { + opts := newTestDiskOpts() + var ( + workspace string + extraRepos []string + flashCmd string + exporterSelector string + ) + opts.Workspace = &workspace + opts.ExtraRepos = &extraRepos + opts.FlashCmd = &flashCmd + opts.ExporterSelector = &exporterSelector + return opts +} + +// fakeBuildServer creates an httptest.Server that responds to /v1/builds/ +// with the given BuildResponse sequence. Each call to GetBuild returns the next +// response; once exhausted it repeats the last one. Progress endpoint returns 404. +func fakeBuildServer(t *testing.T, responses []buildapitypes.BuildResponse) *httptest.Server { + t.Helper() + callIdx := 0 + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/progress") { + w.WriteHeader(http.StatusNotFound) + return + } + idx := callIdx + if idx < len(responses)-1 { + callIdx++ + } + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(responses[idx]); err != nil { + t.Errorf("failed to encode response: %v", err) + } + })) +} + +func TestWaitForBuildCompletion_BuildFailedNoDiskImage(t *testing.T) { + responses := []buildapitypes.BuildResponse{ + { + Name: "test-build", + Phase: "Failed", + Message: `"step-build-image" exited with code 1`, + }, + } + srv := fakeBuildServer(t, responses) + defer srv.Close() + + opts := newTestOpts() + *opts.ServerURL = srv.URL + timeout := 1 + opts.Timeout = &timeout + + var capturedErr error + opts.HandleError = func(err error) { capturedErr = err } + h := NewHandler(opts) + + api, err := buildapiclient.New(srv.URL) + if err != nil { + t.Fatalf("failed to create client: %v", err) + } + + // Capture stdout to verify no flash instructions printed + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + waitErr := h.waitForBuildCompletion(t.Context(), api, "test-build") + + _ = w.Close() + out, _ := io.ReadAll(r) + os.Stdout = old + + if waitErr == nil { + t.Fatal("expected error from waitForBuildCompletion") + } + + output := string(out) + if strings.Contains(output, "Flash failed") { + t.Errorf("should not show flash instructions when build itself failed, got: %s", output) + } + if strings.Contains(output, "flash manually") { + t.Errorf("should not show manual flash guidance when build failed, got: %s", output) + } + if capturedErr == nil { + t.Fatal("expected HandleError to be called") + } + if !strings.Contains(capturedErr.Error(), "step-build-image") { + t.Errorf("error should contain build failure message, got: %v", capturedErr) + } +} + +func TestWaitForBuildCompletion_BuildFailedWithDiskImage_NotFlashFailure(t *testing.T) { + // Edge case: server returns DiskImage on a build failure (shouldn't happen + // with server fix, but tests client-side defense-in-depth). + responses := []buildapitypes.BuildResponse{ + { + Name: "test-build", + Phase: "Failed", + Message: `"step-build-image" exited with code 1`, + DiskImage: "registry.example.com/img:disk", + }, + } + srv := fakeBuildServer(t, responses) + defer srv.Close() + + opts := newTestOpts() + *opts.ServerURL = srv.URL + *opts.FlashAfterBuild = true + timeout := 1 + opts.Timeout = &timeout + + var capturedErr error + opts.HandleError = func(err error) { capturedErr = err } + h := NewHandler(opts) + + api, err := buildapiclient.New(srv.URL) + if err != nil { + t.Fatalf("failed to create client: %v", err) + } + + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + waitErr := h.waitForBuildCompletion(t.Context(), api, "test-build") + + _ = w.Close() + out, _ := io.ReadAll(r) + os.Stdout = old + + if waitErr == nil { + t.Fatal("expected error from waitForBuildCompletion") + } + + output := string(out) + // Even though DiskImage is set and FlashAfterBuild is true, the message + // does not indicate a flash failure, so no flash instructions should appear. + if strings.Contains(output, "Flash failed") { + t.Errorf("should not show flash instructions for build failure (not flash failure), got: %s", output) + } + if capturedErr == nil { + t.Fatal("expected HandleError to be called") + } +} + +func TestWaitForBuildCompletion_FlashFailure_ShowsFlashInstructions(t *testing.T) { + responses := []buildapitypes.BuildResponse{ + { + Name: "test-build", + Phase: "Failed", + Message: "Flash to device failed: timeout waiting for device", + DiskImage: "registry.example.com/ns/test-build:disk", + Jumpstarter: &buildapitypes.JumpstarterInfo{ + Available: true, + ExporterSelector: "board-type=renesas-rcar-s4,enabled=true", + FlashCmd: "j storage flash oci://registry.example.com/ns/test-build:disk", + }, + }, + } + srv := fakeBuildServer(t, responses) + defer srv.Close() + + opts := newTestOpts() + *opts.ServerURL = srv.URL + *opts.FlashAfterBuild = true + timeout := 1 + opts.Timeout = &timeout + + var capturedErr error + opts.HandleError = func(err error) { capturedErr = err } + h := NewHandler(opts) + + api, err := buildapiclient.New(srv.URL) + if err != nil { + t.Fatalf("failed to create client: %v", err) + } + + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + waitErr := h.waitForBuildCompletion(t.Context(), api, "test-build") + + _ = w.Close() + out, _ := io.ReadAll(r) + os.Stdout = old + + if waitErr == nil { + t.Fatal("expected error from waitForBuildCompletion") + } + + output := string(out) + if !strings.Contains(output, "Flash failed") { + t.Errorf("expected flash failure message, got: %s", output) + } + if !strings.Contains(output, "flash manually") { + t.Errorf("expected manual flash guidance, got: %s", output) + } + if !strings.Contains(output, "j storage flash") { + t.Errorf("expected flash command in output, got: %s", output) + } + if capturedErr == nil { + t.Fatal("expected HandleError to be called") + } + if !strings.Contains(capturedErr.Error(), "Flash to device failed") { + t.Errorf("error should contain flash failure message, got: %v", capturedErr) + } +} + +func TestWaitForBuildCompletion_FlashFailure_NoJumpstarter(t *testing.T) { + // Flash failure detected but no Jumpstarter info — should still call + // handleFlashError (which calls handleError) but not print flash instructions. + responses := []buildapitypes.BuildResponse{ + { + Name: "test-build", + Phase: "Failed", + Message: "Flash to device failed: connection refused", + DiskImage: "registry.example.com/ns/test-build:disk", + }, + } + srv := fakeBuildServer(t, responses) + defer srv.Close() + + opts := newTestOpts() + *opts.ServerURL = srv.URL + *opts.FlashAfterBuild = true + timeout := 1 + opts.Timeout = &timeout + + var capturedErr error + opts.HandleError = func(err error) { capturedErr = err } + h := NewHandler(opts) + + api, err := buildapiclient.New(srv.URL) + if err != nil { + t.Fatalf("failed to create client: %v", err) + } + + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + waitErr := h.waitForBuildCompletion(t.Context(), api, "test-build") + + _ = w.Close() + out, _ := io.ReadAll(r) + os.Stdout = old + + if waitErr == nil { + t.Fatal("expected error from waitForBuildCompletion") + } + + output := string(out) + // No Jumpstarter info, so no "flash manually" instructions should appear + if strings.Contains(output, "flash manually") { + t.Errorf("should not show flash instructions without Jumpstarter info, got: %s", output) + } + if capturedErr == nil { + t.Fatal("expected HandleError to be called") + } + if !strings.Contains(capturedErr.Error(), "Flash to device failed") { + t.Errorf("error should contain flash failure message, got: %v", capturedErr) + } +} diff --git a/internal/buildapi/server.go b/internal/buildapi/server.go index 692e72212..ea15ca210 100644 --- a/internal/buildapi/server.go +++ b/internal/buildapi/server.go @@ -95,6 +95,19 @@ const ( maxManifestSize = 900 * 1024 ) +// buildProducedArtifacts reports whether artifact URLs point to real objects. +// For Failed builds, only true when flash was attempted (proving push succeeded). +func buildProducedArtifacts(build *automotivev1alpha1.ImageBuild) bool { + switch build.Status.Phase { + case phaseCompleted, phaseFlashing: + return true + case phaseFailed: + return build.Status.FlashTaskRunName != "" + default: + return false + } +} + var getClientFromRequestFn = getClientFromRequest var getRESTConfigFromRequestFn = getRESTConfigFromRequest var createInternalRegistrySecretFn = createInternalRegistrySecret @@ -1330,8 +1343,11 @@ func listBuilds(c *gin.Context) { compStr = b.Status.CompletionTime.Format(time.RFC3339) } - containerImage := b.Spec.GetContainerPush() - diskImage := b.Spec.GetExportOCI() + var containerImage, diskImage string + if buildProducedArtifacts(&b) { + containerImage = b.Spec.GetContainerPush() + diskImage = b.Spec.GetExportOCI() + } if b.Spec.GetUseServiceAccountAuth() && externalRoute != "" { if containerImage != "" { containerImage = translateToExternalURL(containerImage, externalRoute) @@ -1369,8 +1385,13 @@ func (a *APIServer) getBuild(c *gin.Context, name string) { return } - containerImage := build.Spec.GetContainerPush() - diskImage := build.Spec.GetExportOCI() + // Only report artifact URLs when the build progressed past the Building + // phase — otherwise no image was produced and the URLs are misleading. + var containerImage, diskImage string + if buildProducedArtifacts(build) { + containerImage = build.Spec.GetContainerPush() + diskImage = build.Spec.GetExportOCI() + } var warning string if build.Spec.GetUseServiceAccountAuth() { diff --git a/internal/buildapi/server_test.go b/internal/buildapi/server_test.go index d048c5b4e..40f90dca7 100644 --- a/internal/buildapi/server_test.go +++ b/internal/buildapi/server_test.go @@ -689,4 +689,29 @@ var _ = Describe("APIServer Performance", func() { Expect(w.Code).To(Equal(http.StatusOK)) }) + + Context("buildProducedArtifacts", func() { + DescribeTable("returns correct result for each phase", + func(phase string, pushTaskRun string, flashTaskRun string, expected bool) { + build := &automotivev1alpha1.ImageBuild{ + Status: automotivev1alpha1.ImageBuildStatus{ + Phase: phase, + PushTaskRunName: pushTaskRun, + FlashTaskRunName: flashTaskRun, + }, + } + Expect(buildProducedArtifacts(build)).To(Equal(expected)) + }, + Entry("Pending", phasePending, "", "", false), + Entry("Uploading", phaseUploading, "", "", false), + Entry("Building", phaseBuilding, "", "", false), + Entry("Pushing (in progress)", phasePushing, "", "", false), + Entry("Flashing", phaseFlashing, "", "", true), + Entry("Completed", phaseCompleted, "", "", true), + Entry("Cancelled", phaseCancelled, "", "", false), + Entry("Failed during build (no push/flash)", phaseFailed, "", "", false), + Entry("Failed during push", phaseFailed, "push-taskrun", "", false), + Entry("Failed during flash", phaseFailed, "push-taskrun", "flash-taskrun", true), + ) + }) })