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

Deprecation of kustomize cfg and kustomize fn alpha commands #3953

Closed
KnVerey opened this issue Jun 2, 2021 · 18 comments
Closed

Deprecation of kustomize cfg and kustomize fn alpha commands #3953

KnVerey opened this issue Jun 2, 2021 · 18 comments
Assignees
Labels
area/cli Issues for kustomize CLI interface kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. triage/under-consideration

Comments

@KnVerey
Copy link
Contributor

KnVerey commented Jun 2, 2021

Kustomize currently has a kustomize cfg subcommand that contains a variety of alpha yaml manipulation tools:

Commands for reading and writing configuration.

Usage:
  kustomize cfg [command]

Available Commands:
  annotate      [Alpha] Set an annotation on Resources.
  cat           [Alpha] Print Resource Config from a local directory.
  count         [Alpha] Count Resources Config from a local directory.
  create-setter [Alpha] Create a custom setter for a Resource field
  create-subst
  fmt           [Alpha] Format yaml configuration files.
  grep          [Alpha] Search for matching Resources in a directory or from stdin
  init          [Alpha] Initialize a directory with a Krmfile.
  list-setters  [Alpha] List setters for Resources.
  merge         [Alpha] Merge Resource configuration files
  merge3        [Alpha] Merge diff of Resource configuration files into a destination (3-way)
  set           [Alpha] Set values on Resources fields values.
  tree          [Alpha] Display Resource structure from a directory or stdin.

As part of our efforts to stabilize Kustomize APIs, we should decide what to do with these. They're loosely related to Kustomize in that keeping your configuration expressed in the canonical APIs (template-free) enables you to use such tools on your raw config. But at the same time, these tools aren't specific to Kustomize or part of recommended Kustomize workflows AFAIK.

Proposal:

  • keep the read-only commands (cat, count, grep, tree)
  • remove the init and setter commands--aren't setters more of a kpt concept? if we'd recommend using them with Kustomize, we'd need to document and explain this. E.g. when to use a setter vs patch vs replacement transformer
  • remove the other mutating commands (annotate, fmt, merge, merge3). These seem handy, but do they belong in Kustomize? E.g. to format kustomize output, you should probably declare a transformer in your Kustomization.

The above is a starting point for discussion rather than a strong opinion. Other options that come to mind: publish current cfg as a separate binary (kyaml?), or move to cli-experimental repo.

/triage under-consideration
/area cli
/kind deprecation

@k8s-ci-robot k8s-ci-robot added triage/under-consideration area/cli Issues for kustomize CLI interface kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. labels Jun 2, 2021
@bgrant0607
Copy link
Member

bgrant0607 commented Jun 18, 2021

I thought the experimental cfg command group had already been removed, but since it hasn't...

At least the tree command is useful, from what I've heard, but there's also the kubectl plugin:
https://github.com/ahmetb/kubectl-tree

The readonly commands could make a reasonable "view", "browse", or "scan" command group parallel with edit and build, though.

Setters (set) would fit as a transformer. The replacement transformer is powerful, but is verbose and requires centrally identifying targets. Setters would work more like the other transformers in that targets wouldn't need to be specified in kustomization.yaml. A target-identification transformer could also be added later, if necessary, but restricting to targets already specified in base resources is pretty useful already. I think of setters as an ad hoc transformer for cases where a dedicated transformer (like images, replicas) hasn't been built yet. A command to set setter values in kustomization.yaml would fit under edit.

I agree regarding removing the mutation commands.

cc @monopole @mortent @natasha41575

@bgrant0607
Copy link
Member

I don't think it makes sense to keep the Krmfile.

@natasha41575
Copy link
Contributor

natasha41575 commented Jul 8, 2021

+1 to keeping the readonly commands and removing the others.

There are KRM functions for creating and applying setters that kustomize could theoretically reuse with kustomize fn run, and I agree that mutating commands should be declared as transformers.

@marshall007
Copy link

I think I disagree that setters should be re-implemented as a transformer. See kptdev/kpt#2369 (comment)

It seems much more reasonable to have a hard distinction between in-place and out-of-place hydration. In combination with something like #3980 and a declarative function execution pipeline in kustomize I see no reason why setters would ever need to be performed out-of-place.

If setters are applied in a transformer I foresee similar issues to #4031 where changes to a resource's GVKNN are not tracked appropriately.

@natasha41575
Copy link
Contributor

@marshall007 WDYT about setters as a KRM function?

@bgrant0607
Copy link
Member

bgrant0607 commented Jul 8, 2021

