Skip to content

Commit

Permalink
feat(KONFLUX-2072): prepare the pkg to be able to detect issues outsi…
Browse files Browse the repository at this point in the history 
…de e2e workflow (#85)

* feat(KONFLUX-2072): be able to detect issues outside e2e workflow

* Improved the handling of iterator of e2e directory. The changes ensure
  that the iterator's pointer is not advanced when checking for
  `iterator.Done`, preventing potential data loss.
* When the first call to `it.Next()` returns `iterator.Done`, that means
  there's no files found within the current directory with given prefix.
* This implies there are no files present within the directory that's
  supposed to store artifacts related to e2e tests.
* So, we initialise a new iterator for a new directory with prefix that
  points to "build-log.txt" file. And accordingly we init the maps.
* This commit also refactors some logic to move them inside a function.

Signed-off-by: Dheeraj<[email protected]>

* refactor(KONFLUX-2072): make 'Run()' method more readable and clean

Co-authored-by: Pavel Sturc <[email protected]>

* style(KONFLUX-2072): address Golang warning

Co-authored-by: Tomáš Nevrlka <[email protected]>

* refactor(KONFLUX-2072): avoid else branches by using an early "return nil"

Co-authored-by: Tomáš Nevrlka <[email protected]>

* fix(KONFLUX-2072): check the size of array before accessing it

Signed-off-by: Dheeraj<[email protected]>

---------

Signed-off-by: Dheeraj<[email protected]>
Co-authored-by: Pavel Sturc <[email protected]>
Co-authored-by: Tomáš Nevrlka <[email protected]>
  • Loading branch information
@dheerajodha @psturc @tnevrlka
3 people authored Mar 18, 2024
1 parent ce50aa0 commit 142df51
Showing 1 changed file with 172 additions and 63 deletions.
235 changes: 172 additions & 63 deletions pkg/prow/prow.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package prow
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -34,110 +35,190 @@ func NewArtifactScanner(cfg ScannerConfig) (*ArtifactScanner, error) {
}, nil
}

// Run processes the artifacts associated with the Prow job and store required files
// with their associated openshift-ci step names and their content in ArtifactStepMap
// Run processes the artifacts associated with the Prow job and stores required files
// with their associated openshift-ci step names and their content in ArtifactStepMap.
func (as *ArtifactScanner) Run() error {
var jobTarget, pjURL string
var pjYAML *v1.ProwJob
var err error
// Determine job target and Prow job URL.
jobTarget, pjURL, err := as.determineJobDetails()
if err != nil {
return fmt.Errorf("failed to determine job details: %+v", err)
}

artifactDirectoryPrefix, err := getArtifactsDirectoryPrefix(as, pjURL, jobTarget)
if err != nil {
return fmt.Errorf("failed to get artifact directory prefix: %+v", err)
}

// Iterate over storage objects.
ctx, cancel := context.WithTimeout(context.Background(), time.Minute*2)
defer cancel()
as.bucketHandle = as.Client.Bucket(bucketName)
it := as.bucketHandle.Objects(ctx, &storage.Query{Prefix: artifactDirectoryPrefix})

// Process storage objects.
if err := as.processStorageObjects(ctx, it, artifactDirectoryPrefix, pjURL); err != nil {
return fmt.Errorf("failed to process storage objects: %+v", err)
}

return nil
}

// Helper function to determine job details.
func (as *ArtifactScanner) determineJobDetails() (jobTarget, pjURL string, err error) {
switch {
case as.config.ProwJobID != "":
pjYAML, err = getProwJobYAML(as.config.ProwJobID)
pjYAML, err := getProwJobYAML(as.config.ProwJobID)
if err != nil {
return fmt.Errorf("failed to get prow job yaml: %+v", err)
return "", "", fmt.Errorf("failed to get Prow job YAML: %+v", err)
}

jobTarget, err = determineJobTargetFromYAML(pjYAML)
if err != nil {
return fmt.Errorf("failed to determine job target: %+v", err)
return "", "", fmt.Errorf("failed to determine job target from YAML: %+v", err)
}

pjURL = pjYAML.Status.URL
klog.Infof("got the prow job URL: %s", pjURL)

case as.config.ProwJobURL != "":
pjURL = as.config.ProwJobURL
klog.Infof("got the prow job URL: %s", pjURL)

jobTarget, err = determineJobTargetFromProwJobURL(pjURL)
if err != nil {
return fmt.Errorf("failed to determine job target: %+v", err)
return "", "", fmt.Errorf("failed to determine job target from Prow job URL: %+v", err)
}

default:
return fmt.Errorf("ScannerConfig doesn't contain neither ProwJobID nor ProwJobURL")
return "", "", fmt.Errorf("ScannerConfig doesn't contain either ProwJobID or ProwJobURL")
}

artifactDirectoryPrefix, err := getArtifactsDirectoryPrefix(as, pjURL, jobTarget)
if err != nil {
return err
return jobTarget, pjURL, nil
}

// Helper function to process storage objects.
func (as *ArtifactScanner) processStorageObjects(ctx context.Context, it *storage.ObjectIterator, artifactDirectoryPrefix, pjURL string) error {
var objectAttrs *storage.ObjectAttrs
var err error

objectAttrs, err = it.Next()
if errors.Is(err, iterator.Done) {
// No files present within the target directory - get the root build-log.txt instead.
if err := as.handleEmptyDirectory(ctx, pjURL, artifactDirectoryPrefix); err != nil {
return err
}
return nil
}

ctx, cancel := context.WithTimeout(context.Background(), time.Minute*2)
defer cancel()
as.bucketHandle = as.Client.Bucket(bucketName)
// Iterate over storage objects.
for {
if err != nil {
return fmt.Errorf("failed to iterate over storage objects: %+v", err)
}
fullArtifactName := objectAttrs.Name
if as.isRequiredFile(fullArtifactName) {
if err := as.processRequiredFile(fullArtifactName, artifactDirectoryPrefix); err != nil {
return err
}
}

it := as.bucketHandle.Objects(ctx, &storage.Query{Prefix: artifactDirectoryPrefix})
objectAttrs, err = it.Next()
if errors.Is(err, iterator.Done) {
break
}
}

return nil
}

// Helper function to handle an empty directory.
func (as *ArtifactScanner) handleEmptyDirectory(ctx context.Context, pjURL, artifactDirectoryPrefix string) error {
klog.Infof("For the job (%s), there are no files present within the directory with prefix: `%s`", pjURL, artifactDirectoryPrefix)

// Set up default file name filter.
fileName := "build-log.txt"
as.config.FileNameFilter = []string{fileName}

// Check for build log file.
sp := strings.Split(pjURL, "/"+bucketName+"/")
if len(sp) != 2 {
return fmt.Errorf("failed to determine artifact directory's prefix - Prow job URL: '%s', bucket name: '%s'", pjURL, bucketName)
}
buildLogPrefix := sp[1] + "/" + fileName

// Iterate over build log files.
it := as.bucketHandle.Objects(ctx, &storage.Query{Prefix: buildLogPrefix})
for {
attrs, err := it.Next()
if err == iterator.Done {
if errors.Is(err, iterator.Done) {
break
}
if err != nil {
return fmt.Errorf("failed to iterate over storage objects: %+v", err)
}
fullArtifactName := attrs.Name
if as.isRequiredFile(fullArtifactName) {
klog.Infof("found required file %s", fullArtifactName)
// => e.g. [ "", "redhat-appstudio-e2e/artifacts/e2e-report.xml" ]
sp := strings.Split(fullArtifactName, artifactDirectoryPrefix)
if len(sp) != 2 {
return fmt.Errorf("cannot determine filepath - object name: %s, object prefix: %s", fullArtifactName, artifactDirectoryPrefix)
}
parentStepFilePath := sp[1]

// => e.g. [ "redhat-appstudio-e2e", "artifacts", "e2e-report.xml" ]
sp = strings.Split(parentStepFilePath, "/")
parentStepName := sp[0]
if err := as.initArtifactStepMap(ctx, fileName, fullArtifactName, "/"); err != nil {
return err
}
}

return nil
}

if slices.Contains(as.config.StepsToSkip, parentStepName) {
klog.Infof("skipping step name %s", parentStepName)
continue
}
// Helper function to process a required file.
func (as *ArtifactScanner) processRequiredFile(fullArtifactName, artifactDirectoryPrefix string) error {
parentStepName, err := getParentStepName(fullArtifactName, artifactDirectoryPrefix)
if err != nil {
return err
}

fileName := sp[len(sp)-1]
if slices.Contains(as.config.StepsToSkip, parentStepName) {
klog.Infof("Skipping step name %s", parentStepName)
return nil
}

rc, err := as.bucketHandle.Object(fullArtifactName).NewReader(ctx)
if err != nil {
return fmt.Errorf("failed to create objecthandle for %s: %+v", fullArtifactName, err)
}
data, err := io.ReadAll(rc)
if err != nil {
return fmt.Errorf("cannot read from storage reader: %+v", err)
}
fileName, err := getFileName(fullArtifactName, artifactDirectoryPrefix)
if err != nil {
return err
}

artifact := Artifact{Content: string(data), FullName: fullArtifactName}

// No artifact step map not initialized yet
if as.ArtifactStepMap == nil {
newArtifactMap := ArtifactFilenameMap{ArtifactFilename(fileName): artifact}
as.ArtifactStepMap = map[ArtifactStepName]ArtifactFilenameMap{ArtifactStepName(parentStepName): newArtifactMap}
} else {
// Already have a record of an artifact being mapped to a step name
if afMap, ok := as.ArtifactStepMap[ArtifactStepName(parentStepName)]; ok {
afMap[ArtifactFilename(fileName)] = artifact
as.ArtifactStepMap[ArtifactStepName(parentStepName)] = afMap
} else { // Artifact map initialized, but the artifact filename does not belong to any collected step
as.ArtifactStepMap[ArtifactStepName(parentStepName)] = ArtifactFilenameMap{ArtifactFilename(fileName): artifact}
}
}
}
if err := as.initArtifactStepMap(context.Background(), fileName, fullArtifactName, parentStepName); err != nil {
return err
}

return nil
}

// Helper function to initialise/update the ArtifactStepMap with content
// of a file with given 'fileName', within the given 'parentStepName'
func (as *ArtifactScanner) initArtifactStepMap(ctx context.Context, fileName, fullArtifactName, parentStepName string) error {
rc, err := as.bucketHandle.Object(fullArtifactName).NewReader(ctx)
if err != nil {
return fmt.Errorf("failed to create objecthandle for %s: %+v", fullArtifactName, err)
}
data, err := io.ReadAll(rc)
if err != nil {
return fmt.Errorf("cannot read from storage reader: %+v", err)
}

artifact := Artifact{Content: string(data), FullName: fullArtifactName}
newArtifactMap := ArtifactFilenameMap{ArtifactFilename(fileName): artifact}

// No artifact step map not initialized yet
if as.ArtifactStepMap == nil {
as.ArtifactStepMap = map[ArtifactStepName]ArtifactFilenameMap{ArtifactStepName(parentStepName): newArtifactMap}
return nil
}

// Already have a record of an artifact being mapped to a step name
if afMap, ok := as.ArtifactStepMap[ArtifactStepName(parentStepName)]; ok {
afMap[ArtifactFilename(fileName)] = artifact
as.ArtifactStepMap[ArtifactStepName(parentStepName)] = afMap
} else { // Artifact map initialized, but the artifact filename does not belong to any collected step
as.ArtifactStepMap[ArtifactStepName(parentStepName)] = newArtifactMap
}

return nil
}

// Helper function to check if a file with given 'fullArtifactName',
// matches the file-name filter(s) defined within ScannerConfig struct
func (as *ArtifactScanner) isRequiredFile(fullArtifactName string) bool {
return slices.ContainsFunc(as.config.FileNameFilter, func(s string) bool {
re := regexp.MustCompile(s)
Expand Down Expand Up @@ -223,3 +304,31 @@ func getArtifactsDirectoryPrefix(artifactScanner *ArtifactScanner, prowJobURL, j

return artifactDirectoryPrefix, nil
}

func getParentStepName(fullArtifactName, artifactDirectoryPrefix string) (string, error) {
// => e.g. [ "", "redhat-appstudio-e2e/artifacts/e2e-report.xml" ]
sp := strings.Split(fullArtifactName, artifactDirectoryPrefix)
if len(sp) != 2 {
return "", fmt.Errorf("cannot determine filepath - object name: %s, object prefix: %s", fullArtifactName, artifactDirectoryPrefix)
}
parentStepFilePath := sp[1]

// => e.g. [ "redhat-appstudio-e2e", "artifacts", "e2e-report.xml" ]
sp = strings.Split(parentStepFilePath, "/")
parentStepName := sp[0]

return parentStepName, nil
}

func getFileName(fullArtifactName, artifactDirectoryPrefix string) (string, error) {
sp := strings.Split(fullArtifactName, artifactDirectoryPrefix)
if len(sp) != 2 {
return "", fmt.Errorf("cannot determine filepath - object name: %s, object prefix: %s", fullArtifactName, artifactDirectoryPrefix)
}
parentStepFilePath := sp[1]

sp = strings.Split(parentStepFilePath, "/")
fileName := sp[len(sp)-1]

return fileName, nil
}

0 comments on commit 142df51

Please sign in to comment.