From dbe61d281520d20dba477ddb430139338afabde6 Mon Sep 17 00:00:00 2001 From: "Nathan (Blaise) Bruer" Date: Tue, 12 Dec 2023 16:40:53 -0600 Subject: [PATCH] [Breaking] Rename entrypoint_cmd->entrypoint and precondition_script (#475) In prep for the release we are renaming a few config variables to make things more future-proof. `entrypoint_cmd` is now `entrypoint`. `precondition_script` is now `experimental_precondition_script`. --- nativelink-config/src/cas_server.rs | 26 +++++++++---------- nativelink-worker/src/local_worker.rs | 10 +++---- .../src/running_actions_manager.rs | 6 ++--- nativelink-worker/tests/local_worker_test.rs | 4 +-- .../tests/running_actions_manager_test.rs | 10 +++---- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/nativelink-config/src/cas_server.rs b/nativelink-config/src/cas_server.rs index 5842857ed..b44d23d08 100644 --- a/nativelink-config/src/cas_server.rs +++ b/nativelink-config/src/cas_server.rs @@ -439,7 +439,7 @@ pub struct LocalWorkerConfig { #[serde(default, deserialize_with = "convert_numeric_with_shellexpand")] pub max_action_timeout: usize, - /// If timeout is handled in `entrypoint_cmd` or another wrapper script. + /// If timeout is handled in `entrypoint` or another wrapper script. /// If set to true NativeLink will not honor the timeout the action requested /// and instead will always force kill the action after max_action_timeout /// has been reached. If this is set to false, the smaller value of the action's @@ -449,7 +449,7 @@ pub struct LocalWorkerConfig { /// The real timeout can be received via an environment variable set in: /// `EnvironmentSource::TimeoutMillis`. /// - /// Example on where this is useful: `entrypoint_cmd` launches the action inside + /// Example on where this is useful: `entrypoint` launches the action inside /// a docker container, but the docker container may need to be downloaded. Thus /// the timer should not start until the docker container has started executing /// the action. In this case, action will likely be wrapped in another program, @@ -465,7 +465,17 @@ pub struct LocalWorkerConfig { /// command like: "run.sh sleep 5". /// Default: {Use the command from the job request}. #[serde(default, deserialize_with = "convert_string_with_shellexpand")] - pub entrypoint_cmd: String, + pub entrypoint: String, + + /// An optional script to run before every action is processed on the worker. + /// The value should be the full path to the script to execute and will pause + /// all actions on the worker if it returns an exit code other than 0. + /// If not set, then the worker will never pause and will continue to accept + /// jobs according to the scheduler configuration. + /// This is useful, for example, if the worker should not take any more + /// actions until there is enough resource available on the machine to + /// handle them. + pub experimental_precondition_script: Option, /// Underlying CAS store that the worker will use to download CAS artifacts. /// This store must be a `FastSlowStore`. The `fast` store must be a @@ -493,16 +503,6 @@ pub struct LocalWorkerConfig { /// worker. pub platform_properties: HashMap, - /// An optional script to run before every action is processed on the worker. - /// The value should be the full path to the script to execute and will pause - /// all actions on the worker if it returns an exit code other than 0. - /// If not set, then the worker will never pause and will continue to accept - /// jobs according to the scheduler configuration. - /// This is useful, for example, if the worker should not take any more - /// actions until there is enough resource available on the machine to - /// handle them. - pub precondition_script: Option, - /// An optional mapping of environment names to set for the execution /// as well as those specified in the action itself. If set, will set each /// key as an environment variable before executing the job with the value diff --git a/nativelink-worker/src/local_worker.rs b/nativelink-worker/src/local_worker.rs index 024218753..dd3b402bb 100644 --- a/nativelink-worker/src/local_worker.rs +++ b/nativelink-worker/src/local_worker.rs @@ -92,7 +92,7 @@ async fn preconditions_met(precondition_script: Option) -> Result<(), Er // so that's not currently possible. Perhaps we'll move this in // future to pass useful information through? Or perhaps we'll // have a pre-condition and a pre-execute script instead, although - // arguably entrypoint_cmd already gives us that. + // arguably entrypoint already gives us that. let precondition_process = process::Command::new(precondition_script) .kill_on_drop(true) .stdin(Stdio::null()) @@ -221,7 +221,7 @@ impl<'a, T: WorkerApiClientTrait, U: RunningActionsManager> LocalWorkerImpl<'a, .err_tip(|| "In LocalWorkerImpl::new()"); let running_actions_manager = self.running_actions_manager.clone(); let worker_id_clone = worker_id.clone(); - let precondition_script_cfg = self.config.precondition_script.clone(); + let precondition_script_cfg = self.config.experimental_precondition_script.clone(); let actions_in_transit = self.actions_in_transit.clone(); let start_action_fut = self.metrics.clone().wrap(move |metrics| async move { metrics.preconditions.wrap(preconditions_met(precondition_script_cfg)) @@ -349,10 +349,10 @@ pub async fn new_local_worker( fs::create_dir_all(&config.work_directory) .await .err_tip(|| format!("Could not make work_directory : {}", config.work_directory))?; - let entrypoint_cmd = if config.entrypoint_cmd.is_empty() { + let entrypoint = if config.entrypoint.is_empty() { None } else { - Some(config.entrypoint_cmd.clone()) + Some(config.entrypoint.clone()) }; let max_action_timeout = if config.max_action_timeout == 0 { DEFAULT_MAX_ACTION_TIMEOUT @@ -362,7 +362,7 @@ pub async fn new_local_worker( let running_actions_manager = Arc::new(RunningActionsManagerImpl::new(RunningActionsManagerArgs { root_work_directory: config.work_directory.clone(), execution_configuration: ExecutionConfiguration { - entrypoint_cmd, + entrypoint, additional_environment: config.additional_environment.clone(), }, cas_store: fast_slow_store, diff --git a/nativelink-worker/src/running_actions_manager.rs b/nativelink-worker/src/running_actions_manager.rs index 4a7301dfc..7dfb7889d 100644 --- a/nativelink-worker/src/running_actions_manager.rs +++ b/nativelink-worker/src/running_actions_manager.rs @@ -652,8 +652,8 @@ impl RunningActionImpl { return Err(make_input_err!("No arguments provided in Command proto")); } let args: Vec<&OsStr> = - if let Some(entrypoint_cmd) = &self.running_actions_manager.execution_configuration.entrypoint_cmd { - std::iter::once(entrypoint_cmd.as_ref()) + if let Some(entrypoint) = &self.running_actions_manager.execution_configuration.entrypoint { + std::iter::once(entrypoint.as_ref()) .chain(command_proto.arguments.iter().map(AsRef::as_ref)) .collect() } else { @@ -1218,7 +1218,7 @@ pub struct ExecutionConfiguration { /// If set, will be executed instead of the first argument passed in the /// ActionInfo with all of the arguments in the ActionInfo passed as /// arguments to this command. - pub entrypoint_cmd: Option, + pub entrypoint: Option, /// The only environment variables that will be specified when the command /// executes other than those in the ActionInfo. On Windows, SystemRoot /// and PATH are also assigned (see inner_execute). diff --git a/nativelink-worker/tests/local_worker_test.rs b/nativelink-worker/tests/local_worker_test.rs index 14cbf0cb5..29faa310e 100644 --- a/nativelink-worker/tests/local_worker_test.rs +++ b/nativelink-worker/tests/local_worker_test.rs @@ -470,7 +470,7 @@ mod local_worker_tests { } #[tokio::test] - async fn precondition_script_fails() -> Result<(), Box> { + async fn experimental_precondition_script_fails() -> Result<(), Box> { let temp_path = make_temp_path("scripts"); fs::create_dir_all(temp_path.clone()).await?; #[cfg(target_family = "unix")] @@ -497,7 +497,7 @@ mod local_worker_tests { precondition_script }; let local_worker_config = LocalWorkerConfig { - precondition_script: Some(precondition_script), + experimental_precondition_script: Some(precondition_script), ..Default::default() }; diff --git a/nativelink-worker/tests/running_actions_manager_test.rs b/nativelink-worker/tests/running_actions_manager_test.rs index 9a78a9b50..be5d49579 100644 --- a/nativelink-worker/tests/running_actions_manager_test.rs +++ b/nativelink-worker/tests/running_actions_manager_test.rs @@ -1321,7 +1321,7 @@ mod running_actions_manager_tests { // print to stdout. We then check the results of both to make sure the shell script was // invoked and the actual command was invoked under the shell script. #[tokio::test] - async fn entrypoint_cmd_does_invoke_if_set() -> Result<(), Box> { + async fn entrypoint_does_invoke_if_set() -> Result<(), Box> { #[cfg(target_family = "unix")] const TEST_WRAPPER_SCRIPT_CONTENT: &str = "\ #!/bin/bash @@ -1377,7 +1377,7 @@ exit 0 let running_actions_manager = Arc::new(RunningActionsManagerImpl::new(RunningActionsManagerArgs { root_work_directory: root_work_directory.clone(), execution_configuration: ExecutionConfiguration { - entrypoint_cmd: Some(test_wrapper_script.into_string().unwrap()), + entrypoint: Some(test_wrapper_script.into_string().unwrap()), additional_environment: None, }, cas_store: Pin::into_inner(cas_store.clone()), @@ -1450,7 +1450,7 @@ exit 0 } #[tokio::test] - async fn entrypoint_cmd_injects_properties() -> Result<(), Box> { + async fn entrypoint_injects_properties() -> Result<(), Box> { #[cfg(target_family = "unix")] const TEST_WRAPPER_SCRIPT_CONTENT: &str = "\ #!/bin/bash @@ -1506,7 +1506,7 @@ exit 0 let running_actions_manager = Arc::new(RunningActionsManagerImpl::new(RunningActionsManagerArgs { root_work_directory: root_work_directory.clone(), execution_configuration: ExecutionConfiguration { - entrypoint_cmd: Some(test_wrapper_script.into_string().unwrap()), + entrypoint: Some(test_wrapper_script.into_string().unwrap()), additional_environment: Some(HashMap::from([ ( "PROPERTY".to_string(), @@ -1647,7 +1647,7 @@ exit 1 let running_actions_manager = Arc::new(RunningActionsManagerImpl::new(RunningActionsManagerArgs { root_work_directory: root_work_directory.clone(), execution_configuration: ExecutionConfiguration { - entrypoint_cmd: Some(test_wrapper_script.into_string().unwrap()), + entrypoint: Some(test_wrapper_script.into_string().unwrap()), additional_environment: Some(HashMap::from([( "SIDE_CHANNEL_FILE".to_string(), EnvironmentSource::side_channel_file,