diff --git a/pkg/reconciler/namespace/namespace.go b/pkg/reconciler/namespace/namespace.go index 11df3a5e2b2..d7a6aa76eaa 100644 --- a/pkg/reconciler/namespace/namespace.go +++ b/pkg/reconciler/namespace/namespace.go @@ -50,7 +50,9 @@ const ( // controllerAgentName is the string used by this controller to identify // itself when creating events. - controllerAgentName = "knative-eventing-namespace-controller" + controllerAgentName = "knative-eventing-namespace-controller" + namespaceReconciled = "NamespaceReconciled" + namespaceReconcileFailure = "NamespaceReconcileFailure" // Name of the corev1.Events emitted from the reconciliation process. brokerCreated = "BrokerCreated" @@ -93,7 +95,6 @@ func NewController( namespaceLister: namespaceInformer.Lister(), } impl := controller.NewImpl(r, r.Logger, ReconcilerName, reconciler.MustNewStatsReporter(ReconcilerName, r.Logger)) - // TODO: filter label selector: on InjectionEnabledLabels() r.Logger.Info("Setting up event handlers") @@ -138,7 +139,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { if original.Labels[resources.InjectionLabelKey] != resources.InjectionEnabledLabelValue { logging.FromContext(ctx).Debug("Not reconciling Namespace") - // TODO: this does not handle cleanup of unwanted brokers in namespace. return nil } @@ -148,20 +148,24 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { // Reconcile this copy of the Namespace and then write back any status updates regardless of // whether the reconcile error out. err = r.reconcile(ctx, ns) + // Requeue if the resource is not ready: if err != nil { logging.FromContext(ctx).Error("Error reconciling Namespace", zap.Error(err)) - } else { - logging.FromContext(ctx).Debug("Namespace reconciled") + r.Recorder.Eventf(ns, corev1.EventTypeWarning, namespaceReconcileFailure, "Failed to reconcile Namespace: %v", err) + return err } - // Requeue if the resource is not ready: - return err + logging.FromContext(ctx).Debug("Namespace reconciled") + r.Recorder.Eventf(ns, corev1.EventTypeNormal, namespaceReconciled, "Namespace reconciled: %q", ns.Name) + + return nil } func (r *Reconciler) reconcile(ctx context.Context, ns *corev1.Namespace) error { if ns.DeletionTimestamp != nil { return nil } + sa, err := r.reconcileBrokerFilterServiceAccount(ctx, ns) if err != nil { logging.FromContext(ctx).Error("Unable to reconcile the Broker Filter Service Account for the namespace", zap.Error(err)) diff --git a/pkg/reconciler/namespace/namespace_test.go b/pkg/reconciler/namespace/namespace_test.go index 3a54fc1d4a8..78691f96a0e 100644 --- a/pkg/reconciler/namespace/namespace_test.go +++ b/pkg/reconciler/namespace/namespace_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/knative/eventing/pkg/apis/eventing/v1alpha1" eventingv1alpha1 "github.com/knative/eventing/pkg/apis/eventing/v1alpha1" fakeclientset "github.com/knative/eventing/pkg/client/clientset/versioned/fake" eventinginformers "github.com/knative/eventing/pkg/client/informers/externalversions" @@ -92,154 +93,138 @@ func TestNew(t *testing.T) { } func TestAllCases(t *testing.T) { - 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, - }, { - 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, - }, { - Name: "Namespace enabled", - Objects: []runtime.Object{ - NewNamespace(testNS, - WithNamespaceLabeled(resources.InjectionEnabledLabels()), - ), - }, - Key: testNS, - SkipNamespaceValidation: true, - WantErr: false, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountCreated", "Service account created for the Broker 'eventing-broker-filter'"), - Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountRBACCreated", "Service account RBAC created for the Broker Filter 'eventing-broker-filter'"), - Eventf(corev1.EventTypeNormal, "BrokerCreated", "Default eventing.knative.dev Broker created."), - }, - WantCreates: []metav1.Object{ - resources.MakeBroker(testNS), - resources.MakeServiceAccount(testNS), - resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), - }, - }, { - Name: "Namespace enabled, broker exists", - Objects: []runtime.Object{ - NewNamespace(testNS, - WithNamespaceLabeled(resources.InjectionEnabledLabels()), - ), - resources.MakeBroker(testNS), - }, - Key: testNS, - SkipNamespaceValidation: true, - WantErr: false, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountCreated", "Service account created for the Broker 'eventing-broker-filter'"), - Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountRBACCreated", "Service account RBAC created for the Broker Filter 'eventing-broker-filter'"), - }, - WantCreates: []metav1.Object{ - resources.MakeServiceAccount(testNS), - resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), - }, + 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), }, - { - Name: "Namespace enabled, service account exists", - Objects: []runtime.Object{ - NewNamespace(testNS, - WithNamespaceLabeled(resources.InjectionEnabledLabels()), - ), - resources.MakeServiceAccount(testNS), - }, - Key: testNS, - SkipNamespaceValidation: true, - WantErr: false, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountRBACCreated", "Service account RBAC created for the Broker Filter 'eventing-broker-filter'"), - Eventf(corev1.EventTypeNormal, "BrokerCreated", "Default eventing.knative.dev Broker created."), - }, - WantCreates: []metav1.Object{ - resources.MakeBroker(testNS), - resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), - }, + Key: testNS, + }, { + Name: "Namespace is labeled disabled", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionDisabledLabels())), }, - { - Name: "Namespace enabled, role binding exists", - Objects: []runtime.Object{ - NewNamespace(testNS, - WithNamespaceLabeled(resources.InjectionEnabledLabels()), - ), - resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), - }, - Key: testNS, - SkipNamespaceValidation: true, - WantErr: false, - WantEvents: []string{ - Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountCreated", "Service account created for the Broker 'eventing-broker-filter'"), - Eventf(corev1.EventTypeNormal, "BrokerCreated", "Default eventing.knative.dev Broker created."), - }, - WantCreates: []metav1.Object{ - resources.MakeBroker(testNS), - resources.MakeServiceAccount(testNS), + Key: testNS, + }, { + Name: "Namespace is deleted no resources", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + WithNamespaceDeleted, + ), + }, + Key: testNS, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "NamespaceReconciled", "Namespace reconciled: \"test-namespace\""), + }, + }, { + Name: "Namespace enabled", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + ), + }, + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountCreated", "Service account created for the Broker 'eventing-broker-filter'"), + Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountRBACCreated", "Service account RBAC created for the Broker Filter 'eventing-broker-filter'"), + Eventf(corev1.EventTypeNormal, "BrokerCreated", "Default eventing.knative.dev Broker created."), + Eventf(corev1.EventTypeNormal, "NamespaceReconciled", "Namespace reconciled: \"test-namespace\""), + }, + WantCreates: []metav1.Object{ + resources.MakeBroker(testNS), + resources.MakeServiceAccount(testNS), + resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), + }, + }, { + Name: "Namespace enabled, broker exists", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + ), + resources.MakeBroker(testNS), + }, + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountCreated", "Service account created for the Broker 'eventing-broker-filter'"), + Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountRBACCreated", "Service account RBAC created for the Broker Filter 'eventing-broker-filter'"), + Eventf(corev1.EventTypeNormal, "NamespaceReconciled", "Namespace reconciled: \"test-namespace\""), + }, + WantCreates: []metav1.Object{ + resources.MakeServiceAccount(testNS), + resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), + }, + }, { + 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, + }, }, }, - //{ TODO: this test should work but there is no clean-up in the controller. - // Name: "Namespace disabled, cleanup", - // Objects: []runtime.Object{ - // NewNamespace(testNS, - // WithNamespaceLabeled(resources.InjectionDisabledLabels()), - // ), - // resources.MakeBroker(testNS), - // resources.MakeServiceAccount(testNS), - // resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), - // }, - // Key: testNS, - // SkipNamespaceValidation: true, - // WantErr: false, - // WantDeletes: []clientgotesting.DeleteActionImpl{{ - // ActionImpl: clientgotesting.ActionImpl{ - // Namespace: testNS, - // Verb: "delete", - // Resource: brokerGVR, - // }, - // Name: resources.DefaultBrokerName, - // }, { - // ActionImpl: clientgotesting.ActionImpl{ - // Namespace: testNS, - // Verb: "delete", - // Resource: roleBindingGVR, - // }, - // Name: resources.RoleBindingName, - // }, { - // ActionImpl: clientgotesting.ActionImpl{ - // Namespace: testNS, - // Verb: "delete", - // Resource: serviceAccountGVR, - // }, - // Name: resources.DefaultBrokerName, - // }}, - //}, - // TODO: we need a existing default un-owned test. + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + }, { + Name: "Namespace enabled, service account exists", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + ), + resources.MakeServiceAccount(testNS), + }, + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountRBACCreated", "Service account RBAC created for the Broker Filter 'eventing-broker-filter'"), + Eventf(corev1.EventTypeNormal, "BrokerCreated", "Default eventing.knative.dev Broker created."), + Eventf(corev1.EventTypeNormal, "NamespaceReconciled", "Namespace reconciled: \"test-namespace\""), + }, + WantCreates: []metav1.Object{ + resources.MakeBroker(testNS), + resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), + }, + }, { + Name: "Namespace enabled, role binding exists", + Objects: []runtime.Object{ + NewNamespace(testNS, + WithNamespaceLabeled(resources.InjectionEnabledLabels()), + ), + resources.MakeRoleBinding(resources.MakeServiceAccount(testNS)), + }, + Key: testNS, + SkipNamespaceValidation: true, + WantErr: false, + WantEvents: []string{ + Eventf(corev1.EventTypeNormal, "BrokerFilterServiceAccountCreated", "Service account created for the Broker 'eventing-broker-filter'"), + Eventf(corev1.EventTypeNormal, "BrokerCreated", "Default eventing.knative.dev Broker created."), + Eventf(corev1.EventTypeNormal, "NamespaceReconciled", "Namespace reconciled: \"test-namespace\""), + }, + WantCreates: []metav1.Object{ + resources.MakeBroker(testNS), + resources.MakeServiceAccount(testNS), + }, + }, + // TODO: we need a existing default un-owned test. } defer logtesting.ClearAll() diff --git a/pkg/reconciler/namespace/resources/labels.go b/pkg/reconciler/namespace/resources/labels.go index 79fa71c82db..0bfeade1497 100644 --- a/pkg/reconciler/namespace/resources/labels.go +++ b/pkg/reconciler/namespace/resources/labels.go @@ -21,12 +21,13 @@ const ( 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{ - "eventing.knative.dev/namespaceInjected": "true", + InjectedResourceLabel: "true", } }