Add support for submitting a prebuilt image

[kiendang]

• [x] Rename submit_dockerfile to submit_container_image.
• [x] submit_container_image now accepts a docker_config of a more generic WorkerConfig type which could be DockerWorkerConfig or PrebuiltWorkerConfig
• [x] Add unit tests

Works with (modified) notebook 11. Only need to

• [ ] Add integration test

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[shubham3121]

We might also have to generalize the get_by_docker_config stash method to get both Prebuilt and DockerConfig

[kiendang]

@yashgorana @shubham3121 keep getting TimeOut exception running test_pool_launch with a prebuilt image even though I used a tiny image and increased timeout to 3 mins

https://github.com/OpenMined/PySyft/blob/33aef2718f9e430aaf98f91ce3f64489bc90b60a/packages/syft/src/syft/custom_worker/runner_k8s.py#L48-L62

This just raised a TimeOut error with no other information. Do you know what might caused it or how to find out more?

[kiendang]

Ok I think I might know what the problem is. I think it's because we set the registry to the local k3d registry so k8s wouldn't be able to pull from docker.io.

[kiendang]

~hmm after looking into the k8s json file generation I don't think that's the issue.~

I think it might be lol.

[shubham3121]

Nice catch @kiendang . Your right its the registry. What we need to do is create a image registry object during submission of pre built config file and indicate the same registry during image pull. I think you're correct that its using k3d registry by default dueing worker spawn

[kiendang]

@yashgorana @shubham3121 May I use this opportunity to correct some inconsistencies in arg names of various API endpoints?

• worker_image_service.push(image: UID =) where everywhere else uses image_uid
• worker_image_service.build(pull: bool =), worker_pool_service.create_image_and_pool_request(pull_image: bool =)
• worker_pool_service.launch(name: str = ) where everywhere else uses pool_name
• reg in reg_username and reg_password is very ambiguous. We should change to registry_username and registry_password. They're not much longer and we already use registry_uid.

[kiendang]

We also use tag in a lot of places where it should be image name instead. python:3-slim is not the image tag, it's the image name. 3-slim is the tag.

[madhavajay]

We also use tag in a lot of places where it should be image name instead. python:3-slim is not the image tag, it's the image name. 3-slim is the tag.

Yes although, a tag by itself and an image by itself arent really useful, we can extract the image and repo from the tag, but not the other way around.

I would call this image_tag or something where image and tag are mandatory but but repo is optional.

[kiendang]

@yashgorana @shubham3121 May I use this opportunity to correct some inconsistencies in arg names of various API endpoints?

* `worker_image_service.push(image: UID =)` where everywhere else uses `image_uid`

* `worker_image_service.build(pull: bool =)`, `worker_pool_service.create_image_and_pool_request(pull_image: bool =)`

* `worker_pool_service.launch(name: str = )` where everywhere else uses `pool_name`

* `reg` in `reg_username` and `reg_password` is very ambiguous. We should change to `registry_username` and `registry_password`. They're not much longer and we already use `registry_uid`.

Will put these in a separate PR.