Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align eval output with render #1907

Merged
merged 6 commits into from
May 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion commands/fncmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions e2e/testdata/fn-eval/missing-fn-config/.expected/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
3 changes: 3 additions & 0 deletions e2e/testdata/fn-eval/missing-fn-image/.expected/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
5 changes: 3 additions & 2 deletions internal/cmdrender/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
19 changes: 0 additions & 19 deletions internal/fnruntime/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not happy with this name since the name represent lower level than FunctionRunner.

But unable to find a good name, not worth blocking the PR :)

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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
11 changes: 7 additions & 4 deletions thirdparty/cmdconfig/commands/cmdeval/cmdeval.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package cmdeval

import (
"context"
"fmt"
"io"
"os"
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion thirdparty/cmdconfig/commands/cmdeval/cmdeval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package cmdeval

import (
"context"
"io"
"os"
"strings"
Expand Down Expand Up @@ -183,6 +184,7 @@ apiVersion: v1
ResultsDir: "foo/",
Env: []string{},
ContinueOnEmptyResult: true,
Ctx: context.TODO(),
},
expected: `
metadata:
Expand Down Expand Up @@ -219,6 +221,7 @@ apiVersion: v1
LogSteps: true,
Env: []string{},
ContinueOnEmptyResult: true,
Ctx: context.TODO(),
},
expected: `
metadata:
Expand All @@ -239,6 +242,7 @@ apiVersion: v1
Path: "dir",
Env: []string{"FOO=BAR", "BAR"},
ContinueOnEmptyResult: true,
Ctx: context.TODO(),
},
expected: `
metadata:
Expand All @@ -260,6 +264,7 @@ apiVersion: v1
AsCurrentUser: true,
Env: []string{},
ContinueOnEmptyResult: true,
Ctx: context.TODO(),
},
expected: `
metadata:
Expand Down Expand Up @@ -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 }
Expand Down
17 changes: 12 additions & 5 deletions thirdparty/kyaml/runfn/runfn.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package runfn

import (
"context"
"fmt"
"io"
"os"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -438,27 +443,29 @@ func (r *RunFns) defaultFnFilterProvider(spec runtimeutil.FunctionSpec, fnConfig
AllowMount: true,
},
}
cf := &runtimeutil.FunctionFilter{
fltr = &runtimeutil.FunctionFilter{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to get away from crafting this filter here manually but it will be easier once my structured result changes are merged in.

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
}
Loading