canonical / operator

Pure Python framework for writing Juju charms
Apache License 2.0
244 stars 119 forks source link

Charm venv shadows system python dependencies #1389

Closed silverdrake11 closed 1 hour ago

silverdrake11 commented 2 hours ago

When running subprocesses within the charm code, the Juju Python virtual environment /var/lib/juju/agents/unit-landscape-server-0/charm/venv/ is included in the system’s PYTHONPATH. This causes conflicts, as we need to use the system Python and its dependencies, not those from the Juju environment.

For example this bug https://bugs.launchpad.net/landscape-charm/+bug/2076926, installing a later version of the pydantic package via apt doesn't do anything because the version from the Juju charm virtual environment takes precedence. A similar issue occurred with the distutils package, where the system version was overshadowed.

We used this workaround to make sure the system Python paths are used by removing Juju's virtual environment from PYTHONPATH before invoking subprocesses:

def get_modified_env_vars():
    """
    Because the python path gets munged by the juju env, this grabs the current
    env vars and returns a copy with the juju env removed from the python paths
    """
    env_vars = os.environ.copy()
    logging.info("Fixing python paths")
    new_paths = [path for path in sys.path if "juju" not in path]
    env_vars["PYTHONPATH"] = ":".join(new_paths)
    return env_vars

subprocess.check_call(call, env=get_modified_env_vars())
tonyandrewmeyer commented 2 hours ago

Hi @silverdrake11! The packaging of the dependencies and setting the PYTHONPATH up in the dispatch script is handled by charmcraft, not by ops, so you probably have to report this over there.

Note that the recommendation going forward is to not use the system packages at all, and to specify all dependencies (including indirect, transitive ones) in a lockfile that charmcraft uses.

The discussion probably belongs over in charmcraft or in Matrix, but is there a reason that you're installing pydantic with apt rather than specifying it in the requirements for the charm? If there's some third-party software that's using the system packages, then you should still use the charm requirements and environment for the charm, rather than trying to have both use the same one.

Perfect5th commented 2 hours ago

Hi @tonyandrewmeyer , just chiming in here.

The charm executes a script from an installed system package in a subprocess, which fails because it inherits PYTHONPATH (well the whole env) from the charm, hence the workaround @silverdrake11 listed above.

If there's a recommended way to execute a script from a system package without needing to mangle the env, we can definitely just do that.

Either way, based on what you've said about the dispatch script re: charmcraft, doesn't seem like this is an issue with ops. Thanks for the response!

tonyandrewmeyer commented 1 hour ago

If there's a recommended way to execute a script from a system package without needing to mangle the env, we can definitely just do that.

My recommendation would be that you always pass in an explicitly defined environment in a situation like this. I feel that's the safer choice - you're ensuring that you're not inadvertently leaking anything that the third-party code shouldn't see and ensuring that there aren't any collisions or unintended effects.

Either way, based on what you've said about the dispatch script re: charmcraft, doesn't seem like this is an issue with ops. Thanks for the response!

:+1: