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

chore!: add containerization and packaging manifest lints #937

Merged
merged 67 commits into from
Sep 11, 2024

Conversation

justinthelaw
Copy link
Contributor

@justinthelaw justinthelaw commented Aug 20, 2024

Description

Adds linting for Helm, Docker, Zarf, and UDS manifests to improve the quality and consistency of all containerization and Kubernetes related logic.

Adds workflows and pre-commits to ensure consistency before and after committing to a PR.

Remaining lower-priority Dockerfile lint errors are in issue #984

BREAKING CHANGES

CHANGES

  • Adds Dockerfile linitng
  • Adds Helm manifest linting
  • Adds UDS bundle linting
  • Adds Zarf package linting
    • Moves Zarf variables to override values files (upstream-values.yaml)
  • Centralizes linting schemas and configs at the root of the project
    • UDS and Zarf lints rely on a downloaded schema from upstream repository uds-cli
    • Pre-commit checks for and locally downloads for the schema, schema is gitignored
    • Workflow downloads the schema file fresh every time
    • Schemas are pinned to our UDS CLI version of v0.14.0
    • Dockerfile linting config is defined in the .hadolint.yaml
    • Helm linting uses the helm binary lint
  • Refactors applicable manifests based on linting errors and warnings
  • Removes GPU_ENABLED python code/env from llama-cpp-python
    • CPU-only package and image (no CUDA deps)
    • No exposure or setting of GPU_ENABLED via values nor Zarf variables
    • Was previously always set to 0 anyway, with no kosher way to change it
  • Adds docs related to UDS bundle overrides based on the new values files
  • Deactivates e2e_registry1_weekly test on draft PRs (mostly to stop it from running here all the time)
  • Removes duplicate check-merge-conflict pre-commit
  • More comprehensive clean Make target for build artifact cleaning
  • Updates the Registry1 weekly to 0.12.2

Related Issue

Fixes #965 #706 #705 #367 #365

Checklist before merging

@justinthelaw justinthelaw added tech-debt Not a feature, but still necessary chore labels Aug 20, 2024
@justinthelaw justinthelaw self-assigned this Aug 20, 2024
Copy link

netlify bot commented Aug 20, 2024

Deploy Preview for leapfrogai-docs canceled.

Name Link
🔨 Latest commit 71476f4
🔍 Latest deploy log https://app.netlify.com/sites/leapfrogai-docs/deploys/66e09c49a8cf7400083adf6c

@justinthelaw
Copy link
Contributor Author

Instead of using an outdated and/or sketchy helm-lint action use this to manually install helm v3.15.1 and manually perform a helm lint [sub-directory].

@justinthelaw
Copy link
Contributor Author

Blocked by #920

@justinthelaw justinthelaw added the blocked 🛑 Something needs to happen before this issues is worked label Aug 29, 2024
@justinthelaw justinthelaw changed the title chore: add containerization manifest lints chore: add containerization and packaging manifest lints Aug 29, 2024
@justinthelaw justinthelaw force-pushed the 706-lint-uds-helm-docker-zarf-lints branch from 42ffbd5 to fac06e9 Compare August 30, 2024 16:36
jalling97
jalling97 previously approved these changes Sep 9, 2024
Copy link
Contributor

@jalling97 jalling97 left a comment

Choose a reason for hiding this comment

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

Awesome job cleaning things up!

jalling97
jalling97 previously approved these changes Sep 9, 2024
@vanakema
Copy link
Contributor

vanakema commented Sep 9, 2024

some awesome cleanup, ty justin. I didn't review the code closely, but thanks for giving the thorough changelog. This is all great stuff in making the "ux" of contributing and deploying lfai better

@justinthelaw justinthelaw removed the blocked 🛑 Something needs to happen before this issues is worked label Sep 10, 2024
@justinthelaw justinthelaw linked an issue Sep 10, 2024 that may be closed by this pull request
@justinthelaw justinthelaw dismissed YrrepNoj’s stale review September 10, 2024 17:30

All changes completed and explanations reviewed by others

@justinthelaw
Copy link
Contributor Author

justinthelaw commented Sep 10, 2024

Merge this one in first: #977

Then merge with main and change e2e registry1 weekly to point to the commit SHA as a result of the above merge.

@justinthelaw justinthelaw requested review from jalling97 and a team and removed request for a team, vanakema, YrrepNoj, americanthinker, gscallon and jalling97 September 10, 2024 20:14
@justinthelaw justinthelaw merged commit c4d0835 into main Sep 11, 2024
34 checks passed
@justinthelaw justinthelaw deleted the 706-lint-uds-helm-docker-zarf-lints branch September 11, 2024 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment