Add simple application pattern package #26
Conversation
k8s-ci-robot
added
cncf-cla: yes
pwittrock
changed the title
pwittrock
force-pushed
the
operator
branch
2 times, most recently
from
6d37af6 to
6df4f01
Compare
k8s-ci-robot
added
size/L
pwittrock
force-pushed
the
operator
branch
4 times, most recently
from
225aba7 to
66c7717
Compare
pwittrock
changed the title
pwittrock
changed the title
|
|
||
| // application is a very simple Controller that embeds a Manager. | ||
| type application struct { | ||
| mrg manager.Manager |
| ctrl *internalcontroller.Controller | ||
| } | ||
|
|
||
| var noop = reconcile.Func(func(reconcile.Request) (reconcile.Result, error) { |
| "sigs.k8s.io/controller-runtime/pkg/source" | ||
| ) | ||
|
|
||
| // application is a very simple Controller that embeds a Manager. |
There was a problem hiding this comment.
write why in the docs.
| return reconcile.Result{}, nil | ||
| }) | ||
|
|
||
| var getConfig = config.GetConfig |
There was a problem hiding this comment.
these for testing purposes? Write that in a comment.
| return b | ||
| } | ||
|
|
||
| // WithEventFilter sets the event filters. Defaults to the empty list. |
There was a problem hiding this comment.
... the event filters, to filter which create/update/delete/generic events eventually trigger reconciliations
There was a problem hiding this comment.
for example, filtering by a particular label selector or annotation
| // WithHandler sets th Reconcile called in response to create / update / delete events for the | ||
| // BuilderFor type or Created object types. | ||
| func (b *Builder) WithHandler(r reconcile.Reconciler) *Builder { | ||
| b.r = r |
There was a problem hiding this comment.
give this a proper name please. You have autocomplete, so you have no excuse :-P
| if err != nil { | ||
| return nil, err | ||
| } | ||
| a.ctrl = ctrl.(*internalcontroller.Controller) |
There was a problem hiding this comment.
eeewww. why is this cast necessary?
|
|
||
| // Watch the managed types | ||
| for _, t := range b.manages { | ||
| err = a.ctrl.Watch(&source.Kind{Type: t}, &handler.EnqueueRequestForOwner{ |
There was a problem hiding this comment.
is there/should there be a way to add non-owns relationships with this builder?
There was a problem hiding this comment.
Not right now. The point of this package is to offer a simplified view for the most common way to build application controllers. If arbitrary mappings become common for operators, then we can add them. Otherwise users can break out and use the underlying libraries directly.
| // | ||
| // An application is a Controller that implements the operational logic for an application. It is often used | ||
| // to take off-the-shelf OSS applications, and make them Kubernetes native. | ||
| package application |
There was a problem hiding this comment.
if it's for operators, maybe just call it operator?
There was a problem hiding this comment.
Calling this Application because that is the common response when folks as "what is an operator". I created an operator package to explain this.
| // List the Pods matching the PodTemplate Labels | ||
| pods := &corev1.PodList{} | ||
| err = a.List(context.TODO(), | ||
| &client.ListOptions{ |
There was a problem hiding this comment.
I believe we have a builder for this (if we don't have one for the namespace, we should), so you should be able to write something like
client.InNamespace(req.Namespace).WithLabels(rs.Spec.Template.Labels), which is a bit easier to read and doesn't require pulling out labels.SelectorFromSet.
k8s-ci-robot
added
size/XL
|
@DirectXMan12 LMK if there is anything else. |
pwittrock
added
approved
|
@DirectXMan12 Merging for now since @mengqiy needs to create something similar. Let me know if anything was unaddressed. |
Add simple application pattern package
Add string method for ReconcileKey
✨ Add ClusterAwareBuilderWithOptions to the wrapper
Looking to get feedback on simplifying this pattern.