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

feat!: add lockfile generation support #318

Merged
merged 5 commits into from
Sep 29, 2023
Merged

feat!: add lockfile generation support #318

merged 5 commits into from
Sep 29, 2023

Conversation

maxrake
Copy link
Contributor

@maxrake maxrake commented Sep 28, 2023

The changes in this PR are born out of the research spike to find the
best way to add lockfile generation support to the phylum-ci image.
Each required tool
was examined, to find out how much overhead it would add to the image in
each of the various ways it could be installed. The results of which are
documented in the comment trails of this private story:

https://github.com/phylum-dev/roadmap/issues/380

The actions taken here include:

  • Update the existing Dockerfile to install lockfile generation tools
  • Retain the previous functionality with a new Dockerfile.slim file
  • Switch base image from python:3.11-slim-bullseye to -bookworm
  • Format and refactor throughout the dockerfiles
    • Be DRY about the version of poetry
    • Ensure RUN command contents adhere to shellcheck QA findings
  • Create scripts/docker_tests.sh to provide a basic set of tests
    • Confirm each of the expected tools is present within the image
    • The path and version of the tool is shown
    • Sometimes additional data is provided, like help or info output
  • Update workflow files
    • Update the Test workflow
      • Turn the docker job into a true matrix job
        • Add different dockerfile inputs
        • Add build inputs to allow for building from a wheel or source
        • Test each built image with the new docker_tests.sh script
      • Simplify the logic in the test-rollup job
    • Update Docker and Release workflows
      • Build with both the default and slim dockerfiles
      • Test with the new docker_tests.sh script
      • Create specific docker tags that include the -slim suffix
      • Create slim docker tags mirroring the latest slim image
    • Adhere to actionlint and shellcheck QA tools
  • Update documentation
    • Update README.md with information about the slim tag options
    • Update dockerfiles with information about how to perform testing

Closes #317

BREAKING CHANGE: The phylum-ci docker image created from the default
Dockerfile is much larger, containing all the required tools for
lockfile generation across all supported ecosystems. To retain the
previous functionality, a new slim tag is offered for those instances
where no manifest files are present and/or only lockfiles are used.

Checklist

  • Does this PR have an associated issue (i.e., closes #<issueNum> in description above)?
  • Have you ensured that you have met the expected acceptance criteria?
  • Have you created sufficient tests?
  • Have you updated all affected documentation?

Testing

Tags for the images created with the updated/new dockerfiles in this PR can be found on Docker Hub for my account:

  • manifest_support tag is built with Dockerfile
  • slim is built with Dockerfile.slim

The changes in this PR are born out of the research spike to find the
best way to add lockfile generation support to the `phylum-ci` image.
Each [required tool](https://docs.phylum.io/docs/lockfile_generation)
was examined, to find out how much overhead it would add to the image in
each of the various ways it could be installed. The results of which are
documented in the comment trails of this private story:

phylum-dev/roadmap#380

The actions taken here include:

* Update the existing `Dockerfile` to install lockfile generation tools
* Retain the previous functionality with a new `Dockerfile.slim` file
* Switch base image from `python:3.11-slim-bullseye` to `-bookworm`
* Format and refactor throughout
  * Be DRY about the version of `poetry`
  * Ensure `RUN` command contents adhere to `shellcheck` QA findings

BREAKING CHANGE: The `phylum-ci` docker image created from the default
`Dockerfile` is much larger, containing *all* the required tools for
lockfile generation across all supported ecosystems. To retain the
previous functionality, a new `slim` tag is offered for those instances
where *no* manifest files are present and/or *only* lockfiles are used.
* Create `tests/docker_tests.sh` script to provide a basic set of tests
  * Confirm each of the expected tools is present within the image
  * The path and version of the tool is shown
  * Sometimes additional data is provided, like help or info output
* Update the `Test` workflow
  * Turn the `docker` job into a true matrix job
    * Add different dockerfile inputs
    * Add build inputs to allow for building from a wheel or source
    * Test each built image with the new `docker_tests.sh` script
  * Simplify the logic in the `test-rollup` job
@maxrake maxrake self-assigned this Sep 28, 2023
* Update `Docker` and `Release` workflows
  * Build with both the default and slim dockerfiles
  * Test with the new `docker_tests.sh` script
  * Create specific docker tags that include the `-slim` suffix
  * Create `slim` docker tags mirroring the `latest` slim image
* Adhere to `actionlint` and `shellcheck` QA tools
* Update the `README.md` with information about the `slim` tag options
* Update the dockerfiles with information about how to perform testing
@maxrake maxrake marked this pull request as ready for review September 29, 2023 02:04
@maxrake maxrake requested a review from a team as a code owner September 29, 2023 02:04
@maxrake maxrake requested a review from cd-work September 29, 2023 02:04
Copy link

@cd-work cd-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't have time to read through all of the documentation, but the rest looks good.

Dockerfile Show resolved Hide resolved
@cd-work
Copy link

cd-work commented Sep 29, 2023

What's the image size difference between "fat" and slim?

@maxrake
Copy link
Contributor Author

maxrake commented Sep 29, 2023

What's the image size difference between "fat" and slim?

The "slim" image is 125MB compressed and 340MB uncompressed.

The full details on the new, "fat" image can be found in this comment, with the direct answer to the question repeated here since that is in a private repo:

Some basic features of the new image:

  • Generation time (on my macOS system): 8m13s
    • NOTE: this time is "slow" for the system due to building the non-native --platform linux/amd64
    • See the status check runs in this PR for current "native" build times...which are ~2-4 minutes
  • Size
    • Compressed: 871MB
    • Uncompressed: 2.4GB
    • Pushing this image to Docker Hub from my home network took about 10 minutes, but could be faster when done from CI
  • Run time (using GitLab as the example)
    • New image: 44s to download the image and 1m23s overall runtime (with a manifest)
    • The previous image: 10s to download the image and 25s overall runtime (with a lockfile)

@maxrake maxrake merged commit f96ff48 into main Sep 29, 2023
11 checks passed
@maxrake maxrake deleted the manifest_support branch September 29, 2023 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add lockfile generation support
2 participants