Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(3125): merge parent build meta order by end time #481

Merged
merged 9 commits into from
Nov 23, 2024
Merged

fix(3125): merge parent build meta order by end time #481

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6eceb96
feat: merge parent build meta by end time order
y-oksaku Nov 14, 2024
8ab8d74
feat: add multi parent build meta test
y-oksaku Nov 14, 2024
98039b7
fix: endTime
y-oksaku Nov 14, 2024
7ef8a2a
fix: indent
y-oksaku Nov 14, 2024
70c9e08
Merge branch 'master' into fix-parent-build-meta-priority
y-oksaku Nov 14, 2024
901c421
fix: review
y-oksaku Nov 19, 2024
631632d
fix: review
y-oksaku Nov 21, 2024
c08e161
Merge branch 'master' into fix-parent-build-meta-priority
y-oksaku Nov 21, 2024
d8a5428
Merge branch 'master' into fix-parent-build-meta-priority
kumada626 Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 62 additions & 50 deletions launch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"path/filepath"
"regexp"
"runtime/debug"
"sort"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -293,53 +294,71 @@ func writeMetafile(metaSpace, metaFile, metaLog string, mergedMeta map[string]in
return nil
}

// SetExternalMeta checks if parent build is external and sets meta in external file accordingly
func SetExternalMeta(api screwdriver.API, pipelineID, parentBuildID int, mergedMeta map[string]interface{}, metaSpace, metaLog string, join bool) (map[string]interface{}, error) {
// SetParentBuildMeta checks if parent build is external and sets meta in external file accordingly
func SetParentBuildMeta(api screwdriver.API, pipelineID int, parentBuildIds []int, mergedMeta map[string]interface{}, metaSpace, metaLog string) (map[string]interface{}, error) {
var resultMeta = mergedMeta
log.Printf("Fetching Parent Build %d", parentBuildID)
parentBuild, err := api.BuildFromID(parentBuildID)
if err != nil {
return resultMeta, fmt.Errorf("Fetching Parent Build ID %d: %v", parentBuildID, err)
var join = len(parentBuildIds) > 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var join = len(parentBuildIds) > 1
var isJoin = len(parentBuildIds) > 1


parentBuilds := []screwdriver.Build{}

for _, parentBuildId := range parentBuildIds {
log.Printf("Fetching Parent Build %d", parentBuildId)

parentBuild, err := api.BuildFromID(parentBuildId)

if err != nil {
return resultMeta, fmt.Errorf("Fetching Parent Build ID %d: %v", parentBuildId, err)
}

parentBuilds = append(parentBuilds, parentBuild)
}

log.Printf("Fetching Parent Job %d", parentBuild.JobID)
parentJob, err := api.JobFromID(parentBuild.JobID)
if err != nil {
return resultMeta, fmt.Errorf("Fetching Job ID %d: %v", parentBuild.JobID, err)
}

if parentBuild.Meta != nil {
// Check if build is from external pipeline
if pipelineID != parentJob.PipelineID {
// Write to "sd@123:component.json", where sd@123:component is the triggering job
externalMetaFile := "sd@" + strconv.Itoa(parentJob.PipelineID) + ":" + parentJob.Name + ".json"
writeMetafile(metaSpace, externalMetaFile, metaLog, parentBuild.Meta)
if join {
marshallValue, err := json.Marshal(parentBuild.Meta)
if err != nil {
return resultMeta, fmt.Errorf("Cloning meta of Parent Build ID %d: %v", parentBuildID, err)
}
var externalParentBuildMeta map[string]interface{}
json.Unmarshal(marshallValue, &externalParentBuildMeta)
// Sort by Endtime ASC
// Last finished parent build is priority
sort.SliceStable(parentBuilds, func(i, j int) bool {
return parentBuilds[i].Endtime.Before(parentBuilds[j].Endtime)
})

// Always exclude parameters from external meta
delete(externalParentBuildMeta, "parameters")
for _, parentBuild := range parentBuilds {
log.Printf("Fetching Parent Job %d", parentBuild.JobID)
parentJob, err := api.JobFromID(parentBuild.JobID)
if err != nil {
return resultMeta, fmt.Errorf("Fetching Job ID %d: %v", parentBuild.JobID, err)
}

resultMeta = deepMergeJSON(resultMeta, externalParentBuildMeta)
}
if parentBuild.Meta != nil {
// Check if build is from external pipeline
if pipelineID != parentJob.PipelineID {
// Write to "sd@123:component.json", where sd@123:component is the triggering job
externalMetaFile := "sd@" + strconv.Itoa(parentJob.PipelineID) + ":" + parentJob.Name + ".json"
writeMetafile(metaSpace, externalMetaFile, metaLog, parentBuild.Meta)
if join {
marshallValue, err := json.Marshal(parentBuild.Meta)
if err != nil {
return resultMeta, fmt.Errorf("Cloning meta of Parent Build ID %d: %v", parentBuild.ID, err)
}
var externalParentBuildMeta map[string]interface{}
json.Unmarshal(marshallValue, &externalParentBuildMeta)

// Always exclude parameters from external meta
delete(externalParentBuildMeta, "parameters")

// delete local version of external meta
pIDString := strconv.Itoa(parentJob.PipelineID)
pjn := parentJob.Name
if sdMeta, ok := resultMeta["sd"]; ok {
if externalPipelineMeta, ok := sdMeta.(map[string]interface{})[pIDString]; ok {
if _, ok := externalPipelineMeta.(map[string]interface{})[pjn]; ok {
delete(externalPipelineMeta.(map[string]interface{}), pjn)
resultMeta = deepMergeJSON(resultMeta, externalParentBuildMeta)
}

// delete local version of external meta
pIDString := strconv.Itoa(parentJob.PipelineID)
pjn := parentJob.Name
if sdMeta, ok := resultMeta["sd"]; ok {
if externalPipelineMeta, ok := sdMeta.(map[string]interface{})[pIDString]; ok {
if _, ok := externalPipelineMeta.(map[string]interface{})[pjn]; ok {
delete(externalPipelineMeta.(map[string]interface{}), pjn)
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please separate this logic to a different function, for example:

func handleExternalPipelineMeta(api screwdriver.API, parentBuild screwdriver.Build, parentJob screwdriver.Job, resultMeta map[string]interface{}, metaSpace, metaLog string, isJoin bool) (map[string]interface{}, error) {
    externalMetaFile := ExternalMetaFilePrefix + strconv.Itoa(parentJob.PipelineID) + ":" + parentJob.Name + ExternalMetaFileSuffix
    writeMetafile(metaSpace, externalMetaFile, metaLog, parentBuild.Meta)

    if isJoin {
        marshallValue, err := json.Marshal(parentBuild.Meta)
        if err != nil {
            return resultMeta, fmt.Errorf("Cloning meta of Parent Build ID %d: %v", parentBuild.ID, err)
        }
        var externalParentBuildMeta map[string]interface{}
        json.Unmarshal(marshallValue, &externalParentBuildMeta)
        delete(externalParentBuildMeta, "parameters")
        resultMeta = deepMergeJSON(resultMeta, externalParentBuildMeta)
    }

    pIDString := strconv.Itoa(parentJob.PipelineID)
    pjn := parentJob.Name
    if sdMeta, ok := resultMeta["sd"]; ok {
        if externalPipelineMeta, ok := sdMeta.(map[string]interface{})[pIDString]; ok {
            if _, ok := externalPipelineMeta.(map[string]interface{})[pjn]; ok {
                delete(externalPipelineMeta.(map[string]interface{}), pjn)
            }
        }
    }

    return resultMeta, nil
}

this will help the code to be more readable.

Copy link
Contributor Author

@y-oksaku y-oksaku Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion. I fixed it 🙇

} else {
resultMeta = deepMergeJSON(resultMeta, parentBuild.Meta)
}
} else {
resultMeta = deepMergeJSON(resultMeta, parentBuild.Meta)
}
}

Expand Down Expand Up @@ -499,23 +518,16 @@ func launch(api screwdriver.API, buildID int, rootDir, emitterPath, metaSpace, s
}
}

if len(parentBuildIDs) > 1 { // If has multiple parent build IDs, merge their metadata (join case)
// If has parent build IDs, merge their metadata
if len(parentBuildIDs) > 0 {
// Get meta from all parent builds
for _, pbID := range parentBuildIDs {
mergedMeta, err = SetExternalMeta(api, pipeline.ID, pbID, mergedMeta, metaSpace, metaLog, true)
if err != nil {
return fmt.Errorf("Setting meta for Parent Build ID %d: %v", pbID, err), "", ""
}
}
mergedMeta, err = SetParentBuildMeta(api, pipeline.ID, parentBuildIDs, mergedMeta, metaSpace, metaLog)

metaLog = fmt.Sprintf(`Builds(%v)`, parentBuildIDs)
} else if len(parentBuildIDs) == 1 { // If has parent build, fetch from parent build
mergedMeta, err = SetExternalMeta(api, pipeline.ID, parentBuildIDs[0], mergedMeta, metaSpace, metaLog, false)
if err != nil {
return fmt.Errorf("Setting meta for Parent Build ID %d: %v", parentBuildIDs[0], err), "", ""
return fmt.Errorf("Setting meta for Parent Build ID %d: %v", parentBuildIDs, err), "", ""
}

metaLog = fmt.Sprintf(`Build(%v)`, parentBuildIDs[0])
metaLog = fmt.Sprintf(`Builds(%v)`, parentBuildIDs)
}

// Initialize pr comments (Issue #1858)
Expand Down
Loading