Skip to content

Commit

Permalink
Set a ceiling on the number of extra files to attach.
Browse files Browse the repository at this point in the history 
Otherwise the server will reject invocations describing large jobs (which have tens of thousands of files).
  • Loading branch information
@fejta
fejta committed Mar 6, 2020
1 parent 0c48097 commit 10252eb
Show file tree
Hide file tree
Showing 4 changed files with 293 additions and 104 deletions.
20 changes: 19 additions & 1 deletion experiment/resultstore/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func convertSuiteMeta(suiteMeta gcs.SuitesMeta) resultstore.Suite {
}

// Convert converts build metadata stored in gcp into the corresponding ResultStore Invocation, Target and Test.
func convert(project, details string, url gcs.Path, result downloadResult) (resultstore.Invocation, resultstore.Target, resultstore.Test) {
func convert(project, details string, url gcs.Path, result downloadResult, maxFiles int) (resultstore.Invocation, resultstore.Target, resultstore.Test) {
started := result.started
finished := result.finished
artifacts := result.artifactURLs
Expand Down Expand Up @@ -175,13 +175,18 @@ func convert(project, details string, url gcs.Path, result downloadResult) (resu
artifacts[i] = "gs://" + bucket + "/" + a
}

var total int
for _, a := range artifacts { // add started.json, etc to the invocation artifact list.
if total >= maxFiles {
continue
}
if strings.HasPrefix(a, artifactsPath) {
continue // things under artifacts/ are owned by the test
}
if a == buildLog {
continue // Handle this in InvocationLog
}
total++
inv.Files = append(inv.Files, resultstore.File{
ID: uniqPath(a),
ContentType: "text/plain",
Expand Down Expand Up @@ -237,6 +242,9 @@ func convert(project, details string, url gcs.Path, result downloadResult) (resu
}

for _, a := range artifacts {
if total >= maxFiles {
continue
}
if !strings.HasPrefix(a, artifactsPath) {
continue // Non-artifacts (started.json, etc) are owned by the invocation
}
Expand All @@ -254,13 +262,23 @@ func convert(project, details string, url gcs.Path, result downloadResult) (resu
if found {
continue
}
total++
test.Suite.Files = append(test.Suite.Files, resultstore.File{
ID: uniqPath(a),
ContentType: "text/plain",
URL: a,
})
}

if total >= maxFiles {
// TODO(fejta): expose this to edge case to user in a better way
inv.Files = append(inv.Files, resultstore.File{
ID: fmt.Sprintf("exceeded %d files", maxFiles),
ContentType: "text/plain",
URL: basePath,
})
}

test.Suite.Start = inv.Start
test.Action.Start = inv.Start
test.Suite.Duration = inv.Duration
Expand Down
193 changes: 192 additions & 1 deletion experiment/resultstore/convert_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func TestConvertProjectMetadataToResultStoreArtifacts(t *testing.T) {
details string
url string
result downloadResult
maxFiles int
expectedInvocation resultstore.Invocation
expectedTarget resultstore.Target
expectedTest resultstore.Test
Expand Down Expand Up @@ -396,6 +397,193 @@ func TestConvertProjectMetadataToResultStoreArtifacts(t *testing.T) {
},
},
},
{
name: "Reject excessive artifacts",
maxFiles: 3,
project: "projectX",
details: "detailY",
url: "gs://bucket/logs/jobA/1234567890123456789",
result: downloadResult{
started: gcs.Started{
Started: metadata.Started{
Timestamp: 1234567890,
Repos: map[string]string{
"org/repoA": "branchB",
},
DeprecatedRepoVersion: "aadb2b88d190a38b59f512b4d8c508a88cf839e1",
},
Pending: false,
},
finished: gcs.Finished{
Finished: metadata.Finished{
Result: "SUCCESS",
DeprecatedRevision: "master",
},
Running: false,
},
artifactURLs: []string{
"logs/jobA/1234567890123456789/artifacts/junit_runner.xml",
"logs/jobA/1234567890123456789/artifacts/1",
"logs/jobA/1234567890123456789/artifacts/2",
"logs/jobA/1234567890123456789/artifacts/3",
"logs/jobA/1234567890123456789/artifacts/4",
"logs/jobA/1234567890123456789/artifacts/5",
"logs/jobA/1234567890123456789/build-log.txt",
},
suiteMetas: []gcs.SuitesMeta{
{
Suites: junit.Suites{
XMLName: xml.Name{},
Suites: []junit.Suite{
{
XMLName: xml.Name{Space: "testsuite", Local: ""},
Time: 10.5,
Failures: 0,
Tests: 2,
Results: []junit.Result{
{
Name: "Result1",
Time: 3.2,
ClassName: "test1",
Properties: &junit.Properties{
PropertyList: []junit.Property{
{Name: "p1", Value: "v1"},
},
},
},
{
Name: "Result2",
Time: 7.3,
ClassName: "test2",
Properties: &junit.Properties{
PropertyList: []junit.Property{
{Name: "p2", Value: "v2"},
},
},
},
},
},
},
},
Metadata: map[string]string{
"Context": "runner",
},
Path: "gs://bucket/logs/jobA/1234567890123456789/artifacts/junit_runner.xml",
},
},
},
expectedInvocation: resultstore.Invocation{
Project: "projectX",
Details: "detailY",
Files: []resultstore.File{
{
ID: resultstore.InvocationLog,
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/build-log.txt",
},
{
ID: "exceeded 3 files",
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/",
},
},
Properties: []resultstore.Property{
{Key: "Job", Value: "jobA"},
{Key: "Pull", Value: ""},
{Key: "Org", Value: "org"},
{Key: "Branch", Value: "branchB"},
{Key: "Repo", Value: "repoA"},
{Key: "Repo", Value: "org/repoA"},
{Key: "Repo", Value: "org/repoA:branchB"},
},
Start: time.Unix(1234567890, 0),
Status: resultstore.Running,
Description: "In progress...",
},
expectedTarget: resultstore.Target{
Status: resultstore.Running,
Description: "In progress...",
Start: time.Unix(1234567890, 0),
Properties: []resultstore.Property{
{Key: "Result1:p1", Value: "v1"},
{Key: "Result2:p2", Value: "v2"},
},
},
expectedTest: resultstore.Test{
Suite: resultstore.Suite{
Name: "test",
Start: time.Unix(1234567890, 0),
Files: []resultstore.File{
{
ID: resultstore.TargetLog,
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/build-log.txt",
},
{
ID: "artifacts/junit_runner.xml",
ContentType: "text/xml",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/junit_runner.xml",
},
{
ID: "artifacts/1",
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/1",
},
{
ID: "artifacts/2",
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/2",
},
{
ID: "artifacts/3",
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/3",
},
},
Suites: []resultstore.Suite{
{
Name: "junit_runner.xml",
Duration: dur(10.5),
Files: []resultstore.File{
{
ID: "junit_runner.xml",
ContentType: "text/xml",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/junit_runner.xml",
},
},
Suites: []resultstore.Suite{
{
Cases: []resultstore.Case{
{
Name: "Result1",
Class: "test1",
Result: resultstore.Completed,
Duration: dur(3.2),
},
{
Name: "Result2",
Class: "test2",
Result: resultstore.Completed,
Duration: dur(7.3),
},
},
Duration: dur(10.5),
Properties: []resultstore.Property{
{Key: "Result1:p1", Value: "v1"},
{Key: "Result2:p2", Value: "v2"},
},
},
},
},
},
},
Action: resultstore.Action{
Start: time.Unix(1234567890, 0),
Status: resultstore.Running,
Description: "In progress...",
},
},
},
{
name: "Convert full project metadata",
project: "projectX",
Expand Down Expand Up @@ -566,7 +754,10 @@ func TestConvertProjectMetadataToResultStoreArtifacts(t *testing.T) {
if err != nil {
t.Errorf("incorrect url: %v", err)
}
invocation, target, test := convert(tc.project, tc.details, *urlPath, tc.result)
if tc.maxFiles == 0 {
tc.maxFiles = 40000
}
invocation, target, test := convert(tc.project, tc.details, *urlPath, tc.result, tc.maxFiles)
if diff := cmp.Diff(invocation, tc.expectedInvocation, cmpOption); diff != "" {
t.Errorf("%s:%s mismatch (-got +want):\n%s", tc.name, "invocation", diff)
}
Expand Down
Loading

0 comments on commit 10252eb

Please sign in to comment.