Skip to content

Commit

Permalink
Store exporter responses in a single map.
Browse files Browse the repository at this point in the history
  • Loading branch information
Dmitry Shelenin authored and a-palchikov committed Apr 11, 2022
1 parent dd4e8d6 commit 083e4c5
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 288 deletions.
289 changes: 111 additions & 178 deletions api/services/control/control.pb.go

Large diffs are not rendered by default.

5 changes: 0 additions & 5 deletions api/services/control/control.proto
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,7 @@ message ExporterResponse {
}

message SolveResponse {
// ExporterResponse is the metadata aggregated from
// alternative sources like cache exporter and frontend
ExporterResponse ExporterResponse = 1;
// ExportersResponse groups metadata from individual
// output exporters
repeated ExporterResponse ExportersResponse = 2;
}

message StatusRequest {
Expand Down
64 changes: 15 additions & 49 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -745,11 +745,10 @@ func testPushByDigest(t *testing.T, sb integration.Sandbox) {
_, _, err = contentutil.ProviderFromRef(name + ":latest")
require.Error(t, err)

md := metadataForExporter(t, resp.ExportersResponse, ExporterImage)
desc, _, err := contentutil.ProviderFromRef(name + "@" + md[exptypes.ExporterImageDigestKey])
desc, _, err := contentutil.ProviderFromRef(name + "@" + resp.ExporterResponse[exptypes.ExporterImageDigestKey])
require.NoError(t, err)

require.Equal(t, md[exptypes.ExporterImageDigestKey], desc.Digest.String())
require.Equal(t, resp.ExporterResponse[exptypes.ExporterImageDigestKey], desc.Digest.String())
require.Equal(t, images.MediaTypeDockerSchema2Manifest, desc.MediaType)
require.True(t, desc.Size > 0)
}
Expand Down Expand Up @@ -912,13 +911,12 @@ func testFrontendImageNaming(t *testing.T, sb integration.Sandbox) {
}
require.NoError(t, err)

checkImageName := map[string]func(out, imageName string, resp *SolveResponse){
ExporterOCI: func(out, imageName string, resp *SolveResponse) {
checkImageName := map[string]func(out, imageName string, exporterResponse map[string]string){
ExporterOCI: func(out, imageName string, exporterResponse map[string]string) {
// Nothing to check
return
},
ExporterDocker: func(out, imageName string, resp *SolveResponse) {
exporterResponse := metadataForExporter(t, resp.ExportersResponse, ExporterDocker)
ExporterDocker: func(out, imageName string, exporterResponse map[string]string) {
require.Contains(t, exporterResponse, "image.name")
require.Equal(t, exporterResponse["image.name"], "docker.io/library/"+imageName)

Expand Down Expand Up @@ -946,8 +944,7 @@ func testFrontendImageNaming(t *testing.T, sb integration.Sandbox) {
require.Equal(t, 1, len(dockerMfst[0].RepoTags))
require.Equal(t, imageName, dockerMfst[0].RepoTags[0])
},
ExporterImage: func(_, imageName string, resp *SolveResponse) {
exporterResponse := metadataForExporter(t, resp.ExportersResponse, ExporterImage)
ExporterImage: func(_, imageName string, exporterResponse map[string]string) {
require.Contains(t, exporterResponse, "image.name")
require.Equal(t, exporterResponse["image.name"], imageName)

Expand Down Expand Up @@ -1042,7 +1039,7 @@ func testFrontendImageNaming(t *testing.T, sb integration.Sandbox) {
resp, err := c.Build(sb.Context(), so, "", frontend, nil)
require.NoError(t, err)

checkImageName[exp](out, imageName, resp)
checkImageName[exp](out, imageName, resp.ExporterResponse)
})
}
})
Expand Down Expand Up @@ -1957,20 +1954,13 @@ func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
{
Type: ExporterImage,
Attrs: map[string]string{
"name": target1,
},
},
{
Type: ExporterImage,
Attrs: map[string]string{
"name": target2,
"name": strings.Join([]string{target1, target2}, ","),
},
},
},
}, nil)
require.NoError(t, err)

validateMetadataContains(t, resp.ExportersResponse, "image.name", []string{target1, target2})
require.Equal(t, resp.ExporterResponse["image.name"], target1+","+target2)
}

func testOCIExporter(t *testing.T, sb integration.Sandbox) {
Expand Down Expand Up @@ -2208,14 +2198,12 @@ func testExporterTargetExists(t *testing.T, sb integration.Sandbox) {
},
}, nil)
require.NoError(t, err)

md := metadataForExporter(t, res.ExportersResponse, ExporterOCI)
dgst := md[exptypes.ExporterImageDigestKey]
dgst := res.ExporterResponse[exptypes.ExporterImageDigestKey]

require.True(t, strings.HasPrefix(dgst, "sha256:"))
require.Equal(t, dgst, mdDgst)

require.True(t, strings.HasPrefix(md[exptypes.ExporterImageConfigDigestKey], "sha256:"))
require.True(t, strings.HasPrefix(res.ExporterResponse[exptypes.ExporterImageConfigDigestKey], "sha256:"))
}

func testTarExporterWithSocket(t *testing.T, sb integration.Sandbox) {
Expand Down Expand Up @@ -3873,8 +3861,7 @@ func testBasicInlineCacheImportExport(t *testing.T, sb integration.Sandbox) {
}, nil)
require.NoError(t, err)

md := metadataForExporter(t, resp.ExportersResponse, ExporterImage)
dgst, ok := md[exptypes.ExporterImageDigestKey]
dgst, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
require.Equal(t, ok, true)

unique, err := readFileInImage(sb.Context(), c, target+"@"+dgst, "/unique")
Expand Down Expand Up @@ -3910,8 +3897,7 @@ func testBasicInlineCacheImportExport(t *testing.T, sb integration.Sandbox) {
}, nil)
require.NoError(t, err)

md = metadataForExporter(t, resp.ExportersResponse, ExporterImage)
dgst2, ok := md[exptypes.ExporterImageDigestKey]
dgst2, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
require.Equal(t, ok, true)

require.Equal(t, dgst, dgst2)
Expand Down Expand Up @@ -3949,8 +3935,7 @@ func testBasicInlineCacheImportExport(t *testing.T, sb integration.Sandbox) {
}, nil)
require.NoError(t, err)

md = metadataForExporter(t, resp.ExportersResponse, ExporterImage)
dgst2uncompress, ok := md[exptypes.ExporterImageDigestKey]
dgst2uncompress, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
require.Equal(t, ok, true)

// dgst2uncompress != dgst, because the compression type is different
Expand Down Expand Up @@ -3981,8 +3966,7 @@ func testBasicInlineCacheImportExport(t *testing.T, sb integration.Sandbox) {
}, nil)
require.NoError(t, err)

md = metadataForExporter(t, resp.ExportersResponse, ExporterImage)
dgst3, ok := md[exptypes.ExporterImageDigestKey]
dgst3, ok := resp.ExporterResponse[exptypes.ExporterImageDigestKey]
require.Equal(t, ok, true)

// dgst3 != dgst, because inline cache is not exported for dgst3
Expand Down Expand Up @@ -5860,24 +5844,6 @@ func makeSSHAgentSock(agent agent.Agent) (p string, cleanup func() error, err er
}, nil
}

func metadataForExporter(t *testing.T, mds []map[string]string, exporter string) map[string]string {
for _, md := range mds {
if typ, ok := md[exptypes.ExporterTypeKey]; ok && typ == exporter {
return md
}
}
require.FailNowf(t, "no exporter found", "exporter %s", exporter)
return nil
}

func validateMetadataContains(t *testing.T, mds []map[string]string, key string, expectedValues []string) {
var values []string
for _, md := range mds {
values = append(values, md[key])
}
require.ElementsMatch(t, values, expectedValues)
}

type server struct {
l net.Listener
}
Expand Down
2 changes: 0 additions & 2 deletions client/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,4 @@ type SolveStatus struct {
type SolveResponse struct {
// ExporterResponse is also used for CacheExporter
ExporterResponse map[string]string
// ExportersResponse lists metadata from output exporters
ExportersResponse []map[string]string
}
8 changes: 1 addition & 7 deletions client/solve.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,8 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
return errors.Wrap(err, "failed to solve")
}

exportersResponse := make([]map[string]string, 0, len(resp.ExportersResponse))
for _, resp := range resp.ExportersResponse {
exportersResponse = append(exportersResponse, resp.Response)
}

