[WF] Speed up slow tests (#11376)

These remote runner tests are slow because [we upload 2 50MB
binaries](https://github.com/buildbuddy-io/buildbuddy/blob/master/enterprise/server/util/ci_runner_util/ci_runner_util.go#L48)
to the in-memory cache, which is very resource constrained in CI

I disabled this behavior for most tests, and added one test that tests
this behavior. Tests are now running much faster. With `--config=race`:
//enterprise/server/test/integration/remote_bazel:
[156s](https://app.buildbuddy.io/invocation/563922f0-40a7-4e3f-b018-9481cda8de4b)
=>
[70.561s](https://app.buildbuddy.io/invocation/28df438c-cdd7-41fb-8917-1161b63201a1)
//enterprise/server/workflow/service:
[82.7s](https://app.buildbuddy.io/invocation/32e25dc1-000a-454e-b9d5-1a135624801f)
=> [14.7s
](https://app.buildbuddy.io/invocation/62af68fd-7cef-4591-b91a-a3ad6e771a46)
etc...

The new test still runs really slowly
([139s](https://app.buildbuddy.io/invocation/e1413d9c-b096-49a3-a6e3-8e794042f94e)),
but I think it's worth it because this is the prod behavior. In
production I don't expect this to be an issue because the upload should
short circuit if the binary already exists in the cache

Author: maggie-lou

enterprise/server/hostedrunner/BUILD

@@ -59,6 +59,7 @@ go_test(
         "//server/testutil/testenv",
         "//server/util/authutil",
         "//server/util/platform",
+        "//server/util/testing/flags",
         "@com_github_stretchr_testify//require",
         "@com_google_cloud_go_longrunning//autogen/longrunningpb",
         "@org_golang_google_genproto_googleapis_bytestream//:bytestream",

enterprise/server/hostedrunner/hostedrunner_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testenv"
 	"github.com/buildbuddy-io/buildbuddy/server/util/authutil"
 	"github.com/buildbuddy-io/buildbuddy/server/util/platform"
+	"github.com/buildbuddy-io/buildbuddy/server/util/testing/flags"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/metadata"
@@ -28,6 +29,10 @@ import (
 )
 
 func getEnv(t *testing.T) (*testenv.TestEnv, context.Context) {
+	// Avoid uploading embedded CI runner binaries to the in-memory cache in
+	// each test because it's very slow. The executor will add these binaries locally instead.
+	flags.Set(t, "remote_execution.init_ci_runner_from_cache", false)
+
 	te := enterprise_testenv.New(t)
 	enterprise_testauth.Configure(t, te)
 	ctx := context.Background()

enterprise/server/test/integration/remote_bazel/BUILD

@@ -30,10 +30,12 @@ go_test(
         "//cli/remotebazel",
         "//cli/testutil/testcli",
         "//enterprise/server/backends/kms",
+        "//enterprise/server/cmd/ci_runner/bundle",
         "//enterprise/server/execution_service",
         "//enterprise/server/githubapp",
         "//enterprise/server/hostedrunner",
         "//enterprise/server/invocation_search_service",
+        "//enterprise/server/remote_execution/snaputil",
         "//enterprise/server/secrets",
         "//enterprise/server/test/integration/remote_execution/rbetest",
         "//enterprise/server/util/keystore",
@@ -45,19 +47,21 @@ go_test(
         "//proto:eventlog_go_proto",
         "//proto:invocation_go_proto",
         "//proto:invocation_status_go_proto",
+        "//proto:remote_execution_go_proto",
         "//proto:secrets_go_proto",
         "//proto:user_id_go_proto",
         "//server/backends/memory_kvstore",
         "//server/backends/repo_downloader",
         "//server/interfaces",
+        "//server/remote_cache/digest",
         "//server/tables",
-        "//server/testutil/quarantine",
         "//server/testutil/testbazel",
         "//server/testutil/testenv",
         "//server/testutil/testfs",
         "//server/testutil/testgit",
         "//server/testutil/testshell",
         "//server/util/bazel",
+        "//server/util/bb",
         "//server/util/git",
         "//server/util/log",
         "//server/util/testing/flags",

enterprise/server/test/integration/remote_bazel/remote_bazel_test.go

@@ -16,19 +16,21 @@ import (
 	"github.com/buildbuddy-io/buildbuddy/cli/remotebazel"
 	"github.com/buildbuddy-io/buildbuddy/cli/testutil/testcli"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/backends/kms"
+	"github.com/buildbuddy-io/buildbuddy/enterprise/server/cmd/ci_runner/bundle"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/execution_service"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/githubapp"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/hostedrunner"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/invocation_search_service"
+	"github.com/buildbuddy-io/buildbuddy/enterprise/server/remote_execution/snaputil"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/secrets"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/test/integration/remote_execution/rbetest"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/util/keystore"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/workflow/service"
 	"github.com/buildbuddy-io/buildbuddy/server/backends/memory_kvstore"
 	"github.com/buildbuddy-io/buildbuddy/server/backends/repo_downloader"
 	"github.com/buildbuddy-io/buildbuddy/server/interfaces"
+	"github.com/buildbuddy-io/buildbuddy/server/remote_cache/digest"
 	"github.com/buildbuddy-io/buildbuddy/server/tables"
-	"github.com/buildbuddy-io/buildbuddy/server/testutil/quarantine"
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testbazel"
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testenv"
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testfs"
@@ -49,8 +51,10 @@ import (
 	elpb "github.com/buildbuddy-io/buildbuddy/proto/eventlog"
 	inpb "github.com/buildbuddy-io/buildbuddy/proto/invocation"
 	inspb "github.com/buildbuddy-io/buildbuddy/proto/invocation_status"
+	repb "github.com/buildbuddy-io/buildbuddy/proto/remote_execution"
 	spb "github.com/buildbuddy-io/buildbuddy/proto/secrets"
 	uidpb "github.com/buildbuddy-io/buildbuddy/proto/user_id"
+	cli_bundle "github.com/buildbuddy-io/buildbuddy/server/util/bb"
 )
 
 func init() {
@@ -216,6 +220,10 @@ func TestWithPrivateRepo(t *testing.T) {
 }
 
 func runLocalServerAndExecutor(t *testing.T, mockPrivateGithubToken bool, envModifier func(rbeEnv *rbetest.Env, e *testenv.TestEnv)) (*rbetest.Env, *rbetest.BuildBuddyServer, *rbetest.Executor) {
+	// Avoid uploading embedded CI runner binaries to the in-memory cache in
+	// each test because it's very slow. The executor will add these binaries locally instead.
+	flags.Set(t, "remote_execution.init_ci_runner_from_cache", false)
+
 	githubToken := ""
 	repoURL := ""
 	if mockPrivateGithubToken {
@@ -281,7 +289,6 @@ func runLocalServerAndExecutor(t *testing.T, mockPrivateGithubToken bool, envMod
 }
 
 func TestCancel(t *testing.T) {
-	quarantine.SkipQuarantinedTest(t)
 	repoDir, _ := makeLocalGitRepo(t, map[string]string{
 		"BUILD": `
 sh_binary(
@@ -656,3 +663,68 @@ func TestBashScript(t *testing.T) {
 	require.NoError(t, err)
 	require.Contains(t, string(logResp.GetBuffer()), "Hello from the remote runner!")
 }
+
+// In production, the apps upload the ci_runner and bb binaries to the cache so
+// executors can fetch the latest versions without upgrading. Writing to the cache
+// for tests is very slow, so this behavior is disabled by default.
+//
+// This test enables the behavior and verifies the binaries can be used correctly.
+func TestEmbeddedBinariesFromApp(t *testing.T) {
+	repoDir, _ := makeLocalGitRepo(t, map[string]string{})
+	env, bbServer, _ := runLocalServerAndExecutor(t, false, nil)
+	ctx := env.WithUserID(context.Background(), env.UserID1)
+
+	// The flag is disabled in `runLocalServerAndExecutor`. Enable it here.
+	flags.Set(t, "remote_execution.init_ci_runner_from_cache", true)
+
+	require.NotEmpty(t, bundle.CiRunnerBytes)
+	require.NotEmpty(t, cli_bundle.CLIBytes)
+	runnerBinDigest, err := digest.Compute(bytes.NewReader(bundle.CiRunnerBytes), repb.DigestFunction_BLAKE3)
+	require.NoError(t, err)
+	cliDigest, err := digest.Compute(bytes.NewReader(cli_bundle.CLIBytes), repb.DigestFunction_BLAKE3)
+	require.NoError(t, err)
+
+	// The binaries should not be in the cache to begin with.
+	casClient := env.GetContentAddressableStorageClient()
+	findReq := &repb.FindMissingBlobsRequest{
+		// Hosted runner uploads the CI runner inputs to the snapshot partition.
+		InstanceName:   snaputil.SnapshotPartitionPrefix,
+		BlobDigests:    []*repb.Digest{runnerBinDigest, cliDigest},
+		DigestFunction: repb.DigestFunction_BLAKE3,
+	}
+	findResp, err := casClient.FindMissingBlobs(ctx, findReq)
+	require.NoError(t, err)
+	require.Len(t, findResp.GetMissingBlobDigests(), 2)
+
+	runRemoteBazelInSeparateProcess(t, repoDir, bbServer.GRPCAddress(),
+		"--os=linux",
+		"--arch=amd64",
+		"--script=echo HELLO!",
+		fmt.Sprintf("--remote_header=x-buildbuddy-api-key=%s", env.APIKey1),
+	)
+
+	// Verify invocation logs.
+	bbClient := env.GetBuildBuddyServiceClient()
+	reqCtx := &ctxpb.RequestContext{
+		UserId:  &uidpb.UserId{Id: env.UserID1},
+		GroupId: env.GroupID1,
+	}
+	searchRsp, err := bbClient.SearchInvocation(ctx, &inpb.SearchInvocationRequest{
+		RequestContext: reqCtx,
+		Query:          &inpb.InvocationQuery{GroupId: env.GroupID1},
+	})
+	require.NoError(t, err)
+	require.Equal(t, 1, len(searchRsp.GetInvocation()))
+
+	logResp, err := bbClient.GetEventLogChunk(ctx, &elpb.GetEventLogChunkRequest{
+		InvocationId: searchRsp.Invocation[0].InvocationId,
+		MinLines:     math.MaxInt32,
+	})
+	require.NoError(t, err)
+	require.Contains(t, string(logResp.GetBuffer()), "HELLO!")
+
+	// Verify that the binaries were uploaded to the cache.
+	findResp, err = casClient.FindMissingBlobs(ctx, findReq)
+	require.NoError(t, err)
+	require.Empty(t, findResp.GetMissingBlobDigests())
+}

enterprise/server/test/integration/workflow/workflow_test.go

@@ -107,6 +107,10 @@ func makeRepo(t *testing.T, contents map[string]string) (string, string) {
 }
 
 func setup(t *testing.T, gp interfaces.GitProvider) (*rbetest.Env, interfaces.WorkflowService) {
+	// Avoid uploading embedded CI runner binaries to the in-memory cache in
+	// each test because it's very slow. The executor will add these binaries locally instead.
+	flags.Set(t, "remote_execution.init_ci_runner_from_cache", false)
+
 	env := rbetest.NewRBETestEnv(t)
 	var workflowService interfaces.WorkflowService
 

enterprise/server/util/ci_runner_util/ci_runner_util.go

@@ -25,6 +25,7 @@ const CLIBinaryName = "bb"
 var (
 	RecycledCIRunnerMaxWait = flag.Duration("remote_execution.ci_runner_recycling_max_wait", 3*time.Second, "Max duration that a ci_runner task should wait for a warm runner before running on a potentially cold runner.")
 	CIRunnerDefaultTimeout  = flag.Duration("remote_execution.ci_runner_default_timeout", 8*time.Hour, "Default timeout applied to all ci runners.")
+	InitCIRunnerFromCache   = flag.Bool("remote_execution.init_ci_runner_from_cache", true, "Whether the apps should upload ci_runner binaries to the cache so executors can fetch the latest versions without upgrading.")
 )
 
 // CanInitFromCache The apps are built for linux/amd64. If the ci_runner will run on linux/amd64
@@ -38,7 +39,9 @@ var (
 // a ci_runner task. This will guarantee the binary is built for the correct os/arch,
 // but it will not update automatically when the apps are upgraded.
 func CanInitFromCache(os, arch string) bool {
-	return (os == "" || os == platform.LinuxOperatingSystemName) && (arch == "" || arch == platform.AMD64ArchitectureName)
+	return *InitCIRunnerFromCache &&
+		(os == "" || os == platform.LinuxOperatingSystemName) &&
+		(arch == "" || arch == platform.AMD64ArchitectureName)
 }
 
 // UploadInputRoot If the ci_runner can be properly initialized from the cache,

enterprise/server/workflow/service/service_test.go

@@ -73,6 +73,10 @@ actions:
 )
 
 func newTestEnv(t *testing.T) *testenv.TestEnv {
+	// Avoid uploading embedded CI runner binaries to the in-memory cache in
+	// each test because it's very slow. The executor will add these binaries locally instead.
+	flags.Set(t, "remote_execution.init_ci_runner_from_cache", false)
+
 	flags.Set(t, "github.app.enabled", true)
 	te := enterprise_testenv.New(t)
 	enterprise_testauth.Configure(t, te)