intel-device-plugins-for-kubernetes icon indicating copy to clipboard operation
intel-device-plugins-for-kubernetes copied to clipboard

build: build demo/app images using the same Dockerfile naming as plugins

Open ttzeng opened this issue 2 years ago • 5 comments

  1. The "demos" make target in the Makefile implies the docker build materials of demo images must be in the level-1 subdirectories under the demo folder, which prevents us to categorize demo images by plugin types, and leads to an unorganized demo directory.
  2. To ease of Dockerfile maintenance, the current design uses single directory build/docker/ to host all Dockerfiles of plugin images, and uses the filename to define the image name. We'd like to pursue the same logic for building the demo images, though the mechanism has to be extended to build images with given contexts.
  3. The current build-image.sh script assumes the image Dockerfiles are in the same folder as the script. To reuse the script for building both the plugin images and the app images, we need to pass the paths of the Dockerfiles to the script and decouple the use of the script's pathname.
  4. Some demo images have been used in the e2e validation, which are more than just demos, so we'd like to move them out of the demo directory.

This commit creates a layout and make rules for building demo/app images using a similar mechanism used for building the plugin images as illustrated:

build +- /build-image.sh <-- modified script to decouple the script's path from the Dockerfiles folder
      +- /plugin-images/ <-- previous build/docker/ directory containing Dockerfiles of plugin images
      |
      +- /app-images +- /image1.Dockerfile  <-- Dockerfile only for images with empty build context
                     |- /image2.Dockerfile  <-- If a peer directory with the same image name is present, then
                     |                          that directory will be used as the context for docker build
                     +- /image2 +- /file    <-- files/scripts used by docker build
                                +- /...     <-- other files in the build context

Signed-off-by: Tonny Tzeng [email protected] Closes: #954

ttzeng avatar Apr 03 '22 14:04 ttzeng

Codecov Report

Merging #956 (b5aa8c9) into main (7598c0d) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #956   +/-   ##
=======================================
  Coverage   52.97%   52.97%           
=======================================
  Files          40       40           
  Lines        4232     4232           
=======================================
  Hits         2242     2242           
  Misses       1870     1870           
  Partials      120      120           

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

codecov-commenter avatar Apr 03 '22 17:04 codecov-commenter

@ttzeng thanks! I think this is the right approach. I could we have only one build-image.sh?

mythi avatar Apr 04 '22 04:04 mythi

@ttzeng thanks! I think this is the right approach. I could we have only one build-image.sh?

@mythi , since the original script uses its path to locate the Dockerfiles, I think we would need to pass that path to the script, so that we can reuse it for both plugin-images and app-images. I've changed the calling convention of the script, please review and comment. Thanks.

ttzeng avatar Apr 06 '22 02:04 ttzeng

@ttzeng thank you for the PR! The approach looks good to me so far. It's much more structured and easy to use than the current one.

bart0sh avatar Apr 06 '22 11:04 bart0sh

@bart0sh , this article is also worthy to read:

You should audit your use of docker build to keep your build contexts small. Accidentally including unnecessary files can
result in an excessively large build context, which will lead to longer builds.

Most importantly, just learn a key security issue from the Other Build Context Issues section in the article:

Not using .dockerignore can introduce other issues, too. A Dockerfile with this line is particularly problematic:

    COPY . /my-app

This will copy everything in your working directory. This might seem like a good idea until you realize that your .git
history and any secret files will also end up within your container.

Copying an unfiltered build context also prevents Docker layer caching from working effectively. As something in your working
directory will probably change between builds, Docker would need to run the COPY instruction every time. This would create a
new layer—and new layers for any subsequent instructions—even if the assets you’re interested in haven’t changed.

ttzeng avatar Apr 14 '22 00:04 ttzeng

close due to inactivity

mythi avatar Aug 24 '22 06:08 mythi