Skip to content

Commit

Permalink
[RB] Fix fetching remote build outputs (#7770)
Browse files Browse the repository at this point in the history
Remote bazel can be used to build a target on a remote runner, and then
the CLI will fetch the build outputs to be used locally.

A couple things were broken:
* The CLI uses certain build events to identify and fetch build outputs,
but [this PR](#4490)
moved target-level events from the `Event` field to `TargetGroups`. This
PR updates the CLI to use the new field
* Verifies that the build uses `--remote_upload_local_results` so that
remote build outputs can be fetched from the cache
* Command parsing failed if there were startup commands
* The error code returned when we build remotely and run locally
  • Loading branch information
maggie-lou authored Oct 28, 2024
1 parent 095a257 commit 089a79a
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 70 deletions.
1 change: 1 addition & 0 deletions cli/remotebazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ go_library(
"//proto:invocation_go_proto",
"//proto:remote_execution_go_proto",
"//proto:runner_go_proto",
"//proto/api/v1:common_go_proto",
"//server/cache/dirtools",
"//server/environment",
"//server/real_environment",
Expand Down
55 changes: 29 additions & 26 deletions cli/remotebazel/remotebazel.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"golang.org/x/sys/unix"
"google.golang.org/grpc/metadata"

cmnpb "github.com/buildbuddy-io/buildbuddy/proto/api/v1/common"
bespb "github.com/buildbuddy-io/buildbuddy/proto/build_event_stream"
bbspb "github.com/buildbuddy-io/buildbuddy/proto/buildbuddy_service"
elpb "github.com/buildbuddy-io/buildbuddy/proto/eventlog"
Expand All @@ -55,7 +56,7 @@ import (
)

const (
buildBuddyArtifactDir = "bb-out"
BuildBuddyArtifactDir = "bb-out"

escapeSeq = "\u001B["
gitConfigSection = "buildbuddy"
Expand Down Expand Up @@ -437,7 +438,7 @@ func generatePatches(baseCommit string) ([][]byte, error) {
untrackedFiles = strings.Trim(untrackedFiles, "\n")
if untrackedFiles != "" {
for _, uf := range strings.Split(untrackedFiles, "\n") {
if strings.HasPrefix(uf, buildBuddyArtifactDir+"/") {
if strings.HasPrefix(uf, BuildBuddyArtifactDir+"/") {
continue
}
patch, err := diffUntrackedFile(uf)
Expand Down Expand Up @@ -588,28 +589,21 @@ func lookupBazelInvocationOutputs(ctx context.Context, bbClient bbspb.BuildBuddy
return nil, fmt.Errorf("could not retrieve invocation %q: %s", invocationID, err)
}

fileSets := make(map[string][]*bespb.File)
outputFileSetNames := make(map[string]struct{})
for _, e := range childInRsp.GetInvocation()[0].GetEvent() {
switch t := e.GetBuildEvent().GetPayload().(type) {
case *bespb.BuildEvent_NamedSetOfFiles:
fileSets[e.GetBuildEvent().GetId().GetNamedSet().GetId()] = t.NamedSetOfFiles.GetFiles()
case *bespb.BuildEvent_Completed:
for _, og := range t.Completed.GetOutputGroup() {
for _, fs := range og.GetFileSets() {
outputFileSetNames[fs.GetId()] = struct{}{}
}
}
}
if len(childInRsp.GetInvocation()) < 1 {
return nil, fmt.Errorf("invocation %s not found", invocationID)
}
inv := childInRsp.GetInvocation()[0]

var outputs []*bespb.File
for fsID := range outputFileSetNames {
fs, ok := fileSets[fsID]
if !ok {
return nil, fmt.Errorf("could not find file set with ID %q while fetching outputs", fsID)
for _, g := range inv.TargetGroups {
// The `GetTarget` API only fetches file data for the general
// STATUS_UNSPECIFIED status. For other statuses, it only returns metadata.
if g.Status != cmnpb.Status_STATUS_UNSPECIFIED {
continue
}
for _, t := range g.Targets {
outputs = append(outputs, t.Files...)
}
outputs = append(outputs, fs...)
}

return outputs, nil
Expand Down Expand Up @@ -639,7 +633,7 @@ func downloadOutputs(ctx context.Context, env environment.Env, mainOutputs []*be
if err != nil {
return "", nil
}
outFile := filepath.Join(outputBaseDir, buildBuddyArtifactDir)
outFile := filepath.Join(outputBaseDir, BuildBuddyArtifactDir)
for _, p := range f.GetPathPrefix() {
outFile = filepath.Join(outFile, p)
}
Expand Down Expand Up @@ -671,7 +665,7 @@ func downloadOutputs(ctx context.Context, env environment.Env, mainOutputs []*be
if err := cachetools.GetBlobAsProto(ctx, bsClient, rn, tree); err != nil {
return nil, err
}
outDir := filepath.Join(outputBaseDir, buildBuddyArtifactDir, d.GetName())
outDir := filepath.Join(outputBaseDir, BuildBuddyArtifactDir, d.GetName())
if err := os.MkdirAll(outDir, 0755); err != nil {
return nil, err
}
Expand Down Expand Up @@ -725,9 +719,10 @@ func Run(ctx context.Context, opts RunOpts, repoConfig *RepoConfig) (int, error)
}
fetchOutputs := false
runOutput := false
if len(bazelArgs) > 0 && (bazelArgs[0] == "build" || (bazelArgs[0] == "run" && !*runRemotely)) {
bazelCmd, _ := parser.GetBazelCommandAndIndex(bazelArgs)
if bazelCmd == "build" || (bazelCmd == "run" && !*runRemotely) {
fetchOutputs = true
if bazelArgs[0] == "run" {
if bazelCmd == "run" {
runOutput = true
}
}
Expand Down Expand Up @@ -940,14 +935,16 @@ func Run(ctx context.Context, opts RunOpts, repoConfig *RepoConfig) (int, error)
execArgs = append(execArgs, arg.GetExecutableArgs(opts.Args)...)
log.Debugf("Executing %q with arguments %s", binPath, execArgs)
cmd := exec.CommandContext(ctx, binPath, execArgs...)
cmd.Dir = filepath.Join(outputsBaseDir, buildBuddyArtifactDir, runfilesRoot)
cmd.Dir = filepath.Join(outputsBaseDir, BuildBuddyArtifactDir, runfilesRoot)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
err = cmd.Run()
if e, ok := err.(*exec.ExitError); ok {
return e.ExitCode(), nil
} else if err != nil {
return 1, err
}
return 1, err
return 0, nil
}
} else {
log.Warnf("Cannot download outputs - no child invocations found")
Expand Down Expand Up @@ -1046,6 +1043,12 @@ func parseArgs(commandLineArgs []string) (bazelArgs []string, execArgs []string,
bazelArgs = append(bazelArgs, "--config=buildbuddy_bes_results_url")
bazelArgs = append(bazelArgs, "--config=buildbuddy_remote_cache")

// If the CLI needs to fetch build outputs, make sure the remote runner uploads them.
bazelCmd, _ := parser.GetBazelCommandAndIndex(bazelArgs)
if (!*runRemotely && bazelCmd == "run") || bazelCmd == "build" {
bazelArgs = append(bazelArgs, "--remote_upload_local_results")
}

return bazelArgs, execArgs, nil
}

Expand Down
150 changes: 106 additions & 44 deletions enterprise/server/test/integration/remote_bazel/remote_bazel_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package remote_bazel_test

import (
"bytes"
"context"
"fmt"
"math"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -87,6 +90,33 @@ func waitForInvocationStatus(t *testing.T, ctx context.Context, bb bbspb.BuildBu
require.FailNowf(t, "timeout", "Timed out waiting for invocation to reach expected status %v", expectedStatus)
}

func clonePrivateTestRepo(t *testing.T) {
repoName := "private-test-repo"
// If you need to re-generate this PAT, it should only have read access to
// `private-test-repo`, and should be saved as a BB secret in all environments.
username := "maggie-lou"
personalAccessToken := os.Getenv("PRIVATE_TEST_REPO_GIT_ACCESS_TOKEN")
repoURLWithToken := fmt.Sprintf("https://%s:%[email protected]/buildbuddy-io/private-test-repo.git", username, personalAccessToken)

// Use a dir that is persisted on recycled runners
rootDir := "/root/workspace/remote-bazel-integration-test"
err := os.Setenv("HOME", rootDir)
require.NoError(t, err)

err = os.MkdirAll(rootDir, 0755)
require.NoError(t, err)

if _, err := os.Stat(fmt.Sprintf("%s/%s", rootDir, repoName)); os.IsNotExist(err) {
output := testshell.Run(t, rootDir, fmt.Sprintf("git clone %s --filter=blob:none --depth=1", repoURLWithToken))
require.NotContains(t, output, "fatal")
}

repoDir := fmt.Sprintf("%s/%s", rootDir, repoName)
err = os.Chdir(repoDir)
require.NoError(t, err)
testshell.Run(t, repoDir, "git pull")
}

func TestWithPublicRepo(t *testing.T) {
// Use a dir that is persisted on recycled runners
rootDir := "/root/workspace/remote-bazel-integration-test"
Expand Down Expand Up @@ -154,36 +184,17 @@ func TestWithPublicRepo(t *testing.T) {
}

func TestWithPrivateRepo(t *testing.T) {
repoName := "private-test-repo"
// If you need to re-generate this PAT, it should only have read access to
// `private-test-repo`, and should be saved as a BB secret in all environments.
username := "maggie-lou"
personalAccessToken := os.Getenv("PRIVATE_TEST_REPO_GIT_ACCESS_TOKEN")
repoURLWithToken := fmt.Sprintf("https://%s:%[email protected]/buildbuddy-io/private-test-repo.git", username, personalAccessToken)
clonePrivateTestRepo(t)

// Use a dir that is persisted on recycled runners
rootDir := "/root/workspace/remote-bazel-integration-test"
err := os.Setenv("HOME", rootDir)
require.NoError(t, err)

err = os.MkdirAll(rootDir, 0755)
require.NoError(t, err)

if _, err := os.Stat(fmt.Sprintf("%s/%s", rootDir, repoName)); os.IsNotExist(err) {
output := testshell.Run(t, rootDir, fmt.Sprintf("git clone %s --filter=blob:none --depth=1", repoURLWithToken))
require.NotContains(t, output, "fatal")
}

err = os.Chdir(fmt.Sprintf("%s/%s", rootDir, repoName))
require.NoError(t, err)
personalAccessToken := os.Getenv("PRIVATE_TEST_REPO_GIT_ACCESS_TOKEN")

// Run a server and executor locally to run remote bazel against
env, bbServer, _ := runLocalServerAndExecutor(t, personalAccessToken)

// Create a workflow for the same repo - will be used to fetch the git token
dbh := env.GetDBHandle()
require.NotNil(t, dbh)
err = dbh.NewQuery(context.Background(), "create_git_repo_for_test").Create(&tables.GitRepository{
err := dbh.NewQuery(context.Background(), "create_git_repo_for_test").Create(&tables.GitRepository{
RepoURL: "https://github.com/buildbuddy-io/private-test-repo",
GroupID: env.GroupID1,
})
Expand Down Expand Up @@ -265,28 +276,9 @@ func runLocalServerAndExecutor(t *testing.T, githubToken string) (*rbetest.Env,
}

func TestCancel(t *testing.T) {
repoName := "private-test-repo"
// If you need to re-generate this PAT, it should only have read access to
// `private-test-repo`, and should be saved as a BB secret in all environments.
username := "maggie-lou"
personalAccessToken := os.Getenv("PRIVATE_TEST_REPO_GIT_ACCESS_TOKEN")
repoURLWithToken := fmt.Sprintf("https://%s:%[email protected]/buildbuddy-io/private-test-repo.git", username, personalAccessToken)

// Use a dir that is persisted on recycled runners
rootDir := "/root/workspace/remote-bazel-integration-test"
err := os.Setenv("HOME", rootDir)
require.NoError(t, err)

err = os.MkdirAll(rootDir, 0755)
require.NoError(t, err)
clonePrivateTestRepo(t)

if _, err := os.Stat(fmt.Sprintf("%s/%s", rootDir, repoName)); os.IsNotExist(err) {
output := testshell.Run(t, rootDir, fmt.Sprintf("git clone %s --filter=blob:none --depth=1", repoURLWithToken))
require.NotContains(t, output, "fatal")
}

err = os.Chdir(fmt.Sprintf("%s/%s", rootDir, repoName))
require.NoError(t, err)
personalAccessToken := os.Getenv("PRIVATE_TEST_REPO_GIT_ACCESS_TOKEN")

// Run a server and executor locally to run remote bazel against
env, bbServer, _ := runLocalServerAndExecutor(t, personalAccessToken)
Expand All @@ -299,7 +291,7 @@ func TestCancel(t *testing.T) {
// Create a workflow for the same repo - will be used to fetch the git token
dbh := env.GetDBHandle()
require.NotNil(t, dbh)
err = dbh.NewQuery(context.Background(), "create_git_repo_for_test").Create(&tables.GitRepository{
err := dbh.NewQuery(context.Background(), "create_git_repo_for_test").Create(&tables.GitRepository{
RepoURL: "https://github.com/buildbuddy-io/private-test-repo",
GroupID: env.GroupID1,
})
Expand Down Expand Up @@ -366,3 +358,73 @@ func TestCancel(t *testing.T) {

waitForInvocationStatus(t, ctx, bbClient, reqCtx, invocationID, inspb.InvocationStatus_DISCONNECTED_INVOCATION_STATUS)
}

func TestFetchRemoteBuildOutputs(t *testing.T) {
clonePrivateTestRepo(t)

// Run a server and executor locally to run remote bazel against
personalAccessToken := os.Getenv("PRIVATE_TEST_REPO_GIT_ACCESS_TOKEN")
env, bbServer, _ := runLocalServerAndExecutor(t, personalAccessToken)

// Create a workflow for the same repo - will be used to fetch the git token
dbh := env.GetDBHandle()
require.NotNil(t, dbh)
err := dbh.NewQuery(context.Background(), "create_git_repo_for_test").Create(&tables.GitRepository{
RepoURL: "https://github.com/buildbuddy-io/private-test-repo",
GroupID: env.GroupID1,
})
require.NoError(t, err)

// Run remote bazel
randomStr := time.Now().String()
exitCode, err := remotebazel.HandleRemoteBazel([]string{
fmt.Sprintf("--remote_runner=%s", bbServer.GRPCAddress()),
// Have the ci runner use the "none" isolation type because it's simpler
// to setup than a firecracker runner
"--runner_exec_properties=workload-isolation-type=none",
"--runner_exec_properties=container-image=",
// Ensure the build is happening on a clean runner, because if the build
// artifact is locally cached, we won't upload it to the remote cache
// and we won't be able to fetch it.
"--runner_exec_properties=instance_name=" + randomStr,
// Pass a startup flag to test parsing
"--digest_function=BLAKE3",
"build",
":hello_world_go",
fmt.Sprintf("--remote_header=x-buildbuddy-api-key=%s", env.APIKey1)})
require.NoError(t, err)
require.Equal(t, 0, exitCode)

// Check that the remote build output was fetched locally.
// The outputs will be downloaded to a directory that may change with the platform,
// so recursively search for the build output named `hello_world_go`.
findFile := func(rootDir, targetFile string) (string, error) {
var outputPath string
err := filepath.WalkDir(rootDir, func(path string, d os.DirEntry, err error) error {
if err != nil {
return err
}

if !d.IsDir() && d.Name() == targetFile {
outputPath = path
return filepath.SkipAll // Stop searching further once the file is found
}

return nil
})
return outputPath, err
}
downloadedOutputPath, err := findFile(remotebazel.BuildBuddyArtifactDir, "hello_world_go")
require.NoError(t, err)

// Make sure we can successfully run the fetched binary.
err = os.Chmod(downloadedOutputPath, 0755)
require.NoError(t, err)

var buf bytes.Buffer
cmd := exec.Command(downloadedOutputPath)
cmd.Stdout = &buf
err = cmd.Run()
require.NoError(t, err)
require.Equal(t, "Hello! I'm a go program.\n", buf.String())
}

0 comments on commit 089a79a

Please sign in to comment.