kustomize performs out-of-place transformation/hydration, driven by a declarative specification (https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2377-Kustomize#capabilities). That's its model. Out-of-place transformations are especially useful for variant generation. In fitting with that model and usage, it should be possible to generate multiple variants from the same base. As seen with vars (#2052) and replacements (#1631), there is demand for a substitution mechanism that works with the out-of-place model. That setters used an in-place model was an accident of implementation while exploring (#1113 (comment)) alternatives to inline substitution (https://github.com/kubernetes/community/blob/master/contributors/design-proposals/architecture/declarative-application-management.md#what-about-parameterization). More generally, the in-place transformations provided an experimental method of exercising the new yaml transformation libraries (#1749), but lacked a holistic design for how they would fit with existing out-of-place functionality. Hence this issue.

@KnVerey
Copy link
Contributor Author

KnVerey commented Jul 8, 2021

@natasha41575 thanks for pointing out those KRM functions. Couldn't the "apply-setters" function be invoked declaratively as a KRM-style transformer today then? If so, the question is whether we need to bring that functionality in-tree vs. promote the use of that extensions transformer to those interested. Personally I like the idea of it remaining a extensions feature rather than bringing the additional concept (and need to explain it and contrast it with other features) into Kustomize core.

Re: the read-only commands, @bgrant0607 makes a good point about kubectl plugins. It seems to me none of those really have to do with Kustomize so much as with resource inspection in general. This makes me lean towards removing them as well, and possibly publishing the code as kubectl (krew) plugins from the experimental repo, if there's interest (and maybe even move some to kubectl itself some day if they take off?). That republishing could be an interesting standalone project for someone.

@marshall007
Copy link

@natasha41575 I think the UX of the new create-setters and apply-setters KRM functions is rather clunky in its current state but the overall direction makes sense. My main concern right now is that there are too many different options for manipulating setters and kustomize is now lagging behind:

  • declarative function pipelines defined in a Kptfile executed using kpt fn render
  • declarative "config functions" defined in a directory executed using kustomize fn run <dir>
  • the imperative versions of both of those (which do not persist setter values to declarative configuration)
  • all the kustomize cfg setter commands that have not been deprecated yet

As an author of generic CI/GitOps tooling there is no longer an easy way to know which mechanism should be used by introspecting a given package. It wasn't necessarily ideal, but it used to be that you could check for the presence of a Kptfile and/or Krmfile and safely attempt to apply setters against both.

As a package author, I used to be able to provide both a Kptfile and a Krmfile, with the setters defined in both places, allowing the consumer to use whichever tooling they wanted.

I am a fan of how kpt fn render works now, it is much better than kpt/kustomize fn run <dir>. Like I said in kptdev/kpt#2369 (comment), I just don't understand why this is not being developed in kustomize or cli-utils.


I had already typed all that out before @bgrant0607 's response. So it sounds like in addition to the cfg command group we should also be looking to deprecate kustomize fn commands? That picture would make more sense to me.

@marshall007
Copy link

I agree with @KnVerey that the core introspection command functionality doesn't really belong in kustomize either. It never really made sense to me that the output didn't reflect any out-of-place hydration defined in the kustomization(s). There would still be value in a kustomize-aware wrapper around the core functionality, but the core implementation belongs elsewhere for sure.

@KnVerey
Copy link
Contributor Author

KnVerey commented Jul 8, 2021

So it sounds like in addition to the cfg command group we should also be looking to deprecate kustomize fn commands?

🤔 I'm open to this. With that deprecation, it looks like we'd be left with commands that interact with Kustomization, which makes a lot of sense to me. That said, I can see how kustomize fn run might be very useful for people authoring extensions transformers as KRM functions, which is something we want to facilitate and encourage... but then again, as a function author myself, I can't say I've actually used it. Perhaps more importantly, I see that fn wrap and fn xargs are not marked as alpha, and I don't have the context as to why.

@bgrant0607
Copy link
Member

Yes, the imperative fn commands are also worthy of scrutiny. The commands should have been alpha.

KEPs for plugins and functions:
https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/993-kustomize-generators-transformers
https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2299-kustomize-plugin-composition

@natasha41575
Copy link
Contributor

natasha41575 commented Jul 16, 2021

thanks for pointing out those KRM functions. Couldn't the "apply-setters" function be invoked declaratively as a KRM-style transformer today then?

I just tested it out with the following:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: my-nginx
spec:
  replicas: 4 # kpt-set: ${nginx-replicas}

and

kind: ConfigMap
metadata:
  name: my-config-map
  annotations:
    config.kubernetes.io/function: |-
      container:
        image: gcr.io/kpt-fn/apply-setters:v0.1
data:
  nginx-replicas: "3"

and received the error no matches for the input list of setter.

My hunch is that it is because kustomize doesn't preserve comments and some debugging shows that the setter comments are lost by the time the function is executed on the yaml. If we want to support setters as a transformer, we might need to look into comment preservation during unmarshaling.

The same issue happens when using create-setters declaratively; although the setter comments get added at the function level, they are lost in the output.

It does work with kustomize fn run . --image=gcr.io/kpt-fn/apply-setters:v0.1 -- nginx-replicas=3, though - maybe it's worth holding off on deprecating the imperative fn commands until we have a better story around setters (unless we don't care about supporting them at all).

@KnVerey
Copy link
Contributor Author

KnVerey commented Jul 22, 2021

/retitle Deprecation of kustomize cfg and kustomize fn alpha commands

We've merged the deprecation of most of the cfg alpha commands in #4048 . I'm thinking we don't need to hold off deprecating alpha commands that aren't integrated with Kustomization just because we may in the future implement similar functionality that IS integrated.

@natasha41575 wdyt about deprecating the readonly commands as well, to ultimately get rid of the whole cfg group? They're neat, but not really related to Kustomize. If we keep them, we should open an issue about making them inflate a Kustomization. I agree with @marshall007 that it's reasonable to expect they would, if they live here.

Does anyone object to also deprecating kustomize fn? I could see keeping kustomize fn run to help people try out extensions, but I'm having trouble understanding the value the others add for a user of Kustomize's declarative workflows (at least at a glance, they seem designed to effect one-time imperative operations). Perhaps someone who knows the history better can clarify the intent.

Re: comment preservation, it sounds like we must not be using kyaml at our entry point. That would certainly be worth a try, but I suspect it won't be smooth sailing. E.g. if that effectively changes us over to a yaml 1.2 unmarshaler, booleans will get treated differently.

@k8s-ci-robot k8s-ci-robot changed the title [Question] Future of kustomize cfg alpha commands Deprecation of kustomize cfg and kustomize fn alpha commands Jul 22, 2021
@natasha41575
Copy link
Contributor

natasha41575 commented Jul 22, 2021

wdyt about deprecating the readonly commands as well, to ultimately get rid of the whole cfg group? They're neat, but not really related to Kustomize. If we keep them, we should open an issue about making them inflate a Kustomization. I agree with @marshall007 that it's reasonable to expect they would, if they live here.

Between deprecation and enhancing them to inflate a kustomization, I slightly prefer the latter but I don't have a strong opinion. Would we try to find another home for the commands or just get rid of them altogether? IMO they're kind of handy to have around and are easy to maintain.

Does anyone object to also deprecating kustomize fn? I could see keeping kustomize fn run to help people try out extensions, but I'm having trouble understanding the value the others add for a user of Kustomize's declarative workflows (at least at a glance, they seem designed to effect one-time imperative operations). Perhaps someone who knows the history better can clarify the intent.

I don't see the value of the others, but I do see value in kustomize fn run as a tool for function authors. I'm not sure that we should deprecate it until an alternative method of running functions moves into GA. In general I agree that kustomize's hydration should aim to stay declarative and out of place.

@KnVerey
Copy link
Contributor Author

KnVerey commented Jul 27, 2021

Between deprecation and enhancing them to inflate a kustomization, I slightly prefer the latter but I don't have a strong opinion. Would we try to find another home for the commands or just get rid of them altogether? IMO they're kind of handy to have around and are easy to maintain.

I'm ok with that, and I've opened an issue about it here: #4090

I don't see the value of the others, but I do see value in kustomize fn run as a tool for function authors. I'm not sure that we should deprecate it until an alternative method of running functions moves into GA. In general I agree that kustomize's hydration should aim to stay declarative and out of place.

Ok, let's keep fn run and deprecate the others. Can you take care of that @natasha41575?

@natasha41575
Copy link
Contributor

/assign

@natasha41575
Copy link
Contributor

We have deprecated the fn commands (except for run), and the cfg commands except for the ones specified in #4090. Unless I am mistaken, I believe the work and discussion here has either been completed or moved to separate, more specific tracking issues.

@james-callahan
Copy link
Contributor

remove the other mutating commands (annotate, fmt, merge, merge3). These seem handy, but do they belong in Kustomize? E.g. to format kustomize output, you should probably declare a transformer in your Kustomization.

Are there any tools around to format kustomize files/patches?
I'd expect such a tool would need to have knowledge specific knowledge of fields to order on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Issues for kustomize CLI interface kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. triage/under-consideration
Projects
None yet
Development

No branches or pull requests

6 participants