-
Notifications
You must be signed in to change notification settings - Fork 175
more pkgs: no more package bumping #5219
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
base: master
Are you sure you want to change the base?
more pkgs: no more package bumping #5219
Conversation
691cf2a
to
4612ff5
Compare
24d3514
to
9ec7b48
Compare
I will squash the commits before merge. |
1e8e66e
to
fc6556c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modernizes package builds by using automatic hashes provided by LinuxKit to eliminate manual package bumping. It standardizes the approach across multiple EVE packages by introducing build arguments and parameterized base image references.
- Adds
REL_HASH_%=@lkt:pkgs:../*
build arguments to package build configurations - Replaces hardcoded image references with parameterized ARG variables in Dockerfiles
- Removes unnecessary hadolint ignores and updates hadolint configuration
Reviewed Changes
Copilot reviewed 101 out of 101 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
pkg/*/build.yml | Add build arguments for automatic hash resolution |
pkg/*/Dockerfile | Replace hardcoded image references with ARG variables |
.hadolint.yaml | Add DL3006 to ignored hadolint rules |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
FROM ucode-build-${TARGETARCH} AS ucode-build | ||
|
||
FROM lfedge/eve-alpine:745ae9066273c73b0fd879c4ba4ff626a8392d04 AS compactor-common | ||
FROM ${REL_HASH_LFEDGE_EVE_ALPINE} AS compactor-common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 142 correctly uses the parameterized ARG variable, but line 214 appears to be duplicating the same FROM statement with the same alias name 'compactor-common' changed to 'compactor-full'. This inconsistency should be reviewed to ensure the correct stage naming and purpose.
Copilot uses AI. Check for mistakes.
|
||
|
||
FROM lfedge/eve-alpine:745ae9066273c73b0fd879c4ba4ff626a8392d04 AS compactor-full | ||
FROM ${REL_HASH_LFEDGE_EVE_ALPINE} AS compactor-full |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 142 correctly uses the parameterized ARG variable, but line 214 appears to be duplicating the same FROM statement with the same alias name 'compactor-common' changed to 'compactor-full'. This inconsistency should be reviewed to ensure the correct stage naming and purpose.
FROM ${REL_HASH_LFEDGE_EVE_ALPINE} AS compactor-full | |
FROM build AS compactor-full |
Copilot uses AI. Check for mistakes.
aa1951a
to
67a3867
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 101 out of 101 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
image: eve-external-boot-image | ||
|
||
buildArgs: | ||
- REL_HASH_%=@lkt:pkgs:../* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, actually this change is not needed, as obviously Dockerfile
is not using any REL_HASH_*
, but I would like to add it anyways for two reasons:
- being consistent
- if somebody adds
REL_HASH_*
to the Dockerfile then they might forget to add these lines to build.yml, so to keep the frustration low, let's add it already
furthermore I don't expect any disadvantages from this
7977a69
to
c29c97c
Compare
arm64/nvidia: |
/rerun red |
1 similar comment
/rerun red |
@christoph-zededa I see you have some commits in the PR that were picked up from master, do you need to rebase the branch? |
c29c97c
to
d93268f
Compare
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's see how it goes builds and tests...
@christoph-zededa I guess #5228 can be closed then in favor of this one |
ok, feel free to close it then. |
d93268f
to
614650c
Compare
Actually I removed that commit right now because of #5228 (comment) |
this change runs lkt only for the targets `build-docker` and `build-docker-test`, it does not run anymore for all the other targets as the global variable `DOCKER_LIKE_LKT` now holds the command and not the output of the command otherwise it leads to: ``` 35 [build 7/14] RUN --mount=type=cache,target=/root/.cache/go-build echo "Running go vet" && make HV="$HV" vet && echo "Running go fmt" && ERR="$(find . -name \*.go | grep -v /vendor/ | xargs gofmt -d -e -l -s)" && if [ -n "$ERR" ] ; then printf 'go fmt Failed - ERR: %s' "$ERR" ; exit 1 ; fi && make ZARCH=amd64 HV="$HV" DEV="n" RSTATS=n RSTATS_ENDPOINT= RSTATS_TAG= DISTDIR=/final/opt/zededa/bin BUILD_VERSION=v0.0.0-20250911170914-280122ca6335 build 35 1.824 Running go vet 35 1.828 /bin/sh: ../../build-tools/bin/linuxkit: not found 35 1.830 go vet ./... ``` as inside of the pillar docker image there is no linuxkit Signed-off-by: Christoph Ostarek <[email protected]>
614650c
to
c21410b
Compare
$(shell ...) is evaluated immediately and not on the line it is executed on, therefore build-docker-test needs to be split into three different targets otherwise an error message like this shows up: ``` Dockerfile:19 -------------------- 17 | 18 | FROM ${REL_HASH_LFEDGE_EVE_UEFI} AS uefi-build 19 | >>> FROM ${REL_HASH_LFEDGE_EVE_DOM0_ZTOOLS} AS zfs 20 | RUN mkdir /out 21 | # copy zfs-related files from dom0-ztools using prepared list of files -------------------- ERROR: failed to build: failed to solve: lfedge/eve-dom0-ztools:000adaabe42b4f34dc196b722f143d3ae39f47fa-dirty-cb3d84e: failed to resolve source metadata for docker.io/lfedge/eve-dom0-ztools:000adaabe42b4f34dc196b722f143d3ae39f47fa-dirty-cb3d84e: docker.io/lfedge/eve-dom0-ztools:000adaabe42b4f34dc196b722f143d3ae39f47fa-dirty-cb3d84e: not found ``` Signed-off-by: Christoph Ostarek <[email protected]>
it is used to prevent using docker images without tag, but for us linuxkit fills in the tag Signed-off-by: Christoph Ostarek <[email protected]>
provided by linuxkit to avoid having to bump manually like crazy Signed-off-by: Christoph Ostarek <[email protected]>
c21410b
to
cb405b3
Compare
similar to pillar the docker image with the test target has to be built with docker directly and the build-args have to be extracted from linuxkit Signed-off-by: Christoph Ostarek <[email protected]>
|
/rerun red |
Which runner? |
Unfortunately I cannot find it anymore - it was in 'Go Tests'. Let's see if it fails again ... |
Unfortunately when I click on it, I get to a pipeline running for about 30 minutes. For sure 2:43am GMT+2 is more than 30 minutes ago. |
We run go tests on free GitHub provided runners, they have limitation on disk size, if it becomes too much we can always switch to SH runners |
it happened again; seems like we should do that ... |
Moving this into draft as we should first solve linuxkit/linuxkit#4176 |
Description
use automatic hashes provided by linuxkit to avoid having to bump manually like crazy
this is doing the same as #5190 but for other packages
PR dependencies
How to test and validate this PR
There are no specific steps to validate, common EVE Test Plan should cover the functionality.
Changelog notes
internal change
PR Backports
For all current LTS branches, please state explicitly if this PR should be
backported or not. This section is used by our scripts to track the backports,
so, please, do not omit it.
Here is the list of current LTS branches (it should be always up to date):
Checklist
And the last but not least:
check them.
Please, check the boxes above after submitting the PR in interactive mode.