Minimal addon and declarative pattern implementations for addon operators #241

justinsb · 2018-12-07T03:27:08Z

@johnsonj and I are working to create operators for kubernetes addons - we aim to use kubebuilder to replace the addon-manager in https://github.com/kubernetes/kubernetes/tree/master/cluster/addons.

We would like to contribute it to controller-runtime & kubebuilder. This PR is the minimal first step, starting on an example for dashboard, and including the minimal functionality of the supporting library. We are putting this into an alpha/patterns directory, but hopefully we can figure out if this is indeed the most appropriate place, and/or whether this is a good place to start.

We have three main components:

The skeleton example dashboard controller
A declarative pattern for implementing a reconciler based on a manifest that is manipulated and the kubectl applyd (later this will likely use server-side-apply)
An addon pattern building on declarative which is opinionated as to how those manifests should be found, and some standard manipulations of the objects and surfacing of status that makes sense for addons.

Future PRs will include:

Adding an Application CRD to wrap the manifest objects
Setting labels & owner references on objects
Incorporating "golden yaml" output testing, which has proved powerful and expressive for us
Automatically watching deployed objects and surfacing status

It might be that declarative ends up as core functionality of controller-runtime, as the goal is to have it be much less opinionated than the addon pattern.

However, we though this was a good place to share the first step and figure out how best to proceed - input is greatly appreciated!

cc @DirectXMan12 @droot @mengqiy @pwittrock

k8s-ci-robot · 2018-12-07T03:27:13Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: justinsb
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: droot

If they are not already assigned, you can assign the PR to them by writing /assign @droot in a comment when ready.

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

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

DirectXMan12 · 2018-12-07T05:27:35Z

for the record: I would very much like to have this be managed in a separate pattern repo, and then migrate stuff as needed. I makes management, determining staleness, releasing, etc much easier in my experience.

justinsb · 2018-12-11T16:17:55Z

I think that makes sense @DirectXMan12 ... we're doing it under the alpha/patterns directory, but clearly that would be better as a controller-patterns repo or similar. While some of the patterns are likely applicable generally, we can (as you say) promote them as we demonstrate that.

That said, if sig-apimachinery's stance is that fewer repos is better, we can work with that, and just have bigger repos that are less focused.

(Fixed up the tests as well!)

droot

I would echo the overall sentiments about this living under a different repo/org controller-patterns. Took a quick look and have a few observations/questions.

droot · 2018-12-11T18:56:24Z

alpha/patterns/addon/common_types.go

@@ -0,0 +1,27 @@
+package addon


Is copyright missing ?

droot · 2018-12-11T18:57:34Z

alpha/patterns/addon/common_types.go

+// +k8s:deepcopy-gen=true
+type CommonStatus struct {
+	Healthy bool `json:"healthy,omitempty"`
+}


We should document what these type definition are and how are they suppose to be used.

Also, would be good to include pkg level comments which makes Godoc useful in this case. I think some content from walkthrough be used in the pkg level doc.go in this case.

droot · 2018-12-11T19:01:27Z

alpha/patterns/addon/examples/dashboard-operator/.gitignore

@@ -0,0 +1,27 @@
+


I am wondering if we should move the examples outside the alpha/patterns/... to maintain separation between the packages user import and the examples that they refer. This will also help tools because tools like dep considers examples to be part of the imported code.

droot · 2018-12-11T19:02:50Z

alpha/patterns/addon/examples/dashboard-operator/Gopkg.toml

+
+[[constraint]]
+  name="sigs.k8s.io/controller-tools"
+  version="v0.1.1"


Does the presence of this file introduces deps on controller-tools ? That can introduce the deadlock between controller-tools and controller-runtime.

droot · 2018-12-11T19:03:53Z

alpha/patterns/addon/pkg/loaders/fs.go

+
+func init() {
+	// TODO: Yuk - global flags are ugly
+	flag.StringVar(&FlagChannel, "channel", FlagChannel, "location of channel to use")


droot · 2018-12-11T19:05:17Z

alpha/patterns/declarative/options.go

@@ -0,0 +1,151 @@
+package declarative


We should add pkg level doc.go explaining this pattern and the types defined below.

droot · 2018-12-11T19:07:27Z

alpha/patterns/declarative/reconciler.go

+
+var _ reconcile.Reconciler = &Reconciler{}
+
+type Reconciler struct {


We should explain this in detail.

johnsonj · 2019-01-03T22:12:59Z

Code review-wise I think we have a few comments to address that should be straightforward.

The open question is if this PR is destin for this repo or another. Any progress on that decisions @DirectXMan12 @droot ?

fejta-bot · 2019-04-28T00:04:06Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

mengqiy · 2019-05-06T21:19:05Z

closing this now since we have a separate repo for the add-on pattern.

Add parsing for required field and corresponding test

Created dashboard skeleton with kubebuilder

fef9da4

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 7, 2018

k8s-ci-robot requested review from DirectXMan12 and droot December 7, 2018 03:27

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 7, 2018

Create basic addon & declarative patterns

f4122e8

justinsb force-pushed the minimal_addon_and_declarative_patterns branch from bffc2c9 to f4122e8 Compare December 11, 2018 16:04

droot reviewed Dec 11, 2018

View reviewed changes

johnsonj mentioned this pull request Jan 24, 2019

New repo: kubernetes-sigs/kubebuilder-declarative-pattern kubernetes/org#418

Closed

johnsonj mentioned this pull request Feb 20, 2019

REQUEST: New membership for johnsonj kubernetes/org#510

Closed

6 tasks

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 28, 2019

mengqiy closed this May 6, 2019

DirectXMan12 pushed a commit that referenced this pull request Jan 31, 2020

Merge pull request #241 from Liujingfang1/required

8a31fe0

Add parsing for required field and corresponding test

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Minimal addon and declarative pattern implementations for addon operators #241

Minimal addon and declarative pattern implementations for addon operators #241

Uh oh!

justinsb commented Dec 7, 2018 •

edited

Loading

Uh oh!

k8s-ci-robot commented Dec 7, 2018

Uh oh!

DirectXMan12 commented Dec 7, 2018

Uh oh!

justinsb commented Dec 11, 2018

Uh oh!

droot left a comment

Uh oh!

droot Dec 11, 2018

Uh oh!

droot Dec 11, 2018

Uh oh!

droot Dec 11, 2018

Uh oh!

droot Dec 11, 2018

Uh oh!

droot Dec 11, 2018

Uh oh!

droot Dec 11, 2018

Uh oh!

droot Dec 11, 2018

Uh oh!

johnsonj commented Jan 3, 2019

Uh oh!

fejta-bot commented Apr 28, 2019

Uh oh!

mengqiy commented May 6, 2019

Uh oh!

Uh oh!


		var _ reconcile.Reconciler = &Reconciler{}

		type Reconciler struct {

Minimal addon and declarative pattern implementations for addon operators #241

Minimal addon and declarative pattern implementations for addon operators #241

Uh oh!

Conversation

justinsb commented Dec 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Dec 7, 2018

Uh oh!

DirectXMan12 commented Dec 7, 2018

Uh oh!

justinsb commented Dec 11, 2018

Uh oh!

droot left a comment

Choose a reason for hiding this comment

Uh oh!

droot Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

droot Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

droot Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

droot Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

droot Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

droot Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

droot Dec 11, 2018

Choose a reason for hiding this comment

Uh oh!

johnsonj commented Jan 3, 2019

Uh oh!

fejta-bot commented Apr 28, 2019

Uh oh!

mengqiy commented May 6, 2019

Uh oh!

Uh oh!

justinsb commented Dec 7, 2018 •

edited

Loading