duynguyen 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_r325299534
##########
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:
I guess you meant it to be:
```
val registryConfig =
ContainerFactory.resolveRegisterConfig(userProvidedImage,
runtimesRegistryConfig, userImagesRegistryConfig)
val imageRef = actionImage.localImageName(registryConfig.url)
val image = if (userProvidedImage) Left(actionImage) else Right(imageRef)
```
(`actionImage` in Left, not `imageRef`)
However the condition is not the same as before anymore. In the case of
`userProvidedImage==true`, the raw `imageName` object is passed into
DockerContainer, which will pull the "public image name" (without custom
blackbox registry), see
[DockerContainer.scala#L105](https://github.com/apache/openwhisk/blob/master/core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/docker/DockerContainer.scala#L105).
Hence I added a condition for registryConfig being empty or not.
In general this flow does not fit well into the existing flow of
DockerContainerFactory. I am working on an enhancement to get rid of
`publicImageName`, make `localImageName` (to be renamed) a single image name
resolver, and apply it throughout the code base. I will push that change for
review.
----------------------------------------------------------------
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
