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

Can't compose generated Kustomizations - due to use of vars #1007

Closed
rehevkor5 opened this issue Mar 13, 2020 · 18 comments
Closed

Can't compose generated Kustomizations - due to use of vars #1007

rehevkor5 opened this issue Mar 13, 2020 · 18 comments

Comments

@rehevkor5
Copy link

rehevkor5 commented Mar 13, 2020

Due to the use of "vars" in the generated kustomizations, it's not possible to compose the generated kustomizations together in order to use kubectl -k to deploy the whole thing plus additional variant info like env-specific stuff. If you do, errors such as "var clusterDomain already encountered" and "error: found 2 resId matches for var {project ~G_v1_ConfigMap {data.project}} (unable to disambiguate)" occur.

Can all use of vars be removed to prevent this issue?

I am new to Kustomize, so I may be misunderstanding how this stuff should work. But, the var problem is making it difficult to figure out how to maintain kustomizations in source control with additional layers for environment-specific stuff. kfctl by itself doesn't provide any mechanism for user-level additions/changes, without manually modifying the output every time it's regenerated (not desirable). It seems like for now I may have to maintain a separate kustomization.yaml for each generated kustomization, so that I can apply a shared base and/or any env-specific config. I guess this means that if I have 36 generated kustomizations, that I need to have 36 * (env count) individual kustomization files to provide env-specific concerns? If so, this is not a very practical approach.

@issue-label-bot
Copy link

Issue Label Bot is not confident enough to auto-label this issue.
See dashboard for more details.

@jlewi
Copy link
Contributor

jlewi commented Mar 15, 2020

Related issue: /issues/774

@jlewi jlewi changed the title Can't compose generated Kustomizations Can't compose generated Kustomizations - due to use of vars Mar 15, 2020
@jlewi
Copy link
Contributor

jlewi commented Mar 15, 2020

This is one of the disadvantages of vars as noted in: kubernetes-sigs/kustomize#2052

Our biggest use case for vars is for substituting namespace and clusterDomain into virtual services (example). As suggested in kubernetes-sigs/kustomize#205 using creating plugins for virtual services might be a better long term option.

We could define a VirtualService transformer similar to the built in image transformer

A custom transformer would allow users to add a virtualServices section to their kustomization.yaml files to change hosts; e.g.

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
virtualServices: 
 - host: jupyter-web-app-service.kubeflow.svc.cluster.local   
    newHost: jupyter-web-app-service.customnamespace.svc.some-other-domain
--


@rehevkor5
Copy link
Author

After renaming all vars to be unique across kustomizations, I am seeing now that the same problem exists with configMapGenerator, too. Multiple kustomizations declare a configMapGenerator called "parameters", but the content is different among all of them! I don't understand how this is supposed to work? How can Kubeflow generate the same resource twice, with different content, and still have a predictable result?

Example error:

error: found 2 resId matches for var {clusterDomain_25 ~G_v1_ConfigMap {data.clusterDomain}} (unable to disambiguate)

@jlewi
Copy link
Contributor

jlewi commented Mar 16, 2020

This is a bug in how our kustomize manifests are structured right now. The way our kustomize manifests are structured doesn't support composing the individual applications.

This works because the params configMapGenerator is doing client side (using kustomize) substitution of vars into manifests. So it doesn't depend on the config map being unique across applications.

There's ongoing work to clean this up.

I'll send out a doc this week or next about fixing some of our patterns.

There's an example PR: #962

@jlewi
Copy link
Contributor

jlewi commented Mar 19, 2020

Anyone interested in exploring a custom transformer/plugin to replace virtual services?

Maybe take a look at the example replacement transformer
https://github.com/kubernetes-sigs/kustomize/blob/master/plugin/someteam.example.com/v1/replacementtransformer/ReplacementTransformer.go

@jlewi
Copy link
Contributor

jlewi commented Apr 22, 2020

kpt setters might provide another option for making it easy for folks to change the cluster domain.

Is it possible to use kpt setters in a manner compatible with people still using global vars? i.e. could we just set the default value for the setter to "$(clusterDomain)" so that people using kpt setters can change it easily.

@kkasravi
Copy link
Contributor

@jlewi i'll implement this.

For your comment above, we should have the plugin transformer(s) reference the KfDef to resolve vars and remove all configMapGeneration then a single source of truth is the KfDef config file

@jlewi
Copy link
Contributor

jlewi commented Apr 24, 2020

@kkasravi what is "this" referring to? Is it kpt setters for clusterDomain?

we should have the plugin transformer(s) reference the KfDef to resolve vars and remove all configMapGeneration then a single source of truth is the KfDef config file

