From 885542fa2d185dae3ade703a706846430c591858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Ma=C5=82achowski?= <38684517+KacperMalachowski@users.noreply.github.com> Date: Fri, 31 Jan 2025 11:19:08 +0100 Subject: [PATCH] Fix getting report (#12619) * Strip timestamps before searching report string * Move regex up, to match reprotRegex * Fix parsing build report * Add debug logs, cean up code --- cmd/image-builder/main.go | 16 +++++----- pkg/imagebuilder/report.go | 15 ++------- pkg/imagebuilder/report_test.go | 56 ++++++++++++++++++++++++++------- 3 files changed, 55 insertions(+), 32 deletions(-) diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index bc37e09edbbd..0f468502b50b 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -383,6 +383,8 @@ func buildInADO(o options) error { if err != nil { return fmt.Errorf("build in ADO failed, failed parsing build report from ADO pipeline run logs, err: %s", err) } + + o.logger.Debugw("Parsed build report from ADO logs", "buildReport", buildReport) } else { dryRunPipelineRunResult := pipelines.RunResult("Succeeded") pipelineRunResult = &dryRunPipelineRunResult @@ -394,27 +396,25 @@ func buildInADO(o options) error { if o.ciSystem == GithubActions { fmt.Println("Setting GitHub outputs.") images := buildReport.GetImages() - if !o.dryRun { - fmt.Printf("Extracted built images from ADO logs: %v\n", images) - } else { - fmt.Println("Running in dry-run mode. Skipping extracting images and results from ADO.") - images = []string{"registry/repo/image1:tag1", "registry/repo/image2:tag2"} - } + + o.logger.Debugw("Extracted built images from ADO logs", "images", images) + data, err := json.Marshal(images) if err != nil { return fmt.Errorf("cannot marshal list of images: %w", err) } + o.logger.Debugw("Set GitHub outputs", "images", string(data), "adoResult", string(*pipelineRunResult)) + err = actions.SetOutput("images", string(data)) if err != nil { return fmt.Errorf("cannot set images GitHub output: %w", err) } - fmt.Println("images GitHub output set") + err = actions.SetOutput("adoResult", string(*pipelineRunResult)) if err != nil { return fmt.Errorf("cannot set adoResult GitHub output: %w", err) } - fmt.Println("adoResult GitHub output set") } if o.buildReportPath != "" { diff --git a/pkg/imagebuilder/report.go b/pkg/imagebuilder/report.go index 8af1cac90442..6f51d5ca6a4d 100644 --- a/pkg/imagebuilder/report.go +++ b/pkg/imagebuilder/report.go @@ -11,7 +11,7 @@ import ( var ( reportRegex = regexp.MustCompile(`(?s)---IMAGE BUILD REPORT---\n(.*)\n---END OF IMAGE BUILD REPORT---`) - timestampRegex = regexp.MustCompile(`\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z\s+`) + timestampRegex = regexp.MustCompile(`\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+Z\s`) ) type BuildReport struct { @@ -36,10 +36,6 @@ type ImageSpec struct { func (br *BuildReport) GetImages() []string { var images []string - if br == nil { - return images - } - for _, tag := range br.ImageSpec.Tags { images = append(images, fmt.Sprintf("%s%s:%s", br.ImageSpec.RepositoryPath, br.ImageSpec.Name, tag)) } @@ -54,7 +50,7 @@ func NewBuildReportFromLogs(log string) (*BuildReport, error) { // Find the report in the log matches := reportRegex.FindStringSubmatch(log) if len(matches) < 2 { - return nil, nil + return nil, fmt.Errorf("no image build report found in log") } // Parse the report data @@ -72,13 +68,8 @@ func WriteReportToFile(report *BuildReport, path string) error { return fmt.Errorf("failed to marshal report: %w", err) } - file, err := os.Open(path) + err = os.WriteFile(path, data, os.ModePerm) if err != nil { - return fmt.Errorf("failed to open file: %w", err) - } - defer file.Close() - - if _, err := file.Write(data); err != nil { return fmt.Errorf("failed to write report to file: %w", err) } diff --git a/pkg/imagebuilder/report_test.go b/pkg/imagebuilder/report_test.go index be99455821d6..5bb7e085de11 100644 --- a/pkg/imagebuilder/report_test.go +++ b/pkg/imagebuilder/report_test.go @@ -51,18 +51,26 @@ var _ = Describe("Report", func() { Expect(actual).To(Equal(expectedReport)) }) + + It("returns an error if the log does not contain the image build report", func() { + logs := `2025-01-31T08:32:23.5327056Z ##[section]Starting: prepare_image_build_report` + + _, err := NewBuildReportFromLogs(logs) + Expect(err).To(HaveOccurred()) + }) }) Describe("GetImages", func() { - report := &BuildReport{ - ImageSpec: ImageSpec{ - Name: "ginkgo-test-image/ginkgo", - Tags: []string{"1.23.0-50049457", "wartosc", "innytag", "v20250129-50049457", "1.23.0"}, - RepositoryPath: "europe-docker.pkg.dev/kyma-project/prod/", - }, - } - It("returns the list of images", func() { + It("returns the list of images from build report", func() { + report := &BuildReport{ + ImageSpec: ImageSpec{ + Name: "ginkgo-test-image/ginkgo", + Tags: []string{"1.23.0-50049457", "wartosc", "innytag", "v20250129-50049457", "1.23.0"}, + RepositoryPath: "europe-docker.pkg.dev/kyma-project/prod/", + }, + } + expectedImages := []string{ "europe-docker.pkg.dev/kyma-project/prod/ginkgo-test-image/ginkgo:1.23.0-50049457", "europe-docker.pkg.dev/kyma-project/prod/ginkgo-test-image/ginkgo:wartosc", @@ -75,13 +83,37 @@ var _ = Describe("Report", func() { }) It("returns an empty list if there are no tags", func() { - report.ImageSpec.Tags = []string{} + report := &BuildReport{ + ImageSpec: ImageSpec{ + Name: "ginkgo-test-image/ginkgo", + Tags: []string{}, + RepositoryPath: "europe-docker.pkg.dev/kyma-project/prod/", + }, + } + Expect(report.GetImages()).To(BeEmpty()) }) + }) + + Describe("WriteReportToFile", func() { + report := &BuildReport{ + Status: "Succeeded", + IsPushed: true, + IsSigned: false, + IsProduction: false, + ImageSpec: ImageSpec{ + Name: "github-tools-sap/conduit-cli", + Tags: []string{"PR-477"}, + RepositoryPath: "europe-docker.pkg.dev/kyma-project/dev/", + }, + } + + It("writes the report to a file", func() { + path := "/tmp/report.json" + err := WriteReportToFile(report, path) + Expect(err).ToNot(HaveOccurred()) - It("returns an empty list if build report is nil", func() { - var nilReport *BuildReport - Expect(nilReport.GetImages()).To(BeEmpty()) + Expect(path).To(BeAnExistingFile()) }) }) })