feat: print build logs on verbose mode#499
Conversation
🦋 Changeset detectedLatest commit: 6c4c342 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Coverage Report
📁 File Coverage (19 files)
|
brunomenezes
force-pushed
the
feat/print-build-logs-on-verbose-opt
branch
from
b508cef to
2243094
Compare
| fs.readFileSync(tmpFile, "utf-8"), | ||
| ) as BuildxMetadata; | ||
|
|
||
| const imageName = metadata["image.name"]?.split(",")[0]; |
There was a problem hiding this comment.
I'd stick with what is in the documentation, which is "containerimage.digest"
https://docs.docker.com/reference/cli/docker/buildx/build/#metadata-file
There was a problem hiding this comment.
I will remove image.name and leave the existing digest below. But the correct one is containerimage.config.digest here is the first build that was only using the one mentioned https://github.com/cartesi/cli/actions/runs/28516129205/job/84528221902#step:9:697. Looks like in the OCI image spec they're used for different purposes, and the containerimage.config.digest is the one we see when running the command docker images under the ID column.
There was a problem hiding this comment.
Hmm, that's weird. Will double check this later.
There was a problem hiding this comment.
containerimage.digest is the right one
config namespace if something else
There was a problem hiding this comment.
@endersonmaia I linked the build above. Using that containerimage.digest causes problems as the Docker daemon reports that it is not able to find the image requested with the hash passed.
For context, when using locally and running the tests, it runs fine, and upon inspecting the metadata, both containerimage.digest and containerimage.config.digest have the same SHA256, but once pushed and the CI runs with QEMU, the story is different, and both options have different hashes, and the option discussed is the one that cause the error in the build link I shared above.
There was a problem hiding this comment.
The image id is being used to run against docker image inspect, to get some data like command, entrypoint, env var, workdir, to be used during the cartesi machine build. I think that is the only reason, but it's worth to check.
So, based on the Claude discussion above it looks like containerimage.config.digest is the correct one.
There was a problem hiding this comment.
For me the test is also failing locally.
It works with
containerimage.digest.
If you can run that again locally after a git pull --rebase, I cleaned up the test fixup commits we did earlier.
I included an order of precedence in the returns: containerimage.config.digest > containerimage.digest.
Why
What I understood so far is that locally, we use an embedded buildkit directly on the Docker daemon, so when it builds, it saves the image in the docker image store, but it behaves differently between macOS and Linux, as for me both properties are generated locally with the same hash, but on @endersonmaia it only generates the containerimage.digest. In the CI, the docker/setup-buildx-action creates a new driver in a container, so inside the container there is a buildkit running standalone (i.e. buildkitd), with no dockerd inside it. Furthermore, it will generate the metadata file based on the OCI spec, and in that case the containerimage.config.digest is what we want. Because of the type=docker argument, the built image was streamed to the host Docker as a tarball, unpacked and indexed in the host Docker images store. The containerimage.digest generated is essentially BuildKit's internal tracking ID of the build result in that docker-container, and the host Docker daemon never sees it nor stores it.
I could not find a configuration that would meet our needs, i.e., multiple outputs (type=docker and type=tar) and Cross-platform (riscv64) at the same time. For example, we can change the drive to be docker to run in the host or control BuildKit's version, but as far as I understand, neither will help in that situation and the cartesi build will fail in the test step.
cc: @tuler, @endersonmaia
There was a problem hiding this comment.
What if we stop supporting these info coming from docker (CMD, ENTRYPOINT, ENV, WORKDIR), and use only what is defined in the cartesi.toml. Then maybe we don't even need the image id and the docker output.
There was a problem hiding this comment.
@tuler, I would like to take a closer look at that specific part later. Perhaps in a separate PR/branch.
There was a problem hiding this comment.
But still I did not understand yet in what circumstances using containerimage.digest is correct.
brunomenezes
force-pushed
the
feat/print-build-logs-on-verbose-opt
branch
5 times, most recently
from
685f739 to
389efa2
Compare
* Improve the --verbose flag to stream logs from the build process. * Mainly affects the docker usage that was silent by default.
brunomenezes
force-pushed
the
feat/print-build-logs-on-verbose-opt
branch
from
389efa2 to
6c4c342
Compare
3 participants
Summary
Currently, the
cartesi buildcommand, when using Docker under the hood, is in silent mode even when the--verboseflag is passed. If the build takes a while to happen, the user has no idea what is going on unless they realize to go intodocker desktop -> builds -> click tab active build -> tab logsand have a look at the current situation. However, in a CI environment, that is not an option.Now, by passing the
--verboseflag, you will be able to see the logs of the Docker build.PS: We're dogfooding by enabling it on our own integration tests that require an application to be built before the cases are run against it.