Skip to content

Commit

Permalink
Use alternative exporter metadata representation to accommodate for m…
Browse files Browse the repository at this point in the history
…ulitple exporters with the same ID

Signed-off-by: Dmitri Shelenin <[email protected]>
  • Loading branch information
a-palchikov committed Mar 30, 2022
1 parent d26ef62 commit 9d597fe
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 63 deletions.
67 changes: 46 additions & 21 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func TestIntegration(t *testing.T) {
testTarExporterSymlink,
testMultipleRegistryCacheImportExport,
testMultipleExporters,
testPreventsMultipleExportWithSameExporter,
testSourceMap,
testSourceMapFromRef,
testLazyImagePush,
Expand Down Expand Up @@ -259,7 +260,6 @@ func testBridgeNetworking(t *testing.T, sb integration.Sandbox) {
_, err = c.Solve(sb.Context(), def, SolveOpt{}, nil)
require.Error(t, err)
}

func testHostNetworking(t *testing.T, sb integration.Sandbox) {
if os.Getenv("BUILDKIT_RUN_NETWORK_INTEGRATION_TESTS") == "" {
t.SkipNow()
Expand Down Expand Up @@ -1894,7 +1894,7 @@ func testUser(t *testing.T, sb integration.Sandbox) {
checkAllReleasable(t, c, sb, true)
}

func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
func testPreventsMultipleExportWithSameExporter(t *testing.T, sb integration.Sandbox) {
integration.SkipIfDockerd(t, sb, "multiple exporters")
requiresLinux(t)

Expand All @@ -1919,31 +1919,54 @@ func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
},
},
}, nil)
require.Error(t, err)
require.Errorf(t, err, "using multiple ExporterLocal is not supported")
}

func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
integration.SkipIfDockerd(t, sb, "multiple exporters")
requiresLinux(t)

c, err := New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

def, err := llb.Image("busybox").Marshal(context.TODO())
require.NoError(t, err)

destDir := t.TempDir()

registry, err := sb.NewRegistry()
if errors.Is(err, integration.ErrRequirements) {
t.Skip(err.Error())
}
require.NoError(t, err)

target := registry + "/buildkit/build/exporter:image"
target1, target2 := registry+"/buildkit/build/exporter:image",
registry+"/buildkit/build/alternative:image"

