Skip to content

Commit

Permalink
Address review comments:
Browse files Browse the repository at this point in the history
 * assign the input source in exporters to avoid re-using the stale
   reference
 * remove the FSSend gRPC service - instead rely on gRPC metadata to
   communicate the functional protocol
  • Loading branch information
a-palchikov committed Aug 2, 2023
1 parent dbe146e commit 079a81f
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 199 deletions.
7 changes: 2 additions & 5 deletions client/solve.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,8 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
s.Allow(sessioncontent.NewAttachable(contentStores))
}

if len(exporterConfig.outputDirs) > 0 {
s.Allow(filesync.NewFSSyncTargetDir(exporterConfig.outputDirs))
}
if len(exporterConfig.outputs) > 0 {
s.Allow(filesync.NewFSSyncTarget(exporterConfig.outputs))
if len(exporterConfig.outputDirs) > 0 || len(exporterConfig.outputs) > 0 {
s.Allow(filesync.NewFSSyncTarget(exporterConfig.outputs, exporterConfig.outputDirs))
}

eg.Go(func() error {
Expand Down
15 changes: 8 additions & 7 deletions exporter/containerimage/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,17 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source
return e.ExportImage(ctx, src, exptypes.InlineCache{}, sessionID)
}

func (e *imageExporterInstance) ExportImage(ctx context.Context, inp *exporter.Source, inlineCache exptypes.InlineCache, sessionID string) (_ map[string]string, descref exporter.DescriptorReference, err error) {
func (e *imageExporterInstance) ExportImage(ctx context.Context, src *exporter.Source, inlineCache exptypes.InlineCache, sessionID string) (_ map[string]string, descref exporter.DescriptorReference, err error) {
meta := make(map[string][]byte)
for k, v := range inp.Metadata {
for k, v := range src.Metadata {
meta[k] = v
}
for k, v := range e.meta {
meta[k] = v
}
src := *inp
src.Metadata = meta
inp := *src
inp.Metadata = meta
src = &inp

opts := e.opts
as, _, err := ParseAnnotations(meta)
Expand All @@ -220,7 +221,7 @@ func (e *imageExporterInstance) ExportImage(ctx context.Context, inp *exporter.S
}
}()

desc, err := e.opt.ImageWriter.Commit(ctx, &src, sessionID, &opts)
desc, err := e.opt.ImageWriter.Commit(ctx, src, sessionID, &opts)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -282,7 +283,7 @@ func (e *imageExporterInstance) ExportImage(ctx context.Context, inp *exporter.S
tagDone(nil)

if e.unpack {
if err := e.unpackImage(ctx, img, &src, session.NewGroup(sessionID)); err != nil {
if err := e.unpackImage(ctx, img, src, session.NewGroup(sessionID)); err != nil {
return nil, nil, err
}
}
Expand Down Expand Up @@ -318,7 +319,7 @@ func (e *imageExporterInstance) ExportImage(ctx context.Context, inp *exporter.S
}
}
if e.push {
err := e.pushImage(ctx, &src, sessionID, targetName, desc.Digest)
err := e.pushImage(ctx, src, sessionID, targetName, desc.Digest)
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to push %v", targetName)
}
Expand Down
20 changes: 8 additions & 12 deletions exporter/oci/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,32 +117,28 @@ func (e *imageExporterInstance) Export(ctx context.Context, src *exporter.Source
return e.ExportImage(ctx, src, exptypes.InlineCache{}, sessionID)
}

func (e *imageExporterInstance) ExportImage(ctx context.Context, inp *exporter.Source, inlineCache exptypes.InlineCache, sessionID string) (_ map[string]string, descref exporter.DescriptorReference, err error) {
if e.opt.Variant == VariantDocker && len(inp.Refs) > 0 {
func (e *imageExporterInstance) ExportImage(ctx context.Context, src *exporter.Source, inlineCache exptypes.InlineCache, sessionID string) (_ map[string]string, descref exporter.DescriptorReference, err error) {
if e.opt.Variant == VariantDocker && len(src.Refs) > 0 {
return nil, nil, errors.Errorf("docker exporter does not currently support exporting manifest lists")
}

meta := make(map[string][]byte)
for k, v := range inp.Metadata {
for k, v := range src.Metadata {
meta[k] = v
}
for k, v := range e.meta {
meta[k] = v
}
src := *inp
src.Metadata = meta
inp := *src
inp.Metadata = meta
src = &inp

opts := e.opts
as, _, err := containerimage.ParseAnnotations(meta)
if err != nil {
return nil, nil, err
}
opts.Annotations = opts.Annotations.Merge(as)

if err != nil {
return nil, nil, err
}
opts.Annotations = as.Merge(opts.Annotations)
opts.InlineCache = inlineCache

ctx, done, err := leaseutil.WithLease(ctx, e.opt.LeaseManager, leaseutil.MakeTemporary)
Expand All @@ -155,7 +151,7 @@ func (e *imageExporterInstance) ExportImage(ctx context.Context, inp *exporter.S
}
}()

desc, err := e.opt.ImageWriter.Commit(ctx, &src, sessionID, &opts)
desc, err := e.opt.ImageWriter.Commit(ctx, src, sessionID, &opts)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -190,7 +186,7 @@ func (e *imageExporterInstance) ExportImage(ctx context.Context, inp *exporter.S
}
resp[exptypes.ExporterImageDescriptorKey] = base64.StdEncoding.EncodeToString(dtdesc)

if n, ok := meta["image.name"]; e.opts.ImageName == "*" && ok {
if n, ok := src.Metadata["image.name"]; e.opts.ImageName == "*" && ok {
e.opts.ImageName = string(n)
}

Expand Down
69 changes: 24 additions & 45 deletions session/filesync/filesync.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const (
keyExcludePatterns = "exclude-patterns"
keyFollowPaths = "followpaths"
keyDirName = "dir-name"
keyDirSync = "dir-sync"
keyExporterMetaPrefix = "exporter-md-"
)

Expand Down Expand Up @@ -67,6 +68,7 @@ func (sp *fsSyncProvider) Register(server *grpc.Server) {
func (sp *fsSyncProvider) DiffCopy(stream FileSync_DiffCopyServer) error {
return sp.handle("diffcopy", stream)
}

func (sp *fsSyncProvider) TarStream(stream FileSync_TarStreamServer) error {
return sp.handle("tarstream", stream)
}
Expand Down Expand Up @@ -229,76 +231,52 @@ func FSSync(ctx context.Context, c session.Caller, opt FSSendRequestOpt) error {
return pr.recvFn(stream, opt.DestDir, opt.CacheUpdater, opt.ProgressCb, opt.Differ, opt.Filter)
}

// NewFSSyncTargetDir allows writing into a directory
func NewFSSyncTargetDir(outdirs []string) session.Attachable {
p := &fsSyncTargetDir{
outdirs: outdirs,
}
return p
}

type fsSyncTargetDir struct {
outdirs []string
}

func (sp *fsSyncTargetDir) Register(server *grpc.Server) {
RegisterFSSendServer(server, sp)
}

func (sp *fsSyncTargetDir) DiffCopy(stream FSSend_DiffCopyServer) (err error) {
return syncTargetDiffCopy(stream, sp.outdirs)
}

// NewFSSyncTarget allows writing into an io.WriteCloser
func NewFSSyncTarget(fs map[string]FileOutputFunc) session.Attachable {
// or stream a file system to the client
func NewFSSyncTarget(fs map[string]FileOutputFunc, outdirs []string) session.Attachable {
p := &fsSyncTarget{
fs: fs,
fs: fs,
outdirs: outdirs,
}
return p
}

type fsSyncTarget struct {
// fs maps exporter ID -> output func
fs map[string]FileOutputFunc
fs map[string]FileOutputFunc
outdirs []string
}

func (sp *fsSyncTarget) Register(server *grpc.Server) {
RegisterFileSendServer(server, sp)
}

func (sp *fsSyncTarget) DiffCopy(stream FileSend_DiffCopyServer) (err error) {
if len(sp.fs) == 0 {
return errors.New("empty outfile and outdir")
if len(sp.fs) == 0 && len(sp.outdirs) == 0 {
return errors.New("empty outfile and outdirs")
}
opts, _ := metadata.FromIncomingContext(stream.Context()) // if no metadata continue with empty object

if _, ok := opts[keyDirSync]; ok {
return syncTargetDiffCopy(stream, sp.outdirs)
}

return writeTargetFile(stream, sp.fs, opts)
}

func CopyToCaller(ctx context.Context, fs fsutil.FS, c session.Caller, progress func(int, bool)) error {
method := session.MethodURL(_FSSend_serviceDesc.ServiceName, "diffcopy")
method := session.MethodURL(_FileSend_serviceDesc.ServiceName, "diffcopy")
if !c.Supports(method) {
return copyToCallerLegacy(ctx, fs, c, progress)
return errors.Errorf("method %s not supported by the client", method)
}

client := NewFSSendClient(c.Conn())

cc, err := client.DiffCopy(ctx)
if err != nil {
return errors.WithStack(err)
opts := map[string][]string{
keyDirSync: {"true"},
}

return sendDiffCopy(cc, fs, progress)
}

func copyToCallerLegacy(ctx context.Context, fs fsutil.FS, c session.Caller, progress func(int, bool)) error {
method := session.MethodURL(_FileSend_serviceDesc.ServiceName, "diffcopy")
if !c.Supports(method) {
return errors.Errorf("method %s not supported by the client", method)
}
ctx = metadata.NewOutgoingContext(ctx, opts)

client := NewFileSendClient(c.Conn())

cc, err := client.DiffCopy(ctx)
if err != nil {
return errors.WithStack(err)
Expand Down Expand Up @@ -358,13 +336,13 @@ func decodeOpts(opts map[string][]string) map[string][]string {
md := make(map[string][]string, len(opts))
for k, v := range opts {
out := make([]string, len(v))
var isDecoded bool
var isEncoded bool
if v, ok := opts[k+"-encoded"]; ok && len(v) > 0 {
if b, _ := strconv.ParseBool(v[0]); b {
isDecoded = true
isEncoded = true
}
}
if isDecoded {
if isEncoded {
for i, s := range v {
out[i], _ = url.QueryUnescape(s)
}
Expand All @@ -380,13 +358,14 @@ func decodeOpts(opts map[string][]string) map[string][]string {
// is backwards compatible and avoids encoding ASCII characters.
func encodeStringForHeader(inputs []string) ([]string, bool) {
var encode bool
L:
for _, input := range inputs {
for _, runeVal := range input {
// Only encode non-ASCII characters, and characters that have special
// meaning during decoding.
if runeVal > unicode.MaxASCII {
encode = true
break
break L
}
}
}
Expand Down
Loading

0 comments on commit 079a81f

Please sign in to comment.