-
Notifications
You must be signed in to change notification settings - Fork 164
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
noop comments to force rebuilding of packages and get sboms #3595
noop comments to force rebuilding of packages and get sboms #3595
Conversation
pkg/acrn/Dockerfile
Outdated
@@ -1,4 +1,5 @@ | |||
# syntax=docker/dockerfile-upstream:1.5.0-rc2-labs | |||
# Dockerfile to build acrn |
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.
I think it is worth adding license/ copyright header for all files while we are here
# Copyright (c) 2023 Zededa, Inc.
# SPDX-License-Identifier: Apache-2.0
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.
Sure, that is even cleaner.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3595 +/- ##
=======================================
Coverage 19.43% 19.43%
=======================================
Files 232 232
Lines 50533 50533
=======================================
Hits 9819 9819
Misses 39994 39994
Partials 720 720 ☔ View full report in Codecov by Sentry. |
Why did the eden tests fail? |
I don't understand why these are not building? It is complaining about the image not being in the cache, but it builds it locally. |
did you rebase on top of latest master? there was a fix for eden yesterday |
Possible I am out of date. I will do so now. |
533f001
to
d69eb7c
Compare
I tried a rebase, nothing changed. I did update the comments you suggested. Let it run again. |
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
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
I converted this to draft. I found a bug in the |
Bug in |
d69eb7c
to
46ad212
Compare
This is now ready for review. |
ef6a361
to
46ad212
Compare
Signed-off-by: Avi Deitcher <avi@deitcher.net>
46ad212
to
b1004e5
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.
LGTM
So it did rebuild, but the pushed-out packages are missing the sboms. I do not yet know why, but I will investigate. |
It looks like this is an issue in buildkit running in GHA, but I cannot be sure. Still working on it. |
It gets stranger. I just uploaded the lkt cache as an artifact, so I could download it and see it. The manifest has the sboms. Which makes me wonder, perhaps it is working at build, but not at push? |
OK, I have it. Bug in linuxkit, a path that I didn't realize we used and wasn't properly checked. Expect a PR soon enough. |
These changes are all a noop; they just add a simple comment to the pkg Dockerfile.
This does, however, change the git tree hash for those packages. With #3592 merged in, linuxkit will add an SBoM to those container images (via buildkit), which will be consumed in future PRs.
I went through
images/rootfs.yml.in
and found all of the images referenced there; those are the ones I updated. These are: