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

Add a linter to check rtf test cases are using latest images #3661

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dave-tucker
Copy link
Member

- What I did
Added a shell script to ensure that all the tests use up-to-date images

- How I did it
Shell script magic and adjusted Makefile.
Also changed the hashing logic for tools/guestfs

- How to verify it
CI

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)
image

This commit improves the hash function to include the Dockerfile
and, like tools/alpine, persists the last hash in version.$(ARCH).
This allows the addition of a `show-tag` target which will retrieve
the tag from the last time the package was pushed.

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
Signed-off-by: Dave Tucker <dave@dtucker.co.uk>
@dave-tucker
Copy link
Member Author

dave-tucker commented May 6, 2021

@rn @deitch I've just noticed that make show-tag for alpine/guestfs will show the tag for a given arch, which is useful, but we don't have any way to get the tag used for the multi-arch manifest... which is really what I want to lint! any suggestions?

Also this will fail as it found that the test-ns image is out of date and needs building and pushing.

pr: check-deps test-pr
PACKAGES=$(dir $(shell find ../pkg ./pkg ../tools/ -maxdepth 2 -mindepth 2 -type f -name build.yml))
TOOLS=$(dir $(shell find ../tools -maxdepth 2 -mindepth 2 -type f -name Makefile))
echo "ERROR: expected latest tag ${full_tag}"
Copy link
Member Author

Choose a reason for hiding this comment

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

ignore this copypasta will fix on next push.

@deitch
Copy link
Collaborator

deitch commented May 6, 2021

@dave-tucker wrote:

make show-tag for alpine/guestfs will show the tag for a given arch, which is useful, but we don't have any way to get the tag used for the multi-arch manifest

I think (I have been digging more deeply into alpine lately) because they use different algorithms:

  • lkt pkg <command> gets its image tag by using the git tree hash of the directory. It relies on anything that might change the final image being picked up by a change in the source. For the most part, that works except for:
  • tools/alpine gets its image tag by calculating the sums of the packages in the image itself (and a few other things). I am assuming this is because the builds depend on apk add <pkg>, and that is not always 100% reproducible, even for the same version of alpine.

If you want to see it in action, I have a small PR open to simplify it and the build process. It doesn't change the logic, just easier to get at and have alpine carry it along the way, see #3650 . Actually, that is so small and simple that it should get approved and merged quickly.

@rn
Copy link
Member

rn commented May 6, 2021

@deitch, tools/alpine should be the only package which pulls stuff from the network without out us knowing exactly which bits and that's why it always will have a special case. All other package are either build exclusively from linuxkit/alpine:<hash> or, if they download some from the network we tend to ckeck out a specific immutable git commit or, when we download, verify the hash. So the content hash from the package directly completely describes the contents of the package.

@deitch
Copy link
Collaborator

deitch commented May 6, 2021

I don't disagree. I don't know what the story is with the guestfs one.

@rn
Copy link
Member

rn commented May 9, 2021

I think tools/guestfs is x86 only and for some reason is not linuxkit pkg based (possibly because we did not have the right version in alpine)...well it also pulls a random debian:stretch...

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.

None yet

3 participants