[GitHub] sven-lange-last commented on issue #4176: additional container factory dns configuration

[GitBox <gi...@apache.org>]

sven-lange-last commented on issue #4176: additional container factory dns configuration
URL: https://github.com/apache/incubator-openwhisk/pull/4176#issuecomment-452724465
 
 
   While using the `DOCKER_VERSION` variable should basically work, we have to keep in mind that this variable is not generally defined on systems where a `docker` cli client is installed. It's just defined in the OpenWhisk invoker image - see here: https://github.com/apache/incubator-openwhisk/blob/master/core/invoker/Dockerfile#L8.
   
   For this reason, if somebody should use the Docker container factory in an invoker that is not running in a container based on that image, we cannot warrant the variable is defined.
   
   In order to have a more robust solution, I suggest to consider using this command to determine the `docker` cli client version:
   
   ```
   # docker version --format '{{.Client.Version}}'
   17.10.0-ce
   ```
   
   This requires more code changes but should be much safer. The version info is required when creating a new Docker container, i.e. in `DockerContainer.create()`. Said method receives a `DockerApiWithFileAccess` instance as implicit parameter so that we can extend the `DockerApi` trait and its implementation to determine the `docker` cli command version. It would be good to make version determination a singleton such that the command only needs to be run once. Otherwise, container creation would get slower.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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