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

Log output parsing #441

Closed
6 tasks done
guineveresaenger opened this issue Dec 8, 2022 · 1 comment · Fixed by #450
Closed
6 tasks done

Log output parsing #441

guineveresaenger opened this issue Dec 8, 2022 · 1 comment · Fixed by #450
Assignees
Labels
4.x.x kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Milestone

Comments

@guineveresaenger
Copy link
Contributor

guineveresaenger commented Dec 8, 2022

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

We have a lot of log output returned from the Docker client, for both BuilderV1 as well as BuilderBuildKit modes of the Docker client. Currently, a lot of this output is written to the console, and we should decide which ones, if any, to write to the console vs writing to the ephemeral log window (a log level setting on the Pulumi logger).

We do also need to parse logs for failure statuses, since the client doesn't error on build errors, but we'll want the provider to do so.

Initial implementation for BuilderV1 logs for reference

Classic Docker logs on Build

  • Current logs should (mostly?) be written to Pulumi Ephemeral Logs
  • Additionally, "Downloading" field should be written to Pulumi Ephemeral Logs
  • Additionally, "Extracting" field should be written to Pulumi Ephemeral Logs
  • Determine which status logs should be written to Info, if any

BuildKit logs on Build

  • Determine how to parse the BuildKit logs and handle any error statuses (these logs are structured differently so will need to be parsed differently)
  • Write logs to ephemeral logs

Affected area/feature

4.x.x log output

@guineveresaenger guineveresaenger added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team 4.x.x and removed needs-triage Needs attention from the triage team labels Dec 8, 2022
@guineveresaenger guineveresaenger self-assigned this Jan 4, 2023
@guineveresaenger guineveresaenger added this to the 0.83 milestone Jan 4, 2023
@guineveresaenger
Copy link
Contributor Author

UPDATE:

Working on this PR exposed a bug?missing feature? for running the Docker SDK in buildkit mode.

Because we parsed the build logs for an error and returned that error from Create, for the first time, CI caught and reported that a buildkit build did not succeed. Prior to the linked PR, we only logged the build output, which unfortunately gets suppressed in CI, and Create returned as a success, despite the image not having been built.

The error in CI was error: failed to solve with frontend dockerfile.v0: failed to create LLB definition: no active sessions, which is a combination of a few things.

First, a base image needed to not be available locally for the buildkit builder to attempt to connect to a registry and pull the image. This was a major sticking point in reproducing this error at first. Who doesn't have a stray pythin image lying around on their images list? 😝

Next, it seems as though Buildkit needs to have auth configs passed along explicitly, unlike BuilderV1, which defaults to DockerHub.

Finally, we needed a moby/buildkit/session and pass along a session ID to the build options.

The work here is almost done; there is a bit of final tweaking necessary on the log parsing since all of buildkit's logs are encoded and written to unstructured JSON in docker's jsonmessage.JSONMessage.Aux field.

@AaronFriel AaronFriel modified the milestones: 0.83, 0.84 Jan 17, 2023
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x.x kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants