Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

global remote execution mode is broken for bootstrap stage #17262

Open
Eric-Arellano opened this issue Oct 18, 2022 · 7 comments
Open

global remote execution mode is broken for bootstrap stage #17262

Eric-Arellano opened this issue Oct 18, 2022 · 7 comments
Milestone

Comments

@Eric-Arellano
Copy link
Contributor

#17135 needs to run in local execution mode when the bootstrap stage is happening. Otherwise we get:

Exception caught: (pants.engine.internals.scheduler.ExecutionError)
  File "/Users/ericarellano/code/pants/src/python/pants/bin/pants_loader.py", line 127, in <module>
    main()
  File "/Users/ericarellano/code/pants/src/python/pants/bin/pants_loader.py", line 123, in main
    PantsLoader.main()
  File "/Users/ericarellano/code/pants/src/python/pants/bin/pants_loader.py", line 110, in main
    cls.run_default_entrypoint()
  File "/Users/ericarellano/code/pants/src/python/pants/bin/pants_loader.py", line 92, in run_default_entrypoint
    exit_code = runner.run(start_time)
  File "/Users/ericarellano/code/pants/src/python/pants/bin/pants_runner.py", line 99, in run
    runner = LocalPantsRunner.create(
  File "/Users/ericarellano/code/pants/src/python/pants/bin/local_pants_runner.py", line 127, in create
    build_config = options_initializer.build_config(options_bootstrapper, env)
  File "/Users/ericarellano/code/pants/src/python/pants/init/options_initializer.py", line 111, in build_config
    return _initialize_build_configuration(self._plugin_resolver, options_bootstrapper, env)
  File "/Users/ericarellano/code/pants/src/python/pants/init/options_initializer.py", line 48, in _initialize_build_configuration
    working_set = plugin_resolver.resolve(options_bootstrapper, env)
  File "/Users/ericarellano/code/pants/src/python/pants/init/plugin_resolver.py", line 137, in resolve
    for resolved_plugin_location in self._resolve_plugins(
  File "/Users/ericarellano/code/pants/src/python/pants/init/plugin_resolver.py", line 164, in _resolve_plugins
    session.product_request(ResolvedPluginDistributions, [params])[0],
  File "/Users/ericarellano/code/pants/src/python/pants/engine/internals/scheduler.py", line 561, in product_request
    return self.execute(request)
  File "/Users/ericarellano/code/pants/src/python/pants/engine/internals/scheduler.py", line 505, in execute
    self._raise_on_error([t for _, t in throws])
  File "/Users/ericarellano/code/pants/src/python/pants/engine/internals/scheduler.py", line 489, in _raise_on_error
    raise ExecutionError(

Exception message: 1 Exception encountered:

Engine traceback:
  in Prepare environment for running PEXes
  in Scheduling: Extract environment variables from the remote execution environment

Exception: Failed to execute: Process {
    argv: [
        "env",
        "-0",
    ],
    env: {},
    working_directory: None,
    input_digests: InputDigests {
        complete: DirectoryDigest {
            digest: Digest {
                hash: Fingerprint<e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855>,
                size_bytes: 0,
            },
            tree: "Some(..)",
        },
        nailgun: DirectoryDigest {
            digest: Digest {
                hash: Fingerprint<e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855>,
                size_bytes: 0,
            },
            tree: "Some(..)",
        },
        input_files: DirectoryDigest {
            digest: Digest {
                hash: Fingerprint<e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855>,
                size_bytes: 0,
            },
            tree: "Some(..)",
        },
        immutable_inputs: {},
        use_nailgun: {},
    },
    output_files: {},
    output_directories: {},
    timeout: None,
    execution_slot_variable: None,
    concurrency_available: 0,
    description: "Extract environment variables from the remote execution environment",
    level: Debug,
    append_only_caches: {},
    jdk_home: None,
    platform: Linux_x86_64,
    cache_scope: Successful,
    execution_strategy: RemoteExecution(
        [],
    ),
    remote_cache_speculation_delay: 0ns,
}

Error launching process: Os { code: 2, kind: NotFound, message: "No such file or directory" }

I'm not sure how to know it's the bootstrap stage.

@stuhood
Copy link
Member

stuhood commented Oct 18, 2022

"Bootstrap" uses a BootstrapScheduler, which is configured a particular way:

def create_bootstrap_scheduler(
options_bootstrapper: OptionsBootstrapper, executor: PyExecutor | None = None
) -> BootstrapScheduler:
bc_builder = BuildConfiguration.Builder()
# To load plugins, we only need access to the Python/PEX rules.
load_build_configuration_from_source(bc_builder, ["pants.backend.python"])
# And to plugin-loading-specific rules.
bc_builder.register_rules("_dummy_for_bootstrapping_", plugin_resolver_rules())
# We allow unrecognized options to defer any option error handling until post-bootstrap.
bc_builder.allow_unknown_options()
return BootstrapScheduler(
EngineInitializer.setup_graph(
options_bootstrapper.bootstrap_options.for_global_scope(),
bc_builder.create(),
DynamicRemoteOptions.disabled(),
executor,
ignore_unrecognized_build_file_symbols=True,
).scheduler
)
... it should always fully disable remote execution due to the DynamicRemoteOptions.disabled() line there.

@Eric-Arellano
Copy link
Contributor Author

@stuhood it looks like that only impacts our Rust values, but it does not overwrite the values of OptionsBootstrapper used here:

def _resolve_plugins(
self,
options_bootstrapper: OptionsBootstrapper,
env: CompleteEnvironmentVars,
request: PluginsRequest,
) -> ResolvedPluginDistributions:
session = self._scheduler.scheduler.new_session(
"plugin_resolver",
session_values=SessionValues(
{
OptionsBootstrapper: options_bootstrapper,
CompleteEnvironmentVars: env,
}
),
)
params = Params(request, determine_bootstrap_environment(session))
return cast(
ResolvedPluginDistributions,
session.product_request(ResolvedPluginDistributions, [params])[0],
)

options_bootstrapper.get_bootstrap_options().for_global_scope().remote_execution in this method returns True during the plugin resolution stage.

This is an issue because we now determine whether to use RE from PyProcessConfigFromEnvironment, whereas before it was a global toggle controlled by Rust.

I think we have two fixes:

  1. Trick OptionsBootstrapper -> GlobalOptions into disabling all the dynamic remote options, e.g. add OptionsBootstrapper.remote_options_disabled()
  2. Or, have a way to know that the session is the plugin resolver, and then update our environment-specific rules to handle it.

Suggestions?

@stuhood
Copy link
Member

stuhood commented Oct 19, 2022

There is a pattern of adding static resolve methods to GlobalOptions which take the inputs needed to compute a value, rather than accessing it directly:

@staticmethod
def resolve_keep_sandboxes(
bootstrap_options: OptionValueContainer,
) -> KeepSandboxes:
resolved_value = resolve_conflicting_options(
old_option="process_cleanup",
new_option="keep_sandboxes",
old_scope="",
new_scope="",
old_container=bootstrap_options,
new_container=bootstrap_options,
)
if isinstance(resolved_value, bool):
# Is `process_cleanup`.
return KeepSandboxes.never if resolved_value else KeepSandboxes.always
elif isinstance(resolved_value, KeepSandboxes):
return resolved_value
else:
raise TypeError(f"Unexpected option value for `keep_sandboxes`: {resolved_value}")
@staticmethod
def compute_pants_ignore(buildroot, global_options):
"""Computes the merged value of the `--pants-ignore` flag.
This inherently includes the workdir and distdir locations if they are located under the
buildroot.
"""
pants_ignore = list(global_options.pants_ignore)
def add(absolute_path, include=False):
# To ensure that the path is ignored regardless of whether it is a symlink or a directory, we
# strip trailing slashes (which would signal that we wanted to ignore only directories).
maybe_rel_path = fast_relpath_optional(absolute_path, buildroot)
if maybe_rel_path:
rel_path = maybe_rel_path.rstrip(os.path.sep)
prefix = "!" if include else ""
pants_ignore.append(f"{prefix}/{rel_path}")
add(global_options.pants_workdir)
add(global_options.pants_distdir)
add(global_options.pants_subprocessdir)
return pants_ignore
@staticmethod
def compute_pantsd_invalidation_globs(
buildroot: str, bootstrap_options: OptionValueContainer
) -> tuple[str, ...]:
"""Computes the merged value of the `--pantsd-invalidation-globs` option.
Combines --pythonpath and --pants-config-files files that are in {buildroot} dir with those
invalidation_globs provided by users.
"""
invalidation_globs: OrderedSet[str] = OrderedSet()
# Globs calculated from the sys.path and other file-like configuration need to be sanitized
# to relative globs (where possible).
potentially_absolute_globs = (
*sys.path,
*bootstrap_options.pythonpath,
*bootstrap_options.pants_config_files,
)
for glob in potentially_absolute_globs:
# NB: We use `relpath` here because these paths are untrusted, and might need to be
# normalized in addition to being relativized.
glob_relpath = (
os.path.relpath(glob, buildroot) if os.path.isabs(glob) else os.path.normpath(glob)
)
if glob_relpath == "." or glob_relpath.startswith(".."):
logger.debug(
f"Changes to {glob}, outside of the buildroot, will not be invalidated."
)
continue
invalidation_globs.update([glob_relpath, glob_relpath + "/**"])
# Explicitly specified globs are already relative, and are added verbatim.
invalidation_globs.update(
("!*.pyc", "!__pycache__/", ".gitignore", *bootstrap_options.pantsd_invalidation_globs)
)
return tuple(invalidation_globs)

... but they're used purely by convention. In this case that might look like calling: GlobalOptions.resolve_remote_execution(global_options, dynamic_remote_options) in a @rule.

There isn't a way to "overlay" the DynamicRemoteOptions onto the options bootstrapper... but you could think of them as another options "Rank"... "DYNAMIC_OPTIONS" or "PLUGIN_OPTIONS" or something:

@total_ordering
class Rank(Enum):
# The ranked value sources. Higher ranks override lower ones.
NONE = (0, "NONE") # The value None.
HARDCODED = (1, "HARDCODED") # The default provided at option registration.
CONFIG_DEFAULT = (2, "CONFIG_DEFAULT") # The value from the DEFAULT section of the config file.
CONFIG = (3, "CONFIG") # The value from the relevant section of the config file.
ENVIRONMENT = (4, "ENVIRONMENT") # The value from the appropriately-named environment variable.
FLAG = (5, "FLAG") # The value from the appropriately-named command-line flag.
... and then have a facility for plugins to add options? ...pretty advanced.

A hacky way to accomplish something similar would be to essentially convert the DynamicRemoteOptions back into CLI args, and append those during bootstrap. Oy.

@Eric-Arellano
Copy link
Contributor Author

Thanks for the feedback!

In this case that might look like calling: GlobalOptions.resolve_remote_execution(global_options, dynamic_remote_options) in a @rule.

The issue is we would need in the rule to still know whether we're in the plugin resolver or not. Generally, we'd want to use DynamicRemoteOptions as is. But in same cases, we want to use its .disabled() variant. Now, that could be injected into the graph!

But at that point, maybe it's easier to have some other sentinel mechanism so that these 3 relevant rules can check if they're in the plugin resolver and change their logic accordingly?

@Eric-Arellano Eric-Arellano added this to the 2.15.x milestone Oct 21, 2022
@Eric-Arellano
Copy link
Contributor Author

But at that point, maybe it's easier to have some other sentinel mechanism so that these 3 relevant rules can check if they're in the plugin resolver and change their logic accordingly?

@stuhood thoughts on how we could know inside platform_rules.py that it's the plugin resolver session?

@stuhood
Copy link
Member

stuhood commented Oct 27, 2022

Now, that could be injected into the graph!

Anything can be injected as a singleton without a lot of fanfare in EngineInitializer.setup_graph_extended... you could pass a wrapped boolean, pass the entire DynamicRemoteOptions in and then use it to call static methods, etc.

@Eric-Arellano Eric-Arellano removed their assignment Dec 28, 2022
@Eric-Arellano
Copy link
Contributor Author

@stuhood it looks like this is still an issue, right? I thought I had seen you fix something related, but can't find it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants