Skip to content

Commit

Permalink
api/GetAction: add handling for failed build action
Browse files Browse the repository at this point in the history 
When an invocation failed because of a non-test(aka build) action, the
TargetCompleted event will not contains the relevant output files.
Relevant stderr/stdout of the failed action can be found in the
ActionCompleted event instead.

Enhance the GetAction rpc logic by only handling successful
TargetCompleted events. For the failed cases, add additional logic to
create Action entries from ActionCompleted events.

Refactor FillActionFromBuildEvent to switch on event id instead of event
payload.
  • Loading branch information
@sluongng
sluongng committed Feb 20, 2025
1 parent b1985fc commit 32060a6
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 31 deletions.
1 change: 1 addition & 0 deletions enterprise/server/api/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ go_test(
"//proto:api_key_go_proto",
"//proto:build_event_stream_go_proto",
"//proto:build_events_go_proto",
"//proto:failure_details_go_proto",
"//proto:publish_build_event_go_proto",
"//proto:resource_go_proto",
"//proto/api/v1:api_v1_go_proto",
Expand Down
54 changes: 51 additions & 3 deletions enterprise/server/api/api_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/buildbuddy-io/buildbuddy/proto/api_key"
"github.com/buildbuddy-io/buildbuddy/proto/build_event_stream"
"github.com/buildbuddy-io/buildbuddy/proto/failure_details"
"github.com/buildbuddy-io/buildbuddy/server/build_event_protocol/build_event_handler"
"github.com/buildbuddy-io/buildbuddy/server/interfaces"
"github.com/buildbuddy-io/buildbuddy/server/tables"
Expand Down Expand Up @@ -157,9 +158,10 @@ func TestGetAction(t *testing.T) {
resp, err := s.GetAction(ctx, &apipb.GetActionRequest{Selector: &apipb.ActionSelector{InvocationId: testInvocationID}})
require.NoError(t, err)
require.NotNil(t, resp)
assert.Equal(t, 3, len(resp.Action))
require.Equal(t, 4, len(resp.GetAction()))
assert.Equal(t, resp.Action[0].File[0].Hash, "5dee5f7b2ecaf0365ae2811ab98cb5ba306e72fb088787e176e3b4afd926a55b")
assert.Equal(t, resp.Action[0].File[0].SizeBytes, int64(152092))
assert.Equal(t, resp.Action[3].File[0].Name, "stderr")
}

func TestGetActionWithTargetID(t *testing.T) {
Expand All @@ -175,7 +177,7 @@ func TestGetActionWithTargetID(t *testing.T) {
resp, err := s.GetAction(ctx, &apipb.GetActionRequest{Selector: &apipb.ActionSelector{InvocationId: testInvocationID, TargetId: testTargetID}})
require.NoError(t, err)
require.NotNil(t, resp)
assert.Equal(t, 1, len(resp.Action))
require.Equal(t, 1, len(resp.Action))
assert.Equal(t, resp.Action[0].File[0].Hash, "5dee5f7b2ecaf0365ae2811ab98cb5ba306e72fb088787e176e3b4afd926a55b")
assert.Equal(t, resp.Action[0].File[0].SizeBytes, int64(152092))
}
Expand Down Expand Up @@ -473,7 +475,10 @@ func streamBuild(t *testing.T, te *testenv.TestEnv, iid string) {
err = channel.HandleEvent(streamRequest(targetCompletedEvent("//my/third/target:foo"), iid, 10))
assert.NoError(t, err)

err = channel.HandleEvent(streamRequest(finishedEvent(), iid, 11))
err = channel.HandleEvent(streamRequest(actionCompleteEvent("//failed:target"), iid, 11))
assert.NoError(t, err)

err = channel.HandleEvent(streamRequest(finishedEvent(), iid, 12))
assert.NoError(t, err)

err = channel.FinalizeInvocation(iid)
Expand Down Expand Up @@ -541,6 +546,7 @@ func targetCompletedEvent(label string) *anypb.Any {
},
Payload: &build_event_stream.BuildEvent_Completed{
Completed: &build_event_stream.TargetComplete{
Success: true,
DirectoryOutput: []*build_event_stream.File{
{
Name: "my-output.txt",
Expand All @@ -555,6 +561,48 @@ func targetCompletedEvent(label string) *anypb.Any {
return progressAny
}

func actionCompleteEvent(label string) *anypb.Any {
actionCompleteEvent := &anypb.Any{}
actionCompleteEvent.MarshalFrom(&build_event_stream.BuildEvent{
Id: &build_event_stream.BuildEventId{
Id: &build_event_stream.BuildEventId_ActionCompleted{
ActionCompleted: &build_event_stream.BuildEventId_ActionCompletedId{
Label: label,
Configuration: &build_event_stream.BuildEventId_ConfigurationId{
Id: "config1",
},
},
},
},
Payload: &build_event_stream.BuildEvent_Action{
Action: &build_event_stream.ActionExecuted{
Type: "actionMnemonic",
ExitCode: 1,
Stderr: &build_event_stream.File{
Name: "stderr",
File: &build_event_stream.File_Uri{
Uri: "bytestream://localhost:8080/buildbuddy-io/buildbuddy/ci/blobs/5dee5f7b2ecaf0365ae2811ab98cb5ba306e72fb088787e176e3b4afd926a55b/152092",
},
},
Label: label,
Configuration: &build_event_stream.BuildEventId_ConfigurationId{
Id: "config1",
},
CommandLine: []string{"exit", "1"},
FailureDetail: &failure_details.FailureDetail{
Message: "action failed",
Category: &failure_details.FailureDetail_Spawn{
Spawn: &failure_details.Spawn{
Code: *failure_details.Spawn_NON_ZERO_EXIT.Enum(),
},
},
},
},
},
})
return actionCompleteEvent
}

func progressEvent(stdout string) *anypb.Any {
progressAny := &anypb.Any{}
progressAny.MarshalFrom(&build_event_stream.BuildEvent{
Expand Down
65 changes: 37 additions & 28 deletions server/api/common/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ func ActionLabelKey(groupID, iid, targetLabel string) string {
func filesFromOutput(output []*bespb.File) []*apipb.File {
files := []*apipb.File{}
for _, output := range output {
if output == nil {
continue
}
uri := ""
switch file := output.File.(type) {
case *bespb.File_Uri:
Expand All @@ -65,43 +68,49 @@ func filesFromOutput(output []*bespb.File) []*apipb.File {
}

func FillActionFromBuildEvent(event *bespb.BuildEvent, action *apipb.Action) *apipb.Action {
switch event.Payload.(type) {
case *bespb.BuildEvent_Completed:
{
action.TargetLabel = event.GetId().GetTargetCompleted().GetLabel()
action.Id.TargetId = event.GetId().GetTargetCompleted().GetLabel()
action.Id.ConfigurationId = event.GetId().GetTargetCompleted().GetConfiguration().GetId()
action.Id.ActionId = EncodeID("build")
return action
}
case *bespb.BuildEvent_TestResult:
{
testResultID := event.GetId().GetTestResult()
action.TargetLabel = event.GetId().GetTestResult().GetLabel()
action.Id.TargetId = event.GetId().GetTestResult().GetLabel()
action.Id.ConfigurationId = event.GetId().GetTestResult().GetConfiguration().Id
action.Id.ActionId = EncodeID(fmt.Sprintf("test-S_%d-R_%d-A_%d", testResultID.Shard, testResultID.Run, testResultID.Attempt))
action.Shard = int64(testResultID.Shard)
action.Run = int64(testResultID.Run)
action.Attempt = int64(testResultID.Attempt)
return action
switch id := event.GetId().GetId().(type) {
case *bespb.BuildEventId_ActionCompleted:
action.TargetLabel = id.ActionCompleted.GetLabel()
action.Id.TargetId = id.ActionCompleted.GetLabel()
action.Id.ConfigurationId = id.ActionCompleted.GetConfiguration().GetId()
action.Id.ActionId = EncodeID("build")
return action
case *bespb.BuildEventId_TargetCompleted:
p := event.GetPayload().(*bespb.BuildEvent_Completed)
if !p.Completed.GetSuccess() {
// Don't create an action for failed targets as they don't have any interesting output files
// We have already captured them via an ActionCompleted events earlier in the stream.
return nil
}
action.TargetLabel = id.TargetCompleted.GetLabel()
action.Id.TargetId = id.TargetCompleted.GetLabel()
action.Id.ConfigurationId = id.TargetCompleted.GetConfiguration().GetId()
action.Id.ActionId = EncodeID("build")
return action
case *bespb.BuildEventId_TestResult:
action.TargetLabel = id.TestResult.GetLabel()
action.Id.TargetId = id.TestResult.GetLabel()
action.Id.ConfigurationId = id.TestResult.GetConfiguration().GetId()
action.Id.ActionId = EncodeID(fmt.Sprintf("test-S_%d-R_%d-A_%d", id.TestResult.GetShard(), id.TestResult.GetRun(), id.TestResult.GetAttempt()))
action.Shard = int64(id.TestResult.GetShard())
action.Run = int64(id.TestResult.GetRun())
action.Attempt = int64(id.TestResult.GetAttempt())
return action
}
return nil
}

func FillActionOutputFilesFromBuildEvent(event *bespb.BuildEvent, action *apipb.Action) *apipb.Action {
switch p := event.Payload.(type) {
case *bespb.BuildEvent_Action:
action.File = filesFromOutput([]*bespb.File{p.Action.GetStderr(), p.Action.GetStdout()})
return action
case *bespb.BuildEvent_Completed:
{
action.File = filesFromOutput(p.Completed.DirectoryOutput)
return action
}
action.File = filesFromOutput(p.Completed.DirectoryOutput)
return action
case *bespb.BuildEvent_TestResult:
{
action.File = filesFromOutput(p.TestResult.TestActionOutput)
return action
}
action.File = filesFromOutput(p.TestResult.TestActionOutput)
return action
}
return nil
}
Expand Down

0 comments on commit 32060a6

Please sign in to comment.