openwhisk-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [openwhisk] sven-lange-last commented on a change in pull request #4503: Add optional config for appending custom registry to user provided images
Date Wed, 18 Sep 2019 09:36:05 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message