Skip to content

Commit

Permalink
fall back to Rust for experimental commands
Browse files Browse the repository at this point in the history
Summary:
# Context

Our last attempt at removing the Python version of `eden redirect` failed (see the SEV attached to D55426809). Our next attempt should be done in a phased manner to avoid similar breakages.

# Solution

We will slowly remove all possible fallbacks to Python so that we can be sure no commands are running the Python version of `eden redirect` prior to deleting the code. New rollout plan is as follows:

1) Remove the Chef recipe that writes `/etc/eden/edenfsctl_rollout.json` (Done)
2) Remove "redirect" from the default list of experimental commands in the Rust code.
3) Add a fallback to Rust if Python parsing fails for experimental commands
4) Add an EdenFS event that's logged every time we fallback to the Python `eden redirect` codepath
5) Monitor Scuba data to ensure the Python codepath isn't being hit
6) Delete Python `eden redirect` altogether

# This diff

Accomplishes #3 by ensuring any experimental command that fails to parse as a valid Python command attempts to run as a Rust command before reporting an error. This would have prevented the SEVs today (since there *was* a valid Rust version of `eden redirect`, we just skipped it altogether).

The next diff will add an EdenFS event that we can add alerting for to ensure this path is never hit. If it is hit, we can investigate why the rollout is failing and adjust accordingly.

Reviewed By: kmancini

Differential Revision: D56282193

fbshipit-source-id: 54c4ff1fc6808f6d44e5d98294302274f34598da
  • Loading branch information
MichaelCuevas authored and facebook-github-bot committed Apr 24, 2024
1 parent 29096c7 commit 512503a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 5 deletions.
15 changes: 12 additions & 3 deletions eden/fs/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,17 @@ def stop_internal_processes(_: str) -> None:
# For a non-unix system (like Windows), we will define our own error codes.
try:
EX_OK: int = os.EX_OK
EX_USAGE: int = os.EX_USAGE
EX_SOFTWARE: int = os.EX_SOFTWARE
EX_OSFILE: int = os.EX_OSFILE
except AttributeError: # On a non-unix system
EX_OK: int = 0
EX_USAGE: int = 64
EX_SOFTWARE: int = 70
EX_OSFILE: int = 72

# The Rust CLI depends on this value staying constant. Instead of fetching it
# from the os library, let's just define it here.
EX_USAGE: int = 64


# We have different mitigations on different platforms due to cmd differences
def _get_unmount_timeout_suggestions(path: str) -> str:
Expand Down Expand Up @@ -2690,7 +2692,14 @@ def main() -> int:
# Increase how often it's sampled.
par_telemetry.set_sample_rate(automation=10000)
parser = create_parser()
args = parser.parse_args()
try:
args = parser.parse_args()
except SystemExit:
# For some reason argparse calls sys.exit(2) when it encounters a parse
# error... This makes it hard for us to distinguish between edenfsctl
# failing w/ exit code 2 and a parse error. Let's catch the parse
# error and return a more appropriate exit code.
return EX_USAGE

# The default event loop on 3.8+ will cause an ugly backtrace when
# edenfsctl is interrupted. Switching back to the selector event loop.
Expand Down
12 changes: 11 additions & 1 deletion eden/fs/cli_rs/edenfsctl/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ use edenfs_utils::strip_unc_prefix;
use fbinit::FacebookInit;
use tracing_subscriber::filter::EnvFilter;

/// Value used in Python to indicate a command failed to parse
pub const PYTHON_EDENFSCTL_EX_USAGE: i32 = 64;

fn python_fallback() -> Result<Command> {
if let Ok(args) = std::env::var("EDENFSCTL_REAL") {
// We might get a command starting with python.exe here instead of a simple path.
Expand Down Expand Up @@ -142,7 +145,14 @@ fn wrapper_main() -> Result<i32> {
if cmd.is_enabled() {
rust_main(cmd)
} else {
fallback(None)
match fallback(None) {
// If the Python version of edenfsctl exited with a
// parse error, we should see if the Rust version
// exists. This helps prevent cases where rollouts
// are not working correctly.
Ok(PYTHON_EDENFSCTL_EX_USAGE) => rust_main(cmd),
res => res,
}
}
}
// If the command is defined in Rust, then --help will cause
Expand Down
2 changes: 1 addition & 1 deletion eden/scm/tests/test-eden-commands.t
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ test eden commands help
$ eden --help > /dev/null
$ eden du --help > /dev/null
$ eden imnotacommandiswear --help > /dev/null 2>&1
[2]
[64]

Test a few simple eden commands

Expand Down

0 comments on commit 512503a

Please sign in to comment.