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

Dockerfiles: bump eve-alpine image, eve-cross-compilers and eve-dom0-ztools #4334

Merged

Conversation

christoph-zededa
Copy link
Contributor

Sync eve-alpine image across all packages with the latest image.

> docker inspect lfedge/eve-alpine:1f7685f95a475c6bbe682f0b976f12180b6c8726 | grep -i create
        "Created": "2024-05-14T20:55:51.41185787Z",

> docker inspect lfedge/eve-alpine:82df60e43ab9f8c935584b8c7b4d0a4b0271d608 | grep -i create
        "Created": "2024-09-05T21:14:17.854134914Z",

@christoph-zededa
Copy link
Contributor Author

@deitch so after #4220 (comment) now bumping the installer makes problems? ;-)

#12 [build  2/22] RUN eve-alpine-deploy.sh
#12 0.241 ERROR: unable to select packages:
#12 0.242   so:libhd.so.21 (no such package):
#12 0.244     required by: hwinfo-21.81-r0[so:libhd.so.21]
#12 0.244   i2c-tools (no such package):
#12 0.244     required by: i2c-tools-dev-4.3-r1[i2c-tools=4.3-r1]
#12 0.244   so:libtss2-mu.so.0 (no such package):
#12 0.244     required by: tpm2-tss-sys-3.1.0-r0[so:libtss2-mu.so.0]
#12 0.244                  tpm2-tss-fapi-3.1.0-r0[so:libtss2-mu.so.0]
#12 0.244                  tpm2-tss-esys-3.1.0-r0[so:libtss2-mu.so.0]
#12 0.244   tpm2-tss-mu (no such package):
#12 0.244     required by: tpm2-tss-dev-3.1.0-r0[tpm2-tss-mu=3.1.0-r0]
#12 0.244   tpm2-tss-tcti-cmd (no such package):
#12 0.244     required by: tpm2-tss-dev-3.1.0-r0[tpm2-tss-tcti-cmd=3.1.0-r0]
#12 0.244   tpm2-tss-tcti-device (no such package):
#12 0.244     required by: tpm2-tss-dev-3.1.0-r0[tpm2-tss-tcti-device=3.1.0-r0]
#12 0.244   tpm2-tss-tcti-mssim (no such package):
#12 0.244     required by: tpm2-tss-dev-3.1.0-r0[tpm2-tss-tcti-mssim=3.1.0-r0]
#12 0.244   tpm2-tss-tcti-pcap (no such package):
#12 0.244     required by: tpm2-tss-dev-3.1.0-r0[tpm2-tss-tcti-pcap=3.1.0-r0]
#12 0.244   tpm2-tss-tcti-swtpm (no such package):
#12 0.244     required by: tpm2-tss-dev-3.1.0-r0[tpm2-tss-tcti-swtpm=3.1.0-r0]
#12 ERROR: process "/bin/ash -eo pipefail -c eve-alpine-deploy.sh" did not complete successfully: exit code: 15

Copy link
Member

@OhmSpectator OhmSpectator left a comment

Choose a reason for hiding this comment

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

We have to automate the check on proper image hashes...

@deitch
Copy link
Contributor

deitch commented Oct 10, 2024

@deitch so after #4220 (comment) now bumping the installer makes problems? ;-)

That dependency from mkverification must have moved into installer. We should check the installer to see if it is something we still actually need.

@christoph-zededa
Copy link
Contributor Author

We have to automate the check on proper image hashes...

there is https://github.com/lf-edge/eve-build-tools/blob/main/src/dockerfile-from-checker/main.go but it needs some changes

@OhmSpectator
Copy link
Member

OhmSpectator commented Oct 11, 2024

We have to automate the check on proper image hashes...

there is https://github.com/lf-edge/eve-build-tools/blob/main/src/dockerfile-from-checker/main.go but it needs some changes

I thought about a GH action that can report inconsistency in the hashes in different components.

@christoph-zededa
Copy link
Contributor Author

We have to automate the check on proper image hashes...

there is https://github.com/lf-edge/eve-build-tools/blob/main/src/dockerfile-from-checker/main.go but it needs some changes

I thought about a GH action that can report inconsistency in the hashes in different components.

if you can run it from a GH action, then it should work, just run it with:

dockerfile-checker ./ -i ./eve-tools/bpftrace-compiler/Dockerfile -i ./pkg/optee-os/Dockerfile -i pkg/bsp-imx/Dockerfile -i pkg/vtpm/Dockerfile

because there are still some deviations:

tags differ for image lfedge/eve-dom0-ztools:b8eaeec19d373228a4a842374e5de0d50f050853 in files /home/christoph/projects/eve/pkg/vtpm/Dockerfile and /home/christoph/projects/eve/pkg/pillar/Dockerfile
tags differ for image lfedge/eve-cross-compilers:2a1d062fce410865e7024a83de327a68e52db26c in files /home/christoph/projects/eve/pkg/pillar/Dockerfile and /home/christoph/projects/eve/pkg/optee-os/Dockerfile
tags differ for image alpine:3.20 in files /home/christoph/projects/eve/eve-tools/bpftrace-compiler/Dockerfile and /home/christoph/projects/eve/eve-tools/bpftrace-compiler/root/Dockerfile

@christoph-zededa christoph-zededa force-pushed the eve-alpine_use_same_image branch 2 times, most recently from 965e099 to 4e71198 Compare October 11, 2024 09:58
@OhmSpectator
Copy link
Member

because there are still some deviations:

Do we have a good reason for these deviations?

@christoph-zededa
Copy link
Contributor Author

because there are still some deviations:

Do we have a good reason for these deviations?

For bpftrace-compiler I just want to use the newer alpine as we will move to it anyways, so I didn't start with the old one.

For the other ones I don't know.

@christoph-zededa christoph-zededa changed the title Dockerfiles: bump eve-alpine image Dockerfiles: bump eve-alpine image and check for future inconsistencies Oct 11, 2024
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
}

func main() {
rootCmd.Flags().StringSliceP("ignore-files", "i", []string{}, "ignores specified Dockerfile; can be used several times")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to have a minimal description of what the tool does...

Copy link
Member

@OhmSpectator OhmSpectator Oct 11, 2024

Choose a reason for hiding this comment

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

In Python, it would be fewer than 100 lines and no huge go.sum files. =(
But let it be if we already have it implemented...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But can python evaluate the dockerfile variables?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I got the idea... You parse AST there to get the values. Okay-okay.

@OhmSpectator
Copy link
Member

I would suggest to make it in two different PRs: the version bump (as the original version) and new check introduction.

@christoph-zededa christoph-zededa force-pushed the eve-alpine_use_same_image branch 2 times, most recently from ee69756 to 8595e96 Compare October 11, 2024 14:39
@OhmSpectator
Copy link
Member

Now I really like it.
But I'm a little bit confused. How is this PR affected by #4340? Shouldn't we merge this one before #4340?

@christoph-zededa
Copy link
Contributor Author

Now I really like it. But I'm a little bit confused. How is this PR affected by #4340? Shouldn't we merge this one before #4340?

Once #4340 is merged, the installer/Dockerfile can be removed from the list of ignored Dockerfiles. Then only bpftrace is left (until we bump everything to alpine 3.20).

@deitch
Copy link
Contributor

deitch commented Oct 11, 2024

Probably 4340 first, but I can rebase that one next week if this goes in.

@OhmSpectator
Copy link
Member

Probably 4340 first, but I can rebase that one next week if this goes in.

Let's merge this PR first, as it's already in good shape. And rebase #4334 later (it should also remove the ignoring of the pkg/installer).

@OhmSpectator
Copy link
Member

Why are the Eden tests skipped all the time?.. @yash-zededa, any idea?

@yash-zededa
Copy link
Collaborator

Why are the Eden tests skipped all the time?.. @yash-zededa, any idea?

It's because of the WF modifications.

@christoph-zededa would it be possible to create a separate PR for the WF and Makefile

@OhmSpectator
Copy link
Member

Why are the Eden tests skipped all the time?.. @yash-zededa, any idea?

It's because of the WF modifications.

@christoph-zededa would it be possible to create a separate PR for the WF and Makefile

@yash-zededa, do we skip the tests run if workflow files are changed? I would expect the tests to be skipped if ONLY workflows are changed... Do you think we could fix it?

@yash-zededa
Copy link
Collaborator

Why are the Eden tests skipped all the time?.. @yash-zededa, any idea?

It's because of the WF modifications.
@christoph-zededa would it be possible to create a separate PR for the WF and Makefile

@yash-zededa, do we skip the tests run if workflow files are changed? I would expect the tests to be skipped if ONLY workflows are changed... Do you think we could fix it?

I think so, I will work on the logic where in we run tests for PR which has WF as well as code changes but ideally for all the new WF’s/modification to the existing one’s should be part of separate PR since it will help us with a clean history.

@christoph-zededa
Copy link
Contributor Author

@christoph-zededa would it be possible to create a separate PR for the WF and Makefile

like #4341 ?

@OhmSpectator
Copy link
Member

@christoph-zededa would it be possible to create a separate PR for the WF and Makefile

like #4341 ?

Merged. Rebase this one, please.

@christoph-zededa christoph-zededa changed the title Dockerfiles: bump eve-alpine image and check for future inconsistencies Dockerfiles: bump eve-alpine image, eve-cross-compilers and eve-dom0-ztools Oct 11, 2024
Makefile Outdated
-i pkg/udev/Dockerfile \
-i pkg/xen-tools/Dockerfile \
-i pkg/xen/Dockerfile
-i pkg/installer/Dockerfile
Copy link
Member

Choose a reason for hiding this comment

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

Should it be rebased to use the new variables introduced by @deitch, I guess?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup. And I just opened a PR based on that to bump the alpines as well, but remove pkg/installer from the exceptions list 😄

Not sure if that helps, hinders or is neutral to this one. Feel free to throw mine away if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's best to wait for rebase until #4348 is merged ...

Copy link
Member

Choose a reason for hiding this comment

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

I think it's best to wait for rebase until #4348 is merged ...

And that PR is dependent on #4350

@OhmSpectator
Copy link
Member

@christoph-zededa, it's time to rebase! =) #4348 is merged!

Sync eve-alpine image across all packages with the latest image.

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
bsp-imx, vtpm and optee-os for cross-compilers and ztools in order to
use the same image tag everywhere

Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@eriknordmark eriknordmark merged commit 7a045f6 into lf-edge:master Oct 16, 2024
39 checks passed
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.

6 participants