Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify registry build #20622

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kariya-mitsuru
Copy link

Comprehensive Summary of your change

  1. make/photon/Makefile

    Remove cd command since the current directory is changed in make/photon/registry/builder.

  2. make/photon/registry/Dockerfile.base

    Change the owner of /etc/pki/tls/certs when building the base image.
    (Since files under /etc/pki/tls/certs are not affected by binary build.)

  3. make/photon/registry/Dockerfile

    • Remove changing the owner of /etc/pki/tls/certs since the change has been moved to the base image.
    • The owner/permission changes of files copied from the context are now performed simultaneously when the COPY command is executed.
      (If COPY and the owner/permission changes were separated, both image layers before and after change would be created, making the image unnecessarily large.)
    • Add --link option to COPY command.
      (This will improve image build efficiency since the base image will not be extracted at build time.)
  4. make/photon/registry/builder.

    • Move set -e (exit immediately on error) to the top.
    • There is no error command, so change it to the echo command.
    • Remove cur variables that are no longer used by using ~- and cd -.
    • Add --depth 1 option to git clone command.
      (Since we only need the specified version of the source to build, we don't need the whole history, and this reduces the amount of transfer at clone time.)
    • Change the docker build command to specify the source file directly with the -f option instead of copying Dockerfile.binary.
    • Change the docker build command to directly output the binary file without creating a container image and container by specifying the output directory with the -o option.
  5. make/photon/registry/Dockerfile.binary.

    • Remove PREFIX variable specified at make command line, since it is not used.
    • Change make build target from binaries to bin/registry.
      (Since binaries other than bin/registry are not used.)
    • Add a stage to extract only binary files, since make/photon/registry/builder now outputs binary files directly.

Issue being fixed

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.
1. `make/photon/Makefile`

   Remove `cd` command since the current directory is changed in
   `make/photon/registry/builder`.

2. `make/photon/registry/Dockerfile.base`

   Change the owner of `/etc/pki/tls/certs` when building the base image.
   (Since files under `/etc/pki/tls/certs` are not affected by binary build.)

3. `make/photon/registry/Dockerfile`

   - Remove changing the owner of `/etc/pki/tls/certs` since the change
     has been moved to the base image.
   - The owner/permission changes of files copied from the context are now
     performed simultaneously when the `COPY` command is executed.
     (If `COPY` and the owner/permission changes were separated, both image
     layers before and after change would be created, making the image
     unnecessarily large.)
   - Add `--link` option to `COPY` command.
     (This will improve image build efficiency since the base image will
     not be extracted at build time.)

4. `make/photon/registry/builder`.

   - Move `set -e` (exit immediately on error) to the top.
   - There is no `error` command, so change it to the `echo` command.
   - Remove `cur` variables that are no longer used by using `~-` and `cd -`.
   - Add `--depth 1` option to `git clone` command.
     (Since we only need the specified version of the source to build,
     we don't need the whole history, and this reduces the amount of transfer
     at clone time.)
   - Change the `docker build` command to specify the source file directly
     with the `-f` option instead of copying `Dockerfile.binary`.
   - Change the `docker build` command to directly output the binary file
     without creating a container image and container by specifying the
     output directory with the `-o` option.

5. `make/photon/registry/Dockerfile.binary`.

   - Remove `PREFIX` variable specified at `make` command line, since it is
     not used.
   - Change `make` build target from `binaries` to `bin/registry`.
     (Since `binaries` other than `bin/registry` are not used.)
   - Add a stage to extract only binary files, since
     `make/photon/registry/builder` now outputs binary files directly.

Signed-off-by: Mitsuru Kariya <mitsuru.kariya@nttdata.com>
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.25%. Comparing base (b7b8847) to head (64956e3).
Report is 239 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #20622      +/-   ##
==========================================
- Coverage   67.56%   66.25%   -1.32%     
==========================================
  Files         991     1045      +54     
  Lines      109181   113476    +4295     
  Branches     2719     2845     +126     
==========================================
+ Hits        73768    75183    +1415     
- Misses      31449    34189    +2740     
- Partials     3964     4104     +140     
Flag Coverage Δ
unittests 66.25% <ø> (-1.32%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 571 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants