hundreds-father-404
09/13/2022, 11:00 PMplatform
field on docker_environment
. We could either make it required, or linux_x86_64
seems like a reasonable defaultrequired
field is pretty reasonable...it's not very common you're defining new `docker_environment`s fwict
The default scares me because it's subtle and confusing when that default is wrongenough-analyst-54434
09/13/2022, 11:13 PMhundreds-father-404
09/13/2022, 11:19 PMenv
"Architecture": "arm64",
"Variant": "v8",
"Os": "linux",
witty-crayon-22786
09/13/2022, 11:21 PMrather than needing to run a process that invokesyea: but thinking forward to remote execution, @fast-nail-55400 and i discussed not assuming that we have a docker image locally which corresponds to the remote execution environmentenv
hundreds-father-404
09/13/2022, 11:21 PMdocker inspect
to set up CompleteEnvironmentVars
and Platform
, and then system_binaries.py
can proceed like normal w/o modificationwitty-crayon-22786
09/13/2022, 11:21 PMhundreds-father-404
09/13/2022, 11:22 PMyea: but thinking forward to remote execution, @fast-nail-55400 and i discussed not assuming that we have a docker image locally which corresponds to the remote execution environmentWe will know when it's an RE environment thanks to the target type. So perhaps better to solve this the simplest way for Docker, and then we solve RE in the way that makes sense for it, e.g.
env
?witty-crayon-22786
09/13/2022, 11:23 PMhundreds-father-404
09/14/2022, 6:20 PMdocker inspect
approach, it will look like this:
@rule
def current_platform(env_tgt: EnvironmentTarget) -> Platform:
docker_image = env_tgt.docker_image()
if docker_image is None:
return Platform.create_for_localhost()
# TODO(#16852): Use the engine to compute this
return Platform.linux_arm64
I don't think we can call a #[pyfunction]
from native_engine
because we need to use the docker::CommandRunner
so that we can properly pull down missing images; a free function wouldn't have a way to access to the current SchedulerSession/CommandRunner
I don't think SessionValues
is relevant, because we can have n Docker images per session
So I'm thinking it looks something like Rust defining DockerImageInformation
and DockerImageInformationRequest
, and registering an intrinsic rule? Then this rule will do Get(DockerImageInformation, DockerImageInformationRequest)
I think making it a required field is pretty reasonable...it's not very common you're defining new docker_environments fwictThis approach is not reasonable. I didn't realize that
python:3.9
is Linux Arm64 from my mac, but Linux x86-64 from my dev boxfast-nail-55400
09/14/2022, 6:35 PM--platform
argument to docker pull
can be essential to use. If on M1, you docker pull
a python:3.9
image with platform set to x86_64, but then try to run docker run
without the x86_64 argument, Docker will invoke the container as ARM and not x86_64 and warn about the platform mismatch.% docker pull --platform=x86_64 python:3.9
3.9: Pulling from library/python
23858da423a6: Pull complete
326f452ade5c: Pull complete
a42821cd14fb: Pull complete
8471b75885ef: Pull complete
8ffa7aaef404: Pull complete
15132af73342: Pull complete
f4aeaae2f8da: Pull complete
5d8aa591a389: Pull complete
4645a1f8dbad: Pull complete
Digest: sha256:54c4e59cdab21669b1a5417f5b70cb1cf221530f423661d9a8dbee5576160e2a
Status: Downloaded newer image for python:3.9
<http://docker.io/library/python:3.9|docker.io/library/python:3.9>
% docker run python:3.9
WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
% docker pull --platform=linux/amd64 python:3.9
3.9: Pulling from library/python
Digest: sha256:54c4e59cdab21669b1a5417f5b70cb1cf221530f423661d9a8dbee5576160e2a
Status: Image is up to date for python:3.9
<http://docker.io/library/python:3.9|docker.io/library/python:3.9>
% docker run --platform=linux/amd64 python:3.9
hundreds-father-404
09/14/2022, 7:03 PMThis approach is not reasonable. I didn't realize that python:3.9 is Linux Arm64 from my mac, but Linux x86-64 from my dev boxNack, because I forgot that on macOS ARM you can specify the platform so it's x86
fast-nail-55400
09/14/2022, 7:11 PMplatform
for Docker command runner later today.hundreds-father-404
09/14/2022, 7:13 PMplatform
field to docker_environment
2. I fix it so Platform
is always injected into the Process
, whereas now individual call sites have to set it
3. You consume Platform
in the Docker Rust code
That should result in always consistently using --platform
whenever Docker is executedfast-nail-55400
09/14/2022, 7:13 PMhundreds-father-404
09/14/2022, 7:13 PMfast-nail-55400
09/14/2022, 7:14 PMhundreds-father-404
09/14/2022, 7:48 PMwitty-crayon-22786
09/14/2022, 7:49 PM