Initial addons via plugin model #943

justinsb · 2019-08-15T14:45:54Z

Plugin model; --pattern=addon flag that generates an addon operator

We implement the initial plugin model, starting with an in-process
implementation that can support out-of-process later.  As plugins do
not yet form part of the stable interface for kubebuilder, we only
enable them when the KUBEBUILDER_ENABLE_PLUGINS env var is set.

We implement an initial plugin for addons: `--pattern=addon` creates a
controller & resource that follow the style of
https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern

This style of declarative addon operators is being investigated in the
cluster-addons subject, with code at
https://github.com/kubernetes-sigs/addon-operators

justinsb · 2019-08-15T14:59:14Z

/assign @DirectXMan12

I think this is what you had in mind for the plugin model, but wanted to check before it got too far along!

DirectXMan12

looks like a decent start

minor nit, minor comment inline

shall we move this to an experimental/ branch and iterate a bit (or mark it as experimental some other way?)

DirectXMan12 · 2019-08-15T20:45:47Z

pkg/scaffold/v2/gomod.go

@@ -42,6 +42,8 @@ module {{ .Repo }}

 go 1.12

+replace sigs.k8s.io/kubebuilder-declarative-pattern => ../../kubebuilder-declarative-pattern


this shouldn't be here any more, right?

Thanks - removed!

DirectXMan12 · 2019-08-15T20:48:41Z

pkg/model/types.go

