Image Manager trait for Docker #508
Conversation
enclave_build/src/lib.rs
Outdated
| /// If the 'docker_dir' argument is Some (i.e. --docker-dir flag is used), the docker daemon will | ||
| /// always be used to build the image locally and store it in the docker cache. | ||
| /// | ||
| /// If the 'oci_image' argument is Some (i.e. the --image-uri flag is used) and the '--docker-dir' |
There was a problem hiding this comment.
Not sure this part of comment is relevant here: there is no oci_image argument here. probably you wanted to write about docker_image argument and pulling the image with a help of docker daemon?
There was a problem hiding this comment.
Left from older code, missed it.
enclave_build/src/image_manager.rs
Outdated
| @@ -21,6 +22,8 @@ pub trait ImageManager { | |||
| fn architecture(&self) -> Result<String>; | |||
| /// Returns two temp files containing the CMD and ENV expressions extracted from the image | |||
| fn extract_expressions(&self) -> Result<(NamedTempFile, NamedTempFile)>; | |||
| /// Returns true if the image manager uses the Docker daemon | |||
| fn use_docker(&self) -> bool; | |||
There was a problem hiding this comment.
I find this method redundant, then the whole idea of making this trait abstract looses its sense.
enclave_build/src/lib.rs
Outdated
| // If the docker daemon should be used, then call linuxkit with the '-docker' flag, which | ||
| // makes linuxkit search the image in the docker cache first. | ||
| // Otherwise, do not add the flag and let it pull the images itself, without docker. | ||
| if self.image_manager.use_docker() { |
There was a problem hiding this comment.
At this moment we only initialize it as DockerImageManager. What's the point for this if here?
And if you plan to also initialize OciImageManager in this module then can not we store this isDocker externally rather providing a method from the inside?
There was a problem hiding this comment.
Removing this function and replacing it with a variable or a type match.
- Changed docker.rs to contain the implementation of ImageManager trait. The DockerImageManager struct uses the old logic with shiplift. - Changed lib.rs to use a runtime-set image manager, which can be oci or docker, depending if the new --oci-uri flag is used or not. - Refactored the 'inspect_image' method to return OCI-compliant image metadata in case OciImageManager is used, instead of a Docker-specific one. - Changed enclave_build/src/main.rs to include the new additions. - Matched the OCI behavior by pulling or building the Docker image on manager creation based on the present arguments. Signed-off-by: Calin-Alexandru Coman <calincom@amazon.com> Signed-off-by: Raul-Ovidiu Moldovan <raulmldv@amazon.com>
The build EIF process should have the same workflow for both OCI and Docker. In the previous PR #438 I introduced and implemented the Image Manager trait for OCI while changing the workflow to pull/build the required container image on manager instantiation. This way subsequent pulls are no longer needed for inspect operations.
The Docker workflow was using the docker daemon to access the local image storage (still using the pull API in Rust) thus having a different interface that the trait offers. With this refactoring we can manipulate both types of images with the same operations.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.