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

Ease configuring and running the builtin plugins. #1532

Merged
merged 1 commit into from
Sep 13, 2019

Conversation

monopole
Copy link
Contributor

@monopole monopole commented Sep 13, 2019

This PR allows a user to run a builtin plugin via the transformers: or generators: field without needing to also make the .so file for said plugin available.

The PR also makes it possible to run a builtin plugin without specifying the --enable_alpha_plugins flag.

The gist of this PR is to exploit the statically linked factory functions already being created by the pluginator code generator. The PR moves a few interfaces around to avoid package cycles, and adds an enum type for the builtins, and a map associating the enums to the static plugin factory functions.

I had to modify the pluginator, the code generator that provides statically compiled factory methods for making plugins, to return interface types rather than concrete instances so that the factory methods could be invoked from generic code detecting the use of builtin plugins.

Fix #1504
Fix #1508

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: monopole

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

The pull request process is described 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

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 13, 2019
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 13, 2019
@monopole monopole removed the request for review from droot September 13, 2019 20:44
@monopole
Copy link
Contributor Author

@richardmarshall @jcassee PTAL if you have the time.

It's not as complex as it looks. The longish list of files modified is because i had to gather some interface definitions into the resmap package (which ultimately is a better place for them anyway).

@Liujingfang1
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 13, 2019
@k8s-ci-robot k8s-ci-robot merged commit 32be1cf into kubernetes-sigs:master Sep 13, 2019
@jcassee
Copy link
Contributor

jcassee commented Sep 15, 2019

This is great. Treating builtin generators and external generators equally feels nice. One thing I noticed is that labels and annotations on a ConfigMapGenerator are not passed to the ConfigMap, but that never worked, right?

Not sure if you are looking for comments on the code. I never went into this part of the woods. The pluginator seems either a transitional mechanism or a stopgap, as it generates duplicate code by design? I'm not completely sure what the changes do without diving deeper, but they seems to be doing it quite simply. :-)

Any reason for choosing "builtin" as api version, and not something like "kustomize.config.k8s.io"? A conscious decision not to do version management (yet)?

@monopole monopole deleted the runBuiltinSansFlag branch September 18, 2019 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
4 participants