+
+type File struct {
+	// Type is an identifier for the class of the generator
+	Type string `json:"type"`


what's the purpose of this?

Oops - just a leftover! Removed!

justinsb · 2019-08-20T04:22:10Z

Thanks for the review @DirectXMan12 . I made the "whoopsie" changes - thanks for spotting them. I also put --pattern=addon behind a (currently undocumented) feature flag KUBEBUILDER_ENABLE_PLUGINS - does that work?

Still lots to do here, but I really like the more strongly-typed model - I know you said you wanted to rework a lot of the existing generation logic anyway - do you think this approach could help there?

In any case, would be great to get this in so we can start e.g. adding tests and getting going on addons!

shawn-hurley · 2019-08-20T16:24:39Z

I thought that we had talked about adding this as an experimental branch to collaborate on and then submit to master? is that not the case and I am misremembering?

@DirectXMan12

jwforres · 2019-08-20T16:25:46Z

plugins/addon/manifest.go

+	packageName := getPackageName(u)
+
+	m := &model.File{
+		Path:           filepath.Join("channels", "packages", packageName, exampleManifestVersion, "manifest.yaml"),


Is there a design for manifests, channels, packages, etc?

Just what we're discussing in the cluster-addons WG and https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern We're still in development mode, I believe!

jwforres · 2019-08-20T16:30:00Z

I agree with @DirectXMan12 that this should be going to the experimental branch for kubebuilder.
I think this PR is overloading two sets of changes, one set of changes to introduce plugins generally, and one set of changes to propose an experimental concept for an addons plugin

justinsb · 2019-08-20T18:52:02Z

I think this is throwing off some good changes @jwforres , so I was thinking we could keep it feature-flagged off (so it's not easily availably) but still in a place that people can try out. For example, I think we should look at using the strongly-typed model instead of injecting objects, and whether the existing templating code then becomes simpler. What do you think?

And I'm doing an implementation of a plugin at the same time as the plugin model - otherwise the extension interface feels a little abstract... We can also see the problems with it - for example should we combine the controller & api phases so we can actually check that the expected file exists in a plugin? Is there actually any value in a single kubebuilder, vs just having a separate generator CLI .. particularly as we're not really reusing much of the kubebuilder template?

I could split it into 2 commits or 2 PRs if that helps, but I am developing them together!

DirectXMan12 · 2019-08-22T00:42:39Z

I think not having a motivating usecase makes things a lot more abstract, so we'll want something to demonstrate. We could do this cross repo, but that makes things super hard in the initial stages.

I think the plan is:

Do this, make sure it works fine, and that we're good with the interface
Add exec, make sure that works with OSDK
Split the addon stuff back into exec in kubebuilder-declarative-pattern again

Cross-repo iteration is really frustrating to work with, and a demo plugin is a bit too abstract for initial development.

That being said, I have a few more thoughts here on the generator/plugin parts.

As for feature flag vs experimental branch: feature flag is probably fine, and we've been saying "do this in the plugin framework" on the addon PR for a while now (despite being under the KB umbrella), so I think feature flag is prob fine for the moment.

jwforres · 2019-08-22T02:04:24Z

I think what was tripping me up is that I didn't see anything obvious that indicated the addon plugin is still under development. Since @DirectXMan12 agrees with doing the feature flag I'm fine with that. Since this isn't going into experimental, can something else indicate the addons plugin is a prototype under development?

I think it brings up the general question of how do you guys want to document plugins in this repo. Would something like a README in the plugin/addons folder help describe the goal of the plugin and its current development / support state? Or maybe something under a docs/plugins/addons structure?

DirectXMan12 · 2019-08-22T17:09:11Z

sure, something indicating that it's more of an experiment is fine with me

👍 on more docs too

We implement the initial plugin model, starting with an in-process implementation that can support out-of-process later. As plugins do not yet form part of the stable interface for kubebuilder, we only enable them when the KUBEBUILDER_ENABLE_PLUGINS env var is set. We implement an initial plugin for addons: `--pattern=addon` creates a controller & resource that follow the style of https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern This style of declarative addon operators is being investigated in the cluster-addons subject, with code at https://github.com/kubernetes-sigs/addon-operators

justinsb · 2019-09-11T03:02:21Z

Added a readme to the plugins folder! The env-var to enable is currently KUBEBUILDER_ENABLE_PLUGINS but we could rename that to KUBEBUILDER_ENABLE_EXPERIMENTAL_PLUGINS if anyone is worried.

droot

looks good to me.

droot · 2019-09-11T21:38:35Z

plugins/README.md

+and as a real-world use case for driving development of the plugin system.  We
+don't intend for the plugin system to become an emacs competitor, but it must be
+sufficiently flexible to support the various patterns of operators that
+kubebuilder will generate.


emacs competitor 😆

briantopping

I'd like to see that plugins are per-project rather than (what appears to be) global. This is to say if two projects have unique templates, the same kubebuilder binary would work properly in both cases by introspecting the templates directory of each project. The templates would naturally be checked in with the project, making reuse automatic.

DirectXMan12 · 2019-09-16T20:10:13Z

@briantopping yep, we'd like to have pattern information stored in the project file, it's one of the next things we want to add. Feel free to submit a PR once this merges :-)

DirectXMan12 · 2019-09-16T20:10:48Z

/lgtm
/approve

as per discussion in the monthly meeting and the discussions on this thread

k8s-ci-robot · 2019-09-16T20:10:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12, justinsb

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:

~~OWNERS~~ [DirectXMan12]

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 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. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 15, 2019

k8s-ci-robot requested review from DirectXMan12 and Liujingfang1 August 15, 2019 14:46

justinsb force-pushed the plugin_addon_pattern branch 2 times, most recently from f9c87d1 to f96bee5 Compare August 15, 2019 14:55

k8s-ci-robot assigned DirectXMan12 Aug 15, 2019

DirectXMan12 requested changes Aug 15, 2019

View reviewed changes

justinsb force-pushed the plugin_addon_pattern branch from f96bee5 to 6da11ab Compare August 20, 2019 04:18

justinsb changed the title ~~WIP: Addons via plugin model~~ Initial addons via plugin model Aug 20, 2019

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 Aug 20, 2019

justinsb force-pushed the plugin_addon_pattern branch 2 times, most recently from 1e4038f to 49679f8 Compare August 20, 2019 13:52

jwforres reviewed Aug 20, 2019

View reviewed changes

DirectXMan12 mentioned this pull request Sep 3, 2019

Support custom templates #585

Closed

justinsb force-pushed the plugin_addon_pattern branch from 49679f8 to 722b905 Compare September 11, 2019 03:00

droot reviewed Sep 11, 2019

View reviewed changes

briantopping suggested changes Sep 14, 2019

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 16, 2019

k8s-ci-robot merged commit fd0aab7 into kubernetes-sigs:master Sep 16, 2019

dholbach mentioned this pull request Sep 18, 2019

Upgrade coredns to kubebuilder 2 kubernetes-sigs/cluster-addons#24

Closed

DirectXMan12 mentioned this pull request Sep 20, 2019

Configurable scaffolding behaviour #1004

Closed

Adirio mentioned this pull request Jan 20, 2020

[Umbrella issue] Scaffolding enhancements #1218

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial addons via plugin model #943

Initial addons via plugin model #943

justinsb commented Aug 15, 2019 •

edited

Loading

justinsb commented Aug 15, 2019

DirectXMan12 left a comment

DirectXMan12 Aug 15, 2019

justinsb Aug 20, 2019

DirectXMan12 Aug 15, 2019

justinsb Aug 20, 2019

justinsb commented Aug 20, 2019 •

edited

Loading

shawn-hurley commented Aug 20, 2019

jwforres Aug 20, 2019

justinsb Aug 20, 2019

jwforres commented Aug 20, 2019

justinsb commented Aug 20, 2019 •

edited

Loading

DirectXMan12 commented Aug 22, 2019

jwforres commented Aug 22, 2019

DirectXMan12 commented Aug 22, 2019

justinsb commented Sep 11, 2019

droot left a comment

droot Sep 11, 2019

briantopping left a comment

DirectXMan12 commented Sep 16, 2019

DirectXMan12 commented Sep 16, 2019

k8s-ci-robot commented Sep 16, 2019

		@@ -42,6 +42,8 @@ module {{ .Repo }}

		go 1.12

		replace sigs.k8s.io/kubebuilder-declarative-pattern => ../../kubebuilder-declarative-pattern

Initial addons via plugin model #943

Initial addons via plugin model #943

Conversation

justinsb commented Aug 15, 2019 • edited Loading

justinsb commented Aug 15, 2019

DirectXMan12 left a comment

Choose a reason for hiding this comment

DirectXMan12 Aug 15, 2019

Choose a reason for hiding this comment

justinsb Aug 20, 2019

Choose a reason for hiding this comment

DirectXMan12 Aug 15, 2019

Choose a reason for hiding this comment

justinsb Aug 20, 2019

Choose a reason for hiding this comment

justinsb commented Aug 20, 2019 • edited Loading

shawn-hurley commented Aug 20, 2019

jwforres Aug 20, 2019

Choose a reason for hiding this comment

justinsb Aug 20, 2019

Choose a reason for hiding this comment

jwforres commented Aug 20, 2019

justinsb commented Aug 20, 2019 • edited Loading

DirectXMan12 commented Aug 22, 2019

jwforres commented Aug 22, 2019

DirectXMan12 commented Aug 22, 2019

justinsb commented Sep 11, 2019

droot left a comment

Choose a reason for hiding this comment

droot Sep 11, 2019

Choose a reason for hiding this comment

briantopping left a comment

Choose a reason for hiding this comment

DirectXMan12 commented Sep 16, 2019

DirectXMan12 commented Sep 16, 2019

k8s-ci-robot commented Sep 16, 2019

justinsb commented Aug 15, 2019 •

edited

Loading

justinsb commented Aug 20, 2019 •

edited

Loading

justinsb commented Aug 20, 2019 •

edited

Loading