_, err = c.Solve(sb.Context(), def, SolveOpt{
resp, err := c.Solve(sb.Context(), def, SolveOpt{
Exports: []ExportEntry{
{
Type: ExporterLocal,
OutputDir: destDir1,
OutputDir: destDir,
},
{
Type: ExporterImage,
Attrs: map[string]string{
"name": target,
"name": target1,
},
},
{
Type: ExporterImage,
Attrs: map[string]string{
"name": target2,
},
},
},
}, nil)
require.NoError(t, err)

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

func testOCIExporter(t *testing.T, sb integration.Sandbox) {
Expand Down Expand Up @@ -2498,7 +2521,7 @@ func testBuildExportWithUncompressed(t *testing.T, sb integration.Sandbox) {
dt, err := content.ReadBlob(ctx, img.ContentStore(), img.Target())
require.NoError(t, err)

mfst := struct {
var mfst = struct {
MediaType string `json:"mediaType,omitempty"`
ocispecs.Manifest
}{}
Expand Down Expand Up @@ -2745,7 +2768,6 @@ func testPullZstdImage(t *testing.T, sb integration.Sandbox) {
require.NoError(t, err)
require.Equal(t, dt, []byte("zstd"))
}

func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) {
integration.SkipIfDockerd(t, sb, "direct push")
requiresLinux(t)
Expand Down Expand Up @@ -2886,7 +2908,7 @@ func testBuildPushAndValidate(t *testing.T, sb integration.Sandbox) {
dt, err = content.ReadBlob(ctx, img.ContentStore(), img.Target())
require.NoError(t, err)

mfst := struct {
var mfst = struct {
MediaType string `json:"mediaType,omitempty"`
ocispecs.Manifest
}{}
Expand Down Expand Up @@ -3725,8 +3747,7 @@ func testBasicCacheImportExport(t *testing.T, sb integration.Sandbox, cacheOptio
{
Type: ExporterLocal,
OutputDir: destDir,
},
},
}},
CacheImports: cacheOptionsEntryImport,
}, nil)
require.NoError(t, err)
Expand Down Expand Up @@ -4588,7 +4609,7 @@ func testSourceMapFromRef(t *testing.T, sb integration.Sandbox) {

frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
st := llb.Scratch().File(
llb.Mkdir("foo/bar", 0600), // fails because /foo doesn't exist
llb.Mkdir("foo/bar", 0600), //fails because /foo doesn't exist
sm.Location([]*pb.Range{{Start: pb.Position{Line: 3, Character: 1}}}),
)

Expand Down Expand Up @@ -5821,6 +5842,14 @@ func makeSSHAgentSock(agent agent.Agent) (p string, cleanup func() error, err er
}, 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 Expand Up @@ -5848,10 +5877,8 @@ func (*secModeInsecure) UpdateConfigFile(in string) string {
return in + "\n\ninsecure-entitlements = [\"security.insecure\"]\n"
}

var (
securitySandbox integration.ConfigUpdater = &secModeSandbox{}
securityInsecure integration.ConfigUpdater = &secModeInsecure{}
)
var securitySandbox integration.ConfigUpdater = &secModeSandbox{}
var securityInsecure integration.ConfigUpdater = &secModeInsecure{}

type netModeHost struct{}

Expand All @@ -5865,10 +5892,8 @@ func (*netModeDefault) UpdateConfigFile(in string) string {
return in
}

var (
hostNetwork integration.ConfigUpdater = &netModeHost{}
defaultNetwork integration.ConfigUpdater = &netModeDefault{}
)
var hostNetwork integration.ConfigUpdater = &netModeHost{}
var defaultNetwork integration.ConfigUpdater = &netModeDefault{}

func fixedWriteCloser(wc io.WriteCloser) func(map[string]string) (io.WriteCloser, error) {
return func(map[string]string) (io.WriteCloser, error) {
Expand Down
7 changes: 1 addition & 6 deletions client/exporters.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
package client

const (
// TODO: keep these in sync with the corresponding exporter Name constants
// Keep these in sync with the corresponding exporter Name constants
ExporterImage = "image"
ExporterLocal = "local"
ExporterTar = "tar"
ExporterOCI = "oci"
ExporterDocker = "docker"

// ExporterCommon defines the name of the exporter
// with metadata aggregated from alternative sources
// like the cache exporter or the frontend
ExporterCommon = "<auto>"
)
4 changes: 2 additions & 2 deletions client/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ type SolveStatus struct {
type SolveResponse struct {
// ExporterResponse is also used for CacheExporter
ExporterResponse map[string]string
// ExportersResponse maps exporter name -> metadata
ExportersResponse map[string]map[string]string
// ExportersResponse lists metadata from output exporters
ExportersResponse []map[string]string
}
4 changes: 2 additions & 2 deletions client/solve.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,9 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
return errors.Wrap(err, "failed to solve")
}

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

result = &SolveResponse{
Expand Down
10 changes: 5 additions & 5 deletions cmd/buildctl/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func buildAction(clicontext *cli.Context) error {
}

metadataFile := clicontext.String("metadata-file")
if metadataFile != "" && len(resp.ExportersResponse) != 0 {
if metadataFile != "" && (resp.ExporterResponse != nil || len(resp.ExportersResponse) != 0) {
if err := writeMetadataFile(metadataFile, resp); err != nil {
return err
}
Expand All @@ -326,11 +326,11 @@ func writeMetadataFile(filename string, resp *client.SolveResponse) (err error)
}
return continuity.AtomicWriteFile(filename, b, 0666)
}
result := map[string]map[string]interface{}{
client.ExporterCommon: marshalExporterMetadata(resp.ExporterResponse),
result := []map[string]interface{}{
marshalExporterMetadata(resp.ExporterResponse),
}
for name, md := range resp.ExportersResponse {
result[name] = marshalExporterMetadata(md)
for _, md := range resp.ExportersResponse {
result = append(result, marshalExporterMetadata(md))
}
b, err := json.MarshalIndent(result, "", " ")
if err != nil {
Expand Down
16 changes: 13 additions & 3 deletions cmd/buildctl/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,11 @@ func testBuildMetadataFile(t *testing.T, sb integration.Sandbox) {
metadataBytes, err := os.ReadFile(metadataFile)
require.NoError(t, err)

var metadata map[string]map[string]interface{}
var metadata []map[string]interface{}
err = json.Unmarshal(metadataBytes, &metadata)
require.NoError(t, err)

require.Contains(t, metadata, containerimage.Name)
exporterMetadata := metadata[containerimage.Name]
exporterMetadata := validateExporterMetadata(t, metadata, containerimage.Name)

require.Contains(t, exporterMetadata, "image.name")
require.Equal(t, imageName, exporterMetadata["image.name"])
Expand Down Expand Up @@ -183,6 +182,17 @@ func testBuildMetadataFile(t *testing.T, sb integration.Sandbox) {
}
}

func validateExporterMetadata(t *testing.T, metadata []map[string]interface{}, exporter string) (result map[string]interface{}) {
require.NotEmpty(t, metadata)
for _, md := range metadata {
if typ, ok := md[exptypes.ExporterTypeKey]; ok && typ == exporter {
return md
}
}
require.Failf(t, "required exporter metadata not found in the list", "exporter %s", exporter)
return nil
}

func marshal(ctx context.Context, st llb.State) (io.Reader, error) {
def, err := st.Marshal(ctx)
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions control/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@ import (
"sync/atomic"
"time"

"github.com/moby/buildkit/util/bklog"

controlapi "github.com/moby/buildkit/api/services/control"
apitypes "github.com/moby/buildkit/api/types"
"github.com/moby/buildkit/cache/remotecache"
"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"
"github.com/moby/buildkit/solver"
"github.com/moby/buildkit/solver/llbsolver"
"github.com/moby/buildkit/solver/pb"
"github.com/moby/buildkit/util/bklog"
"github.com/moby/buildkit/util/imageutil"
"github.com/moby/buildkit/util/throttle"
"github.com/moby/buildkit/util/tracing/transform"
Expand Down Expand Up @@ -326,9 +326,9 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*
}

resps := make([]*controlapi.ExporterResponse, 0, len(resp.ExportersResponse))
for name, md := range resp.ExportersResponse {
for _, md := range resp.ExportersResponse {
resps = append(resps, &controlapi.ExporterResponse{
Name: name,
Name: md[exptypes.ExporterTypeKey],
Response: md,
})
}
Expand Down
5 changes: 1 addition & 4 deletions exporter/containerimage/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,6 @@ func (e *imageExporterInstance) Name() string {
return "exporting to image"
}

func (e *imageExporterInstance) ID() string {
return Name
}

func (e *imageExporterInstance) Config() exporter.Config {
return exporter.Config{
Compression: e.compression(),
Expand Down Expand Up @@ -378,6 +374,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source,
return nil, err
}
resp[exptypes.ExporterImageDescriptorKey] = base64.StdEncoding.EncodeToString(dtdesc)
resp[exptypes.ExporterTypeKey] = Name

return resp, nil
}
Expand Down
1 change: 1 addition & 0 deletions exporter/containerimage/exptypes/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const (
ExporterInlineCache = "containerimage.inlinecache"
ExporterBuildInfo = "containerimage.buildinfo"
ExporterPlatformsKey = "refs.platforms"
ExporterTypeKey = "exporter.type"
)

type Platforms struct {
Expand Down
1 change: 0 additions & 1 deletion exporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ type Exporter interface {

type ExporterInstance interface {
Name() string
ID() string
Config() Config
Export(ctx context.Context, src Source, sessionID string) (map[string]string, error)
}
Expand Down
4 changes: 0 additions & 4 deletions exporter/local/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ func (e *localExporterInstance) Name() string {
return "exporting to client"
}

func (e *localExporterInstance) ID() string {
return Name
}

func (e *localExporter) Config() exporter.Config {
return exporter.Config{}
}
Expand Down
7 changes: 2 additions & 5 deletions exporter/oci/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,6 @@ func (e *imageExporterInstance) Name() string {
return "exporting to oci image format"
}

func (e *imageExporterInstance) ID() string {
return string(e.opt.Variant)
}

func (e *imageExporterInstance) Config() exporter.Config {
return exporter.Config{
Compression: e.compression(),
Expand Down Expand Up @@ -237,6 +233,7 @@ func (e *imageExporterInstance) Export(ctx context.Context, src exporter.Source,

resp := make(map[string]string)

resp[exptypes.ExporterTypeKey] = string(e.opt.Variant)
resp[exptypes.ExporterImageDigestKey] = desc.Digest.String()
if v, ok := desc.Annotations[exptypes.ExporterConfigDigestKey]; ok {
resp[exptypes.ExporterImageConfigDigestKey] = v
Expand Down Expand Up @@ -357,7 +354,7 @@ func normalizedNames(name string) ([]string, error) {
return nil, nil
}
names := strings.Split(name, ",")
tagNames := make([]string, len(names))
var tagNames = make([]string, len(names))
for i, name := range names {
parsed, err := reference.ParseNormalizedNamed(name)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions exporter/tar/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ func (e *localExporterInstance) Name() string {
return "exporting to client"
}

func (e *localExporterInstance) ID() string {
return Name
}

func (e *localExporterInstance) Config() exporter.Config {
return exporter.Config{}
}
Expand Down
Loading

0 comments on commit 9d597fe

Please sign in to comment.