Propagate container configuration from base image?

[coollog]

Thumbs up if this is useful for you.

Configuration currently propagated from the base image:

• environment variables
• workingdir
• entrypoint
• cmd
• history
• labels
• volumes
• healthcheck
• exposed ports

UPDATE(04/23/2020):

• user (USER not respected / java running as root user #2421)

[chanseokoh]

Hmm... if the images built by docker in the normal way don't inherit the container configuration, I think we don't need to do this. OTOH, if they do, this is something we can consider.

[coollog]

Yes, I believe Dockerfile FROM uses the container configuration from the base image. Any instructions afterwards are extensions on that base container configuration.

[essh]

This is useful for me to pick up on the Env from a base image (specifically PATH and JAVA_HOME). As already mentioned these were previously coming along when using a DockerFile FROM and would save having to specify them when launching a container built with jib.

[sunnychanwork]

Without this feature we will be forced to copy/link the java executables into the /usr/bin directory which might not be desirable.

[coollog]

I think for this feature, we can start with propagating environment variables since that seems to be the most useful case.

[TadCordle]

Adjusting the milestone since we were only planning on propagating environment variables for this next release.

[chanseokoh]

For those who are following this issue, environment variables from the base image are now being propagated into the target image, starting from Jib 0.9.8.

[chanseokoh]

Turns out we also need to propagate WorkingDir to make use of distroless-jetty as a base image for #431. Will work on this.

[chanseokoh]

Also ExposedPorts in the container configuration.

However, this is a bit different: we have a config parameter for setting the value. The question becomes then, should we add ports on top of the ports from the base image? Or, does it still make sense to replace the base value with the value given by our config parameter? Maybe, we inherit ports only when not configured in Jib, but when configured, just replace the ports from the base?

[chanseokoh]

#924: Entrypoint and CMD

[paivagustavo]

@chanseokoh I like using the base image ports if none is specified.

Also, I think volumes must be inherited as well. After the PR for supporting volume configuration (#1215) is accepted we can do it.

[chanseokoh]

@GuustavoPaiva yeah, we actually do need to inherit the ports and the volumes, which docker does with FROM in Dockerfile.

[coollog]

We have two left: volumes and healthcheck

Updated original post with a TODO list.

[chanseokoh]

I think we haven't handled ports either. Updated the TODO list.

[chanseokoh]

Also need to check if User can/should be inherited.

UPDATE(04/23/2020): issue #2421 reported.

[chanseokoh]

Just in case, I am documenting here that there is one subtle to be careful about when enabling inheriting ports, volumes, etc. In BuildImageSteps, the code currently looks like

      // inherit properties
      imageBuilder
          .addEnvironment(baseImage.getEnvironment())
          .addLabels(baseImage.getLabels())
          .setWorkingDirectory(baseImage.getWorkingDirectory());
      ...
      // set properties when explicitly given
      if (containerConfiguration != null) {
        imageBuilder
            .addEnvironment(containerConfiguration.getEnvironmentMap())
            ...
            .setExposedPorts(containerConfiguration.getExposedPorts())
            .setVolumes(containerConfiguration.getVolumes())
            ...
      }

To inherit volumes for example, one would start with

      // inherit properties
      imageBuilder
          ...
          .setWorkingDirectory(baseImage.getWorkingDirectory())
          .setVolumes(baseImage.getVolumes());  // inherit volumes, YEAH!
      ...
      // set properties when explicitly given
      if (containerConfiguration != null) {
        ...
      }

In addition to this, we should update the code to selectively override properties:

      // inherit properties
      imageBuilder
          ...
          .setVolumes(baseImage.getVolumes());  // inherit volumes, YEAH!

      // set properties when explicitly given
      if (containerConfiguration != null) {
        ...
        // should override only when actually given
        if (containerConfiguration.getExposedPorts() != null) {
            imageBuilder.setExposedPorts(containerConfiguration.getExposedPorts());
        }
        // should override only when actually given
        if (containerConfiguration.getVolumes() != null) {
            imageBuilder.setVolumes(containerConfiguration.getVolumes());
        }
      }

UPDATE: we should actually do addExposedPorts() and addVolumes() instead of replacing them. We may still need to do the null check unless the add methods ignore null.

[coollog]

I think we should be adding the volumes/ports rather than replacing them since that's the behavior of Dockerfile builds as well. So, having port 1234 on the base image and setting port 5678 to expose would result in an image with ports 1234 and 5678 exposed.

[chanseokoh]

Ah, you're right. We should actually do addExposedPorts() and addVolumes() instead of replacing them. I wonder if we need to ensure no duplicate.

[coollog]

When generating the container configuration, there would be no duplicates since the ports/volumes are maps to empty objects. We should probably just have them be represented as sets instead of lists in the image builder and other parts of the API too though.

[coollog]

@essh @cwensel @thekguy @sunnychanwork @joelhandwell @jcrossley3 @Tanmayshetty All of these configurations are propagated from the base image now in version 0.10.1!