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

add builtin go-template inflator #3490

Closed
wants to merge 1 commit into from

Conversation

epcim
Copy link
Contributor

@epcim epcim commented Jan 20, 2021

Signed-off-by: Petr Michalec [email protected]

Generates source from remote go-template based k8s manifests or Kustomize "base" resources.

/do-not-merge
/work-in-progress

PLEASE LET ME KNOW WHETHER THE FEATURE IS WANTED

About:

  • This is not replacement for HelmChart generator
  • Makes easy reuse, even "pieces" of existing Helm charts (long-term) or other templated manifests available
  • To simplify complexity in latter Kustomization

Who might want to use it:

  • you have library with existing go-templated manifests already
  • you know your manifests and you want low-level control
  • deployment perspective
    • you live on edge (build from master)
    • you store generated templates as versioned artefacts
    • you store templates (versioned) linked to your image builds (versioned)

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 20, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @epcim. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 20, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: epcim
To complete the pull request process, please assign monopole after the PR has been reviewed.
You can assign the PR to them by writing /assign @monopole in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@epcim
Copy link
Contributor Author

epcim commented Jan 20, 2021

BTW even ti build now I currently struggle with missed load. Did I missed it to add/enable it somewhere?

Error: loading generator plugins: unable to load builtin ~G_builtin_GotplInflator

Well I know it was not generated into

api/internal/plugins/builtinhelpers/builtinplugintype_string.go

but on the other hand it was added to https://github.com/kubernetes-sigs/kustomize/pull/3490/files#diff-e7be08f5853fe648144d9d52862dc670f1aa729cfbbda4ba183608ca4d4e0b93

UPDATE, FIXED: had to run stringer manually in the folder

@epcim epcim force-pushed the gotplinflator branch 4 times, most recently from 68939a5 to 77405b0 Compare January 22, 2021 08:29
@epcim epcim changed the title WIP: add builtin go-template inflator add builtin go-template inflator Jan 24, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 24, 2021
@epcim
Copy link
Contributor Author

epcim commented Jan 29, 2021

@monopole @justinsb can you pls review and share your status the MR, would it get approval assuming it is finished?

@monopole
Copy link
Contributor

monopole commented Feb 6, 2021

there's lots going on so sorry about the slow response.

  • There's already a large KEP outstanding out for discussion which provides another approach to extensions (extensions that can use templates or whatever).
  • We're focusing on getting back into kubectl before code freeze
  • This PR though not large in a LOC sense is large in a conceptual sense.

Does anyone else want this and want to hash it out in a KEP? Any plugni can do whatever, but building it in to native kustomize is the core issue here.

  • as the PR description reminds us, helm does templates already. Also, kustomize has a builtin helm inflator, which recognizes the market success of helm and the wide variety of existing charts that can be used as a base.

  • kustomize started as project to explore config generation and mutation without resorting to templating (aka parameterizatio), for reasons discussed as length elsewhere. The goal is a system that allows - encourages - building variants on a base that is itself complete and useful and requires no further processing. To forget that and bake in templates-as-a-base is quite a pivot.

A KEP laying out use cases that are currently underserved seems warranted - a KEP with multiple authors/sponsors.

@aodinokov
Copy link
Contributor

@monopole @justinsb can you pls review and share your status the MR, would it get approval assuming it is finished?

Meanwhile @epcim , it's not always necessary to include that functionality as builtin plugins to get the desired behavior :)
please refer to this example.
in particular this and this kustomizations. I think it does almost what you need and without need to install any kustomize plugins... This is achieved with using krm-functions as plugins.

@epcim
Copy link
Contributor Author

epcim commented Mar 11, 2021

Thanks @aodinokov. Actually I am not using kpt nor will to use templating as part of the deployment configuration. I tend more to clean Kustomize approach.

Technically I do agree with @monopole not to extend Kustomize base functionality above the original idea. Currently running it as plugin and most probably will offer alternative implementation once above mentioned KEP will land. Most probably other go-plugins will do the same.

Closing as it was here long enough for comments. Obviously not much interesting for Kustomize audience.

--
Less important:

Regarding Templates: (what ever syntax) in my view can be used in the manner to distillate, simplify "base" manifests. That Kustomize only tune or extend for particular environment. Originally I though it could reuse existing Helm manifests without actually using Helm at all. Tahat is still possible but as helm is using imports and .tpl helpers it's making thing more complicated at the end.

Regarding Kustomize plugins. For the time I use Kustomize I do adore it for easy of use and for

  • the fact it allows me quickly set-up and deploy the deployment at early stages (composing/testing)
  • without actually inventing a wheel from scratch (manifests, deployment)
  • with this plugin, reusing any existing/mature sources (especially from helm community)

If I would compare Kustomize with for example Kapitan, the benefit here is the simplicity of configuration in the repo. and the fact that deployment with Kustomize avoids need for config.mgmt (for good and for bad, sure).

Questionable is whether Kustomize tends more to be deployment tool (for git-ops approach) that can tune source manifests or whether it's more tool to render final manifests. Here then comes the topic what plugins it then needs. Obviously K8s deployment needs more thank just *** apply -f ./*.yml. One needs to manage various resources, secrets, cluster configuration, ...

@epcim epcim closed this Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants