Fixes #12063 Add docker compatible output after image build.

[ananthb]

What this PR does / why we need it:

Adds docker compatible output after an image is built.

How to verify it

Building against podman and docker should offer compatible output.

Which issue(s) this PR fixes:

Fixes #12063

Special notes for your reviewer:

[rhatdan]

Thanks @ananthb
You need to either add a test or add the [NO NEW TESTS NEEDED] flag.
@nalind @flouthoc PTAL

[rhatdan]

@jwhonce PTAL

[flouthoc]

@ananthb Few doubts and comment

• To me it looks like aux should be part of https://github.com/containers/podman/blob/main/pkg/api/handlers/compat/images_build.go#L542 as omitempty field. Also adding docker's message struct for your reference https://github.com/containers/podman/blob/main/vendor/github.com/docker/docker/pkg/jsonmessage/jsonmessage.go#L138 ( cc @jwhonce )

• [ I think we could ignore this: ] From the original issue (Podman/Docker API mismatch: cannot get the image ID #12063) it seems that docker is also spitting this particular format at the end of every stage. Could we confirm from author of original issue (@stac47) if we also need this at the end of every stage for SDK to work or a parallel line to successfully built is fine. ( I think we are fine without it but need to confirm )

• +1 to @TomSweeneyRedHat do we need to honor --quiet for this.

Overall LGTM ( could we add a test ? )

[ananthb]

@flouthoc

• Will add aux to the original message itself.
• If we do need it at every stage, then that would be a change in buildah right?

[flouthoc]

If we do need it at every stage, then that would be a change in buildah right?

@ananthb I am not sure if we need this I just raised concern cause i saw it in the output. Personally i don't think it should break SDK but this is just an assumption. @stac47 Could you please confirm if this is needed.

[stac47]

On my side, I am only interested in the final built image id. So this fix will fit my needs and to confirm @flouthoc message it will not break the docker SDK.
But, personally, I would try to reproduce the exact behaviour of Docker so I would emit the aux/ID on every stage. Some users might be interested in all the intermediate image ids (I don't why but it is something that you can achieve with the Docker SDK).

[ananthb]

So @flouthoc if we must print per stage ids, then that's a change in buildah. Podman doesn't get that info from buildah currently, so we'd have to figure out a way of passing that over.

[flouthoc]

@ananthb Since @stac47 confirmed that it does not breaks SDK. I think we should be okay without it as of now.
Since change looks synthetic and ratio of effort : value does not makes lot of sense. I think we should proceed without it.

PR looks good to me. I'd request adding a test and would also request @jwhonce to give a quick glimpse since its an API output change.

[ananthb]

@flouthoc where would I add a test for this? Would test/system/ dir work?

[flouthoc]

@ananthb It should go here and test should be restricted to remote only. There are other tests which are restricted to remote-only for help. https://github.com/containers/podman/blob/main/test/e2e/build_test.go

[vrothberg]

Friendly ping.

[rhatdan]

@ananthb Still working on this?

[ananthb]

@rhatdan yep I'll have tests up in a day or two.

[ananthb]

@flouthoc the HTTP API response isn't available in the podman test session. How do I read that in my test?

[flouthoc]

@ananthb where are you checking that ? it should be available but anyways the first thing your tests are failing because of a different error. It should be FROM scratch not FROM SCRATCH in tests. Could you fix that please.

[flouthoc]

@ananthb You can get API output from a similar test like specified here https://github.com/containers/podman/blob/main/test/apiv2/10-images.at . But I think you can drop checking API output and just add a test where build is successful for podman-remote

[ananthb]

@flouthoc am I reading the test output right? It looks like some other test failed.

[flouthoc]

@ananthb LGTM. Could you please squash the commit , there are two additional commit for a single test.

[rhatdan]

/lgtm
/hold

[edsantiago]

Major concerns with the tests. I'm still looking into them, trying to understand the problems, but for now I'm afraid I need to ask for changes and clarification. I'm sorry not to have caught these earlier.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ananthb, edsantiago, flouthoc, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [edsantiago,flouthoc,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[edsantiago]

/lgtm

Thank you @ananthb !