We currently have two use cases for vars

  1. Setting certain variables consistently across multiple kustomize packages (e.g. clusterDomain)
  2. Internal subsitutions within a package
    • e.g. Getting kustomize to substitute namespace into some custom resource field

For the second use case vars seem like an ok solution. Since the var is local to the package we can just give it a unique enough name so it is unlikely to conflict with vars from other packages.

For #1 I think kpt setters might be a better solution then vars. I suspect we will likely go the route of using kpt setters on GCP as kpt is becoming a standard tool for GCP (e.g. you can install it with gcloud). But I don't intend to mandate the use of kpt.

If your curious I've been experimenting with deploying KF using kpt on GCP here
https://github.com/jlewi/kf-templates-gcp/blob/master/kubeflow/Makefile

@kkasravi
Copy link
Contributor

An alternate to kpt is kustomize functions which are invoked like kustomize config set ...
This is an alpha feature of kustomize and requires KUSTOMIZE_ENABLE_ALPHA_COMMANDS=true. However it could potentially solve a number of problems including vars that might work better than a transformer plugin. I need to look at it more closely.

@kkasravi
Copy link
Contributor

Here is one nice use of kustomize functions https://github.com/kubernetes-sigs/kustomize/tree/master/functions/examples/application-cr where the application cr is added to a set of resources.

@jlewi
Copy link
Contributor

jlewi commented Apr 25, 2020

Thanks @kkasravi any solution which requires running a docker image or installing a plugin (".so") seems less preferable to me than a method which just relies on out of box functionality in stock tools like kustomize or kpt.

  • I think installing plugins or docker images is going to be a major source of friction (e.g. binary incompatibility, forgettng to install it, how to run images in pods, etc...)
  • There is a lot of overlapping functionality between kustomize functions, transformers/plugins, and kpt functions and I'm not sure how its all going to play out.

Right now I think we only have 1-2 global vars see here

  • clusterDomain
  • namespace

Both of which are used to configure virtual services like so

I'd like to get rid of those 2 dependencies on global vars.

For clusterDomain this feels like a really good use case for kpt setters

  • I think we could just use a kpt setter with a default value of $(clusterDomain) to maintain backwards compatibility
  • Alternatively we could create an overlay for each service with a kpt setter so that distributions that want to transition to kpt setters could start using that.

Note: There is a new (backwards compatible) way of creating setters in kpt that is more powerful https://googlecontainertools.github.io/kpt/guides/producer/substitutions/

For namespace, I'm not really sure that needs to be a global var. It looks like in that case we are just using vars to do trick kustomize into substituting namespace into a field kustomize isn't normally aware of. I think in that case we can just make the var names unique enough per package (e.g. "jupyter-web-app-namespace") so they won't conflict when applications are composed.

@jlewi
Copy link
Contributor

jlewi commented May 4, 2020

It looks like another use case for global vars is userId. This is a field used to configure web applications to know what header to look at to get the id of the user.

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the labels:

Label Probability
kind/bug 0.59

Please mark this comment with 👍 or 👎 to give our bot feedback!
Links: app homepage, dashboard and code for this bot.

@jlewi
Copy link
Contributor

jlewi commented May 18, 2020

@kkasravi Have you had a chance to check whether we can easily add KPT annotations that are backwards compatible with vars substitutions?

@jlewi
Copy link
Contributor

jlewi commented May 19, 2020

It looks like kustomize on master now has an alpha config command which supports setters. So it looks like this functionality might be getting upstreamed from kpt.

I haven't checked if this functionality has made it into any of the existing releases. It does not appear to be in kustomize 3.2.1

~/git_kustomize/kustomize/kustomize config help
[Alpha] Utilities for working with Resource Configuration.

...

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
  list-setters              [Alpha] List setters for Resources.
   ...

@jlewi
Copy link
Contributor

jlewi commented Jun 3, 2020

@kkasravi You were right about kpt fns; it looks like those might be a great solution to a lot of our problems e.g. (kubeflow/kfctl#345). The use of docker images looks like it will be advantageous in some cases (eg. allow for more modularity) but will create problems in some cases (e.g. when trying to deploy from within a docker container).

It looks like kpt fns are just go binaries though and can be invoked without a container so that could be one way we solve that problem.

@jlewi
Copy link
Contributor

jlewi commented Jul 10, 2020

Closing this out because there is no clear work remaining.

I think we might still need to figure out a better solution for clusterDomain and namespace. Lets file a separate more specific issue for that if need be.

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

No branches or pull requests

3 participants