From dafc34ce531c2c440749332c6d81df76c1e4d6d4 Mon Sep 17 00:00:00 2001 From: y-oksaku <43719835+y-oksaku@users.noreply.github.com> Date: Sat, 23 Nov 2024 14:13:40 +0900 Subject: [PATCH] fix(3125): merge parent build meta order by end time (#481) * feat: merge parent build meta by end time order * feat: add multi parent build meta test * fix: endTime * fix: indent * fix: review * fix: review --------- Co-authored-by: Keisuke Kumada --- launch.go | 127 +++++++------ launch_test.go | 311 +++++++++++++++++++++++++++++++- screwdriver/screwdriver.go | 1 + screwdriver/screwdriver_test.go | 1 + 4 files changed, 388 insertions(+), 52 deletions(-) diff --git a/launch.go b/launch.go index e3efbed..019d473 100644 --- a/launch.go +++ b/launch.go @@ -12,6 +12,7 @@ import ( "path/filepath" "regexp" "runtime/debug" + "sort" "strconv" "strings" "time" @@ -293,53 +294,84 @@ 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) { +// setParentBuildsMeta checks if parent build is external and sets meta in external file accordingly +func setParentBuildsMeta(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 isJoin = len(parentBuildIDs) > 1 - 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) + 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) } - if parentBuild.Meta != nil { + // 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) + }) + + for _, parentBuild := range parentBuilds { + if parentBuild.Meta == nil { + continue + } + + 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) + } + // 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) - - // Always exclude parameters from external meta - delete(externalParentBuildMeta, "parameters") - - resultMeta = deepMergeJSON(resultMeta, externalParentBuildMeta) + if pipelineID == parentJob.PipelineID { + resultMeta = deepMergeJSON(resultMeta, parentBuild.Meta) + } else { + resultMeta, err = handleExternalPipelineMeta(parentBuild, parentJob, resultMeta, metaSpace, metaLog, isJoin) + + if err != nil { + return resultMeta, fmt.Errorf("Merging meta of External Parent Build ID %d: %v", parentBuild.ID, err) } + } + } + + return resultMeta, nil +} + +func handleExternalPipelineMeta(parentBuild screwdriver.Build, parentJob screwdriver.Job, resultMeta map[string]interface{}, metaSpace, metaLog string, isJoin bool) (map[string]interface{}, error) { + // 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 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) + + // 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) } - } else { - resultMeta = deepMergeJSON(resultMeta, parentBuild.Meta) } } @@ -499,23 +531,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 = setParentBuildsMeta(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) diff --git a/launch_test.go b/launch_test.go index 1e56c6f..05e1f45 100644 --- a/launch_test.go +++ b/launch_test.go @@ -1332,7 +1332,7 @@ func TestFetchParentBuildMetaWriteError(t *testing.T) { } err, sourceDir, shellBin := launch(screwdriver.API(api), TestBuildID, TestWorkspace, TestEmitter, TestMetaSpace, TestStoreURL, TestUIURL, TestShellBin, TestBuildTimeout, TestBuildToken, "", "", "", "", false, false, false, 0, 10000) - expected := fmt.Sprintf(`Writing Parent Build(%d) Meta JSON: Testing writing parent build meta`, TestParentBuildID) + expected := fmt.Sprintf(`Writing Parent Builds([%d]) Meta JSON: Testing writing parent build meta`, TestParentBuildID) if err.Error() != expected { t.Errorf("Error is wrong, got '%v', expected '%v'", err, expected) @@ -2137,6 +2137,315 @@ func TestMetaWhenTriggeredFromPipelinesByANDLogicWithParentBuildMeta(t *testing. } } +func TestMetaWhenTriggeredFromPipelinesByANDLogicWithMultiParentBuildMeta(t *testing.T) { + initCoverageMeta() + oldWriteFile := writeFile + defer func() { writeFile = oldWriteFile }() + var defaultMeta []byte + + var PipelineID = TestPipelineID + var FirstParentBuildID = 1111 + var FirstParentJobID = 1112 + var SecondParentBuildID = 2222 + var SecondParentJobID = 2223 + var ThirdParentBuildID = 3333 + var ThirdParentJobID = 3334 + ParentBuildIds := []float64{ + float64(SecondParentBuildID), + float64(ThirdParentBuildID), + float64(FirstParentBuildID), + } + + IDs = make([]interface{}, len(ParentBuildIds)) + for i, id := range ParentBuildIds { + IDs[i] = id + } + + buildFromIDMeta := map[string]interface{}{ + "build_only": "build_value", // Remain + "build_and_parent_build": "build_value", // Remain + "parameters": map[string]string{ + "build_only": "build_value", // Remain + "build_and_parent_build": "build_value", // Remain + }, + "meta": map[string]interface{}{ + "summary": map[string]string{ // This should be deleted + "build_only": "build_value", + }, + "build_only": "build_value", // Remain + "build_and_parent_build": "build_value", // Remain + }, + "build": map[string]interface{}{ + "buildId": "build_value", // Overwrote by default value + "jobId": "build_value", // Overwrote by default value + "eventId": "build_value", // Overwrote by default value + "pipelineId": "build_value", // Overwrote by default value + "sha": "build_value", // Overwrote by default value + "jobName": "build_value", // Overwrote by default value + "coverageKey": "build_value", // Overwrote by default value + "build_only": "build_value", // Remain + "build_and_parent_build": "build_value", // Remain + }, + "event": map[string]interface{}{ + "creator": "build_value", // Overwrote by default value + }, + } + + eventFromIDMeta := map[string]interface{}{ + "event_only": "event_value", // Remain + "event_and_parent_build": "event_value", // Overwrote by last parent build + "parameters": map[string]string{ + "event_only": "event_value", // This should be deleted + "event_and_parent_build": "event_value", // This souuld be deleted + }, + "meta": map[string]interface{}{ + "summary": map[string]string{ // This should be deleted + "event_only": "event_value", + }, + "event_only": "event_value", // Remain + "event_and_parent_build": "event_value", // Overwrote by last parent build + }, + } + + firstParentBuildMeta := map[string]interface{}{ + "build_and_parent_build": "first_build_value", // Overwrote by build + "event_and_parent_build": "first_build_value", + "build_1": "first_build_value", + "build_12": "first_build_value", + "build_13": "first_build_value", + "build_123": "first_build_value", + "parameters": map[string]string{ + "build_and_parent_build": "first_build_value", // Overwrote by build + "event_and_parent_build": "first_build_value", + "build_1": "first_build_value", + "build_12": "first_build_value", + "build_13": "first_build_value", + "build_123": "first_build_value", + }, + "meta": map[string]interface{}{ + "summary": map[string]string{ // This should be deleted + "build_1": "first_build_value", + }, + "build_and_parent_build": "first_build_value", // Overwrote by build + "event_and_parent_build": "first_build_value", + "build_1": "first_build_value", + "build_12": "first_build_value", + "build_13": "first_build_value", + "build_123": "first_build_value", + }, + "build": map[string]interface{}{ + "buildId": "first_build_value", // Overwrote by default value + "jobId": "first_build_value", // Overwrote by default value + "eventId": "first_build_value", // Overwrote by default value + "pipelineId": "first_build_value", // Overwrote by default value + "sha": "first_build_value", // Overwrote by default value + "jobName": "first_build_value", // Overwrote by default value + "coverageKey": "first_build_value", // Overwrote by default value + "build_and_parent_build": "first_build_value", + "build_1": "first_build_value", + "build_12": "first_build_value", + "build_13": "first_build_value", + "build_123": "first_build_value", + }, + "event": map[string]interface{}{ + "creator": "first_build_value", // Overwrote by default value + }, + } + + secondParentBuildMeta := map[string]interface{}{ + "build_and_parent_build": "second_build_value", // Overwrote by build + "event_and_parent_build": "second_build_value", + "build_2": "second_build_value", + "build_12": "second_build_value", + "build_23": "second_build_value", + "build_123": "second_build_value", + "parameters": map[string]string{ + "build_and_parent_build": "second_build_value", + "event_and_parent_build": "second_build_value", + "build_2": "second_build_value", + "build_12": "second_build_value", + "build_23": "second_build_value", + "build_123": "second_build_value", + }, + "meta": map[string]interface{}{ + "summary": map[string]string{ // This should be deleted + "build_2": "second_build_value", + }, + "build_and_parent_build": "second_build_value", // Overwrote by build + "event_and_parent_build": "second_build_value", + "build_2": "second_build_value", + "build_12": "second_build_value", + "build_23": "second_build_value", + "build_123": "second_build_value", + }, + "build": map[string]interface{}{ + "buildId": "second_build_value", // Overwrote by default value + "jobId": "second_build_value", // Overwrote by default value + "eventId": "second_build_value", // Overwrote by default value + "pipelineId": "second_build_value", // Overwrote by default value + "sha": "second_build_value", // Overwrote by default value + "jobName": "second_build_value", // Overwrote by default value + "coverageKey": "second_build_value", // Overwrote by default value + "build_and_parent_build": "second_build_value", // Overwrote by build + "build_2": "second_build_value", + "build_12": "second_build_value", + "build_23": "second_build_value", + "build_123": "second_build_value", + }, + "event": map[string]interface{}{ + "creator": "second_build_value", // Overwrote by default value + }, + } + + thirdParentBuildMeta := map[string]interface{}{ + "build_and_parent_build": "third_build_value", // Overwrote by build + "event_and_parent_build": "third_build_value", + "build_3": "third_build_value", + "build_13": "third_build_value", + "build_23": "third_build_value", + "build_123": "third_build_value", + "parameters": map[string]string{ + "build_and_parent_build": "third_build_value", // Overwrote by build + "event_and_parent_build": "third_build_value", + "build_3": "third_build_value", + "build_13": "third_build_value", + "build_23": "third_build_value", + "build_123": "third_build_value", + }, + "meta": map[string]interface{}{ + "summary": map[string]string{ // This should be deleted + "build_3": "third_build_value", + }, + "build_and_parent_build": "third_build_value", // Overwrote by build + "event_and_parent_build": "third_build_value", + "build_3": "third_build_value", + "build_13": "third_build_value", + "build_23": "third_build_value", + "build_123": "third_build_value", + }, + "build": map[string]interface{}{ + "buildId": "third_build_value", // Overwrote by default value + "jobId": "third_build_value", // Overwrote by default value + "eventId": "third_build_value", // Overwrote by default value + "pipelineId": "third_build_value", // Overwrote by default value + "sha": "third_build_value", // Overwrote by default value + "jobName": "third_build_value", // Overwrote by default value + "coverageKey": "third_build_value", // Overwrote by default value + "build_and_parent_build": "third_build_value", // Overwrote by build + "build_3": "third_build_value", + "build_13": "third_build_value", + "build_23": "third_build_value", + "build_123": "third_build_value", + }, + "event": map[string]interface{}{ + "creator": "third_build_value", // Overwrote by default value + }, + } + + oldMarshal := marshal + defer func() { marshal = oldMarshal }() + + api := mockAPI(t, TestBuildID, TestJobID, 0, "RUNNING") + api.buildFromID = func(buildID int) (screwdriver.Build, error) { + if buildID == FirstParentBuildID { + return screwdriver.Build(FakeBuild{ID: buildID, JobID: FirstParentJobID, Meta: firstParentBuildMeta, Endtime: time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)}), nil + } + if buildID == SecondParentBuildID { + return screwdriver.Build(FakeBuild{ID: buildID, JobID: SecondParentJobID, Meta: secondParentBuildMeta, Endtime: time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC)}), nil + } + if buildID == ThirdParentBuildID { + return screwdriver.Build(FakeBuild{ID: buildID, JobID: ThirdParentJobID, Meta: thirdParentBuildMeta, Endtime: time.Date(2024, 1, 3, 0, 0, 0, 0, time.UTC)}), nil + } + // target build + return screwdriver.Build(FakeBuild{ID: TestBuildID, JobID: TestJobID, ParentBuildID: IDs, Meta: buildFromIDMeta}), nil + } + api.jobFromID = func(jobID int) (screwdriver.Job, error) { + if jobID == FirstParentJobID { + return screwdriver.Job(FakeJob{ID: jobID, PipelineID: PipelineID, Name: "first_job"}), nil + } + if jobID == SecondParentJobID { + return screwdriver.Job(FakeJob{ID: jobID, PipelineID: PipelineID, Name: "second_job"}), nil + } + if jobID == ThirdParentJobID { + return screwdriver.Job(FakeJob{ID: jobID, PipelineID: PipelineID, Name: "third_job"}), nil + } + // target build + return screwdriver.Job(FakeJob{ID: jobID, PipelineID: PipelineID, Name: "main"}), nil + } + api.eventFromID = func(eventID int) (screwdriver.Event, error) { + return screwdriver.Event(FakeEvent{ID: TestEventID, ParentEventID: TestParentEventID, Meta: eventFromIDMeta, Creator: TestEventCreator}), nil + } + api.pipelineFromID = func(pipelineID int) (screwdriver.Pipeline, error) { + return screwdriver.Pipeline(FakePipeline{ID: pipelineID, ScmURI: TestScmURI, ScmRepo: TestScmRepo}), nil + } + writeFile = func(path string, data []byte, perm os.FileMode) (err error) { + if path == "./data/meta/meta.json" { + defaultMeta = data + } + return nil + } + + assert.Equal(t, 3, len(ParentBuildIds)) + + err, _, _ := launch(screwdriver.API(api), TestBuildID, TestWorkspace, TestEmitter, TestMetaSpace, TestStoreURL, TestUIURL, TestShellBin, TestBuildTimeout, TestBuildToken, "", "", "", "", false, false, false, 0, 10000) + want := fmt.Sprintf(`{ + "build_1":"first_build_value", + "build_12":"second_build_value", + "build_123":"third_build_value", + "build_13":"third_build_value", + "build_2":"second_build_value", + "build_23":"third_build_value", + "build_3":"third_build_value", + "build_and_parent_build":"third_build_value", + "build_only":"build_value", + "event_and_parent_build":"third_build_value", + "event_only":"event_value", + "build": { + "build_1":"first_build_value", + "build_12":"second_build_value", + "build_123":"third_build_value", + "build_13":"third_build_value", + "build_2":"second_build_value", + "build_23":"third_build_value", + "build_3":"third_build_value", + "build_and_parent_build":"third_build_value", + "build_only":"build_value", + "buildId":"%d", + "jobId":"%d", + "pipelineId":"%d", + "coverageKey":"%s", + "eventId":"0", + "jobName":"main", + "sha":"" + }, + "event":{ + "creator":"%s" + }, + "meta":{ + "build_1":"first_build_value", + "build_12":"second_build_value", + "build_123":"third_build_value", + "build_13":"third_build_value", + "build_2":"second_build_value", + "build_23":"third_build_value", + "build_3":"third_build_value", + "build_and_parent_build":"third_build_value", + "build_only":"build_value", + "event_and_parent_build":"third_build_value", + "event_only":"event_value" + }, + "parameters":{ + "build_and_parent_build":"build_value", + "build_only":"build_value" + } + }`, TestBuildID, TestJobID, TestPipelineID, TestEnvVars["SD_SONAR_PROJECT_KEY"], TestEventCreator["username"]) + assert.JSONEq(t, want, string(defaultMeta)) + + if err != nil { + t.Errorf(fmt.Sprintf("err returned: %s", err.Error())) + } +} + func TestMetaWhenTriggeredFromInnerPipelineByORLogicWithParentBuildMeta(t *testing.T) { initCoverageMeta() oldWriteFile := writeFile diff --git a/screwdriver/screwdriver.go b/screwdriver/screwdriver.go index 3931ce0..7b01d90 100644 --- a/screwdriver/screwdriver.go +++ b/screwdriver/screwdriver.go @@ -183,6 +183,7 @@ type Build struct { Meta map[string]interface{} `json:"meta"` EventID int `json:"eventId"` Createtime string `json:"createTime"` + Endtime time.Time `json:"endTime"` Stats struct { QueueEntertime string `json:"queueEnterTime"` } `json:"stats"` diff --git a/screwdriver/screwdriver_test.go b/screwdriver/screwdriver_test.go index 60ace7b..c5a546e 100644 --- a/screwdriver/screwdriver_test.go +++ b/screwdriver/screwdriver_test.go @@ -140,6 +140,7 @@ func TestBuildFromID(t *testing.T) { Commands: testCmds, Environment: testEnv, Createtime: "2020-04-28T20:34:01.907Z", + Endtime: time.Date(2020, 4, 28, 20, 40, 0, 123, time.UTC), Stats: struct { QueueEntertime string `json:"queueEnterTime"` }{