result = &SolveResponse{
ExporterResponse: resp.ExporterResponse.GetResponse(),
ExportersResponse: exportersResponse,
ExporterResponse: resp.ExporterResponse.GetResponse(),
}
return nil
})
Expand Down
43 changes: 12 additions & 31 deletions cmd/buildctl/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,15 +293,13 @@ func buildAction(clicontext *cli.Context) error {
if err != nil {
return err
}
for _, resp := range resp.ExportersResponse {
for k, v := range resp {
logrus.Debugf("exporter response: %s=%s", k, v)
}
for k, v := range resp.ExporterResponse {
logrus.Debugf("exporter response: %s=%s", k, v)
}

metadataFile := clicontext.String("metadata-file")
if metadataFile != "" && (resp.ExporterResponse != nil || len(resp.ExportersResponse) != 0) {
if err := writeMetadataFile(metadataFile, resp); err != nil {
if metadataFile != "" && resp.ExporterResponse != nil {
if err := writeMetadataFile(metadataFile, resp.ExporterResponse); err != nil {
return err
}
}
Expand All @@ -317,31 +315,10 @@ func buildAction(clicontext *cli.Context) error {
return eg.Wait()
}

func writeMetadataFile(filename string, resp *client.SolveResponse) (err error) {
if len(resp.ExportersResponse) == 0 {
// Keep the old metadata file format
b, err := json.MarshalIndent(marshalExporterMetadata(resp.ExporterResponse), "", " ")
if err != nil {
return err
}
return continuity.AtomicWriteFile(filename, b, 0666)
}
result := []map[string]interface{}{
marshalExporterMetadata(resp.ExporterResponse),
}
for _, md := range resp.ExportersResponse {
result = append(result, marshalExporterMetadata(md))
}
b, err := json.MarshalIndent(result, "", " ")
if err != nil {
return err
}
return continuity.AtomicWriteFile(filename, b, 0666)
}

func marshalExporterMetadata(exp map[string]string) map[string]interface{} {
func writeMetadataFile(filename string, exporterResponse map[string]string) error {
var err error
out := make(map[string]interface{})
for k, v := range exp {
for k, v := range exporterResponse {
dt, err := base64.StdEncoding.DecodeString(v)
if err != nil {
out[k] = v
Expand All @@ -354,5 +331,9 @@ func marshalExporterMetadata(exp map[string]string) map[string]interface{} {
}
out[k] = json.RawMessage(dt)
}
return out
b, err := json.MarshalIndent(out, "", " ")
if err != nil {
return err
}
return continuity.AtomicWriteFile(filename, b, 0666)
}
11 changes: 1 addition & 10 deletions control/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/moby/buildkit/client"
controlgateway "github.com/moby/buildkit/control/gateway"
"github.com/moby/buildkit/exporter"
"github.com/moby/buildkit/exporter/containerimage/exptypes"
"github.com/moby/buildkit/frontend"
"github.com/moby/buildkit/session"
"github.com/moby/buildkit/session/grpchijack"
Expand Down Expand Up @@ -334,16 +333,8 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*
return nil, err
}

resps := make([]*controlapi.ExporterResponse, 0, len(resp.ExportersResponse))
for _, md := range resp.ExportersResponse {
resps = append(resps, &controlapi.ExporterResponse{
Name: md[exptypes.ExporterTypeKey],
Response: md,
})
}
return &controlapi.SolveResponse{
ExporterResponse: &controlapi.ExporterResponse{Response: resp.ExporterResponse},
ExportersResponse: resps,
ExporterResponse: &controlapi.ExporterResponse{Response: resp.ExporterResponse},
}, nil
}

Expand Down
17 changes: 11 additions & 6 deletions solver/llbsolver/solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
}
}

exportersResponse := []map[string]string{}
var exporterResponse map[string]string
for _, exp := range out.Exporters {
inp := exporter.Source{
Metadata: res.Metadata,
Expand Down Expand Up @@ -273,7 +273,12 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
return err
}
if resp != nil {
exportersResponse = append(exportersResponse, resp)
if exporterResponse == nil {
exporterResponse = make(map[string]string)
}
for k, v := range resp {
exporterResponse[k] = v
}
}
return nil
}); err != nil {
Expand Down Expand Up @@ -319,8 +324,9 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
return nil, err
}
}

exporterResponse := make(map[string]string)
if exporterResponse == nil {
exporterResponse = make(map[string]string)
}
for k, v := range res.Metadata {
if strings.HasPrefix(k, "frontend.") {
exporterResponse[k] = string(v)
Expand All @@ -336,8 +342,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
}

return &client.SolveResponse{
ExporterResponse: exporterResponse,
ExportersResponse: exportersResponse,
ExporterResponse: exporterResponse,
}, nil
}

Expand Down

0 comments on commit 083e4c5

Please sign in to comment.