sven-lange-last commented on a change in pull request #4503: Add optional
config for appending custom registry to user provided images
URL: https://github.com/apache/openwhisk/pull/4503#discussion_r325577206
##########
File path:
core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainerFactory.scala
##########
@@ -60,10 +62,14 @@ class DockerContainerFactory(instance: InvokerInstanceId,
userProvidedImage: Boolean,
memory: ByteSize,
cpuShares: Int)(implicit config: WhiskConfig,
logging: Logging): Future[Container] = {
+ val registryConfig =
+ ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
+ val image =
+ if (userProvidedImage && registryConfig.url.isEmpty) Left(actionImage)
+ else Right(actionImage.localImageName(registryConfig.url))
Review comment:
My understanding is that the following behaviour is desired for blackbox
(i.e. user provided) images identified by a user provided image reference (see
https://github.com/docker/distribution/blob/master/reference/reference.go):
* If the image reference contains a domain, keep the image reference as is
to pull from the repository in the image reference.
* If the image reference contains NO domain and NO global user image
registry is configured (`registryConfig.url.isEmpty == true`), keep the image
reference as is. It's up to the used container factory to convert to a
canonical reference including a domain and path.
* If the image reference contains NO domain and a global user image registry
is configured (`registryConfig.url.isEmpty == false`), add the global user
image registry as domain to the image reference.
I think this is exactly the behaviour that following code implements:
```
val registryConfig =
ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
val imageRef = ContainerFactory.resolveImage(actionImage, registryConfig)
```
That's why I proposed this code because it already addresses the three cases
listed above.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
