Remove deprecated --remote-execution-server and --remote-store-server options

src/python/pants/engine/internals/native.py

@@ -251,7 +251,7 @@ def new_scheduler(
 
         remoting_options = PyRemotingOptions(
             execution_enable=execution_options.remote_execution,
-            store_addresses=execution_options.remote_store_addresses,
+            store_address=execution_options.remote_store_address,
             execution_address=execution_options.remote_execution_address,
             execution_process_cache_namespace=execution_options.process_execution_cache_namespace,
             instance_name=execution_options.remote_instance_name,

src/python/pants/option/global_options.py

@@ -117,8 +117,8 @@ class ExecutionOptions:
     process_execution_cleanup_local_dirs: bool
     process_execution_use_local_cache: bool
 
-    remote_store_addresses: list[str]
-    remote_store_headers: Dict[str, str]
+    remote_store_address: str | None
+    remote_store_headers: dict[str, str]
     remote_store_chunk_bytes: Any
     remote_store_chunk_upload_timeout_seconds: int
     remote_store_rpc_retries: int
@@ -196,30 +196,20 @@ def from_options(cls, options: Options) -> ExecutionOptions:
                     )
                     remote_instance_name = auth_plugin_result.instance_name
 
-        # Determine the remote servers.
+        # Determine the remote addresses.
         # NB: Tonic expects the schemes `http` and `https`, even though they are gRPC requests.
         # We validate that users set `grpc` and `grpcs` in the options system for clarity, but then
         # normalize to `http`/`https`.
-        remote_address_scheme = "https://" if bootstrap_options.remote_ca_certs_path else "http://"
-        if bootstrap_options.remote_execution_address:
-            remote_execution_address = re.sub(
-                r"^grpc", "http", bootstrap_options.remote_execution_address
-            )
-        elif bootstrap_options.remote_execution_server:
-            remote_execution_address = (
-                f"{remote_address_scheme}{bootstrap_options.remote_execution_server}"
-            )
-        else:
-            remote_execution_address = None
-
-        if bootstrap_options.remote_store_address:
-            remote_store_addresses = [
-                re.sub(r"^grpc", "http", bootstrap_options.remote_store_address)
-            ]
-        else:
-            remote_store_addresses = [
-                f"{remote_address_scheme}{addr}" for addr in bootstrap_options.remote_store_server
-            ]
+        remote_execution_address = (
+            re.sub(r"^grpc", "http", bootstrap_options.remote_execution_address)
+            if bootstrap_options.remote_execution_address
+            else None
+        )
+        remote_store_address = (
+            re.sub(r"^grpc", "http", bootstrap_options.remote_store_address)
+            if bootstrap_options.remote_store_address
+            else None
+        )
 
         return cls(
             # Remote execution strategy.
@@ -236,7 +226,7 @@ def from_options(cls, options: Options) -> ExecutionOptions:
             process_execution_use_local_cache=bootstrap_options.process_execution_use_local_cache,
             process_execution_cache_namespace=bootstrap_options.process_execution_cache_namespace,
             # Remote store setup.
-            remote_store_addresses=remote_store_addresses,
+            remote_store_address=remote_store_address,
             remote_store_headers=remote_store_headers,
             remote_store_chunk_bytes=bootstrap_options.remote_store_chunk_bytes,
             remote_store_chunk_upload_timeout_seconds=bootstrap_options.remote_store_chunk_upload_timeout_seconds,
@@ -271,7 +261,7 @@ def from_options(cls, options: Options) -> ExecutionOptions:
     process_execution_cleanup_local_dirs=True,
     process_execution_use_local_cache=True,
     # Remote store setup.
-    remote_store_addresses=[],
+    remote_store_address=None,
     remote_store_headers={},
     remote_store_chunk_bytes=1024 * 1024,
     remote_store_chunk_upload_timeout_seconds=60,
@@ -904,26 +894,11 @@ def register_bootstrap_options(cls, register):
             ),
         )
 
-        register(
-            "--remote-store-server",
-            advanced=True,
-            type=list,
-            default=DEFAULT_EXECUTION_OPTIONS.remote_store_addresses,
-            help="host:port of grpc server to use as remote execution file store.",
-            removal_version="2.4.0.dev0",
-            removal_hint=(
-                "Use `--remote-store-address` instead.\n\nNote that you must add the prefix "
-                "`grpc://` or `grpcs://` to identify whether TLS should be used.\n\n"
-                "`--remote-store-address` also is a string option, rather than list option; if you "
-                "still need support for multiple servers, please open a GitHub issue or reach out "
-                f"on Slack in the #remoting channel. See {docs_url('community')}."
-            ),
-        )
         register(
             "--remote-store-address",
             advanced=True,
             type=str,
-            default=None,
+            default=DEFAULT_EXECUTION_OPTIONS.remote_store_address,
             help=(
                 "The URI of a server used for the remote file store.\n\nFormat: "
                 "`scheme://host:port`. The supported schemes are `grpc` and `grpcs`, i.e. gRPC "
@@ -1030,23 +1005,11 @@ def register_bootstrap_options(cls, register):
             ),
         )
 
-        register(
-            "--remote-execution-server",
-            advanced=True,
-            type=str,
-            default=DEFAULT_EXECUTION_OPTIONS.remote_execution_address,
-            help="host:port of grpc server to use as remote execution scheduler.",
-            removal_version="2.4.0.dev0",
-            removal_hint=(
-                "Use `--remote-execution-address` instead.\n\nNote that you must add the prefix "
-                "`grpc://` or `grpcs://` to identify whether TLS should be used."
-            ),
-        )
         register(
             "--remote-execution-address",
             advanced=True,
             type=str,
-            default=None,
+            default=DEFAULT_EXECUTION_OPTIONS.remote_execution_address,
             help=(
                 "The URI of a server used as a remote execution scheduler.\n\nFormat: "
                 "`scheme://host:port`. The supported schemes are `grpc` and `grpcs`, i.e. gRPC "
@@ -1216,47 +1179,26 @@ def validate_instance(cls, opts):
                 "enabled, it will already use remote caching."
             )
 
-        if opts.remote_execution_address and opts.remote_execution_server:
-            raise OptionsError(
-                "Conflicting options used. You used the new, preferred remote_execution_address, "
-                "but also used the deprecated remote_execution_server.\n\nPlease use only of these "
-                "(preferably remote_execution_address)."
-            )
-        if opts.remote_store_address and opts.remote_store_server:
-            raise OptionsError(
-                "Conflicting options used. You used the new, preferred remote_store_address, but "
-                "also used the deprecated remote_store_server.\n\nPlease use only of these "
-                "(preferably remote_store_address)."
-            )
-
-        remote_execution_address_configured = (
-            opts.remote_execution_server or opts.remote_execution_address
-        )
-        remote_store_address_configured = opts.remote_store_server or opts.remote_store_address
-        if opts.remote_execution and not remote_execution_address_configured:
+        if opts.remote_execution and not opts.remote_execution_address:
             raise OptionsError(
                 "The `--remote-execution` option requires also setting "
-                "either `--remote-execution-address` or the deprecated "
-                "`--remote-execution-server` to work properly."
+                "`--remote-execution-address` to work properly."
             )
-        if remote_execution_address_configured and not remote_store_address_configured:
+        if opts.remote_execution_address and not opts.remote_store_address:
             raise OptionsError(
-                "The `--remote-execution-address` and deprecated `--remote-execution-server` "
-                "options require also setting `--remote-store-address` or the deprecated "
-                "`--remote-store-server`. Often these have the same value."
+                "The `--remote-execution-address` option requires also setting "
+                "`--remote-store-address`. Often these have the same value."
             )
 
-        if opts.remote_cache_read and not remote_store_address_configured:
+        if opts.remote_cache_read and not opts.remote_store_address:
             raise OptionsError(
                 "The `--remote-cache-read` option requires also setting "
-                "`--remote-store-address` or the deprecated `--remote-store-server` to work "
-                "properly."
+                "`--remote-store-address` to work properly."
             )
-        if opts.remote_cache_write and not remote_store_address_configured:
+        if opts.remote_cache_write and not opts.remote_store_address:
             raise OptionsError(
                 "The `--remote-cache-write` option requires also setting "
-                "`--remote-store-address` or the deprecated `--remote-store-server` to work "
-                "properly."
+                "`--remote-store-address` or to work properly."
             )
 
         def validate_remote_address(opt_name: str) -> None:

src/python/pants/option/global_options_test.py

@@ -67,15 +67,15 @@ def test_execution_options_remote_addresses() -> None:
         remote_execution_address=f"grpc://{host}",
     )
     assert exec_options.remote_execution_address == f"http://{host}"
-    assert exec_options.remote_store_addresses == [f"http://{host}"]
+    assert exec_options.remote_store_address == [f"http://{host}"]
 
     exec_options = create_execution_options(
         initial_headers={},
         remote_store_address=f"grpcs://{host}",
         remote_execution_address=f"grpcs://{host}",
     )
     assert exec_options.remote_execution_address == f"https://{host}"
