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

@@ -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

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"

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"

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
 		}

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 {

internal/cmdrender/pipeline_function.go → 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,38 +22,73 @@ 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"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"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)

internal/cmdrender/pipeline_function_test.go → 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"

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,

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 }

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
 }