From 255569bd0aa00a5f7028cb54859a8b59b1c9458c Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Thu, 6 May 2021 14:57:10 -0700 Subject: [PATCH] Align eval output with render (#1907) * refactor fn runner * update eval output format * omit output when the result is printed to stdout * fix test * improve comment * always use FunctionRunner --- commands/fncmd.go | 2 +- .../missing-fn-config/.expected/config.yaml | 3 + .../missing-fn-image/.expected/config.yaml | 3 + internal/cmdrender/executor.go | 5 +- internal/fnruntime/container.go | 19 ----- .../runner.go} | 63 +++++++++++---- .../runner_test.go} | 14 +--- .../cmdconfig/commands/cmdeval/cmdeval.go | 11 ++- .../commands/cmdeval/cmdeval_test.go | 7 +- thirdparty/kyaml/runfn/runfn.go | 17 ++-- thirdparty/kyaml/runfn/runfn_test.go | 78 ------------------- 11 files changed, 84 insertions(+), 138 deletions(-) rename internal/{cmdrender/pipeline_function.go => fnruntime/runner.go} (60%) rename internal/{cmdrender/pipeline_function_test.go => fnruntime/runner_test.go} (82%) diff --git a/commands/fncmd.go b/commands/fncmd.go index 5bcc49c610..d7e2a93aa3 100644 --- a/commands/fncmd.go +++ b/commands/fncmd.go @@ -46,7 +46,7 @@ func GetFnCommand(ctx context.Context, name string) *cobra.Command { }, } - eval := cmdeval.EvalCommand(name) + eval := cmdeval.EvalCommand(ctx, name) eval.Short = fndocs.RunShort eval.Long = fndocs.RunShort + "\n" + fndocs.RunLong eval.Example = fndocs.RunExamples diff --git a/e2e/testdata/fn-eval/missing-fn-config/.expected/config.yaml b/e2e/testdata/fn-eval/missing-fn-config/.expected/config.yaml index c34ab36e74..e902edfec8 100644 --- a/e2e/testdata/fn-eval/missing-fn-config/.expected/config.yaml +++ b/e2e/testdata/fn-eval/missing-fn-config/.expected/config.yaml @@ -16,3 +16,6 @@ testType: eval exitCode: 1 image: gcr.io/kpt-fn/set-namespace:v0.1 stdErr: "failed to configure function: input namespace cannot be empty" +stdOut: | + [RUNNING] "gcr.io/kpt-fn/set-namespace:v0.1" + [FAIL] "gcr.io/kpt-fn/set-namespace:v0.1" diff --git a/e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml b/e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml index a608401d18..c22d0b65f9 100644 --- a/e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml +++ b/e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml @@ -18,3 +18,6 @@ image: gcr.io/kpt-fn/dne # non-existing image args: namespace: staging stdErr: 'failed to check function image existence: function image "gcr.io/kpt-fn/dne" doesn''t exist' +stdOut: | + [RUNNING] "gcr.io/kpt-fn/dne" + [FAIL] "gcr.io/kpt-fn/dne" diff --git a/internal/cmdrender/executor.go b/internal/cmdrender/executor.go index 246b797554..9bc3b3d686 100644 --- a/internal/cmdrender/executor.go +++ b/internal/cmdrender/executor.go @@ -23,6 +23,7 @@ import ( "strings" "github.com/GoogleContainerTools/kpt/internal/errors" + "github.com/GoogleContainerTools/kpt/internal/fnruntime" "github.com/GoogleContainerTools/kpt/internal/pkg" "github.com/GoogleContainerTools/kpt/internal/printer" "github.com/GoogleContainerTools/kpt/internal/types" @@ -356,7 +357,7 @@ func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, in for i := range pl.Validators { fn := pl.Validators[i] - validator, err := newFnRunner(ctx, &fn, pn.pkg.UniquePath) + validator, err := fnruntime.NewContainerRunner(ctx, &fn, pn.pkg.UniquePath) if err != nil { return err } @@ -410,7 +411,7 @@ func fnChain(ctx context.Context, pkgPath types.UniquePath, fns []kptfilev1alpha var runners []kio.Filter for i := range fns { fn := fns[i] - r, err := newFnRunner(ctx, &fn, pkgPath) + r, err := fnruntime.NewContainerRunner(ctx, &fn, pkgPath) if err != nil { return nil, err } diff --git a/internal/fnruntime/container.go b/internal/fnruntime/container.go index 4a1fe92154..df6dff35e9 100644 --- a/internal/fnruntime/container.go +++ b/internal/fnruntime/container.go @@ -47,25 +47,6 @@ type ContainerFnPermission struct { AllowMount bool } -// ContainerFnWrapper wraps the real function filter, prints -// the function running progress and failures. -type ContainerFnWrapper struct { - Fn *ContainerFn -} - -func (fw *ContainerFnWrapper) Run(r io.Reader, w io.Writer) error { - pr := printer.FromContextOrDie(fw.Fn.Ctx) - printOpt := printer.NewOpt() - pr.OptPrintf(printOpt, "[RUNNING] %q\n", fw.Fn.Image) - err := fw.Fn.Run(r, w) - if err != nil { - pr.OptPrintf(printOpt, "[FAIL] %q\n", fw.Fn.Image) - return err - } - pr.OptPrintf(printOpt, "[PASS] %q\n", fw.Fn.Image) - return nil -} - // ContainerFn implements a KRMFn which run a containerized // KRM function type ContainerFn struct { diff --git a/internal/cmdrender/pipeline_function.go b/internal/fnruntime/runner.go similarity index 60% rename from internal/cmdrender/pipeline_function.go rename to internal/fnruntime/runner.go index 4d1a5eaf7e..a970bb5b12 100644 --- a/internal/cmdrender/pipeline_function.go +++ b/internal/fnruntime/runner.go @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package pipeline provides struct definitions for Pipeline and utility -// methods to read and write a pipeline resource. -package cmdrender +package fnruntime import ( "context" @@ -24,7 +22,7 @@ import ( "path/filepath" "github.com/GoogleContainerTools/kpt/internal/errors" - "github.com/GoogleContainerTools/kpt/internal/fnruntime" + "github.com/GoogleContainerTools/kpt/internal/printer" "github.com/GoogleContainerTools/kpt/internal/types" kptfilev1alpha2 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1alpha2" "sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil" @@ -32,30 +30,65 @@ import ( "sigs.k8s.io/kustomize/kyaml/yaml" ) -// newFnRunner returns a fnRunner from the image and configs of -// this function. -func newFnRunner(ctx context.Context, f *kptfilev1alpha2.Function, pkgPath types.UniquePath) (kio.Filter, error) { +// NewContainerRunner returns a kio.Filter given a specification of a container function +// and it's config. +func NewContainerRunner(ctx context.Context, f *kptfilev1alpha2.Function, pkgPath types.UniquePath) (kio.Filter, error) { config, err := newFnConfig(f, pkgPath) if err != nil { return nil, err } - - cfn := &fnruntime.ContainerFn{ + cfn := &ContainerFn{ Path: pkgPath, Image: f.Image, Ctx: ctx, } - - cfnw := &fnruntime.ContainerFnWrapper{ - Fn: cfn, + fltr := &runtimeutil.FunctionFilter{ + Run: cfn.Run, + FunctionConfig: config, } + return NewFunctionRunner(ctx, fltr, f.Image, false) +} - return &runtimeutil.FunctionFilter{ - Run: cfnw.Run, - FunctionConfig: config, +// NewFunctionRunner returns a kio.Filter given a specification of a function +// and it's config. +func NewFunctionRunner(ctx context.Context, fltr *runtimeutil.FunctionFilter, name string, disableOutput bool) (kio.Filter, error) { + return &FunctionRunner{ + ctx: ctx, + name: name, + filter: fltr, + disableOutput: disableOutput, }, nil } +// FunctionRunner wraps FunctionFilter and implements kio.Filter interface. +type FunctionRunner struct { + ctx context.Context + name string + disableOutput bool + filter *runtimeutil.FunctionFilter +} + +func (fr *FunctionRunner) Filter(input []*yaml.RNode) (output []*yaml.RNode, err error) { + if fr.disableOutput { + output, err = fr.filter.Filter(input) + } else { + pr := printer.FromContextOrDie(fr.ctx) + printOpt := printer.NewOpt() + pr.OptPrintf(printOpt, "[RUNNING] %q\n", fr.name) + output, err = fr.filter.Filter(input) + if err != nil { + pr.OptPrintf(printOpt, "[FAIL] %q\n", fr.name) + return nil, err + } + // capture the result from running the function + pr.OptPrintf(printOpt, "[PASS] %q\n", fr.name) + } + + // TODO(droot): print functionResults + + return +} + func newFnConfig(f *kptfilev1alpha2.Function, pkgPath types.UniquePath) (*yaml.RNode, error) { const op errors.Op = "fn.readConfig" var fn errors.Fn = errors.Fn(f.Image) diff --git a/internal/cmdrender/pipeline_function_test.go b/internal/fnruntime/runner_test.go similarity index 82% rename from internal/cmdrender/pipeline_function_test.go rename to internal/fnruntime/runner_test.go index aa68760387..858b812f80 100644 --- a/internal/cmdrender/pipeline_function_test.go +++ b/internal/fnruntime/runner_test.go @@ -12,21 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - // Package pipeline provides struct definitions for Pipeline and utility // methods to read and write a pipeline resource. -package cmdrender +package fnruntime import ( "io/ioutil" diff --git a/thirdparty/cmdconfig/commands/cmdeval/cmdeval.go b/thirdparty/cmdconfig/commands/cmdeval/cmdeval.go index 9f0ecad620..000202ae34 100644 --- a/thirdparty/cmdconfig/commands/cmdeval/cmdeval.go +++ b/thirdparty/cmdconfig/commands/cmdeval/cmdeval.go @@ -4,6 +4,7 @@ package cmdeval import ( + "context" "fmt" "io" "os" @@ -18,8 +19,8 @@ import ( ) // GetEvalFnRunner returns a EvalFnRunner. -func GetEvalFnRunner(name string) *EvalFnRunner { - r := &EvalFnRunner{} +func GetEvalFnRunner(ctx context.Context, name string) *EvalFnRunner { + r := &EvalFnRunner{Ctx: ctx} c := &cobra.Command{ Use: "eval [DIR | -]", RunE: r.runE, @@ -61,8 +62,8 @@ func GetEvalFnRunner(name string) *EvalFnRunner { return r } -func EvalCommand(name string) *cobra.Command { - return GetEvalFnRunner(name).Command +func EvalCommand(ctx context.Context, name string) *cobra.Command { + return GetEvalFnRunner(ctx, name).Command } // EvalFnRunner contains the run function @@ -81,6 +82,7 @@ type EvalFnRunner struct { Env []string AsCurrentUser bool IncludeMetaResources bool + Ctx context.Context } func (r *EvalFnRunner) runE(c *cobra.Command, _ []string) error { @@ -275,6 +277,7 @@ func (r *EvalFnRunner) preRunE(c *cobra.Command, args []string) error { } r.RunFns = runfn.RunFns{ + Ctx: r.Ctx, Functions: fns, Output: output, Input: input, diff --git a/thirdparty/cmdconfig/commands/cmdeval/cmdeval_test.go b/thirdparty/cmdconfig/commands/cmdeval/cmdeval_test.go index 811e1b9c97..50ae38384e 100644 --- a/thirdparty/cmdconfig/commands/cmdeval/cmdeval_test.go +++ b/thirdparty/cmdconfig/commands/cmdeval/cmdeval_test.go @@ -4,6 +4,7 @@ package cmdeval import ( + "context" "io" "os" "strings" @@ -183,6 +184,7 @@ apiVersion: v1 ResultsDir: "foo/", Env: []string{}, ContinueOnEmptyResult: true, + Ctx: context.TODO(), }, expected: ` metadata: @@ -219,6 +221,7 @@ apiVersion: v1 LogSteps: true, Env: []string{}, ContinueOnEmptyResult: true, + Ctx: context.TODO(), }, expected: ` metadata: @@ -239,6 +242,7 @@ apiVersion: v1 Path: "dir", Env: []string{"FOO=BAR", "BAR"}, ContinueOnEmptyResult: true, + Ctx: context.TODO(), }, expected: ` metadata: @@ -260,6 +264,7 @@ apiVersion: v1 AsCurrentUser: true, Env: []string{}, ContinueOnEmptyResult: true, + Ctx: context.TODO(), }, expected: ` metadata: @@ -287,7 +292,7 @@ apiVersion: v1 for i := range tests { tt := tests[i] t.Run(tt.name, func(t *testing.T) { - r := GetEvalFnRunner("kpt") + r := GetEvalFnRunner(context.TODO(), "kpt") // Don't run the actual command r.Command.Run = nil r.Command.RunE = func(cmd *cobra.Command, args []string) error { return nil } diff --git a/thirdparty/kyaml/runfn/runfn.go b/thirdparty/kyaml/runfn/runfn.go index 059cc420d5..2b3011cc73 100644 --- a/thirdparty/kyaml/runfn/runfn.go +++ b/thirdparty/kyaml/runfn/runfn.go @@ -4,6 +4,7 @@ package runfn import ( + "context" "fmt" "io" "os" @@ -30,6 +31,8 @@ import ( // RunFns runs the set of configuration functions in a local directory against // the Resources in that directory type RunFns struct { + Ctx context.Context + StorageMounts []runtimeutil.StorageMount // Path is the path to the directory containing functions @@ -419,6 +422,8 @@ func (r *RunFns) defaultFnFilterProvider(spec runtimeutil.FunctionSpec, fnConfig return nil, err } } + var fltr *runtimeutil.FunctionFilter + var name string if spec.Container.Image != "" { // TODO: Add a test for this behavior uidgid, err := getUIDGID(r.AsCurrentUser, currentUser) @@ -438,27 +443,29 @@ func (r *RunFns) defaultFnFilterProvider(spec runtimeutil.FunctionSpec, fnConfig AllowMount: true, }, } - cf := &runtimeutil.FunctionFilter{ + fltr = &runtimeutil.FunctionFilter{ Run: c.Run, FunctionConfig: fnConfig, DeferFailure: spec.DeferFailure, ResultsFile: resultsFile, } - return cf, nil + name = spec.Container.Image } if spec.Exec.Path != "" { e := &fnruntime.ExecFn{ Path: spec.Exec.Path, } - ef := &runtimeutil.FunctionFilter{ + fltr = &runtimeutil.FunctionFilter{ Run: e.Run, FunctionConfig: fnConfig, DeferFailure: spec.DeferFailure, ResultsFile: resultsFile, } - return ef, nil + name = spec.Exec.Path } + // if output is not nil we will write the resources to stdout + disableOutput := (r.Output != nil) + return fnruntime.NewFunctionRunner(r.Ctx, fltr, name, disableOutput) - return nil, nil } diff --git a/thirdparty/kyaml/runfn/runfn_test.go b/thirdparty/kyaml/runfn/runfn_test.go index a011587be2..9c9d06797c 100644 --- a/thirdparty/kyaml/runfn/runfn_test.go +++ b/thirdparty/kyaml/runfn/runfn_test.go @@ -5,15 +5,12 @@ package runfn import ( "bytes" - "fmt" "io/ioutil" "os" - "os/user" "path/filepath" "runtime" "testing" - "github.com/GoogleContainerTools/kpt/internal/fnruntime" "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1alpha2" "github.com/stretchr/testify/assert" @@ -55,81 +52,6 @@ metadata: ` ) -func currentUser() (*user.User, error) { - return &user.User{ - Uid: "1", - Gid: "2", - }, nil -} - -func TestRunFns_init(t *testing.T) { - instance := RunFns{} - assert.NoError(t, instance.init()) - if !assert.Equal(t, instance.Input, os.Stdin) { - t.FailNow() - } - if !assert.Equal(t, instance.Output, os.Stdout) { - t.FailNow() - } - - api, err := yaml.Parse(`apiVersion: apps/v1 -kind: -`) - spec := runtimeutil.FunctionSpec{ - Container: runtimeutil.ContainerSpec{ - Image: "example.com:version", - }, - } - if !assert.NoError(t, err) { - return - } - filter, _ := instance.functionFilterProvider(spec, api, currentUser) - c := fnruntime.ContainerFn{ - Image: "example.com:version", - UIDGID: "nobody", - } - cf := &runtimeutil.FunctionFilter{ - Run: c.Run, - FunctionConfig: api, - } - assert.Equal(t, fmt.Sprint(cf), fmt.Sprint(filter)) -} - -func TestRunFns_initAsCurrentUser(t *testing.T) { - instance := RunFns{ - AsCurrentUser: true, - } - assert.NoError(t, instance.init()) - if !assert.Equal(t, instance.Input, os.Stdin) { - t.FailNow() - } - if !assert.Equal(t, instance.Output, os.Stdout) { - t.FailNow() - } - - api, err := yaml.Parse(`apiVersion: apps/v1 -kind: -`) - spec := runtimeutil.FunctionSpec{ - Container: runtimeutil.ContainerSpec{ - Image: "example.com:version", - }, - } - if !assert.NoError(t, err) { - return - } - filter, _ := instance.functionFilterProvider(spec, api, currentUser) - c := fnruntime.ContainerFn{ - Image: "example.com:version", - UIDGID: "1:2", - } - cf := &runtimeutil.FunctionFilter{ - Run: c.Run, - FunctionConfig: api, - } - assert.Equal(t, fmt.Sprint(cf), fmt.Sprint(filter)) -} - func TestRunFns_Execute__initDefault(t *testing.T) { b := &bytes.Buffer{} var tests = []struct {