-    assert exec_options.remote_store_addresses == [f"https://{host}"]
+    assert exec_options.remote_store_address == [f"https://{host}"]
 
     with pytest.raises(OptionsError):
         create_execution_options(

src/rust/engine/fs/fs_util/src/main.rs

@@ -45,7 +45,6 @@ use futures::FutureExt;
 use grpc_util::prost::MessageExt;
 use hashing::{Digest, Fingerprint};
 use parking_lot::Mutex;
-use rand::seq::SliceRandom;
 use serde_derive::Serialize;
 use std::collections::BTreeMap;
 use store::{
@@ -210,8 +209,6 @@ to this directory.",
               .takes_value(true)
               .long("server-address")
               .required(false)
-              .multiple(true)
-              .number_of_values(1)
         )
         .arg(
           Arg::with_name("root-ca-cert-file")
@@ -266,7 +263,7 @@ async fn execute(top_match: &clap::ArgMatches<'_>) -> Result<(), ExitError> {
     .unwrap_or_else(Store::default_path);
   let runtime = task_executor::Executor::new();
   let (store, store_has_remote) = {
-    let (store_result, store_has_remote) = match top_match.values_of("server-address") {
+    let (store_result, store_has_remote) = match top_match.value_of("server-address") {
       Some(cas_address) => {
         let chunk_size =
           value_t!(top_match.value_of("chunk-bytes"), usize).expect("Bad chunk-bytes flag");
@@ -294,15 +291,11 @@ async fn execute(top_match: &clap::ArgMatches<'_>) -> Result<(), ExitError> {
           );
         }
 
-        // Randomize CAS address order to avoid thundering herds from common config.
-        let mut cas_addresses = cas_address.map(str::to_owned).collect::<Vec<_>>();
-        cas_addresses.shuffle(&mut rand::thread_rng());
-
         (
           Store::with_remote(
             runtime.clone(),
             &store_dir,
-            cas_addresses,
+            cas_address,
             top_match
               .value_of("remote-instance-name")
               .map(str::to_owned),

src/rust/engine/fs/store/src/lib.rs

@@ -257,7 +257,7 @@ impl Store {
   pub fn with_remote<P: AsRef<Path>>(
     executor: task_executor::Executor,
     path: P,
-    cas_addresses: Vec<String>,
+    cas_address: &str,
     instance_name: Option<String>,
     root_ca_certs: Option<Vec<u8>>,
     headers: BTreeMap<String, String>,
@@ -268,7 +268,7 @@ impl Store {
     Ok(Store {
       local: local::ByteStore::new(executor, path)?,
       remote: Some(remote::ByteStore::new(
-        cas_addresses,
+        cas_address,
         instance_name,
         root_ca_certs,
         headers,

src/rust/engine/fs/store/src/remote.rs

@@ -14,10 +14,9 @@ use futures::Future;
 use futures::StreamExt;
 use grpc_util::headers_to_interceptor_fn;
 use hashing::Digest;
-use itertools::{Either, Itertools};
 use log::Level;
 use remexec::content_addressable_storage_client::ContentAddressableStorageClient;
-use tonic::transport::{Channel, Endpoint};
+use tonic::transport::Channel;
 use tonic::{Code, Interceptor, Request};
 use workunit_store::{with_workunit, ObservationMetric};
 
@@ -41,40 +40,22 @@ impl fmt::Debug for ByteStore {
 
 impl ByteStore {
   pub fn new(
-    cas_addresses: Vec<String>,
+    cas_address: &str,
     instance_name: Option<String>,
     root_ca_certs: Option<Vec<u8>>,
     headers: BTreeMap<String, String>,
     chunk_size_bytes: usize,
     upload_timeout: Duration,
     rpc_retries: usize,
   ) -> Result<ByteStore, String> {
-    let tls_client_config = if cas_addresses
-      .first()
-      .map(|addr| addr.starts_with("https://"))
-      .unwrap_or(false)
-    {
+    let tls_client_config = if cas_address.starts_with("https://") {
       Some(grpc_util::create_tls_config(root_ca_certs)?)
     } else {
       None
     };
 
-    let (endpoints, errors): (Vec<Endpoint>, Vec<String>) = cas_addresses
-      .iter()
-      .map(|addr| grpc_util::create_endpoint(addr, tls_client_config.as_ref()))
-      .partition_map(|result| match result {
-        Ok(endpoint) => Either::Left(endpoint),
-        Err(err) => Either::Right(err),
-      });
-
-    if !errors.is_empty() {
-      return Err(format!(
-        "Errors while creating gRPC endpoints: {}",
-        errors.join(", ")
-      ));
-    }
-
-    let channel = tonic::transport::Channel::balance_list(endpoints.iter().cloned());
+    let endpoint = grpc_util::create_endpoint(&cas_address, tls_client_config.as_ref())?;
+    let channel = tonic::transport::Channel::balance_list(vec![endpoint].into_iter());
     let interceptor = if headers.is_empty() {
       None
     } else {