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

Conversation

Shell32-Natsu
Copy link
Contributor

@Shell32-Natsu Shell32-Natsu commented May 5, 2021

Align the output of fn eval with fn render.

Example output:

$ kpt fn eval --image gcr.io/kpt-fn/set-namespace:v0.1 -- namespace=staging
[RUNNING] "gcr.io/kpt-fn/set-namespace:v0.1"
[PASS] "gcr.io/kpt-fn/set-namespace:v0.1"
$ kpt fn eval --image gcr.io/kpt-fn/set-namespace:v0.1
[RUNNING] "gcr.io/kpt-fn/set-namespace:v0.1"
[FAIL] "gcr.io/kpt-fn/set-namespace:v0.1"
  Stderr:
    "failed to configure function: input namespace cannot be empty"
  Exit Code: 1

$ kpt fn source | kpt fn eval - --image gcr.io/kpt-fn/set-namespace:v0.1 -- namespace=staging
# Copyright 2021 Google LLC
#
# 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.
apiVersion: apps/v1
kind: Deployment
metadata:
...
$ kpt fn eval --image gcr.io/kpt-fn/set-namespace:v0.1 --dry-run -- namespace=staging
# Copyright 2021 Google LLC
#
# 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.
apiVersion: apps/v1
kind: Deployment
metadata:
...
$ kpt fn eval --image gcr.io/kpt-fn/kubeval:v0.1
[RUNNING] "gcr.io/kpt-fn/kubeval:v0.1"
[FAIL] "gcr.io/kpt-fn/kubeval:v0.1"
  Stderr:
    "[ERROR] Failed to parse raw kubeval output:"
    "Unexpected token E in JSON at position 0"
    ""
    "ERR  - stdin: Failed initializing schema file:///tmp/master-standalone/deploymen-apps-v1.json: open /tmp/master-standalone/deploymen-apps-v1.json: no such file or directory"
    ...(2 line(s) truncated)
  Exit Code: 1
$ kpt fn eval --image gcr.io/kpt-fn/kubeval:v0.1 --truncate-output=false
[RUNNING] "gcr.io/kpt-fn/kubeval:v0.1"
[FAIL] "gcr.io/kpt-fn/kubeval:v0.1"
  Stderr:
    "[ERROR] Failed to parse raw kubeval output:"
    "Unexpected token E in JSON at position 0"
    ""
    "ERR  - stdin: Failed initializing schema file:///tmp/master-standalone/deploymen-apps-v1.json: open /tmp/master-standalone/deploymen-apps-v1.json: no such file or directory"
    " in object 'apps/v1/Deploymen//my-nginx' in file deployment.yaml"
    ""
  Exit Code: 1

close #1861

@Shell32-Natsu Shell32-Natsu added this to the v1.0 m2 milestone May 5, 2021
@frankfarzan
Copy link
Contributor

frankfarzan commented May 5, 2021

Align the output of fn eval with fn render.

Example output:

$ kpt fn eval --image gcr.io/kpt-fn/set-namespace:v0.1 -- namespace=staging
[RUNNING] "gcr.io/kpt-fn/set-namespace:v0.1"
[PASS] "gcr.io/kpt-fn/set-namespace:v0.1"
$ kpt fn eval --image gcr.io/kpt-fn/set-namespace:v0.1
[RUNNING] "gcr.io/kpt-fn/set-namespace:v0.1"
[FAIL] "gcr.io/kpt-fn/set-namespace:v0.1"
  Stderr:
    "failed to configure function: input namespace cannot be empty"
  Exit Code: 1
$ kpt fn source | kpt fn eval - --image gcr.io/kpt-fn/set-namespace:v0.1 -- namespace=staging

Why does this show the resources instead of the PASS/FAIL? Per discussion, we need a document
on how render and fn will work when piped.

