[ci_runner] Clean up bazel_sub_command flag (#7837)

This flag was originally added so that remote bazel could specify the
command to run. Now that we're always sending the commands in
--serialized_action, we can remove it

Should be merged after
#7835 has been deployed

enterprise/server/cmd/ci_runner/main.go

@@ -166,7 +166,6 @@ var (
 	serializedAction   = flag.String("serialized_action", "", "If set, run this b64+yaml encoded action, ignoring trigger conditions.")
 	invocationID       = flag.String("invocation_id", "", "If set, use the specified invocation ID for the workflow action. Ignored if action_name is not set.")
 	visibility         = flag.String("visibility", "", "If set, use the specified value for VISIBILITY build metadata for the workflow invocation.")
-	bazelSubCommand    = flag.String("bazel_sub_command", "", "If set, run the bazel command specified by these args and ignore all triggering and configured actions.")
 	timeout            = flag.Duration("timeout", 0, "Timeout before all commands will be canceled automatically.")
 
 	// Flags to configure setting up git repo
@@ -1284,9 +1283,6 @@ func getActionNameForWorkflowConfiguredEvent() (string, error) {
 		}
 		return a.Name, nil
 	}
-	if *bazelSubCommand != "" {
-		return "run", nil
-	}
 	if *actionName != "" {
 		return *actionName, nil
 	}
@@ -1297,14 +1293,6 @@ func getActionToRun() (*config.Action, error) {
 	if *serializedAction != "" {
 		return deserializeAction(*serializedAction)
 	}
-	if *bazelSubCommand != "" {
-		return &config.Action{
-			Name: "run",
-			DeprecatedBazelCommands: []string{
-				*bazelSubCommand,
-			},
-		}, nil
-	}
 	if *actionName != "" {
 		cfg, err := readConfig()
 		if err != nil {
@@ -1314,7 +1302,7 @@ func getActionToRun() (*config.Action, error) {
 		// actions with a matching action name.
 		return findAction(cfg.Actions, *actionName)
 	}
-	return nil, status.InvalidArgumentError("One of --action or --bazel_sub_command must be specified.")
+	return nil, status.InvalidArgumentError("an action to run must be specified")
 }
 
 func deserializeAction(actionString string) (*config.Action, error) {

enterprise/server/test/integration/ci_runner/BUILD

@@ -27,13 +27,15 @@ go_test(
         "ciRunnerRunfilePath": "$(rlocationpath //enterprise/server/cmd/ci_runner)",
     },
     deps = [
+        "//enterprise/server/workflow/config",
         "//proto:build_event_stream_go_proto",
         "//proto:command_line_go_proto",
         "//proto:eventlog_go_proto",
         "//proto:invocation_go_proto",
         "//proto:invocation_status_go_proto",
         "//proto:remote_execution_go_proto",
         "//proto:remote_execution_log_go_proto",
+        "//proto:runner_go_proto",
         "//server/remote_cache/cachetools",
         "//server/testutil/app",
         "//server/testutil/buildbuddy",
@@ -44,6 +46,7 @@ go_test(
         "@com_github_google_uuid//:uuid",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
+        "@in_gopkg_yaml_v2//:yaml_v2",
         "@io_bazel_rules_go//go/runfiles:go_default_library",
         "@org_golang_google_protobuf//encoding/protodelim",
     ],

enterprise/server/test/integration/ci_runner/ci_runner_test.go

@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"encoding/base64"
 	"fmt"
 	"io"
 	"math"
@@ -19,6 +20,7 @@ import (
 	"time"
 
 	"github.com/bazelbuild/rules_go/go/runfiles"
+	"github.com/buildbuddy-io/buildbuddy/enterprise/server/workflow/config"
 	"github.com/buildbuddy-io/buildbuddy/server/remote_cache/cachetools"
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/app"
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/buildbuddy"
@@ -30,6 +32,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/encoding/protodelim"
+	"gopkg.in/yaml.v2"
 
 	bespb "github.com/buildbuddy-io/buildbuddy/proto/build_event_stream"
 	clpb "github.com/buildbuddy-io/buildbuddy/proto/command_line"
@@ -38,6 +41,7 @@ import (
 	inspb "github.com/buildbuddy-io/buildbuddy/proto/invocation_status"
 	repb "github.com/buildbuddy-io/buildbuddy/proto/remote_execution"
 	rlpb "github.com/buildbuddy-io/buildbuddy/proto/remote_execution_log"
+	rnpb "github.com/buildbuddy-io/buildbuddy/proto/runner"
 )
 
 const (
@@ -1385,14 +1389,24 @@ func TestHostedBazel_ApplyingAndDiscardingPatches(t *testing.T) {
 
 	// Execute a Bazel command with a patched `pass.sh` that should output 'EDIT'.
 	{
+		runAction := &config.Action{
+			Name: "remote run",
+			Steps: []*rnpb.Step{
+				{Run: "bazel test --test_output=streamed --nocache_test_results //..."},
+			},
+		}
+		actionBytes, err := yaml.Marshal(runAction)
+		require.NoError(t, err)
+		serializedAction := base64.StdEncoding.EncodeToString(actionBytes)
+
 		runnerFlags := []string{
 			"--pushed_repo_url=file://" + targetRepoPath,
 			"--pushed_branch=master",
 			"--target_repo_url=file://" + targetRepoPath,
 			"--target_branch=master",
 			"--cache_backend=" + app.GRPCAddress(),
 			"--patch_uri=" + fmt.Sprintf("blobs/%s/%d", patchDigest.GetHash(), patchDigest.GetSizeBytes()),
-			"--bazel_sub_command", "test --test_output=streamed --nocache_test_results //...",
+			"--serialized_action=" + serializedAction,
 			// Disable clean checkout fallback for this test since we expect to sync
 			// without errors.
 			"--fallback_to_clean_checkout=false",
@@ -1411,12 +1425,22 @@ func TestHostedBazel_ApplyingAndDiscardingPatches(t *testing.T) {
 
 	// Re-run Bazel without a patched `pass.sh` which should revert the previous change.
 	{
+		runAction := &config.Action{
+			Name: "remote run",
+			Steps: []*rnpb.Step{
+				{Run: "bazel test --test_output=streamed --nocache_test_results //..."},
+			},
+		}
+		actionBytes, err := yaml.Marshal(runAction)
+		require.NoError(t, err)
+		serializedAction := base64.StdEncoding.EncodeToString(actionBytes)
+
 		runnerFlags := []string{
 			"--pushed_repo_url=file://" + targetRepoPath,
 			"--pushed_branch=master",
 			"--target_repo_url=file://" + targetRepoPath,
 			"--target_branch=master",
-			"--bazel_sub_command", "test --test_output=streamed --nocache_test_results //...",
+			"--serialized_action=" + serializedAction,
 			// Disable clean checkout fallback for this test since we expect to sync
 			// without errors.
 			"--fallback_to_clean_checkout=false",