From 68cfde8b74c1e0b717daeabd9cb5e4028f217c9d Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Wed, 18 Mar 2020 19:57:15 -0700 Subject: [PATCH 1/2] Add a namespace controller for the MT Broker. This adds a namespace controller that creates brokers per namespace without the additional components that are only needed by the single-tenant brokers (ConfigMapProp, RoleBindings, ServiceAccounts). Since the footprint of the MT Broker is effectively nothing, I've made this version of the controller opt-out vs. opt-in. --- cmd/mtchannel_broker/main.go | 2 + pkg/reconciler/mtnamespace/controller.go | 70 +++++++ pkg/reconciler/mtnamespace/controller_test.go | 38 ++++ pkg/reconciler/mtnamespace/namespace.go | 145 +++++++++++++ pkg/reconciler/mtnamespace/namespace_test.go | 191 ++++++++++++++++++ .../mtnamespace/resources/broker.go | 46 +++++ .../mtnamespace/resources/labels.go | 44 ++++ 7 files changed, 536 insertions(+) create mode 100644 pkg/reconciler/mtnamespace/controller.go create mode 100644 pkg/reconciler/mtnamespace/controller_test.go create mode 100644 pkg/reconciler/mtnamespace/namespace.go create mode 100644 pkg/reconciler/mtnamespace/namespace_test.go create mode 100644 pkg/reconciler/mtnamespace/resources/broker.go create mode 100644 pkg/reconciler/mtnamespace/resources/labels.go diff --git a/cmd/mtchannel_broker/main.go b/cmd/mtchannel_broker/main.go index 79e7d772531..b6d6ee4bde5 100644 --- a/cmd/mtchannel_broker/main.go +++ b/cmd/mtchannel_broker/main.go @@ -23,10 +23,12 @@ import ( "knative.dev/pkg/injection/sharedmain" "knative.dev/eventing/pkg/reconciler/mtbroker" + "knative.dev/eventing/pkg/reconciler/mtnamespace" ) func main() { sharedmain.Main("mt-broker-controller", + mtnamespace.NewController, mtbroker.NewController, ) } diff --git a/pkg/reconciler/mtnamespace/controller.go b/pkg/reconciler/mtnamespace/controller.go new file mode 100644 index 00000000000..e2b916bfdec --- /dev/null +++ b/pkg/reconciler/mtnamespace/controller.go @@ -0,0 +1,70 @@ +/* +Copyright 2020 The Knative 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 mtnamespace + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/cache" + "knative.dev/pkg/configmap" + "knative.dev/pkg/controller" + + "knative.dev/eventing/pkg/reconciler" + + "knative.dev/eventing/pkg/client/injection/informers/eventing/v1beta1/broker" + "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace" +) + +const ( + // ReconcilerName is the name of the reconciler + ReconcilerName = "Namespace" // TODO: Namespace is not a very good name for this controller. + + // controllerAgentName is the string used by this controller to identify + // itself when creating events. + controllerAgentName = "knative-eventing-namespace-controller" +) + +// NewController initializes the controller and is called by the generated code +// Registers event handlers to enqueue events +func NewController( + ctx context.Context, + cmw configmap.Watcher, +) *controller.Impl { + + namespaceInformer := namespace.Get(ctx) + brokerInformer := broker.Get(ctx) + + r := &Reconciler{ + Base: reconciler.NewBase(ctx, controllerAgentName, cmw), + namespaceLister: namespaceInformer.Lister(), + brokerLister: brokerInformer.Lister(), + } + + impl := controller.NewImpl(r, r.Logger, ReconcilerName) + // TODO: filter label selector: on InjectionEnabledLabels() + + r.Logger.Info("Setting up event handlers") + namespaceInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)) + brokerInformer.Informer().AddEventHandler( + cache.FilteringResourceEventHandler{ + FilterFunc: controller.FilterGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Namespace")), + Handler: controller.HandleAll(impl.EnqueueControllerOf), + }) + + return impl +} diff --git a/pkg/reconciler/mtnamespace/controller_test.go b/pkg/reconciler/mtnamespace/controller_test.go new file mode 100644 index 00000000000..058607cb91e --- /dev/null +++ b/pkg/reconciler/mtnamespace/controller_test.go @@ -0,0 +1,38 @@ +/* +Copyright 2020 The Knative 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 mtnamespace + +import ( + "testing" + + "knative.dev/pkg/configmap" + . "knative.dev/pkg/reconciler/testing" + + // Fake injection informers + _ "knative.dev/eventing/pkg/client/injection/informers/eventing/v1beta1/broker/fake" + _ "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace/fake" +) + +func TestNew(t *testing.T) { + ctx, _ := SetupFakeContext(t) + + c := NewController(ctx, configmap.NewStaticWatcher()) + + if c == nil { + t.Fatal("Expected NewController to return a non-nil value") + } +} diff --git a/pkg/reconciler/mtnamespace/namespace.go b/pkg/reconciler/mtnamespace/namespace.go new file mode 100644 index 00000000000..2bab595c2eb --- /dev/null +++ b/pkg/reconciler/mtnamespace/namespace.go @@ -0,0 +1,145 @@ +/* +Copyright 2020 The Knative 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 mtnamespace + +import ( + "context" + "fmt" + + "k8s.io/client-go/tools/cache" + "knative.dev/eventing/pkg/reconciler/mtnamespace/resources" + + corev1listers "k8s.io/client-go/listers/core/v1" + eventinglisters "knative.dev/eventing/pkg/client/listers/eventing/v1beta1" + + "go.uber.org/zap" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrs "k8s.io/apimachinery/pkg/api/errors" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + configsv1alpha1 "knative.dev/eventing/pkg/apis/configs/v1alpha1" + "knative.dev/eventing/pkg/apis/eventing/v1alpha1" + "knative.dev/eventing/pkg/apis/eventing/v1beta1" + "knative.dev/eventing/pkg/logging" + "knative.dev/eventing/pkg/reconciler" + "knative.dev/pkg/controller" +) + +const ( + namespaceReconciled = "NamespaceReconciled" + namespaceReconcileFailure = "NamespaceReconcileFailure" + + // Name of the corev1.Events emitted from the reconciliation process. + configMapPropagationCreated = "ConfigMapPropagationCreated" + brokerCreated = "BrokerCreated" + serviceAccountCreated = "BrokerServiceAccountCreated" + serviceAccountRBACCreated = "BrokerServiceAccountRBACCreated" + secretCopied = "SecretCopied" + secretCopyFailure = "SecretCopyFailure" +) + +var ( + serviceAccountGVK = corev1.SchemeGroupVersion.WithKind("ServiceAccount") + roleBindingGVK = rbacv1.SchemeGroupVersion.WithKind("RoleBinding") + brokerGVK = v1alpha1.SchemeGroupVersion.WithKind("Broker") + configMapPropagationGVK = configsv1alpha1.SchemeGroupVersion.WithKind("ConfigMapPropagation") +) + +type Reconciler struct { + *reconciler.Base + + // listers index properties about resources + namespaceLister corev1listers.NamespaceLister + brokerLister eventinglisters.BrokerLister +} + +// Check that our Reconciler implements controller.Reconciler +var _ controller.Reconciler = (*Reconciler)(nil) + +// Reconcile compares the actual state with the desired, and attempts to +// converge the two. It then updates the Status block of the Namespace resource +// with the current status of the resource. +func (r *Reconciler) Reconcile(ctx context.Context, key string) error { + _, name, err := cache.SplitMetaNamespaceKey(key) + if err != nil { + logging.FromContext(ctx).Error("invalid resource key") + return nil + } + + // Get the namespace resource with this namespace/name + original, err := r.namespaceLister.Get(name) + if apierrs.IsNotFound(err) { + // The resource may no longer exist, in which case we stop processing. + logging.FromContext(ctx).Error("namespace key in work queue no longer exists") + return nil + } else if err != nil { + return err + } + + if original.Labels[resources.InjectionLabelKey] == resources.InjectionDisabledLabelValue { + logging.FromContext(ctx).Debug("Not reconciling Namespace") + return nil + } + + // Don't modify the informers copy + ns := original.DeepCopy() + + // Reconcile this copy of the Namespace. + reconcileErr := r.reconcile(ctx, ns) + if reconcileErr != nil { + logging.FromContext(ctx).Error("Error reconciling Namespace", zap.Error(reconcileErr)) + r.Recorder.Eventf(ns, corev1.EventTypeWarning, namespaceReconcileFailure, "Failed to reconcile Namespace: %v", reconcileErr) + } else { + logging.FromContext(ctx).Debug("Namespace reconciled") + r.Recorder.Eventf(ns, corev1.EventTypeNormal, namespaceReconciled, "Namespace reconciled: %q", ns.Name) + } + + return reconcileErr +} + +func (r *Reconciler) reconcile(ctx context.Context, ns *corev1.Namespace) error { + if ns.DeletionTimestamp != nil { + return nil + } + + if _, err := r.reconcileBroker(ctx, ns); err != nil { + return fmt.Errorf("broker: %v", err) + } + + return nil +} + +// reconcileBroker reconciles the default Broker for the Namespace 'ns'. +func (r *Reconciler) reconcileBroker(ctx context.Context, ns *corev1.Namespace) (*v1beta1.Broker, error) { + current, err := r.brokerLister.Brokers(ns.Name).Get(resources.DefaultBrokerName) + + // If the resource doesn't exist, we'll create it. + if k8serrors.IsNotFound(err) { + b := resources.MakeBroker(ns) + b, err = r.EventingClientSet.EventingV1beta1().Brokers(ns.Name).Create(b) + if err != nil { + return nil, err + } + r.Recorder.Event(ns, corev1.EventTypeNormal, brokerCreated, + "Default eventing.knative.dev Broker created.") + return b, nil + } else if err != nil { + return nil, err + } + // Don't update anything that is already present. + return current, nil +} diff --git a/pkg/reconciler/mtnamespace/namespace_test.go b/pkg/reconciler/mtnamespace/namespace_test.go new file mode 100644 index 00000000000..161dc0446bc --- /dev/null +++ b/pkg/reconciler/mtnamespace/namespace_test.go @@ -0,0 +1,191 @@ +/* +Copyright 2020 The Knative 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 mtnamespace + +import ( + "context" + "testing" + + "knative.dev/pkg/configmap" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "knative.dev/eventing/pkg/reconciler/mtnamespace/resources" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + clientgotesting "k8s.io/client-go/testing" + "knative.dev/eventing/pkg/apis/eventing/v1alpha1" + eventingv1alpha1 "knative.dev/eventing/pkg/apis/eventing/v1alpha1" + "knative.dev/eventing/pkg/reconciler" + . "knative.dev/eventing/pkg/reconciler/testing" + "knative.dev/pkg/controller" + logtesting "knative.dev/pkg/logging/testing" + . "knative.dev/pkg/reconciler/testing" +) + +const ( + testNS = "test-namespace" + brokerImagePullSecretName = "broker-image-pull-secret" +) + +var ( + brokerGVR = schema.GroupVersionResource{ + Group: "eventing.knative.dev", + Version: "v1alpha1", + Resource: "brokers", + } + + roleBindingGVR = schema.GroupVersionResource{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Resource: "rolebindings", + } + + serviceAccountGVR = schema.GroupVersionResource{ + Version: "v1", + Resource: "serviceaccounts", + } +) + +func init() { + // Add types to scheme + _ = eventingv1alpha1.AddToScheme(scheme.Scheme) +} + +func TestAllCases(t *testing.T) { + // Events + brokerEvent := Eventf(corev1.EventTypeNormal, "BrokerCreated", "Default eventing.knative.dev Broker created.") + nsEvent := Eventf(corev1.EventTypeNormal, "NamespaceReconciled", "Namespace reconciled: \"test-namespace\"") + + // Object + namespace := NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + ) + broker := resources.MakeBroker(namespace) + + table := TableTest{{ + Name: "bad workqueue key", + // Make sure Reconcile handles bad keys. + Key: "too/many/parts", + }, { + Name: "key not found", + // Make sure Reconcile handles good keys that don't exist. + Key: "foo/not-found", + }, { + Name: "Namespace is not labeled", + Objects: []runtime.Object{ + NewNamespace(testNS), + }, + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + WantEvents: []string{ + brokerEvent, + nsEvent, + }, + WantCreates: []runtime.Object{ + broker, + }, + }, { + Name: "Namespace is labeled disabled", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionDisabledLabels())), + }, + Key: testNS, + }, { + Name: "Namespace is deleted no resources", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + WithNamespaceDeleted, + ), + }, + Key: testNS, + WantEvents: []string{ + nsEvent, + }, + }, { + Name: "Namespace enabled", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + ), + }, + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + WantEvents: []string{ + brokerEvent, + nsEvent, + }, + WantCreates: []runtime.Object{ + broker, + }, + }, { + Name: "Namespace enabled, broker exists", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + ), + resources.MakeBroker(NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + )), + }, + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + WantEvents: []string{ + nsEvent, + }, + }, { + Name: "Namespace enabled, broker exists with no label", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionDisabledLabels()), + ), + &v1alpha1.Broker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNS, + Name: resources.DefaultBrokerName, + }, + }, + }, + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + }} + + logger := logtesting.TestLogger(t) + table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { + return &Reconciler{ + Base: reconciler.NewBase(ctx, controllerAgentName, cmw), + namespaceLister: listers.GetNamespaceLister(), + brokerLister: listers.GetV1Beta1BrokerLister(), + } + }, false, logger)) +} + +func createPatch(namespace string, name string) clientgotesting.PatchActionImpl { + patch := clientgotesting.PatchActionImpl{} + patch.Namespace = namespace + patch.Name = name + patch.Patch = []byte(`{"imagePullSecrets":[{"name":"` + brokerImagePullSecretName + `"}]}`) + return patch +} diff --git a/pkg/reconciler/mtnamespace/resources/broker.go b/pkg/reconciler/mtnamespace/resources/broker.go new file mode 100644 index 00000000000..2c0c5fedd88 --- /dev/null +++ b/pkg/reconciler/mtnamespace/resources/broker.go @@ -0,0 +1,46 @@ +/* +Copyright 2020 The Knative 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 resources + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "knative.dev/eventing/pkg/apis/eventing/v1beta1" +) + +const ( + DefaultBrokerName = "default" +) + +// MakeBroker creates a default Broker object for Namespace 'namespace'. +func MakeBroker(ns *corev1.Namespace) *v1beta1.Broker { + return &v1beta1.Broker{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + *metav1.NewControllerRef(ns.GetObjectMeta(), schema.GroupVersionKind{ + Group: corev1.SchemeGroupVersion.Group, + Version: corev1.SchemeGroupVersion.Version, + Kind: "Namespace", + }), + }, + Namespace: ns.Name, + Name: DefaultBrokerName, + Labels: OwnedLabels(), + }, + } +} diff --git a/pkg/reconciler/mtnamespace/resources/labels.go b/pkg/reconciler/mtnamespace/resources/labels.go new file mode 100644 index 00000000000..9a3f964afbc --- /dev/null +++ b/pkg/reconciler/mtnamespace/resources/labels.go @@ -0,0 +1,44 @@ +/* +Copyright 2020 The Knative 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 resources + +const ( + // Label to enable knative-eventing in a namespace. + InjectionLabelKey = "knative-eventing-injection" + InjectionEnabledLabelValue = "enabled" + InjectionDisabledLabelValue = "disabled" + InjectedResourceLabel = "eventing.knative.dev/namespaceInjected" +) + +// OwnedLabels generates the labels present on injected broker resources. +func OwnedLabels() map[string]string { + return map[string]string{ + InjectedResourceLabel: "true", + } +} + +func InjectionEnabledLabels() map[string]string { + return map[string]string{ + InjectionLabelKey: InjectionEnabledLabelValue, + } +} + +func InjectionDisabledLabels() map[string]string { + return map[string]string{ + InjectionLabelKey: InjectionDisabledLabelValue, + } +} From 3c7d2797f38d5ed2d3f80a9b6fb45857b34c69f9 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Thu, 19 Mar 2020 08:25:19 -0700 Subject: [PATCH 2/2] Moar red lines --- pkg/reconciler/mtnamespace/namespace.go | 14 ++------------ pkg/reconciler/mtnamespace/namespace_test.go | 9 --------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/pkg/reconciler/mtnamespace/namespace.go b/pkg/reconciler/mtnamespace/namespace.go index 2bab595c2eb..745202283fb 100644 --- a/pkg/reconciler/mtnamespace/namespace.go +++ b/pkg/reconciler/mtnamespace/namespace.go @@ -28,10 +28,8 @@ import ( "go.uber.org/zap" corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" k8serrors "k8s.io/apimachinery/pkg/api/errors" - configsv1alpha1 "knative.dev/eventing/pkg/apis/configs/v1alpha1" "knative.dev/eventing/pkg/apis/eventing/v1alpha1" "knative.dev/eventing/pkg/apis/eventing/v1beta1" "knative.dev/eventing/pkg/logging" @@ -44,19 +42,11 @@ const ( namespaceReconcileFailure = "NamespaceReconcileFailure" // Name of the corev1.Events emitted from the reconciliation process. - configMapPropagationCreated = "ConfigMapPropagationCreated" - brokerCreated = "BrokerCreated" - serviceAccountCreated = "BrokerServiceAccountCreated" - serviceAccountRBACCreated = "BrokerServiceAccountRBACCreated" - secretCopied = "SecretCopied" - secretCopyFailure = "SecretCopyFailure" + brokerCreated = "BrokerCreated" ) var ( - serviceAccountGVK = corev1.SchemeGroupVersion.WithKind("ServiceAccount") - roleBindingGVK = rbacv1.SchemeGroupVersion.WithKind("RoleBinding") - brokerGVK = v1alpha1.SchemeGroupVersion.WithKind("Broker") - configMapPropagationGVK = configsv1alpha1.SchemeGroupVersion.WithKind("ConfigMapPropagation") + brokerGVK = v1alpha1.SchemeGroupVersion.WithKind("Broker") ) type Reconciler struct { diff --git a/pkg/reconciler/mtnamespace/namespace_test.go b/pkg/reconciler/mtnamespace/namespace_test.go index 161dc0446bc..536f5060bff 100644 --- a/pkg/reconciler/mtnamespace/namespace_test.go +++ b/pkg/reconciler/mtnamespace/namespace_test.go @@ -29,7 +29,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" - clientgotesting "k8s.io/client-go/testing" "knative.dev/eventing/pkg/apis/eventing/v1alpha1" eventingv1alpha1 "knative.dev/eventing/pkg/apis/eventing/v1alpha1" "knative.dev/eventing/pkg/reconciler" @@ -181,11 +180,3 @@ func TestAllCases(t *testing.T) { } }, false, logger)) } - -func createPatch(namespace string, name string) clientgotesting.PatchActionImpl { - patch := clientgotesting.PatchActionImpl{} - patch.Namespace = namespace - patch.Name = name - patch.Patch = []byte(`{"imagePullSecrets":[{"name":"` + brokerImagePullSecretName + `"}]}`) - return patch -}