I defer to vadim on this one: from my pov, as long as:
  * there's no network access
  * there's no additional risk of failure at service-run-time

Then I think it's fine. I think in option A, we would require puppet to pre-initialize the environments during its run (so that any errors that occur can be reported, and the previous runtime environment could continue to run the system services; rollout of the new service would be blocked on getting the python stuff working).

The other way to do it would be to have puppet fully manage the environments; it invokes vpython to generate a python environment in some determinable folder, and then uses the python in that folder to run any tasks. It would also have rules to assert that none of the other folders exist. Is this possible?

The problem with the "Puppet fully manage" idea is that Puppet runs continuously throughout the lifecycle of the system. Imagine:

1) Puppet runs, installs VENV_A.
1) System runs "service_manager" @ VENV_A
2) Puppet runs, installs VENV_B.

In this case, how would Puppet know that (1) is still running?

TBH I'm in favor of Option A b/c it is complete and simple. Vadim had some concern about initial VirtualEnv setup delay during first-run and potential for failure, though, which is how Option B came about.

OTOH currently Puppet rips "infra_python" out from underneath of running instances. If the VirtualEnv changes, or if the package layout or code changes, it's entirely possible for "service_manager" to start, then later run some code that expects a file or import to work and find that it no longer exists.

On Linux systems, services shut down while they are being upgraded. Is it reasonable at all to have Puppet shut down all "infra_python" services when it determines that an upgrade is warranted?

Even w/ individual "vpython" and lock protection, we're still in a situation where the "infra_python" CIPD package contents, layout, etc., could change while "infra_python" code is running. If the code references any externals after it starts up, this is a fundamental problem. Maybe having root prune the environment after 7 days isn't so bad...

Few independent thoughts:

1. As I said before multiple times, I'm against option A, since it introduces possibility of "delayed" errors (when building per-user environment) that happen at uncontrollable, not-well-monitored moments. As much provisioning steps as possible should be happening within Puppet run (all of them, ideally). E.g if there's not enough disk space to build venv, we want Puppet run to fail (keeping all existing services intact), instead of "succeeding", only to make service_manager fail to start after the next reboot (which happens at unknown moment in time, perhaps delayed for days).

2. > OTOH currently Puppet rips "infra_python" out from underneath of running instances

Yeah, it kind of does. But in practice it looks similar to running "gclient sync" while Buildbot process is still up: it updates files in-place, and doesn't change directory inode or anything like that. So if processes do not load modules during runtime, it is mostly harmless (well.. mostly).

Now, IIUC, vpython will try to _delete_ all files, and then recreate new environment in a new directory with new inode. This is more destructive procedure. Python may freak out if it finds its sys.prefix has been deleted. (Or maybe not).

I think "gc old venvs after N days" is actually better than both what we had and what we have with existing vpython implementation. If this is exposed as "vpython gc", we can also try to hook it up to run after reboot (though this may be complicated).

3. I've been thinking about idempotency of vpython installs w.r.t. Puppet integration. CIPD achieves them by using special "cipd puppet-check-updates" subcommand, which is used as "onlyif" guard in 'exec' resource. This is required to:
  * Make sure puppet rerun cipd if previous install is "dirty".
  * Avoid Puppet emitting "something has changed!!!" signal, when nothing has really changed. Without it, puppet will be restarting all services that depend on cipd-installed packages all the time (thinking their code has changed).

I believe vpython will need similar functionality: a subcommand that says whether the install root is good enough or it needs more work.

Also, I'm curios how you see API for using vpython for installing stuff via Puppet. I can imagine few variants:

vpython { "/opt/python-venv":
  manifest => "<vpython manifest in plain text>"
  ...
}

or 

vpython { "/opt/python-venv":
  bundle  => "infra/venv/crappy-python-env"
  version => "git_revision:...."
  ...
}

(where 'bundle' is a CIPD package with vpython manifest and all necessary wheels).

--

I guess, I see vpython+Puppet as a tool to install isolated virtual environments, not as a tool that runs some programs, which may install environments. I mean, there's no single script that can have the vpython manifest in it, the manifest is supplied from outside as a parameter...

Hm, I forgot that we actually want to also deploy non-wheel code too (for infra_python). Maybe

vpython { "/opt/infra-python":
  package  => "infra/infra_python",  # CIPD package with all the code and manifest
  manifest => "run.py",  # the file that embeds the manifest, or manifest itself
  ...
}

It will either use cipd {...} resource inside to install 'infra/infra_python', or will do necessary cipd calls itself. (I even consider possibility of making vpython its own flavor of CIPD client, so it can be fetched from CIPD backend directly, in same way we bootstrap cipd client now).

The property I very much like to see in puppet-level API is "atomicity": if you want to install something managed by vpython, you declare it as single resource.

Expanding more on the "gc" idea, what about:

1) Add a "managed" boolean flag to the VirtualEnv protobuf. "vpython" automatic pruning will ignore managed VirutalEnv.
2) Add a "vpython provision" subcommand that will accept 1 or more "spec" files, provision those VirtualEnv, mark them all "managed: true", and write the set of VirtualEnv to a manifest file.
3) Add a "vpython gc" command that prune any VirtualEnv, managed or otherwise, that's not in the current manifest.
4) Add support in "run.py" to read a local file (//infra/vpython_root) containing the "vpython" root to use.

We will rig things such that:
1) Puppet uses "vpython provision" to ensure that a set of VirtualEnv are installed into a root.
2) Puppet will drop a "//infra/vpython_root" file into the CIPD package that it provisions to let "run.py" know to use the "infra_python" vpython root.
3) We will run "vpython gc" on boot (as part of starting service manager?).

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot