From 8409bddaa3623cf857aeb42bdfa11884b12a141f Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Wed, 19 Dec 2018 21:03:12 -0800 Subject: [PATCH 1/2] :sparkles: clean up builder package and UX - Rename Builder functions - Encourage providing the manager to the Builder factory - Deprecate discouraged functions - Add alias.go with short cuts for common functions to reduce import line bloat - Fix issue with GetConfigOrDie not exiting - Update examples - Add example at package root --- alias.go | 135 ++++++++++++ example_test.go | 97 +++++++++ pkg/builder/build.go | 154 +++++++++----- pkg/builder/build_test.go | 231 +++++++++++++-------- pkg/builder/example_test.go | 35 ++-- pkg/client/config/config.go | 1 + pkg/controller/controller.go | 16 +- pkg/doc.go | 2 +- pkg/internal/controller/controller_test.go | 7 +- 9 files changed, 513 insertions(+), 165 deletions(-) create mode 100644 alias.go create mode 100644 example_test.go diff --git a/alias.go b/alias.go new file mode 100644 index 0000000000..d5a21b1f38 --- /dev/null +++ b/alias.go @@ -0,0 +1,135 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package controllerruntime alias' common functions and types to improve discoverability and reduce +// the number of imports for simple Controllers. +package controllerruntime + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client/config" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/runtime/log" + "sigs.k8s.io/controller-runtime/pkg/runtime/scheme" + "sigs.k8s.io/controller-runtime/pkg/runtime/signals" +) + +// Builder builds an Application ControllerManagedBy (e.g. Operator) and returns a manager.Manager to start it. +type Builder = builder.Builder + +// Request contains the information necessary to reconcile a Kubernetes object. This includes the +// information to uniquely identify the object - its Name and Namespace. It does NOT contain information about +// any specific Event or the object contents itself. +type Request = reconcile.Request + +// Result contains the result of a Reconciler invocation. +type Result = reconcile.Result + +// Manager initializes shared dependencies such as Caches and Clients, and provides them to Runnables. +// A Manager is required to create Controllers. +type Manager = manager.Manager + +// Options are the arguments for creating a new Manager +type Options = manager.Options + +// Builder builds a new Scheme for mapping go types to Kubernetes GroupVersionKinds. +type SchemeBuilder = scheme.Builder + +// GroupVersion contains the "group" and the "version", which uniquely identifies the API. +type GroupVersion = schema.GroupVersion + +// GroupResource specifies a Group and a Resource, but does not force a version. This is useful for identifying +// concepts during lookup stages without having partially valid types +type GroupResource = schema.GroupResource + +// TypeMeta describes an individual object in an API response or request +// with strings representing the type of the object and its API schema version. +// Structures that are versioned or persisted should inline TypeMeta. +// +// +k8s:deepcopy-gen=false +type TypeMeta = metav1.TypeMeta + +// ObjectMeta is metadata that all persisted resources must have, which includes all objects +// users must create. +type ObjectMeta = metav1.ObjectMeta + +var ( + // GetConfigOrDie creates a *rest.Config for talking to a Kubernetes apiserver. + // If --kubeconfig is set, will use the kubeconfig file at that location. Otherwise will assume running + // in cluster and use the cluster provided kubeconfig. + // + // Will log an error and exit if there is an error creating the rest.Config. + GetConfigOrDie = config.GetConfigOrDie + + // GetConfig creates a *rest.Config for talking to a Kubernetes apiserver. + // If --kubeconfig is set, will use the kubeconfig file at that location. Otherwise will assume running + // in cluster and use the cluster provided kubeconfig. + // + // Config precedence + // + // * --kubeconfig flag pointing at a file + // + // * KUBECONFIG environment variable pointing at a file + // + // * In-cluster config if running in cluster + // + // * $HOME/.kube/config if exists + GetConfig = config.GetConfig + + // NewControllerManagedBy returns a new controller builder that will be started by the provided Manager + NewControllerManagedBy = builder.ControllerManagedBy + + // NewManager returns a new Manager for creating Controllers. + NewManager = manager.New + + // CreateOrUpdate creates or updates the given object obj in the Kubernetes + // cluster. The object's desired state should be reconciled with the existing + // state using the passed in ReconcileFn. obj must be a struct pointer so that + // obj can be updated with the content returned by the Server. + // + // It returns the executed operation and an error. + CreateOrUpdate = controllerutil.CreateOrUpdate + + // SetControllerReference sets owner as a Controller OwnerReference on owned. + // This is used for garbage collection of the owned object and for + // reconciling the owner object on changes to owned (with a Watch + EnqueueRequestForOwner). + // Since only one OwnerReference can be a controller, it returns an error if + // there is another OwnerReference with Controller flag set. + SetControllerReference = controllerutil.SetControllerReference + + // SetupSignalHandler registered for SIGTERM and SIGINT. A stop channel is returned + // which is closed on one of these signals. If a second signal is caught, the program + // is terminated with exit code 1. + SetupSignalHandler = signals.SetupSignalHandler + + // Log is the base logger used by controller-runtime. It delegates + // to another logr.Logger. You *must* call SetLogger to + // get any actual logging. + Log = log.Log + + // SetLogger sets a concrete logging implementation for all deferred Loggers. + SetLogger = log.SetLogger + + // ZapLogger is a Logger implementation. + // If development is true, a Zap development config will be used + // (stacktraces on warnings, no sampling), otherwise a Zap production + // config will be used (stacktraces on errors, sampling). + ZapLogger = log.ZapLogger +) diff --git a/example_test.go b/example_test.go new file mode 100644 index 0000000000..dde560cf63 --- /dev/null +++ b/example_test.go @@ -0,0 +1,97 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllerruntime_test + +import ( + "context" + "fmt" + "os" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + controllers "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// This example creates a simple application Controller that is configured for ReplicaSets and Pods. +// +// * Create a new application for ReplicaSets that manages Pods owned by the ReplicaSet and calls into +// ReplicaSetReconciler. +// +// * Start the application. +// TODO(pwittrock): Update this example when we have better dependency injection support +func Example() { + var log = controllers.Log.WithName("builder-examples") + + manager, err := controllers.NewManager(controllers.GetConfigOrDie(), controllers.Options{}) + if err != nil { + log.Error(err, "could not create manager") + os.Exit(1) + } + + err = controllers. + NewControllerManagedBy(manager). // Create the Controller + For(&appsv1.ReplicaSet{}). // ReplicaSet is the Application API + Owns(&corev1.Pod{}). // ReplicaSet owns Pods created by it + Complete(&ReplicaSetReconciler{Client: manager.GetClient()}) + if err != nil { + log.Error(err, "could not create controller") + os.Exit(1) + } + + if err := manager.Start(controllers.SetupSignalHandler()); err != nil { + log.Error(err, "could not start manager") + os.Exit(1) + } +} + +// ReplicaSetReconciler is a simple Controller example implementation. +type ReplicaSetReconciler struct { + client.Client +} + +// Implement the business logic: +// This function will be called when there is a change to a ReplicaSet or a Pod with an OwnerReference +// to a ReplicaSet. +// +// * Read the ReplicaSet +// * Read the Pods +// * Set a Label on the ReplicaSet with the Pod count +func (a *ReplicaSetReconciler) Reconcile(req controllers.Request) (controllers.Result, error) { + // Read the ReplicaSet + rs := &appsv1.ReplicaSet{} + err := a.Get(context.TODO(), req.NamespacedName, rs) + if err != nil { + return controllers.Result{}, err + } + + // List the Pods matching the PodTemplate Labels + pods := &corev1.PodList{} + err = a.List(context.TODO(), client.InNamespace(req.Namespace).MatchingLabels(rs.Spec.Template.Labels), pods) + if err != nil { + return controllers.Result{}, err + } + + // Update the ReplicaSet + rs.Labels["pod-count"] = fmt.Sprintf("%v", len(pods.Items)) + err = a.Update(context.TODO(), rs) + if err != nil { + return controllers.Result{}, err + } + + return controllers.Result{}, nil +} diff --git a/pkg/builder/build.go b/pkg/builder/build.go index e3be7ea255..fc3e379873 100644 --- a/pkg/builder/build.go +++ b/pkg/builder/build.go @@ -32,133 +32,173 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ) -// application is a simple Controller for a single API type. It will create a Manager for itself -// if one is not provided. -type application struct { - mgr manager.Manager - ctrl controller.Controller -} - // Supporting mocking out functions for testing var getConfig = config.GetConfig var newController = controller.New var newManager = manager.New var getGvk = apiutil.GVKForObject -// Builder builds an Application Controller (e.g. Operator) and returns a manager.Manager to start it. +// Builder builds an Application ControllerManagedBy (e.g. Operator) and returns a manager.Manager to start it. type Builder struct { apiType runtime.Object mgr manager.Manager predicates []predicate.Predicate managedObjects []runtime.Object + watchRequest []watchRequest config *rest.Config ctrl controller.Controller } -// SimpleController returns a new Builder +// SimpleController returns a new Builder. +// Deprecated: Use ControllerManagedBy(Manager) instead. func SimpleController() *Builder { return &Builder{} } -// ForType sets the ForType that generates other types -func (b *Builder) ForType(apiType runtime.Object) *Builder { - b.apiType = apiType - return b +// ControllerManagedBy returns a new controller builder that will be started by the provided Manager +func ControllerManagedBy(m manager.Manager) *Builder { + return SimpleController().WithManager(m) +} + +// ForType defines the type of Object being *reconciled*, and configures the ControllerManagedBy to respond to create / delete / +// update events by *reconciling the object*. +// This is the equivalent of calling +// Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{}) +// Deprecated: Use For +func (blder *Builder) ForType(apiType runtime.Object) *Builder { + return blder.For(apiType) +} + +// For defines the type of Object being *reconciled*, and configures the ControllerManagedBy to respond to create / delete / +// update events by *reconciling the object*. +// This is the equivalent of calling +// Watches(&source.Kind{Type: apiType}, &handler.EnqueueRequestForObject{}) +func (blder *Builder) For(apiType runtime.Object) *Builder { + blder.apiType = apiType + return blder +} + +// Owns defines types of Objects being *generated* by the ControllerManagedBy, and configures the ControllerManagedBy to respond to +// create / delete / update events by *reconciling the owner object*. This is the equivalent of calling +// Watches(&handler.EnqueueRequestForOwner{&source.Kind{Type: }, &handler.EnqueueRequestForOwner{OwnerType: apiType, IsController: true}) +func (blder *Builder) Owns(apiType runtime.Object) *Builder { + blder.managedObjects = append(blder.managedObjects, apiType) + return blder +} + +type watchRequest struct { + src source.Source + eventhandler handler.EventHandler } -// Owns configures the Application Controller to respond to create / delete / update events for objects it managedObjects -// - e.g. creates. apiType is an empty instance of an object matching the managed object type. -func (b *Builder) Owns(apiType runtime.Object) *Builder { - b.managedObjects = append(b.managedObjects, apiType) - return b +// Watches exposes the lower-level ControllerManagedBy Watches functions through the builder. Consider using +// Owns or For instead of Watches directly. +func (blder *Builder) Watches(src source.Source, eventhandler handler.EventHandler) *Builder { + blder.watchRequest = append(blder.watchRequest, watchRequest{src: src, eventhandler: eventhandler}) + return blder } // WithConfig sets the Config to use for configuring clients. Defaults to the in-cluster config or to ~/.kube/config. -func (b *Builder) WithConfig(config *rest.Config) *Builder { - b.config = config - return b +// Deprecated: Use ControllerManagedBy(Manager) and this isn't needed. +func (blder *Builder) WithConfig(config *rest.Config) *Builder { + blder.config = config + return blder } -// WithManager sets the Manager to use for registering the Controller. Defaults to a new manager.Manager. -func (b *Builder) WithManager(m manager.Manager) *Builder { - b.mgr = m - return b +// WithManager sets the Manager to use for registering the ControllerManagedBy. Defaults to a new manager.Manager. +// Deprecated: Use ControllerManagedBy(Manager) and this isn't needed. +func (blder *Builder) WithManager(m manager.Manager) *Builder { + blder.mgr = m + return blder } // WithEventFilter sets the event filters, to filter which create/update/delete/generic events eventually // trigger reconciliations. For example, filtering on whether the resource version has changed. // Defaults to the empty list. -func (b *Builder) WithEventFilter(p predicate.Predicate) *Builder { - b.predicates = append(b.predicates, p) - return b +func (blder *Builder) WithEventFilter(p predicate.Predicate) *Builder { + blder.predicates = append(blder.predicates, p) + return blder } -// Build builds the Application Controller and returns the Manager used to start it. -func (b *Builder) Build(r reconcile.Reconciler) (manager.Manager, error) { +// Complete builds the Application ControllerManagedBy and returns the Manager used to start it. +func (blder *Builder) Complete(r reconcile.Reconciler) error { + _, err := blder.Build(r) + return err +} + +// Build builds the Application ControllerManagedBy and returns the Manager used to start it. +// Deprecated: Use Complete +func (blder *Builder) Build(r reconcile.Reconciler) (manager.Manager, error) { if r == nil { return nil, fmt.Errorf("must call WithReconciler to set Reconciler") } // Set the Config - if err := b.doConfig(); err != nil { + if err := blder.doConfig(); err != nil { return nil, err } // Set the Manager - if err := b.doManager(); err != nil { + if err := blder.doManager(); err != nil { return nil, err } - // Set the Controller - if err := b.doController(r); err != nil { + // Set the ControllerManagedBy + if err := blder.doController(r); err != nil { return nil, err } - a := &application{mgr: b.mgr, ctrl: b.ctrl} - // Reconcile type - s := &source.Kind{Type: b.apiType} - h := &handler.EnqueueRequestForObject{} - err := a.ctrl.Watch(s, h, b.predicates...) + src := &source.Kind{Type: blder.apiType} + hdler := &handler.EnqueueRequestForObject{} + err := blder.ctrl.Watch(src, hdler, blder.predicates...) if err != nil { return nil, err } - // Watch the managed types - for _, t := range b.managedObjects { - s := &source.Kind{Type: t} - h := &handler.EnqueueRequestForOwner{ - OwnerType: b.apiType, + // Watches the managed types + for _, obj := range blder.managedObjects { + src := &source.Kind{Type: obj} + hdler := &handler.EnqueueRequestForOwner{ + OwnerType: blder.apiType, IsController: true, } - if err := a.ctrl.Watch(s, h, b.predicates...); err != nil { + if err := blder.ctrl.Watch(src, hdler, blder.predicates...); err != nil { return nil, err } } - return a.mgr, nil + // Do the watch requests + for _, w := range blder.watchRequest { + if err := blder.ctrl.Watch(w.src, w.eventhandler, blder.predicates...); err != nil { + return nil, err + } + + } + + return blder.mgr, nil } -func (b *Builder) doConfig() error { - if b.config != nil { +func (blder *Builder) doConfig() error { + if blder.config != nil { return nil } var err error - b.config, err = getConfig() + blder.config, err = getConfig() return err } -func (b *Builder) doManager() error { - if b.mgr != nil { +func (blder *Builder) doManager() error { + if blder.mgr != nil { return nil } var err error - b.mgr, err = newManager(b.config, manager.Options{}) + blder.mgr, err = newManager(blder.config, manager.Options{}) return err } -func (b *Builder) getControllerName() (string, error) { - gvk, err := getGvk(b.apiType, b.mgr.GetScheme()) +func (blder *Builder) getControllerName() (string, error) { + gvk, err := getGvk(blder.apiType, blder.mgr.GetScheme()) if err != nil { return "", err } @@ -166,11 +206,11 @@ func (b *Builder) getControllerName() (string, error) { return name, nil } -func (b *Builder) doController(r reconcile.Reconciler) error { - name, err := b.getControllerName() +func (blder *Builder) doController(r reconcile.Reconciler) error { + name, err := blder.getControllerName() if err != nil { return err } - b.ctrl, err = newController(name, b.mgr, controller.Options{Reconciler: r}) + blder.ctrl, err = newController(name, blder.mgr, controller.Options{Reconciler: r}) return err } diff --git a/pkg/builder/build_test.go b/pkg/builder/build_test.go index 07cf85efcb..75c1795e98 100644 --- a/pkg/builder/build_test.go +++ b/pkg/builder/build_test.go @@ -19,6 +19,10 @@ package builder import ( "context" "fmt" + "strings" + + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/source" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -55,7 +59,7 @@ var _ = Describe("application", func() { Describe("New", func() { It("should return success if given valid objects", func() { instance, err := SimpleController(). - ForType(&appsv1.ReplicaSet{}). + For(&appsv1.ReplicaSet{}). Owns(&appsv1.ReplicaSet{}). Build(noop) Expect(err).NotTo(HaveOccurred()) @@ -65,7 +69,7 @@ var _ = Describe("application", func() { It("should return an error if the Config is invalid", func() { getConfig = func() (*rest.Config, error) { return cfg, fmt.Errorf("expected error") } instance, err := SimpleController(). - ForType(&appsv1.ReplicaSet{}). + For(&appsv1.ReplicaSet{}). Owns(&appsv1.ReplicaSet{}). Build(noop) Expect(err).To(HaveOccurred()) @@ -75,7 +79,7 @@ var _ = Describe("application", func() { It("should return an error if there is no GVK for an object", func() { instance, err := SimpleController(). - ForType(&fakeType{}). + For(&fakeType{}). Owns(&appsv1.ReplicaSet{}). Build(noop) Expect(err).To(HaveOccurred()) @@ -83,7 +87,7 @@ var _ = Describe("application", func() { Expect(instance).To(BeNil()) instance, err = SimpleController(). - ForType(&appsv1.ReplicaSet{}). + For(&appsv1.ReplicaSet{}). Owns(&fakeType{}). Build(noop) Expect(err).To(HaveOccurred()) @@ -96,7 +100,7 @@ var _ = Describe("application", func() { return nil, fmt.Errorf("expected error") } instance, err := SimpleController(). - ForType(&appsv1.ReplicaSet{}). + For(&appsv1.ReplicaSet{}). Owns(&appsv1.ReplicaSet{}). Build(noop) Expect(err).To(HaveOccurred()) @@ -110,7 +114,7 @@ var _ = Describe("application", func() { return nil, fmt.Errorf("expected error") } instance, err := SimpleController(). - ForType(&appsv1.ReplicaSet{}). + For(&appsv1.ReplicaSet{}). Owns(&appsv1.ReplicaSet{}). Build(noop) Expect(err).To(HaveOccurred()) @@ -119,94 +123,157 @@ var _ = Describe("application", func() { }) }) - Describe("Start", func() { - It("should Reconcile objects", func(done Done) { - By("Creating the application") - ch := make(chan reconcile.Request) - fn := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) { - defer GinkgoRecover() - ch <- req - return reconcile.Result{}, nil - }) - - instance, err := SimpleController().ForType(&appsv1.Deployment{}). + Describe("Start with SimpleController", func() { + It("should Reconcile Owns objects", func(done Done) { + bldr := SimpleController(). + ForType(&appsv1.Deployment{}). WithConfig(cfg). - Owns(&appsv1.ReplicaSet{}). - Build(fn) + Owns(&appsv1.ReplicaSet{}) + doReconcileTest("1", stop, bldr, nil, false) + + close(done) + }, 10) + + It("should Reconcile Owns objects with a Manager", func(done Done) { + m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) - By("Starting the application") - go func() { - defer GinkgoRecover() - Expect(instance.Start(stop)).NotTo(HaveOccurred()) - By("Stopping the application") - }() - - By("Creating a Deployment") - // Expect a Reconcile when the Deployment is managedObjects. - dep := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "deploy-name", - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"foo": "bar"}, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"foo": "bar"}}, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "nginx", - Image: "nginx", - }, - }, - }, - }, - }, - } - err = instance.GetClient().Create(context.TODO(), dep) + bldr := SimpleController(). + WithManager(m). + For(&appsv1.Deployment{}). + Owns(&appsv1.ReplicaSet{}) + doReconcileTest("2", stop, bldr, m, false) + close(done) + }, 10) + }) + + Describe("Start with ControllerManagedBy", func() { + It("should Reconcile Owns objects", func(done Done) { + m, err := manager.New(cfg, manager.Options{}) + Expect(err).NotTo(HaveOccurred()) + + bldr := ControllerManagedBy(m). + For(&appsv1.Deployment{}). + Owns(&appsv1.ReplicaSet{}) + doReconcileTest("3", stop, bldr, m, false) + close(done) + }, 10) + + It("should Reconcile Watches objects", func(done Done) { + m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) - By("Waiting for the Deployment Reconcile") - Expect(<-ch).To(Equal(reconcile.Request{ - NamespacedName: types.NamespacedName{Namespace: "default", Name: "deploy-name"}})) - - By("Creating a ReplicaSet") - // Expect a Reconcile when an Owned object is managedObjects. - t := true - rs := &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - Name: "rs-name", - Labels: dep.Spec.Selector.MatchLabels, - OwnerReferences: []metav1.OwnerReference{ + bldr := ControllerManagedBy(m). + For(&appsv1.Deployment{}). + Watches( // Equivalent of Owns + &source.Kind{Type: &appsv1.ReplicaSet{}}, + &handler.EnqueueRequestForOwner{OwnerType: &appsv1.Deployment{}, IsController: true}) + doReconcileTest("4", stop, bldr, m, true) + close(done) + }, 10) + }) +}) + +func doReconcileTest(nameSuffix string, stop chan struct{}, blder *Builder, mgr manager.Manager, complete bool) { + deployName := "deploy-name-" + nameSuffix + rsName := "rs-name-" + nameSuffix + + By("Creating the application") + ch := make(chan reconcile.Request) + fn := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) { + defer GinkgoRecover() + if !strings.HasSuffix(req.Name, nameSuffix) { + // From different test, ignore this request. Etcd is shared across tests. + return reconcile.Result{}, nil + } + ch <- req + return reconcile.Result{}, nil + }) + + instance := mgr + if complete { + err := blder.Complete(fn) + Expect(err).NotTo(HaveOccurred()) + } else { + var err error + instance, err = blder.Build(fn) + Expect(err).NotTo(HaveOccurred()) + } + + // Manager should match + if mgr != nil { + Expect(instance).To(Equal(mgr)) + } + + By("Starting the application") + go func() { + defer GinkgoRecover() + Expect(instance.Start(stop)).NotTo(HaveOccurred()) + By("Stopping the application") + }() + + By("Creating a Deployment") + // Expect a Reconcile when the Deployment is managedObjects. + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: deployName, + }, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"foo": "bar"}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"foo": "bar"}}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ { - Name: "deploy-name", - Kind: "Deployment", - APIVersion: "apps/v1", - Controller: &t, - UID: dep.UID, + Name: "nginx", + Image: "nginx", }, }, }, - Spec: appsv1.ReplicaSetSpec{ - Selector: dep.Spec.Selector, - Template: dep.Spec.Template, + }, + }, + } + err := instance.GetClient().Create(context.TODO(), dep) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for the Deployment Reconcile") + Expect(<-ch).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: "default", Name: deployName}})) + + By("Creating a ReplicaSet") + // Expect a Reconcile when an Owned object is managedObjects. + t := true + rs := &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: rsName, + Labels: dep.Spec.Selector.MatchLabels, + OwnerReferences: []metav1.OwnerReference{ + { + Name: deployName, + Kind: "Deployment", + APIVersion: "apps/v1", + Controller: &t, + UID: dep.UID, }, - } - err = instance.GetClient().Create(context.TODO(), rs) - Expect(err).NotTo(HaveOccurred()) + }, + }, + Spec: appsv1.ReplicaSetSpec{ + Selector: dep.Spec.Selector, + Template: dep.Spec.Template, + }, + } + err = instance.GetClient().Create(context.TODO(), rs) + Expect(err).NotTo(HaveOccurred()) - By("Waiting for the ReplicaSet Reconcile") - Expect(<-ch).To(Equal(reconcile.Request{ - NamespacedName: types.NamespacedName{Namespace: "default", Name: "deploy-name"}})) + By("Waiting for the ReplicaSet Reconcile") + Expect(<-ch).To(Equal(reconcile.Request{ + NamespacedName: types.NamespacedName{Namespace: "default", Name: deployName}})) - close(done) - }, 10) - }) -}) +} var _ runtime.Object = &fakeType{} diff --git a/pkg/builder/example_test.go b/pkg/builder/example_test.go index 8339b56087..a966eed456 100644 --- a/pkg/builder/example_test.go +++ b/pkg/builder/example_test.go @@ -21,41 +21,50 @@ import ( "fmt" "os" + logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/config" + "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" - logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" "sigs.k8s.io/controller-runtime/pkg/runtime/signals" ) -// NB: don't call SetLogger in init(), or else you'll mess up logging in the main suite. -var log = logf.Log.WithName("builder-examples") - -// This example creates a simple application Controller that is configured for ReplicaSets and Pods. +// This example creates a simple application ControllerManagedBy that is configured for ReplicaSets and Pods. // // * Create a new application for ReplicaSets that manages Pods owned by the ReplicaSet and calls into // ReplicaSetReconciler. // // * Start the application. func ExampleBuilder() { - rs, err := builder.SimpleController(). - ForType(&appsv1.ReplicaSet{}). // ReplicaSet is the Application API - Owns(&corev1.Pod{}). // ReplicaSet owns Pods created by it - Build(&ReplicaSetReconciler{}) // Build + var log = logf.Log.WithName("builder-examples") + + mgr, err := manager.New(config.GetConfigOrDie(), manager.Options{}) + if err != nil { + log.Error(err, "could not create manager") + os.Exit(1) + } + + _, err = builder. + ControllerManagedBy(mgr). // Create the ControllerManagedBy + For(&appsv1.ReplicaSet{}). // ReplicaSet is the Application API + Owns(&corev1.Pod{}). // ReplicaSet owns Pods created by it + Build(&ReplicaSetReconciler{}) if err != nil { - log.Error(err, "Unable to build controller") + log.Error(err, "could not create controller") os.Exit(1) } - if err := rs.Start(signals.SetupSignalHandler()); err != nil { - log.Error(err, "Unable to run controller") + if err := mgr.Start(signals.SetupSignalHandler()); err != nil { + log.Error(err, "could not start manager") os.Exit(1) } } -// ReplicaSetReconciler is a simple Controller example implementation. +// ReplicaSetReconciler is a simple ControllerManagedBy example implementation. type ReplicaSetReconciler struct { client.Client } diff --git a/pkg/client/config/config.go b/pkg/client/config/config.go index 849ba7af95..a57d48767f 100644 --- a/pkg/client/config/config.go +++ b/pkg/client/config/config.go @@ -89,6 +89,7 @@ func GetConfigOrDie() *rest.Config { config, err := GetConfig() if err != nil { log.Error(err, "unable to get kubeconfig") + os.Exit(1) } return config } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 164167a8c2..64a27a7a22 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -78,15 +78,15 @@ func New(name string, mgr manager.Manager, options Options) (Controller, error) // Create controller with dependencies set c := &controller.Controller{ - Do: options.Reconciler, - Cache: mgr.GetCache(), - Config: mgr.GetConfig(), - Scheme: mgr.GetScheme(), - Client: mgr.GetClient(), - Recorder: mgr.GetRecorder(name), - Queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), name), + Do: options.Reconciler, + Cache: mgr.GetCache(), + Config: mgr.GetConfig(), + Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Recorder: mgr.GetRecorder(name), + Queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), name), MaxConcurrentReconciles: options.MaxConcurrentReconciles, - Name: name, + Name: name, } // Add the controller as a Manager components diff --git a/pkg/doc.go b/pkg/doc.go index 5792a4e6fb..8c9bcf5f49 100644 --- a/pkg/doc.go +++ b/pkg/doc.go @@ -47,7 +47,7 @@ system must be read for each Reconciler. * Controllers require a Reconciler to be provided to perform the work pulled from the work queue. -* Controller require Watches to be configured to enqueue reconcile.Requests in response to events. +* Controllers require Watches to be configured to enqueue reconcile.Requests in response to events. Webhook diff --git a/pkg/internal/controller/controller_test.go b/pkg/internal/controller/controller_test.go index 1269c07a29..8faa1f5eef 100644 --- a/pkg/internal/controller/controller_test.go +++ b/pkg/internal/controller/controller_test.go @@ -18,7 +18,6 @@ package controller import ( "fmt" - "time" . "github.com/onsi/ginkgo" @@ -63,9 +62,9 @@ var _ = Describe("controller", func() { informers = &informertest.FakeInformers{} ctrl = &Controller{ MaxConcurrentReconciles: 1, - Do: fakeReconcile, - Queue: queue, - Cache: informers, + Do: fakeReconcile, + Queue: queue, + Cache: informers, } ctrl.InjectFunc(func(interface{}) error { return nil }) }) From 58c9cd9617c268dafc7579f8948d51cba695e22a Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Sat, 22 Dec 2018 09:46:07 -0800 Subject: [PATCH 2/2] disable metalinter goimports because it is broken (failing on files that haven't changed their imports) --- hack/verify.sh | 2 +- pkg/builder/build.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/verify.sh b/hack/verify.sh index c6748189a1..43278b73af 100755 --- a/hack/verify.sh +++ b/hack/verify.sh @@ -35,7 +35,6 @@ gometalinter.v2 --disable-all \ --enable=structcheck \ --enable=golint \ --enable=deadcode \ - --enable=goimports \ --enable=errcheck \ --enable=varcheck \ --enable=goconst \ @@ -52,6 +51,7 @@ gometalinter.v2 --disable-all \ --skip=atomic \ ./pkg/... # TODO: Enable these as we fix them to make them pass +# --enable=goimports \ # --enable=gosec \ # --enable=maligned \ # --enable=safesql \ diff --git a/pkg/builder/build.go b/pkg/builder/build.go index fc3e379873..d00b036384 100644 --- a/pkg/builder/build.go +++ b/pkg/builder/build.go @@ -38,7 +38,7 @@ var newController = controller.New var newManager = manager.New var getGvk = apiutil.GVKForObject -// Builder builds an Application ControllerManagedBy (e.g. Operator) and returns a manager.Manager to start it. +// Builder builds a Controller. type Builder struct { apiType runtime.Object mgr manager.Manager