# Copyright 2021 Google LLC
#
# 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.
apiVersion: apps/v1
kind: Deployment
metadata:
...
$ kpt fn eval --image gcr.io/kpt-fn/set-namespace:v0.1 --dry-run -- namespace=staging
# Copyright 2021 Google LLC
#
# 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.
apiVersion: apps/v1
kind: Deployment
metadata:
...
$ kpt fn eval --image gcr.io/kpt-fn/kubeval:v0.1
[RUNNING] "gcr.io/kpt-fn/kubeval:v0.1"
[FAIL] "gcr.io/kpt-fn/kubeval:v0.1"
  Stderr:
    "[ERROR] Failed to parse raw kubeval output:"
    "Unexpected token E in JSON at position 0"
    ""
    "ERR  - stdin: Failed initializing schema file:///tmp/master-standalone/deploymen-apps-v1.json: open /tmp/master-standalone/deploymen-apps-v1.json: no such file or directory"
    ...(2 line(s) truncated)
  Exit Code: 1
$ kpt fn eval --image gcr.io/kpt-fn/kubeval:v0.1 --truncate-output=false
[RUNNING] "gcr.io/kpt-fn/kubeval:v0.1"
[FAIL] "gcr.io/kpt-fn/kubeval:v0.1"
  Stderr:
    "[ERROR] Failed to parse raw kubeval output:"
    "Unexpected token E in JSON at position 0"
    ""
    "ERR  - stdin: Failed initializing schema file:///tmp/master-standalone/deploymen-apps-v1.json: open /tmp/master-standalone/deploymen-apps-v1.json: no such file or directory"
    " in object 'apps/v1/Deploymen//my-nginx' in file deployment.yaml"
    ""
  Exit Code: 1

close #1861

@Shell32-Natsu
Copy link
Contributor Author

Why does this show the resources instead of the PASS/FAIL? Per discussion, we need a document
on how render and fn will work when piped.

The output destination here is controlled by - and it's added in #1710. So actually fn eval doesn't know is it in a pipeline but read from stdin and write to stdout if there is an argument -.

@frankfarzan
Copy link
Contributor

Why does this show the resources instead of the PASS/FAIL? Per discussion, we need a document
on how render and fn will work when piped.

The output destination here is controlled by - and it's added in #1710. So actually fn eval doesn't know is it in a pipeline but read from stdin and write to stdout if there is an argument -.

- controls reading from stdin, not necessarily the output behavior.

@Shell32-Natsu
Copy link
Contributor Author

Why does this show the resources instead of the PASS/FAIL? Per discussion, we need a document
on how render and fn will work when piped.

The output destination here is controlled by - and it's added in #1710. So actually fn eval doesn't know is it in a pipeline but read from stdin and write to stdout if there is an argument -.

- controls reading from stdin, not necessarily the output behavior.

Makes sense, but controlling both reading from stdin and writing to stdout keeps consistent with fn run. fn run will read from stdin and write to stdout if no directory specified.

@droot
Copy link
Contributor

droot commented May 6, 2021

Per discussion, we need a document
on how render and fn will work when piped.

We need to prioritized this, filed an issue #1915, will start working on hashing these details out otherwise it will be hard to make progress on these PRs.

@frankfarzan
Copy link
Contributor

frankfarzan commented May 6, 2021

To unblock this, we can address the piping behavior separeately. For now, this output lgtm:

$ kpt fn eval --image gcr.io/kpt-fn/set-namespace:v0.1 -- namespace=staging
[RUNNING] "gcr.io/kpt-fn/set-namespace:v0.1"
[PASS] "gcr.io/kpt-fn/set-namespace:v0.1"

thirdparty/kyaml/runfn/runfn.go Outdated Show resolved Hide resolved
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 :)

thirdparty/kyaml/runfn/runfn.go Outdated Show resolved Hide resolved
@@ -438,27 +443,30 @@ 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.

@Shell32-Natsu Shell32-Natsu merged commit 255569b into kptdev:next May 6, 2021
@Shell32-Natsu Shell32-Natsu deleted the eval-output branch May 6, 2021 21:57
frankfarzan pushed a commit to frankfarzan/kpt that referenced this pull request Jun 3, 2021
* refactor fn runner

* update eval output format

* omit output when the result is printed to stdout

* fix test

* improve comment

* always use FunctionRunner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants