From 641aa1c3575883e76ea432c5a08be9b2ebe255ab Mon Sep 17 00:00:00 2001 From: awgreene Date: Wed, 16 Dec 2020 10:13:20 -0500 Subject: [PATCH] update controllers --- .../operators/adoption_controller.go | 73 +++++++++++-------- .../operators/adoption_controller_test.go | 6 +- .../operators/operator_controller.go | 22 +++--- .../operators/operator_controller_test.go | 11 +-- .../operatorcondition_controller_test.go | 19 +++-- ...ratorconditiongenerator_controller_test.go | 19 ++--- pkg/controller/operators/suite_test.go | 9 ++- 7 files changed, 85 insertions(+), 74 deletions(-) diff --git a/pkg/controller/operators/adoption_controller.go b/pkg/controller/operators/adoption_controller.go index f255e111179..50a2bf20bdc 100644 --- a/pkg/controller/operators/adoption_controller.go +++ b/pkg/controller/operators/adoption_controller.go @@ -2,6 +2,7 @@ package operators import ( "context" + "fmt" "sync" "github.com/go-logr/logr" @@ -45,9 +46,7 @@ type AdoptionReconciler struct { // SetupWithManager adds the operator reconciler to the given controller manager. func (r *AdoptionReconciler) SetupWithManager(mgr ctrl.Manager) error { // Trigger operator events from the events of their compoenents. - enqueueSub := &handler.EnqueueRequestsFromMapFunc{ - ToRequests: handler.ToRequestsFunc(r.mapToSubscriptions), - } + enqueueSub := handler.EnqueueRequestsFromMapFunc(r.mapToSubscriptions) // Create multiple controllers for resource types that require automatic adoption err := ctrl.NewControllerManagedBy(mgr). @@ -60,12 +59,8 @@ func (r *AdoptionReconciler) SetupWithManager(mgr ctrl.Manager) error { } var ( - enqueueCSV = &handler.EnqueueRequestsFromMapFunc{ - ToRequests: handler.ToRequestsFunc(r.mapToClusterServiceVersions), - } - enqueueProviders = &handler.EnqueueRequestsFromMapFunc{ - ToRequests: handler.ToRequestsFunc(r.mapToProviders), - } + enqueueCSV = handler.EnqueueRequestsFromMapFunc(r.mapToClusterServiceVersions) + enqueueProviders = handler.EnqueueRequestsFromMapFunc(r.mapToProviders) ) err = ctrl.NewControllerManagedBy(mgr). For(&operatorsv1alpha1.ClusterServiceVersion{}). @@ -113,13 +108,12 @@ func NewAdoptionReconciler(cli client.Client, log logr.Logger, scheme *runtime.S } // ReconcileSubscription labels the CSVs installed by a Subscription as components of an operator named after the subscribed package and install namespace. -func (r *AdoptionReconciler) ReconcileSubscription(req ctrl.Request) (reconcile.Result, error) { +func (r *AdoptionReconciler) ReconcileSubscription(ctx context.Context, req ctrl.Request) (reconcile.Result, error) { // Set up a convenient log object so we don't have to type request over and over again log := r.log.WithValues("request", req) log.V(4).Info("reconciling subscription") // Fetch the Subscription from the cache - ctx := context.TODO() in := &operatorsv1alpha1.Subscription{} if err := r.Get(ctx, req.NamespacedName, in); err != nil { if apierrors.IsNotFound(err) { @@ -176,13 +170,12 @@ func (r *AdoptionReconciler) ReconcileSubscription(req ctrl.Request) (reconcile. } // ReconcileClusterServiceVersion projects the component labels of a given CSV onto all resources owned by it. -func (r *AdoptionReconciler) ReconcileClusterServiceVersion(req ctrl.Request) (reconcile.Result, error) { +func (r *AdoptionReconciler) ReconcileClusterServiceVersion(ctx context.Context, req ctrl.Request) (reconcile.Result, error) { // Set up a convenient log object so we don't have to type request over and over again log := r.log.WithValues("request", req) log.V(4).Info("reconciling csv") // Fetch the CSV from the cache - ctx := context.TODO() in := &operatorsv1alpha1.ClusterServiceVersion{} if err := r.Get(ctx, req.NamespacedName, in); err != nil { if apierrors.IsNotFound(err) { @@ -265,7 +258,12 @@ func (r *AdoptionReconciler) adopt(ctx context.Context, operator *decorators.Ope return nil } - if err := r.Get(ctx, types.NamespacedName{Namespace: m.GetNamespace(), Name: m.GetName()}, component); err != nil { + cObj, ok := component.(client.Object) + if !ok { + return fmt.Errorf("Unable to typecast runtime.Object to client.Object") + } + + if err := r.Get(ctx, types.NamespacedName{Namespace: m.GetNamespace(), Name: m.GetName()}, cObj); err != nil { if apierrors.IsNotFound(err) { r.log.Error(err, "component not found") err = nil @@ -273,7 +271,7 @@ func (r *AdoptionReconciler) adopt(ctx context.Context, operator *decorators.Ope return err } - candidate := component.DeepCopyObject() + candidate := cObj.DeepCopyObject() adopted, err := operator.AdoptComponent(candidate) if err != nil { @@ -282,14 +280,21 @@ func (r *AdoptionReconciler) adopt(ctx context.Context, operator *decorators.Ope if adopted { // Only update if freshly adopted - r.log.Info("component adopted", "component", candidate) - return r.Patch(ctx, candidate, client.MergeFrom(component)) + pCObj, ok := candidate.(client.Object) + if !ok { + return fmt.Errorf("Unable to typecast runtime.Object to client.Object") + } + return r.Patch(ctx, pCObj, client.MergeFrom(cObj)) } return nil } func (r *AdoptionReconciler) disown(ctx context.Context, operator *decorators.Operator, component runtime.Object) error { + cObj, ok := component.(client.Object) + if !ok { + return fmt.Errorf("Unable to typecast runtime.Object to client.Object") + } candidate := component.DeepCopyObject() disowned, err := operator.DisownComponent(candidate) if err != nil { @@ -303,7 +308,11 @@ func (r *AdoptionReconciler) disown(ctx context.Context, operator *decorators.Op // Only update if freshly disowned r.log.V(4).Info("component disowned", "component", candidate) - return r.Patch(ctx, candidate, client.MergeFrom(component)) + uCObj, ok := candidate.(client.Object) + if !ok { + return fmt.Errorf("Unable to typecast runtime.Object to client.Object") + } + return r.Patch(ctx, uCObj, client.MergeFrom(cObj)) } func (r *AdoptionReconciler) adoptees(ctx context.Context, operator decorators.Operator, csv *operatorsv1alpha1.ClusterServiceVersion) ([]runtime.Object, error) { @@ -335,7 +344,11 @@ func (r *AdoptionReconciler) adoptees(ctx context.Context, operator decorators.O } opt := client.MatchingLabelsSelector{Selector: selector} for _, list := range componentLists { - if err := r.List(ctx, list, opt); err != nil { + cList, ok := list.(client.ObjectList) + if !ok { + return nil, fmt.Errorf("Unable to typecast runtime.Object to client.ObjectList") + } + if err := r.List(ctx, cList, opt); err != nil { return nil, err } } @@ -415,8 +428,8 @@ func (r *AdoptionReconciler) adoptInstallPlan(ctx context.Context, operator *dec return utilerrors.NewAggregate(errs) } -func (r *AdoptionReconciler) mapToSubscriptions(obj handler.MapObject) (requests []reconcile.Request) { - if obj.Meta == nil { +func (r *AdoptionReconciler) mapToSubscriptions(obj client.Object) (requests []reconcile.Request) { + if obj == nil { return } @@ -424,7 +437,7 @@ func (r *AdoptionReconciler) mapToSubscriptions(obj handler.MapObject) (requests // The Subscription reconciler will sort out the important changes ctx := context.TODO() subs := &operatorsv1alpha1.SubscriptionList{} - if err := r.List(ctx, subs, client.InNamespace(obj.Meta.GetNamespace())); err != nil { + if err := r.List(ctx, subs, client.InNamespace(obj.GetNamespace())); err != nil { r.log.Error(err, "error listing subscriptions") } @@ -444,15 +457,15 @@ func (r *AdoptionReconciler) mapToSubscriptions(obj handler.MapObject) (requests return } -func (r *AdoptionReconciler) mapToClusterServiceVersions(obj handler.MapObject) (requests []reconcile.Request) { - if obj.Meta == nil { +func (r *AdoptionReconciler) mapToClusterServiceVersions(obj client.Object) (requests []reconcile.Request) { + if obj == nil { return } // Get all owner CSV from owner labels if cluster scoped - namespace := obj.Meta.GetNamespace() + namespace := obj.GetNamespace() if namespace == metav1.NamespaceAll { - name, ns, ok := ownerutil.GetOwnerByKindLabel(obj.Meta, operatorsv1alpha1.ClusterServiceVersionKind) + name, ns, ok := ownerutil.GetOwnerByKindLabel(obj, operatorsv1alpha1.ClusterServiceVersionKind) if ok { nsn := types.NamespacedName{Namespace: ns, Name: name} requests = append(requests, reconcile.Request{NamespacedName: nsn}) @@ -461,7 +474,7 @@ func (r *AdoptionReconciler) mapToClusterServiceVersions(obj handler.MapObject) } // Get all owner CSVs from OwnerReferences - owners := ownerutil.GetOwnersByKind(obj.Meta, operatorsv1alpha1.ClusterServiceVersionKind) + owners := ownerutil.GetOwnersByKind(obj, operatorsv1alpha1.ClusterServiceVersionKind) for _, owner := range owners { nsn := types.NamespacedName{Namespace: namespace, Name: owner.Name} requests = append(requests, reconcile.Request{NamespacedName: nsn}) @@ -470,8 +483,8 @@ func (r *AdoptionReconciler) mapToClusterServiceVersions(obj handler.MapObject) return } -func (r *AdoptionReconciler) mapToProviders(obj handler.MapObject) (requests []reconcile.Request) { - if obj.Meta == nil { +func (r *AdoptionReconciler) mapToProviders(obj client.Object) (requests []reconcile.Request) { + if obj == nil { return nil } @@ -489,7 +502,7 @@ func (r *AdoptionReconciler) mapToProviders(obj handler.MapObject) (requests []r NamespacedName: types.NamespacedName{Namespace: csv.GetNamespace(), Name: csv.GetName()}, } for _, provided := range csv.Spec.CustomResourceDefinitions.Owned { - if provided.Name == obj.Meta.GetName() { + if provided.Name == obj.GetName() { requests = append(requests, request) break } diff --git a/pkg/controller/operators/adoption_controller_test.go b/pkg/controller/operators/adoption_controller_test.go index c27af0d876d..2c1505e1572 100644 --- a/pkg/controller/operators/adoption_controller_test.go +++ b/pkg/controller/operators/adoption_controller_test.go @@ -12,9 +12,9 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/reference" apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" + "sigs.k8s.io/controller-runtime/pkg/client" operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators" @@ -33,11 +33,11 @@ var _ = Describe("Adoption Controller", func() { Describe("Component label generation", func() { var ( - created []runtime.Object + created []client.Object ) BeforeEach(func() { - created = []runtime.Object{} + created = []client.Object{} }) JustAfterEach(func() { diff --git a/pkg/controller/operators/operator_controller.go b/pkg/controller/operators/operator_controller.go index d5494b34555..edfb27fd3ae 100644 --- a/pkg/controller/operators/operator_controller.go +++ b/pkg/controller/operators/operator_controller.go @@ -2,6 +2,7 @@ package operators import ( "context" + "fmt" "sync" "github.com/go-logr/logr" @@ -58,10 +59,7 @@ type OperatorReconciler struct { // SetupWithManager adds the operator reconciler to the given controller manager. func (r *OperatorReconciler) SetupWithManager(mgr ctrl.Manager) error { // Trigger operator events from the events of their compoenents. - enqueueOperator := &handler.EnqueueRequestsFromMapFunc{ - ToRequests: handler.ToRequestsFunc(r.mapComponentRequests), - } - + enqueueOperator := handler.EnqueueRequestsFromMapFunc(r.mapComponentRequests) // Note: If we want to support resources composed of custom resources, we need to figure out how // to dynamically add resource types to watch. return ctrl.NewControllerManagedBy(mgr). @@ -110,13 +108,12 @@ func NewOperatorReconciler(cli client.Client, log logr.Logger, scheme *runtime.S // Implement reconcile.Reconciler so the controller can reconcile objects var _ reconcile.Reconciler = &OperatorReconciler{} -func (r *OperatorReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { +func (r *OperatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { // Set up a convenient log object so we don't have to type request over and over again log := r.log.WithValues("request", req) log.V(1).Info("reconciling operator") // Get the Operator - ctx := context.TODO() create := false name := req.NamespacedName.Name in := &operatorsv1.Operator{} @@ -210,7 +207,11 @@ func (r *OperatorReconciler) listComponents(ctx context.Context, selector labels opt := client.MatchingLabelsSelector{Selector: selector} for _, list := range componentLists { - if err := r.List(ctx, list, opt); err != nil { + cList, ok := list.(client.ObjectList) + if !ok { + return nil, fmt.Errorf("Unable to typecast runtime.Object to client.ObjectList") + } + if err := r.List(ctx, cList, opt); err != nil { return nil, err } } @@ -245,13 +246,14 @@ func (r *OperatorReconciler) unsetLastResourceVersion(name types.NamespacedName) delete(r.lastResourceVersion, name) } -func (r *OperatorReconciler) mapComponentRequests(obj handler.MapObject) []reconcile.Request { +func (r *OperatorReconciler) mapComponentRequests(obj client.Object) []reconcile.Request { var requests []reconcile.Request - if obj.Meta == nil { + if obj == nil { return requests } - for _, name := range decorators.OperatorNames(obj.Meta.GetLabels()) { + labels := decorators.OperatorNames(obj.GetLabels()) + for _, name := range labels { // unset the last recorded resource version so the Operator will reconcile r.unsetLastResourceVersion(name) requests = append(requests, reconcile.Request{NamespacedName: name}) diff --git a/pkg/controller/operators/operator_controller_test.go b/pkg/controller/operators/operator_controller_test.go index b6c402fefae..296db1f5370 100644 --- a/pkg/controller/operators/operator_controller_test.go +++ b/pkg/controller/operators/operator_controller_test.go @@ -10,6 +10,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators" @@ -83,7 +84,7 @@ var _ = Describe("Operator Controller", func() { ) for _, obj := range objs { - Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + Expect(k8sClient.Create(ctx, obj.(client.Object))).To(Succeed()) } expectedRefs = toRefs(scheme, objs...) @@ -91,8 +92,8 @@ var _ = Describe("Operator Controller", func() { AfterEach(func() { for _, obj := range objs { - Expect(k8sClient.Get(ctx, testobj.NamespacedName(obj), obj)).To(Succeed()) - Expect(k8sClient.Delete(ctx, obj, deleteOpts)).To(Succeed()) + Expect(k8sClient.Get(ctx, testobj.NamespacedName(obj), obj.(client.Object))).To(Succeed()) + Expect(k8sClient.Delete(ctx, obj.(client.Object), deleteOpts)).To(Succeed()) } }) @@ -125,7 +126,7 @@ var _ = Describe("Operator Controller", func() { ) for _, obj := range newObjs { - Expect(k8sClient.Create(ctx, obj)).To(Succeed()) + Expect(k8sClient.Create(ctx, obj.(client.Object))).To(Succeed()) } objs = append(objs, newObjs...) @@ -143,7 +144,7 @@ var _ = Describe("Operator Controller", func() { Context("when component labels are removed", func() { BeforeEach(func() { for _, obj := range testobj.StripLabel(expectedKey, objs...) { - Expect(k8sClient.Update(ctx, obj)).To(Succeed()) + Expect(k8sClient.Update(ctx, obj.(client.Object))).To(Succeed()) } }) diff --git a/pkg/controller/operators/operatorcondition_controller_test.go b/pkg/controller/operators/operatorcondition_controller_test.go index a863efc6e89..4da2fea954e 100644 --- a/pkg/controller/operators/operatorcondition_controller_test.go +++ b/pkg/controller/operators/operatorcondition_controller_test.go @@ -45,22 +45,21 @@ var _ = Describe("OperatorCondition", func() { Context("The OperatorCondition Reconciler", func() { var ( ctx context.Context - namespace string - namespacedName types.NamespacedName operatorCondition *operatorsv1.OperatorCondition + namespace *corev1.Namespace + namespacedName types.NamespacedName ) BeforeEach(func() { ctx = context.Background() - namespace = genName("ns-") - ns := &corev1.Namespace{ + namespace = &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: namespace, + Name: genName("ns-"), }, } - Expect(k8sClient.Create(ctx, ns)).To(Succeed()) + Expect(k8sClient.Create(ctx, namespace)).To(Succeed()) - namespacedName = types.NamespacedName{Name: "test", Namespace: namespace} + namespacedName = types.NamespacedName{Name: "test", Namespace: namespace.GetName()} // Create the deployment labels := map[string]string{ @@ -68,7 +67,7 @@ var _ = Describe("OperatorCondition", func() { } deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ Name: "deployment", - Namespace: namespacedName.Namespace, + Namespace: namespace.GetName(), }, Spec: appsv1.DeploymentSpec{ Selector: &metav1.LabelSelector{ @@ -77,7 +76,7 @@ var _ = Describe("OperatorCondition", func() { Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "nginx-", - Namespace: namespacedName.Namespace, + Namespace: namespace.GetName(), Labels: labels, }, Spec: corev1.PodSpec{ @@ -192,7 +191,7 @@ var _ = Describe("OperatorCondition", func() { It("appends the OPERATOR_CONDITION_NAME environment variable to the containers in the deployments", func() { deployment := &appsv1.Deployment{} Eventually(func() error { - err := k8sClient.Get(ctx, types.NamespacedName{Name: operatorCondition.Spec.Deployments[0], Namespace: namespace}, deployment) + err := k8sClient.Get(ctx, types.NamespacedName{Name: operatorCondition.Spec.Deployments[0], Namespace: namespace.GetName()}, deployment) if err != nil { return err } diff --git a/pkg/controller/operators/operatorconditiongenerator_controller_test.go b/pkg/controller/operators/operatorconditiongenerator_controller_test.go index 05a02d4cfb5..f990031ffd3 100644 --- a/pkg/controller/operators/operatorconditiongenerator_controller_test.go +++ b/pkg/controller/operators/operatorconditiongenerator_controller_test.go @@ -11,7 +11,6 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apiserver/pkg/storage/names" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -25,23 +24,19 @@ var _ = Describe("The OperatorConditionsGenerator Controller", func() { interval = time.Millisecond * 100 ) - var ( - genName = names.SimpleNameGenerator.GenerateName - ) var ( ctx context.Context - namespace string + namespace *corev1.Namespace ) BeforeEach(func() { ctx = context.Background() - namespace = genName("ns-") - ns := &corev1.Namespace{ + namespace = &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: namespace, + Name: genName("ns-"), }, } - Expect(k8sClient.Create(ctx, ns)).To(Succeed()) + Expect(k8sClient.Create(ctx, namespace)).To(Succeed()) }) It("creates an OperatorCondition for a CSV without a ServiceAccount", func() { @@ -53,7 +48,7 @@ var _ = Describe("The OperatorConditionsGenerator Controller", func() { }, ObjectMeta: metav1.ObjectMeta{ Name: genName("csv-"), - Namespace: namespace, + Namespace: namespace.GetName(), }, Spec: operatorsv1alpha1.ClusterServiceVersionSpec{ InstallModes: []operatorsv1alpha1.InstallMode{ @@ -113,7 +108,7 @@ var _ = Describe("The OperatorConditionsGenerator Controller", func() { }, ObjectMeta: metav1.ObjectMeta{ Name: genName("csv-"), - Namespace: namespace, + Namespace: namespace.GetName(), Labels: map[string]string{ operatorsv1alpha1.CopiedLabelKey: "", }, @@ -164,7 +159,7 @@ var _ = Describe("The OperatorConditionsGenerator Controller", func() { }, ObjectMeta: metav1.ObjectMeta{ Name: genName("csv-"), - Namespace: namespace, + Namespace: namespace.GetName(), }, Spec: operatorsv1alpha1.ClusterServiceVersionSpec{ InstallModes: []operatorsv1alpha1.InstallMode{ diff --git a/pkg/controller/operators/suite_test.go b/pkg/controller/operators/suite_test.go index 854c778e171..cedf8037759 100644 --- a/pkg/controller/operators/suite_test.go +++ b/pkg/controller/operators/suite_test.go @@ -1,6 +1,7 @@ package operators import ( + "context" "fmt" "testing" "time" @@ -45,7 +46,7 @@ var ( cfg *rest.Config k8sClient client.Client testEnv *envtest.Environment - stop chan struct{} + ctx context.Context scheme = runtime.NewScheme() gracePeriod int64 = 0 @@ -87,7 +88,7 @@ var _ = BeforeSuite(func() { useExisting := false testEnv = &envtest.Environment{ UseExistingCluster: &useExisting, - CRDs: []runtime.Object{ + CRDs: []client.Object{ crds.CatalogSource(), crds.ClusterServiceVersion(), crds.InstallPlan(), @@ -143,7 +144,7 @@ var _ = BeforeSuite(func() { Expect(operatorConditionReconciler.SetupWithManager(mgr)).ToNot(HaveOccurred()) Expect(operatorConditionGeneratorReconciler.SetupWithManager(mgr)).ToNot(HaveOccurred()) - ctx := ctrl.SetupSignalHandler() + ctx = ctrl.SetupSignalHandler() go func() { defer GinkgoRecover() @@ -160,7 +161,7 @@ var _ = BeforeSuite(func() { var _ = AfterSuite(func() { By("stopping the controller manager") - close(stop) + ctx.Done() By("tearing down the test environment") err := testEnv.Stop()