Enable multiple exporters #2760
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |
| "context" | ||
| "encoding/base64" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "os" | ||
| "path/filepath" | ||
|
|
@@ -55,8 +56,8 @@ type SolveOpt struct { | |
| type ExportEntry struct { | ||
| Type string | ||
| Attrs map[string]string | ||
| Output filesync.FileOutputFunc // for ExporterOCI and ExporterDocker | ||
| OutputDir string // for ExporterLocal | ||
| } | ||
|
|
||
| type CacheOptionsEntry struct { | ||
|
|
@@ -125,12 +126,23 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG | |
| return nil, err | ||
| } | ||
|
|
||
| type exporter struct { | ||
| ExportEntry | ||
| id string | ||
| } | ||
|
|
||
| var exporters []exporter | ||
| ids := make(map[string]int) | ||
| for _, exp := range opt.Exports { | ||
| if id, ok := ids[exp.Type]; !ok { | ||
|
There was a problem hiding this comment. A naive way to auto-id the exporters. Potentially could also be provided |
||
| ids[exp.Type] = 1 | ||
| } else { | ||
| ids[exp.Type] = id + 1 | ||
| } | ||
| exporters = append(exporters, exporter{ | ||
| ExportEntry: exp, | ||
| id: fmt.Sprint(exp.Type, ids[exp.Type]), | ||
| }) | ||
| } | ||
|
|
||
| storesToUpdate := []string{} | ||
|
|
@@ -156,58 +168,70 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG | |
| contentStores[key2] = store | ||
| } | ||
|
|
||
| var exporterConfig struct { | ||
| outputDirs []string | ||
| outputs map[string]filesync.FileOutputFunc | ||
| } | ||
|
|
||
| for _, ex := range exporters { | ||
| var supportFile bool | ||
| var supportDir bool | ||
| switch ex.Type { | ||
| case ExporterLocal: | ||
| supportDir = true | ||
| case ExporterTar: | ||
| supportFile = true | ||
| case ExporterOCI, ExporterDocker: | ||
| supportDir = ex.OutputDir != "" | ||
| supportFile = ex.Output != nil | ||
| } | ||
| if supportFile && supportDir { | ||
| return nil, errors.Errorf("both file and directory output is not supported by %s exporter", ex.Type) | ||
| } | ||
| if !supportFile && ex.Output != nil { | ||
| return nil, errors.Errorf("output file writer is not supported by %s exporter", ex.Type) | ||
| } | ||
| if !supportDir && ex.OutputDir != "" { | ||
| return nil, errors.Errorf("output directory is not supported by %s exporter", ex.Type) | ||
| } | ||
| if supportFile { | ||
| if ex.Output == nil { | ||
| return nil, errors.Errorf("output file writer is required for %s exporter", ex.Type) | ||
| } | ||
| if exporterConfig.outputs == nil { | ||
| exporterConfig.outputs = make(map[string]filesync.FileOutputFunc) | ||
| } | ||
| exporterConfig.outputs[ex.id] = ex.Output | ||
| } | ||
| if supportDir { | ||
| if ex.OutputDir == "" { | ||
| return nil, errors.Errorf("output directory is required for %s exporter", ex.Type) | ||
| } | ||
| switch ex.Type { | ||
| case ExporterOCI, ExporterDocker: | ||
| if err := os.MkdirAll(ex.OutputDir, 0755); err != nil { | ||
| return nil, err | ||
| } | ||
| cs, err := contentlocal.NewStore(ex.OutputDir) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| contentStores["export"] = cs | ||
| storesToUpdate = append(storesToUpdate, ex.OutputDir) | ||
| default: | ||
| exporterConfig.outputDirs = append(exporterConfig.outputDirs, ex.OutputDir) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if len(contentStores) > 0 { | ||
| s.Allow(sessioncontent.NewAttachable(contentStores)) | ||
| } | ||
|
|
||
| if len(exporterConfig.outputDirs) > 0 || len(exporterConfig.outputs) > 0 { | ||
| s.Allow(filesync.NewFSSyncTarget(exporterConfig.outputs, exporterConfig.outputDirs)) | ||
| } | ||
|
|
||
| eg.Go(func() error { | ||
| sd := c.sessionDialer | ||
| if sd == nil { | ||
|
|
@@ -255,11 +279,33 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG | |
| frontendInputs[key] = def.ToPB() | ||
| } | ||
|
|
||
| exports := make([]*controlapi.Exporter, 0, len(opt.Exports)) | ||
| var localExporter *controlapi.Exporter | ||
| for _, exp := range exporters { | ||
| if exp.Type != ExporterLocal { | ||
| exports = append(exports, &controlapi.Exporter{ | ||
| ID: exp.id, | ||
| Type: exp.Type, | ||
| Attrs: exp.Attrs, | ||
| }) | ||
| } else if localExporter == nil { | ||
| // TODO(dima): different options per local exporter? | ||
|
There was a problem hiding this comment. This is still unclear how to deal with potentially different options for multiple local exporters as currently (as implemented in this PR), the local export outputs are duplicated solely on the client, which means that any configuration taken into account on the server (for example, the attestation file prefix) will have to come from a single exporter configuration - e.g. the first (as is also currently implemented). Would appreciate feedback on this. There was a problem hiding this comment. I think if we do my suggestion below for multi-plexing between multiple output directories, this issue should go away? We then shouldn't need to do the de-duplication. There was a problem hiding this comment. I think I would need more context on this. It was a good pointer to avoid a new gRPC service - I did this with metadata instead, but it does not remove the option for users to specify multiple outputs of type local (including options which may vary) - this was my confusion at this specific point. |
||
| localExporter = &controlapi.Exporter{ | ||
| Type: ExporterLocal, | ||
| Attrs: exp.Attrs, | ||
| } | ||
| } | ||
| } | ||
| if localExporter != nil { | ||
| // Add a single instance of the local exporter | ||
| // since it's replicated entirely on the client | ||
| exports = append(exports, localExporter) | ||
| } | ||
|
|
||
| resp, err := c.ControlClient().Solve(ctx, &controlapi.SolveRequest{ | ||
| Ref: ref, | ||
| Definition: pbd, | ||
| Exporters: exports, | ||
| Session: s.ID(), | ||
| Frontend: opt.Frontend, | ||
| FrontendAttrs: frontendAttrs, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add
tarexporters here as well for good measure.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, just realized that this could be a bigger issue with the multiple file sender implementations like local and tar exporters. There needs to be some sort of fan-out for the multiple file exporters on the gRPC level.