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

formatting changes introduced by eval #1973

Closed
frankfarzan opened this issue May 11, 2021 · 4 comments
Closed

formatting changes introduced by eval #1973

frankfarzan opened this issue May 11, 2021 · 4 comments
Assignees
Labels
area/hydrate bug Something isn't working triaged Issue has been triaged by adding an `area/` label
Milestone

Comments

@frankfarzan
Copy link
Contributor

frankfarzan commented May 11, 2021

kpt pkg get https://github.com/GoogleContainerTools/kpt.git/package-examples/wordpress@next
cd wordpress
kpt fn render
kpt fn eval --image gcr.io/kpt-fn/set-namespace:v0.1 -- namespace=wordpress
git diff

Two issues:

  1. Why is set-namespace changing the formatting?
  2. More importantly: This is a general issue that can happen with arbitrary functions. The current answer is that you should run render after eval. But then, why not just have eval perform formatting as well as a convenience?
@frankfarzan frankfarzan added this to the v1.0 m2 milestone May 11, 2021
@frankfarzan frankfarzan added area/hydrate triaged Issue has been triaged by adding an `area/` label labels May 11, 2021
@phanimarupaka
Copy link
Contributor

This is an issue with the kyaml version being used by functions. set-labels set-namespace for instance are having this problem as they are using older version of kyaml. https://github.com/GoogleContainerTools/kpt-functions-catalog/blob/master/functions/go/set-namespace/go.mod#L8

apply-setters, search-replace work fine.

@mengqiy @droot I think we should have consistent usage of kyaml all across the functions. This is not just a problem with formatting but results in general.

This doesn't justify formatting resources by default as part of eval command as eval can be invoked on validation functions like kubeval, seeing formatting related diffs for invoking validator functions is scary.

@mikebz mikebz added the bug Something isn't working label May 11, 2021
@frankfarzan
Copy link
Contributor Author

frankfarzan commented May 11, 2021

But you can have a pipeline with just a validator and render would format?

Not all functions are or will be using Go or kyaml, so the general question about formatting by eval is orthogonal.

@phanimarupaka
Copy link
Contributor

But you can have a pipeline with just a validator and render would format?

Not all functions are or will be using Go or kyaml, so the general question about formatting by eval is orthogonal.

So you say we should format resources as part of eval as well ?

@phanimarupaka
Copy link
Contributor

But you can have a pipeline with just a validator and render would format?

Not all functions are or will be using Go or kyaml, so the general question about formatting by eval is orthogonal.

Agree. So how about formatting resources as part of eval command as with clear messaging. In the following example, search(without replace) is formatting resources with clear messaging.

$ kpt fn eval --image gcr.io/kpt-fn/search-replace:v0.1 -- by-value=4

[RUNNING] "gcr.io/kpt-fn/search-replace:v0.1"
[PASS] "gcr.io/kpt-fn/search-replace:v0.1"
  Results:
    [INFO] Matched field value to "4" in file "resources.yaml" in field "spec.replicas"
    [INFO] Formatted resources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hydrate bug Something isn't working triaged Issue has been triaged by adding an `area/` label
Projects
None yet
Development

No branches or pull requests

3 participants