From b8de4e5049a89b9930be0472035c1119ca2cfaa4 Mon Sep 17 00:00:00 2001 From: Andrea Luzzardi Date: Fri, 19 Mar 2021 15:07:01 -0700 Subject: [PATCH] cache fix: stable ordering of maps (Env, Mount, ...) Maps were causing the same Pipeline to randomly produce slightly different LLB on each run (because they are represented as an array in LLB, wheras they're a map in CueLLB). This forces every Cue field iteration (env, mount, etc) to be predictable by using stable sorting. Signed-off-by: Andrea Luzzardi --- dagger/compiler/value.go | 60 ++++++++++++++++----------------- dagger/pipeline.go | 73 +++++++++++++++++++++++++--------------- 2 files changed, 74 insertions(+), 59 deletions(-) diff --git a/dagger/compiler/value.go b/dagger/compiler/value.go index e04db9a3..0314cfdc 100644 --- a/dagger/compiler/value.go +++ b/dagger/compiler/value.go @@ -1,6 +1,8 @@ package compiler import ( + "sort" + "cuelang.org/go/cue" cueformat "cuelang.org/go/cue/format" ) @@ -72,9 +74,33 @@ func (v *Value) Kind() cue.Kind { return v.val.Kind() } +// Field represents a struct field +type Field struct { + Label string + Value *Value +} + // Proxy function to the underlying cue.Value -func (v *Value) Fields() (*cue.Iterator, error) { - return v.val.Fields() +// Field ordering is guaranteed to be stable. +func (v *Value) Fields() ([]Field, error) { + it, err := v.val.Fields() + if err != nil { + return nil, err + } + + fields := []Field{} + for it.Next() { + fields = append(fields, Field{ + Label: it.Label(), + Value: v.Wrap(it.Value()), + }) + } + + sort.SliceStable(fields, func(i, j int) bool { + return fields[i].Label < fields[j].Label + }) + + return fields, nil } // Proxy function to the underlying cue.Value @@ -121,38 +147,10 @@ func (v *Value) List() ([]*Value, error) { for it.Next() { l = append(l, v.Wrap(it.Value())) } + return l, nil } -// FIXME: deprecate to simplify -func (v *Value) RangeList(fn func(int, *Value) error) error { - it, err := v.val.List() - if err != nil { - return err - } - i := 0 - for it.Next() { - if err := fn(i, v.Wrap(it.Value())); err != nil { - return err - } - i++ - } - return nil -} - -func (v *Value) RangeStruct(fn func(string, *Value) error) error { - it, err := v.Fields() - if err != nil { - return err - } - for it.Next() { - if err := fn(it.Label(), v.Wrap(it.Value())); err != nil { - return err - } - } - return nil -} - // FIXME: receive string path? func (v *Value) Merge(x interface{}, path ...string) (*Value, error) { if xval, ok := x.(*Value); ok { diff --git a/dagger/pipeline.go b/dagger/pipeline.go index e7fe433c..1293d58c 100644 --- a/dagger/pipeline.go +++ b/dagger/pipeline.go @@ -112,12 +112,15 @@ func analyzeOp(fn func(*compiler.Value) error, op *compiler.Value) error { case "load", "copy": return Analyze(fn, op.Get("from")) case "exec": - return op.Get("mount").RangeStruct(func(dest string, mnt *compiler.Value) error { - if from := mnt.Get("from"); from.Exists() { + fields, err := op.Get("mount").Fields() + if err != nil { + return err + } + for _, mnt := range fields { + if from := mnt.Value.Get("from"); from.Exists() { return Analyze(fn, from) } - return nil - }) + } } return nil } @@ -337,7 +340,6 @@ func (p *Pipeline) Exec(ctx context.Context, op *compiler.Value, st llb.State) ( opts := []llb.RunOption{} var cmd struct { Args []string - Env map[string]string Dir string Always bool } @@ -349,10 +351,22 @@ func (p *Pipeline) Exec(ctx context.Context, op *compiler.Value, st llb.State) ( opts = append(opts, llb.Args(cmd.Args)) // dir opts = append(opts, llb.Dir(cmd.Dir)) + // env - for k, v := range cmd.Env { - opts = append(opts, llb.AddEnv(k, v)) + if env := op.Get("env"); env.Exists() { + envs, err := op.Get("env").Fields() + if err != nil { + return st, err + } + for _, env := range envs { + v, err := env.Value.String() + if err != nil { + return st, err + } + opts = append(opts, llb.AddEnv(env.Label, v)) + } } + // always? // FIXME: initialize once for an entire compute job, to avoid cache misses if cmd.Always { @@ -385,14 +399,17 @@ func (p *Pipeline) Exec(ctx context.Context, op *compiler.Value, st llb.State) ( func (p *Pipeline) mountAll(ctx context.Context, mounts *compiler.Value) ([]llb.RunOption, error) { opts := []llb.RunOption{} - err := mounts.RangeStruct(func(dest string, mnt *compiler.Value) error { - o, err := p.mount(ctx, dest, mnt) + fields, err := mounts.Fields() + if err != nil { + return nil, err + } + for _, mnt := range fields { + o, err := p.mount(ctx, mnt.Label, mnt.Value) if err != nil { - return err + return nil, err } opts = append(opts, o) - return nil - }) + } return opts, err } @@ -709,31 +726,31 @@ func (p *Pipeline) DockerBuild(ctx context.Context, op *compiler.Value, st llb.S } if buildArgs := op.Lookup("buildArg"); buildArgs.Exists() { - err := buildArgs.RangeStruct(func(key string, value *compiler.Value) error { - v, err := value.String() - if err != nil { - return err - } - req.FrontendOpt["build-arg:"+key] = v - return nil - }) + fields, err := buildArgs.Fields() if err != nil { return st, err } + for _, buildArg := range fields { + v, err := buildArg.Value.String() + if err != nil { + return st, err + } + req.FrontendOpt["build-arg:"+buildArg.Label] = v + } } if labels := op.Lookup("label"); labels.Exists() { - err := labels.RangeStruct(func(key string, value *compiler.Value) error { - s, err := value.String() - if err != nil { - return err - } - req.FrontendOpt["label:"+key] = s - return nil - }) + fields, err := labels.Fields() if err != nil { return st, err } + for _, label := range fields { + s, err := label.Value.String() + if err != nil { + return st, err + } + req.FrontendOpt["label:"+label.Label] = s + } } if platforms := op.Lookup("platforms"); platforms.Exists() {