From 4109c4d1206a30fe0070e9747868aba837a3afdb Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Tue, 22 Jan 2019 17:00:48 -0800 Subject: [PATCH 1/9] Adding corev1.Events to in-memory-channel and subscription objects for easier debugging. --- .../in-memory-channel/in-memory-channel.yaml | 11 +++++++++++ .../eventing/inmemory/channel/reconcile.go | 12 ++++++++++-- .../inmemory/clusterchannelprovisioner/reconcile.go | 5 +++++ pkg/controller/eventing/subscription/reconcile.go | 11 +++++++++++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/config/provisioners/in-memory-channel/in-memory-channel.yaml b/config/provisioners/in-memory-channel/in-memory-channel.yaml index 51dc7b1a957..bb153102f9b 100644 --- a/config/provisioners/in-memory-channel/in-memory-channel.yaml +++ b/config/provisioners/in-memory-channel/in-memory-channel.yaml @@ -92,6 +92,17 @@ rules: - watch - create - update + - apiGroups: + - "" # Core API Group. + resources: + - events + verbs: + - get + - list + - watch + - create + - update + - patch --- diff --git a/pkg/controller/eventing/inmemory/channel/reconcile.go b/pkg/controller/eventing/inmemory/channel/reconcile.go index c1c0d3d1cce..4ff0d6cf88a 100644 --- a/pkg/controller/eventing/inmemory/channel/reconcile.go +++ b/pkg/controller/eventing/inmemory/channel/reconcile.go @@ -91,6 +91,7 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err err = r.reconcile(ctx, c) if err != nil { logger.Info("Error reconciling Channel", zap.Error(err)) + r.recorder.Eventf(c, corev1.EventTypeWarning, "ChannelReconcileFailed", "Failed to reconcile Channel: %v", err) // Note that we do not return the error here, because we want to update the Status // regardless of the error. } @@ -124,9 +125,11 @@ func (r *reconciler) reconcile(ctx context.Context, c *eventingv1alpha1.Channel) // We always need to sync the Channel config, so do it first. if err := r.syncChannelConfig(ctx); err != nil { - logger.Info("Error updating syncing the Channel config", zap.Error(err)) + logger.Info("Error syncing the Channel config", zap.Error(err)) + r.recorder.Eventf(c, corev1.EventTypeWarning, "ChannelConfigSyncFailed", "Failed to sync Channel config: %v", err) return err } + r.recorder.Eventf(c, corev1.EventTypeNormal, "ChannelConfigSynced", "Channel config synced") if c.DeletionTimestamp != nil { // K8s garbage collection will delete the K8s service and VirtualService for this channel. @@ -140,17 +143,22 @@ func (r *reconciler) reconcile(ctx context.Context, c *eventingv1alpha1.Channel) svc, err := util.CreateK8sService(ctx, r.client, c) if err != nil { logger.Info("Error creating the Channel's K8s Service", zap.Error(err)) + r.recorder.Eventf(c, corev1.EventTypeWarning, "K8sServiceCreateFailed", "Failed to create Channel's K8s Service: %v", err) return err } c.Status.SetAddress(controller.ServiceHostName(svc.Name, svc.Namespace)) + r.recorder.Eventf(c, corev1.EventTypeNormal, "K8sServiceCreated", "Channel's K8s Service created: %q", svc.Name) - _, err = util.CreateVirtualService(ctx, r.client, c, svc) + virtualService, err := util.CreateVirtualService(ctx, r.client, c, svc) if err != nil { logger.Info("Error creating the Virtual Service for the Channel", zap.Error(err)) + r.recorder.Eventf(c, corev1.EventTypeWarning, "VirtualServiceCreateFailed", "Failed to create Virtual Service for the Channel: %v", err) return err } + r.recorder.Eventf(c, corev1.EventTypeNormal, "VirtualServiceCreated", "Virtual Service for the Channel created: %q", virtualService.Name) c.Status.MarkProvisioned() + r.recorder.Eventf(c, corev1.EventTypeNormal, "ChannelProvisioned", "Channel Provisioned") return nil } diff --git a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go index 5eb68ef5250..7dc098b50d7 100644 --- a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go +++ b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go @@ -96,6 +96,7 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err err = r.reconcile(ctx, ccp) if err != nil { logger.Info("Error reconciling ClusterChannelProvisioner", zap.Error(err)) + r.recorder.Eventf(ccp, corev1.EventTypeWarning, "ReconcileClusterChannelProvisionerFailed", "Failed to reconcile ClusterChannelProvisioner: %v", err) // Note that we do not return the error here, because we want to update the Status // regardless of the error. } @@ -140,8 +141,10 @@ func (r *reconciler) reconcile(ctx context.Context, ccp *eventingv1alpha1.Cluste if err != nil { logger.Info("Error creating the ClusterChannelProvisioner's K8s Service", zap.Error(err)) + r.recorder.Eventf(ccp, corev1.EventTypeWarning, "CreateK8sServiceFailed", "Failed to create ClusterChannelProvisioner's K8s Service: %v", err) return err } + r.recorder.Eventf(ccp, corev1.EventTypeNormal, "K8sServiceCreated", "ClusterChannelProvisioner's K8s Service created: %q", svc.Name) // Check if this ClusterChannelProvisioner is the owner of the K8s service. if !metav1.IsControlledBy(svc, ccp) { @@ -153,10 +156,12 @@ func (r *reconciler) reconcile(ctx context.Context, ccp *eventingv1alpha1.Cluste err = r.deleteOldDispatcherService(ctx, ccp) if err != nil { logger.Info("Error deleting the old ClusterChannelProvisioner's K8s Service", zap.Error(err)) + r.recorder.Eventf(ccp, corev1.EventTypeWarning, "DeleteOldK8sServiceFailed", "Failed to delete the old ClusterChannelProvisioner's K8s Service: %v", err) return err } ccp.Status.MarkReady() + r.recorder.Eventf(ccp, corev1.EventTypeNormal, "ClusterChannelProvisionerReady", "ClusterChannelProvisioner ready: %q", ccp.Name) return nil } diff --git a/pkg/controller/eventing/subscription/reconcile.go b/pkg/controller/eventing/subscription/reconcile.go index d867dfd6987..5a4df3a574b 100644 --- a/pkg/controller/eventing/subscription/reconcile.go +++ b/pkg/controller/eventing/subscription/reconcile.go @@ -67,6 +67,7 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err err = r.reconcile(subscription) if _, updateStatusErr := r.updateStatus(subscription.DeepCopy()); updateStatusErr != nil { glog.Warningf("Failed to update subscription status: %v", updateStatusErr) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, "SubscriptionReconcileFailed", "Failed to reconcile Subscription: %v", err) return reconcile.Result{}, updateStatusErr } @@ -93,8 +94,10 @@ func (r *reconciler) reconcile(subscription *v1alpha1.Subscription) error { err := r.syncPhysicalChannel(subscription, true) if err != nil { glog.Warningf("Failed to sync physical from Channel : %s", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, "PhysicalChannelSyncFailed", "Failed to sync physical Channel: %v", err) return err } + r.recorder.Eventf(subscription, corev1.EventTypeNormal, "PhysicalChannelSynced", "Physical Channel synced") } removeFinalizer(subscription) return nil @@ -104,23 +107,29 @@ func (r *reconciler) reconcile(subscription *v1alpha1.Subscription) error { _, err = r.fetchObjectReference(subscription.Namespace, &subscription.Spec.Channel) if err != nil { glog.Warningf("Failed to validate `channel` exists: %+v, %v", subscription.Spec.Channel, err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, "ObjectReferenceFetchFailed", "Failed to validate Channel exists: %v", err) return err } + r.recorder.Eventf(subscription, corev1.EventTypeNormal, "ObjectReferenceFetched", "Validated Channel exists: %q", subscription.Spec.Channel.Name) if subscriberURI, err := r.resolveSubscriberSpec(subscription.Namespace, subscription.Spec.Subscriber); err != nil { glog.Warningf("Failed to resolve Subscriber %+v : %s", *subscription.Spec.Subscriber, err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, "SubscriberResolveFailed", "Failed to resolve Subscriber: %v", err) return err } else { subscription.Status.PhysicalSubscription.SubscriberURI = subscriberURI glog.Infof("Resolved subscriber to: %q", subscriberURI) + r.recorder.Eventf(subscription, corev1.EventTypeNormal, "SubscriberResolved", "Subscriber resolved to: %q", subscriberURI) } if replyURI, err := r.resolveResult(subscription.Namespace, subscription.Spec.Reply); err != nil { glog.Warningf("Failed to resolve Result %v : %v", subscription.Spec.Reply, err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, "ResultResolveFailed", "Failed to resolve Result: %v", err) return err } else { subscription.Status.PhysicalSubscription.ReplyURI = replyURI glog.Infof("Resolved reply to: %q", replyURI) + r.recorder.Eventf(subscription, corev1.EventTypeNormal, "ResultResolved", "Resolved Response to: %q", replyURI) } // Everything that was supposed to be resolved was, so flip the status bit on that. @@ -131,10 +140,12 @@ func (r *reconciler) reconcile(subscription *v1alpha1.Subscription) error { err = r.syncPhysicalChannel(subscription, false) if err != nil { glog.Warningf("Failed to sync physical Channel : %s", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, "PhysicalChannelSyncFailed", "Failed to sync physical Channel: %v", err) return err } // Everything went well, set the fact that subscriptions have been modified subscription.Status.MarkChannelReady() + r.recorder.Eventf(subscription, corev1.EventTypeNormal, "SubscriptionReady", "Subscription Ready") addFinalizer(subscription) return nil } From 70be180a4b463c4c17738ef13b06d108b6a08672 Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Tue, 22 Jan 2019 23:52:06 -0800 Subject: [PATCH 2/9] Adding mock eventRecorder --- .../inmemory/channel/reconcile_test.go | 6 +- .../eventing/subscription/reconcile_test.go | 8 ++- pkg/controller/testing/mock_event_recorder.go | 57 +++++++++++++++++++ pkg/controller/testing/table.go | 34 ++++++++++- 4 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 pkg/controller/testing/mock_event_recorder.go diff --git a/pkg/controller/eventing/inmemory/channel/reconcile_test.go b/pkg/controller/eventing/inmemory/channel/reconcile_test.go index 27bb7298e23..ffa6cd6d114 100644 --- a/pkg/controller/eventing/inmemory/channel/reconcile_test.go +++ b/pkg/controller/eventing/inmemory/channel/reconcile_test.go @@ -38,7 +38,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -466,13 +465,14 @@ func TestReconcile(t *testing.T) { }, }, } - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) + for _, tc := range testCases { configMapKey := types.NamespacedName{ Namespace: cmNamespace, Name: cmName, } c := tc.GetClient() + recorder := tc.GetEventRecorder() r := &reconciler{ client: c, recorder: recorder, @@ -483,7 +483,7 @@ func TestReconcile(t *testing.T) { tc.ReconcileKey = fmt.Sprintf("/%s", cName) } tc.IgnoreTimes = true - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/controller/eventing/subscription/reconcile_test.go b/pkg/controller/eventing/subscription/reconcile_test.go index 4b7383e7583..68d9949218b 100644 --- a/pkg/controller/eventing/subscription/reconcile_test.go +++ b/pkg/controller/eventing/subscription/reconcile_test.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -988,6 +987,9 @@ var testCases = []controllertesting.TestCase{ // out. //getChannelWithOtherSubscription(), }, + WantEvent: []corev1.Event{ + {Reason: "PhysicalChannelSyncFailed", Type: corev1.EventTypeWarning,}, + }, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -1019,11 +1021,11 @@ var testCases = []controllertesting.TestCase{ } func TestAllCases(t *testing.T) { - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) for _, tc := range testCases { c := tc.GetClient() dc := tc.GetDynamicClient() + recorder := tc.GetEventRecorder() r := &reconciler{ client: c, @@ -1033,7 +1035,7 @@ func TestAllCases(t *testing.T) { } tc.ReconcileKey = fmt.Sprintf("%s/%s", testNS, subscriptionName) tc.IgnoreTimes = true - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/controller/testing/mock_event_recorder.go b/pkg/controller/testing/mock_event_recorder.go new file mode 100644 index 00000000000..ae99207ee82 --- /dev/null +++ b/pkg/controller/testing/mock_event_recorder.go @@ -0,0 +1,57 @@ +/* +Copyright 2018 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 testing + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" +) + +var _ record.EventRecorder = (*MockEventRecorder)(nil) + +// mockEventRecorder is a recorder.EventRecorder that allows to save v1 Events emitted. +type MockEventRecorder struct { + events []corev1.Event +} + +func NewEventRecorder() *MockEventRecorder { + return &MockEventRecorder{} +} + +func (m *MockEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { + // Create an event with only the information that we need to verify in the test. + // Should include more information if we want to check other fields + event := corev1.Event{ + Reason: reason, + Type: eventtype, + } + m.events = append(m.events, event) +} + +func (m *MockEventRecorder) Event(object runtime.Object, eventtype, reason, message string) { + panic("not implemented") +} + +func (m *MockEventRecorder) PastEventf(object runtime.Object, timestamp metav1.Time, eventtype, reason, messageFmt string, args ...interface{}) { + panic("not implemented") +} + +func (m *MockEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { + panic("not implemented") +} diff --git a/pkg/controller/testing/table.go b/pkg/controller/testing/table.go index a7691c59d3d..1b22d65552a 100644 --- a/pkg/controller/testing/table.go +++ b/pkg/controller/testing/table.go @@ -19,6 +19,8 @@ package testing import ( "context" "fmt" + corev1 "k8s.io/api/core/v1" + "reflect" "strings" "testing" @@ -70,6 +72,10 @@ type TestCase struct { // after reconciliation completes. WantAbsent []runtime.Object + // WantEvent holds the list of events expected to exist after + // reconciliation completes. + WantEvent []corev1.Event + // Mocks that tamper with the client's responses. Mocks Mocks @@ -93,7 +99,7 @@ type TestCase struct { } // Runner returns a testing func that can be passed to t.Run. -func (tc *TestCase) Runner(t *testing.T, r reconcile.Reconciler, c *MockClient) func(t *testing.T) { +func (tc *TestCase) Runner(t *testing.T, r reconcile.Reconciler, c *MockClient, recorder *MockEventRecorder) func(t *testing.T) { return func(t *testing.T) { result, recErr := tc.Reconcile(r) @@ -116,6 +122,10 @@ func (tc *TestCase) Runner(t *testing.T, r reconcile.Reconciler, c *MockClient) t.Error(err) } + if err := tc.VerifyWantEvent(recorder); err != nil { + t.Error(err) + } + for _, av := range tc.AdditionalVerification { av(t, tc) } @@ -137,6 +147,11 @@ func (tc *TestCase) GetClient() *MockClient { return NewMockClient(innerClient, tc.Mocks) } +// GetEventRecorder returns the mockEventRecorder to use for this test case. +func (tc *TestCase) GetEventRecorder() *MockEventRecorder { + return NewEventRecorder() +} + // Reconcile calls the given reconciler's Reconcile() function with the test // case's reconcile request. func (tc *TestCase) Reconcile(r reconcile.Reconciler) (reconcile.Result, error) { @@ -265,6 +280,23 @@ func (tc *TestCase) VerifyWantAbsent(c client.Client) error { return nil } +// VerifyWantEvent verifies that the eventRecorder does contain the events +// expected to be emitted after reconciliation. +func (tc *TestCase) VerifyWantEvent(eventRecorder *MockEventRecorder) error { + if !reflect.DeepEqual(tc.WantEvent, eventRecorder.events) { + return fmt.Errorf("expected %s, got %s", getEventsAsString(tc.WantEvent), getEventsAsString(eventRecorder.events)) + } + return nil +} + +func getEventsAsString(events []corev1.Event) []string { + eventsAsString := make([]string, 0) + for _, event := range events { + eventsAsString = append(eventsAsString, fmt.Sprintf("(%s,%s)", event.Reason, event.Type)) + } + return eventsAsString +} + func buildAllObjects(objs []runtime.Object) []runtime.Object { builtObjs := []runtime.Object{} for _, obj := range objs { From 822b2785085c0284cff2b371b8667f2e733e2139 Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Wed, 23 Jan 2019 11:50:47 -0800 Subject: [PATCH 3/9] Updating test cases. Removing unnecessary events on the success condition --- .../eventing/inmemory/channel/reconcile.go | 26 ++++-- .../inmemory/channel/reconcile_test.go | 55 +++++++++++ .../clusterchannelprovisioner/reconcile.go | 19 +++- .../reconcile_test.go | 42 ++++++++- .../eventing/subscription/reconcile.go | 34 ++++--- .../eventing/subscription/reconcile_test.go | 93 ++++++++++++++++--- pkg/controller/testing/mock_event_recorder.go | 28 +++--- 7 files changed, 242 insertions(+), 55 deletions(-) diff --git a/pkg/controller/eventing/inmemory/channel/reconcile.go b/pkg/controller/eventing/inmemory/channel/reconcile.go index 4ff0d6cf88a..fe0c0521aea 100644 --- a/pkg/controller/eventing/inmemory/channel/reconcile.go +++ b/pkg/controller/eventing/inmemory/channel/reconcile.go @@ -39,6 +39,14 @@ import ( const ( finalizerName = controllerAgentName + + // Name of the corev1.Events emitted from the reconciliation process + channelReconciled = "ChannelReconciled" + channelReconcileFailed = "ChannelReconcileFailed" + channelUpdateStatusFailed = "ChannelUpdateStatusFailed" + channelConfigSyncFailed = "ChannelConfigSyncFailed" + k8sServiceCreateFailed = "K8sServiceCreateFailed" + virtualServiceCreateFailed = "VirtualServiceCreateFailed" ) type reconciler struct { @@ -91,13 +99,17 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err err = r.reconcile(ctx, c) if err != nil { logger.Info("Error reconciling Channel", zap.Error(err)) - r.recorder.Eventf(c, corev1.EventTypeWarning, "ChannelReconcileFailed", "Failed to reconcile Channel: %v", err) + r.recorder.Eventf(c, corev1.EventTypeWarning, channelReconcileFailed, "Failed to reconcile Channel: %v", err) // Note that we do not return the error here, because we want to update the Status // regardless of the error. + } else { + logger.Info("Channel reconciled") + r.recorder.Eventf(c, corev1.EventTypeNormal, channelReconciled, "Channel reconciled: %q", c.Name) } if updateStatusErr := util.UpdateChannel(ctx, r.client, c); updateStatusErr != nil { logger.Info("Error updating Channel Status", zap.Error(updateStatusErr)) + r.recorder.Eventf(c, corev1.EventTypeWarning, channelUpdateStatusFailed, "Failed to update Channel's status: %v", err) return reconcile.Result{}, updateStatusErr } @@ -126,10 +138,9 @@ func (r *reconciler) reconcile(ctx context.Context, c *eventingv1alpha1.Channel) // We always need to sync the Channel config, so do it first. if err := r.syncChannelConfig(ctx); err != nil { logger.Info("Error syncing the Channel config", zap.Error(err)) - r.recorder.Eventf(c, corev1.EventTypeWarning, "ChannelConfigSyncFailed", "Failed to sync Channel config: %v", err) + r.recorder.Eventf(c, corev1.EventTypeWarning, channelConfigSyncFailed, "Failed to sync Channel config: %v", err) return err } - r.recorder.Eventf(c, corev1.EventTypeNormal, "ChannelConfigSynced", "Channel config synced") if c.DeletionTimestamp != nil { // K8s garbage collection will delete the K8s service and VirtualService for this channel. @@ -143,22 +154,19 @@ func (r *reconciler) reconcile(ctx context.Context, c *eventingv1alpha1.Channel) svc, err := util.CreateK8sService(ctx, r.client, c) if err != nil { logger.Info("Error creating the Channel's K8s Service", zap.Error(err)) - r.recorder.Eventf(c, corev1.EventTypeWarning, "K8sServiceCreateFailed", "Failed to create Channel's K8s Service: %v", err) + r.recorder.Eventf(c, corev1.EventTypeWarning, k8sServiceCreateFailed, "Failed to create Channel's K8s Service: %v", err) return err } c.Status.SetAddress(controller.ServiceHostName(svc.Name, svc.Namespace)) - r.recorder.Eventf(c, corev1.EventTypeNormal, "K8sServiceCreated", "Channel's K8s Service created: %q", svc.Name) - virtualService, err := util.CreateVirtualService(ctx, r.client, c, svc) + _, err = util.CreateVirtualService(ctx, r.client, c, svc) if err != nil { logger.Info("Error creating the Virtual Service for the Channel", zap.Error(err)) - r.recorder.Eventf(c, corev1.EventTypeWarning, "VirtualServiceCreateFailed", "Failed to create Virtual Service for the Channel: %v", err) + r.recorder.Eventf(c, corev1.EventTypeWarning, virtualServiceCreateFailed, "Failed to create Virtual Service for the Channel: %v", err) return err } - r.recorder.Eventf(c, corev1.EventTypeNormal, "VirtualServiceCreated", "Virtual Service for the Channel created: %q", virtualService.Name) c.Status.MarkProvisioned() - r.recorder.Eventf(c, corev1.EventTypeNormal, "ChannelProvisioned", "Channel Provisioned") return nil } diff --git a/pkg/controller/eventing/inmemory/channel/reconcile_test.go b/pkg/controller/eventing/inmemory/channel/reconcile_test.go index ffa6cd6d114..e592c4da857 100644 --- a/pkg/controller/eventing/inmemory/channel/reconcile_test.go +++ b/pkg/controller/eventing/inmemory/channel/reconcile_test.go @@ -177,6 +177,16 @@ var ( }, }, } + + // map of events to set test cases' expectations easier + events = map[string]corev1.Event{ + channelReconcileFailed: {Reason: channelReconcileFailed, Type: corev1.EventTypeWarning}, + channelReconciled: {Reason: channelReconciled, Type: corev1.EventTypeNormal}, + channelUpdateStatusFailed: {Reason: channelUpdateStatusFailed, Type: corev1.EventTypeWarning}, + channelConfigSyncFailed: {Reason: channelConfigSyncFailed, Type: corev1.EventTypeWarning}, + k8sServiceCreateFailed: {Reason: k8sServiceCreateFailed, Type: corev1.EventTypeWarning}, + virtualServiceCreateFailed: {Reason: virtualServiceCreateFailed, Type: corev1.EventTypeWarning}, + } ) func init() { @@ -251,6 +261,9 @@ func TestReconcile(t *testing.T) { makeDeletingChannel(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[channelConfigSyncFailed], events[channelReconcileFailed], + }, }, { Name: "Channel deleted - finalizer removed", @@ -260,6 +273,9 @@ func TestReconcile(t *testing.T) { WantPresent: []runtime.Object{ makeDeletingChannelWithoutFinalizer(), }, + WantEvent: []corev1.Event{ + events[channelReconciled], + }, }, { Name: "Channel config sync fails - can't list Channels", @@ -270,6 +286,9 @@ func TestReconcile(t *testing.T) { MockLists: errorListingChannels(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[channelConfigSyncFailed], events[channelReconcileFailed], + }, }, { Name: "Channel config sync fails - can't get ConfigMap", @@ -280,6 +299,9 @@ func TestReconcile(t *testing.T) { MockGets: errorGettingConfigMap(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[channelConfigSyncFailed], events[channelReconcileFailed], + }, }, { Name: "Channel config sync fails - can't create ConfigMap", @@ -290,6 +312,9 @@ func TestReconcile(t *testing.T) { MockCreates: errorCreatingConfigMap(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[channelConfigSyncFailed], events[channelReconcileFailed], + }, }, { Name: "Channel config sync fails - can't update ConfigMap", @@ -301,6 +326,9 @@ func TestReconcile(t *testing.T) { MockUpdates: errorUpdatingConfigMap(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[channelConfigSyncFailed], events[channelReconcileFailed], + }, }, { Name: "K8s service get fails", @@ -315,6 +343,9 @@ func TestReconcile(t *testing.T) { makeChannelWithFinalizer(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[k8sServiceCreateFailed], events[channelReconcileFailed], + }, }, { Name: "K8s service creation fails", @@ -330,6 +361,9 @@ func TestReconcile(t *testing.T) { makeChannelWithFinalizer(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[k8sServiceCreateFailed], events[channelReconcileFailed], + }, }, { Name: "Virtual service get fails", @@ -348,6 +382,9 @@ func TestReconcile(t *testing.T) { makeChannelWithFinalizerAndAddress(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[virtualServiceCreateFailed], events[channelReconcileFailed], + }, }, { Name: "Virtual service creation fails", @@ -365,6 +402,9 @@ func TestReconcile(t *testing.T) { makeChannelWithFinalizerAndAddress(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[virtualServiceCreateFailed], events[channelReconcileFailed], + }, }, { Name: "Channel get for update fails", @@ -378,6 +418,9 @@ func TestReconcile(t *testing.T) { MockGets: errorOnSecondChannelGet(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[channelReconciled], events[channelUpdateStatusFailed], + }, }, { Name: "Channel update fails", @@ -391,6 +434,9 @@ func TestReconcile(t *testing.T) { MockUpdates: errorUpdatingChannel(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[channelReconciled], events[channelUpdateStatusFailed], + }, }, { Name: "Channel status update fails", InitialState: []runtime.Object{ @@ -403,6 +449,9 @@ func TestReconcile(t *testing.T) { MockStatusUpdates: errorUpdatingChannelStatus(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[channelReconciled], events[channelUpdateStatusFailed], + }, }, { Name: "Channel reconcile successful - Channel list follows pagination", InitialState: []runtime.Object{ @@ -423,6 +472,9 @@ func TestReconcile(t *testing.T) { makeVirtualService(), makeConfigMapWithVerifyConfigMapData(), }, + WantEvent: []corev1.Event{ + events[channelReconciled], + }, }, { Name: "Channel reconcile successful - Channel has no subscribers", @@ -463,6 +515,9 @@ func TestReconcile(t *testing.T) { makeVirtualService(), makeConfigMapWithVerifyConfigMapData(), }, + WantEvent: []corev1.Event{ + events[channelReconciled], + }, }, } diff --git a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go index 7dc098b50d7..6442e4f45c0 100644 --- a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go +++ b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go @@ -40,6 +40,13 @@ const ( // Channel is the name of the Channel resource in eventing.knative.dev/v1alpha1. Channel = "Channel" + + // Name of the corev1.Events emitted from the reconciliation process + ccpReconciled = "CcpReconciled" + ccpReconcileFailed = "CcpReconcileFailed" + ccpUpdateStatusFailed = "CcpUpdateStatusFailed" + k8sServiceCreateFailed = "K8sServiceCreateFailed" + k8sServiceDeleteFailed = "K8sServiceDeleteFailed" ) type reconciler struct { @@ -96,13 +103,17 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err err = r.reconcile(ctx, ccp) if err != nil { logger.Info("Error reconciling ClusterChannelProvisioner", zap.Error(err)) - r.recorder.Eventf(ccp, corev1.EventTypeWarning, "ReconcileClusterChannelProvisionerFailed", "Failed to reconcile ClusterChannelProvisioner: %v", err) + r.recorder.Eventf(ccp, corev1.EventTypeWarning, ccpReconcileFailed, "Failed to reconcile ClusterChannelProvisioner: %v", err) // Note that we do not return the error here, because we want to update the Status // regardless of the error. + } else { + logger.Info("ClusterChannelProvisioner reconciled") + r.recorder.Eventf(ccp, corev1.EventTypeNormal, ccpReconciled, "ClusterChannelProvisioner reconciled: %q", ccp.Name) } if updateStatusErr := util.UpdateClusterChannelProvisionerStatus(ctx, r.client, ccp); updateStatusErr != nil { logger.Info("Error updating ClusterChannelProvisioner Status", zap.Error(updateStatusErr)) + r.recorder.Eventf(ccp, corev1.EventTypeWarning, ccpUpdateStatusFailed, "Failed to update ClusterChannelProvisioner's status: %v", err) return reconcile.Result{}, updateStatusErr } @@ -141,10 +152,9 @@ func (r *reconciler) reconcile(ctx context.Context, ccp *eventingv1alpha1.Cluste if err != nil { logger.Info("Error creating the ClusterChannelProvisioner's K8s Service", zap.Error(err)) - r.recorder.Eventf(ccp, corev1.EventTypeWarning, "CreateK8sServiceFailed", "Failed to create ClusterChannelProvisioner's K8s Service: %v", err) + r.recorder.Eventf(ccp, corev1.EventTypeWarning, k8sServiceCreateFailed, "Failed to create ClusterChannelProvisioner's K8s Service: %v", err) return err } - r.recorder.Eventf(ccp, corev1.EventTypeNormal, "K8sServiceCreated", "ClusterChannelProvisioner's K8s Service created: %q", svc.Name) // Check if this ClusterChannelProvisioner is the owner of the K8s service. if !metav1.IsControlledBy(svc, ccp) { @@ -156,12 +166,11 @@ func (r *reconciler) reconcile(ctx context.Context, ccp *eventingv1alpha1.Cluste err = r.deleteOldDispatcherService(ctx, ccp) if err != nil { logger.Info("Error deleting the old ClusterChannelProvisioner's K8s Service", zap.Error(err)) - r.recorder.Eventf(ccp, corev1.EventTypeWarning, "DeleteOldK8sServiceFailed", "Failed to delete the old ClusterChannelProvisioner's K8s Service: %v", err) + r.recorder.Eventf(ccp, corev1.EventTypeWarning, k8sServiceDeleteFailed, "Failed to delete the old ClusterChannelProvisioner's K8s Service: %v", err) return err } ccp.Status.MarkReady() - r.recorder.Eventf(ccp, corev1.EventTypeNormal, "ClusterChannelProvisionerReady", "ClusterChannelProvisioner ready: %q", ccp.Name) return nil } diff --git a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile_test.go b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile_test.go index 8c299ffb288..cbd4be6d720 100644 --- a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile_test.go +++ b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile_test.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -51,6 +50,15 @@ var ( deletionTime = metav1.Now().Rfc3339Copy() truePointer = true + + // map of events to set test cases' expectations easier + events = map[string]corev1.Event{ + ccpReconciled: {Reason: ccpReconciled, Type: corev1.EventTypeNormal}, + ccpReconcileFailed: {Reason: ccpReconcileFailed, Type: corev1.EventTypeWarning}, + ccpUpdateStatusFailed: {Reason: ccpUpdateStatusFailed, Type: corev1.EventTypeWarning}, + k8sServiceCreateFailed: {Reason: k8sServiceCreateFailed, Type: corev1.EventTypeWarning}, + k8sServiceDeleteFailed: {Reason: k8sServiceDeleteFailed, Type: corev1.EventTypeWarning}, + } ) func init() { @@ -156,6 +164,9 @@ func TestReconcile(t *testing.T) { InitialState: []runtime.Object{ makeDeletingClusterChannelProvisioner(), }, + WantEvent: []corev1.Event{ + events[ccpReconciled], + }, }, { Name: "Create dispatcher fails", @@ -168,6 +179,9 @@ func TestReconcile(t *testing.T) { }, }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[k8sServiceCreateFailed], events[ccpReconcileFailed], + }, }, { Name: "Create dispatcher - already exists", @@ -178,6 +192,9 @@ func TestReconcile(t *testing.T) { WantPresent: []runtime.Object{ makeReadyClusterChannelProvisioner(), }, + WantEvent: []corev1.Event{ + events[ccpReconciled], + }, }, { Name: "Delete old dispatcher", @@ -192,6 +209,9 @@ func TestReconcile(t *testing.T) { WantAbsent: []runtime.Object{ makeOldK8sService(), }, + WantEvent: []corev1.Event{ + events[ccpReconciled], + }, }, { Name: "Create dispatcher - not owned by CCP", @@ -202,6 +222,9 @@ func TestReconcile(t *testing.T) { WantPresent: []runtime.Object{ makeReadyClusterChannelProvisioner(), }, + WantEvent: []corev1.Event{ + events[ccpReconciled], + }, }, { Name: "Create dispatcher succeeds", @@ -212,6 +235,9 @@ func TestReconcile(t *testing.T) { makeReadyClusterChannelProvisioner(), makeK8sService(), }, + WantEvent: []corev1.Event{ + events[ccpReconciled], + }, }, { Name: "Create dispatcher succeeds - request is namespace-scoped", @@ -223,6 +249,9 @@ func TestReconcile(t *testing.T) { makeK8sService(), }, ReconcileKey: fmt.Sprintf("%s/%s", testNS, Name), + WantEvent: []corev1.Event{ + events[ccpReconciled], + }, }, { Name: "Error getting CCP for updating Status", @@ -235,6 +264,9 @@ func TestReconcile(t *testing.T) { MockGets: oneSuccessfulClusterChannelProvisionerGet(), }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[ccpReconciled], events[ccpUpdateStatusFailed], + }, }, { Name: "Error updating Status", @@ -249,11 +281,15 @@ func TestReconcile(t *testing.T) { }, }, WantErrMsg: testErrorMessage, + WantEvent: []corev1.Event{ + events[ccpReconciled], events[ccpUpdateStatusFailed], + }, }, } - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) + for _, tc := range testCases { c := tc.GetClient() + recorder := tc.GetEventRecorder() r := &reconciler{ client: c, recorder: recorder, @@ -263,7 +299,7 @@ func TestReconcile(t *testing.T) { tc.ReconcileKey = fmt.Sprintf("/%s", Name) } tc.IgnoreTimes = true - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/controller/eventing/subscription/reconcile.go b/pkg/controller/eventing/subscription/reconcile.go index 5a4df3a574b..ec449291d0d 100644 --- a/pkg/controller/eventing/subscription/reconcile.go +++ b/pkg/controller/eventing/subscription/reconcile.go @@ -42,6 +42,15 @@ import ( const ( finalizerName = controllerAgentName + + // Name of the corev1.Events emitted from the reconciliation process + subscriptionReconciled = "SubscriptionReconciled" + subscriptionReconcileFailed = "SubscriptionReconcileFailed" + subscriptionUpdateStatusFailed = "SubscriptionUpdateStatusFailed" + physicalChannelSyncFailed = "PhysicalChannelSyncFailed" + objectReferenceFetchFailed = "ObjectReferenceFetchFailed" + subscriberResolveFailed = "SubscriberResolveFailed" + resultResolveFailed = "ResultResolveFailed" ) // Reconcile compares the actual state with the desired, and attempts to @@ -65,9 +74,17 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err // Reconcile this copy of the Subscription and then write back any status // updates regardless of whether the reconcile error out. err = r.reconcile(subscription) + if err != nil { + glog.Warningf("Error reconciling Subscription: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, subscriptionReconcileFailed, "Failed to reconcile Subscription: %v", err) + } else { + glog.Info("Subscription reconciled") + r.recorder.Eventf(subscription, corev1.EventTypeNormal, subscriptionReconciled, "Subscription reconciled: %q", subscription.Name) + } + if _, updateStatusErr := r.updateStatus(subscription.DeepCopy()); updateStatusErr != nil { glog.Warningf("Failed to update subscription status: %v", updateStatusErr) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, "SubscriptionReconcileFailed", "Failed to reconcile Subscription: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, subscriptionUpdateStatusFailed, "Failed to update Subscription's status: %v", err) return reconcile.Result{}, updateStatusErr } @@ -94,10 +111,9 @@ func (r *reconciler) reconcile(subscription *v1alpha1.Subscription) error { err := r.syncPhysicalChannel(subscription, true) if err != nil { glog.Warningf("Failed to sync physical from Channel : %s", err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, "PhysicalChannelSyncFailed", "Failed to sync physical Channel: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, physicalChannelSyncFailed, "Failed to sync physical Channel: %v", err) return err } - r.recorder.Eventf(subscription, corev1.EventTypeNormal, "PhysicalChannelSynced", "Physical Channel synced") } removeFinalizer(subscription) return nil @@ -107,29 +123,26 @@ func (r *reconciler) reconcile(subscription *v1alpha1.Subscription) error { _, err = r.fetchObjectReference(subscription.Namespace, &subscription.Spec.Channel) if err != nil { glog.Warningf("Failed to validate `channel` exists: %+v, %v", subscription.Spec.Channel, err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, "ObjectReferenceFetchFailed", "Failed to validate Channel exists: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, objectReferenceFetchFailed, "Failed to validate Channel exists: %v", err) return err } - r.recorder.Eventf(subscription, corev1.EventTypeNormal, "ObjectReferenceFetched", "Validated Channel exists: %q", subscription.Spec.Channel.Name) if subscriberURI, err := r.resolveSubscriberSpec(subscription.Namespace, subscription.Spec.Subscriber); err != nil { glog.Warningf("Failed to resolve Subscriber %+v : %s", *subscription.Spec.Subscriber, err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, "SubscriberResolveFailed", "Failed to resolve Subscriber: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, subscriberResolveFailed, "Failed to resolve Subscriber: %v", err) return err } else { subscription.Status.PhysicalSubscription.SubscriberURI = subscriberURI glog.Infof("Resolved subscriber to: %q", subscriberURI) - r.recorder.Eventf(subscription, corev1.EventTypeNormal, "SubscriberResolved", "Subscriber resolved to: %q", subscriberURI) } if replyURI, err := r.resolveResult(subscription.Namespace, subscription.Spec.Reply); err != nil { glog.Warningf("Failed to resolve Result %v : %v", subscription.Spec.Reply, err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, "ResultResolveFailed", "Failed to resolve Result: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, resultResolveFailed, "Failed to resolve Result: %v", err) return err } else { subscription.Status.PhysicalSubscription.ReplyURI = replyURI glog.Infof("Resolved reply to: %q", replyURI) - r.recorder.Eventf(subscription, corev1.EventTypeNormal, "ResultResolved", "Resolved Response to: %q", replyURI) } // Everything that was supposed to be resolved was, so flip the status bit on that. @@ -140,12 +153,11 @@ func (r *reconciler) reconcile(subscription *v1alpha1.Subscription) error { err = r.syncPhysicalChannel(subscription, false) if err != nil { glog.Warningf("Failed to sync physical Channel : %s", err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, "PhysicalChannelSyncFailed", "Failed to sync physical Channel: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, physicalChannelSyncFailed, "Failed to sync physical Channel: %v", err) return err } // Everything went well, set the fact that subscriptions have been modified subscription.Status.MarkChannelReady() - r.recorder.Eventf(subscription, corev1.EventTypeNormal, "SubscriptionReady", "Subscription Ready") addFinalizer(subscription) return nil } diff --git a/pkg/controller/eventing/subscription/reconcile_test.go b/pkg/controller/eventing/subscription/reconcile_test.go index 68d9949218b..3f29c7b9fd8 100644 --- a/pkg/controller/eventing/subscription/reconcile_test.go +++ b/pkg/controller/eventing/subscription/reconcile_test.go @@ -40,6 +40,17 @@ var ( // deletionTime is used when objects are marked as deleted. Rfc3339Copy() // truncates to seconds to match the loss of precision during serialization. deletionTime = metav1.Now().Rfc3339Copy() + + // map of events to set test cases' expectations easier + events = map[string]corev1.Event{ + subscriptionReconciled: {Reason: subscriptionReconciled, Type: corev1.EventTypeNormal}, + subscriptionReconcileFailed: {Reason: subscriptionReconcileFailed, Type: corev1.EventTypeWarning}, + subscriptionUpdateStatusFailed: {Reason: subscriptionUpdateStatusFailed, Type: corev1.EventTypeWarning}, + physicalChannelSyncFailed: {Reason: physicalChannelSyncFailed, Type: corev1.EventTypeWarning}, + objectReferenceFetchFailed: {Reason: objectReferenceFetchFailed, Type: corev1.EventTypeWarning}, + subscriberResolveFailed: {Reason: subscriberResolveFailed, Type: corev1.EventTypeWarning}, + resultResolveFailed: {Reason: resultResolveFailed, Type: corev1.EventTypeWarning}, + } ) const ( @@ -77,6 +88,9 @@ var testCases = []controllertesting.TestCase{ Subscription(), }, WantErrMsg: `channels.eventing.knative.dev "fromchannel" not found`, + WantEvent: []corev1.Event{ + events[objectReferenceFetchFailed], events[subscriptionReconcileFailed], + }, }, { Name: "subscription, but From is not subscribable", InitialState: []runtime.Object{ @@ -87,7 +101,10 @@ var testCases = []controllertesting.TestCase{ // failure for now, until upstream is fixed. It should actually fail saying that there is no // Spec.Subscribers field. WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -146,6 +163,9 @@ var testCases = []controllertesting.TestCase{ WantPresent: []runtime.Object{ Subscription().UnknownConditions(), }, + WantEvent: []corev1.Event{ + events[subscriberResolveFailed], events[subscriptionReconcileFailed], + }, Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel @@ -172,7 +192,10 @@ var testCases = []controllertesting.TestCase{ Subscription().UnknownConditions(), }, WantErrMsg: "status does not contain address", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[subscriberResolveFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -212,7 +235,10 @@ var testCases = []controllertesting.TestCase{ Subscription().UnknownConditions().PhysicalSubscriber(targetDNS), }, WantErrMsg: `channels.eventing.knative.dev "resultchannel" not found`, - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[resultResolveFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -257,6 +283,9 @@ var testCases = []controllertesting.TestCase{ // Subscription().ReferencesResolved(), Subscription().UnknownConditions().PhysicalSubscriber(targetDNS), }, + WantEvent: []corev1.Event{ + events[resultResolveFailed], events[subscriptionReconcileFailed], + }, Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel @@ -317,7 +346,10 @@ var testCases = []controllertesting.TestCase{ Subscription().ReferencesResolved().PhysicalSubscriber(targetDNS).Reply(), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -382,7 +414,10 @@ var testCases = []controllertesting.TestCase{ Subscription().NilReply().ReferencesResolved().PhysicalSubscriber(targetDNS), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -428,7 +463,10 @@ var testCases = []controllertesting.TestCase{ Subscription().ReferencesResolved().PhysicalSubscriber(targetDNS).EmptyNonNilReply(), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -474,7 +512,10 @@ var testCases = []controllertesting.TestCase{ Subscription().ReferencesResolved().PhysicalSubscriber(targetDNS).EmptyNonNilReply(), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -519,7 +560,10 @@ var testCases = []controllertesting.TestCase{ Subscription().NilSubscriber().ReferencesResolved().Reply(), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -569,7 +613,10 @@ var testCases = []controllertesting.TestCase{ Subscription().NilReply().ReferencesResolved().PhysicalSubscriber(targetDNS), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -613,7 +660,10 @@ var testCases = []controllertesting.TestCase{ Subscription().NilSubscriber().ReferencesResolved().Reply(), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -662,7 +712,10 @@ var testCases = []controllertesting.TestCase{ Subscription().EmptyNonNilSubscriber().ReferencesResolved().Reply(), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -708,7 +761,10 @@ var testCases = []controllertesting.TestCase{ Subscription().ToK8sService().UnknownConditions(), }, WantErrMsg: "services \"testk8sservice\" not found", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[subscriberResolveFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -739,7 +795,10 @@ var testCases = []controllertesting.TestCase{ Subscription().ToK8sService().ReferencesResolved().PhysicalSubscriber(k8sServiceDNS).Reply(), }, WantErrMsg: "invalid JSON document", - Scheme: scheme.Scheme, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, + Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source channel &unstructured.Unstructured{ @@ -799,6 +858,9 @@ var testCases = []controllertesting.TestCase{ WantPresent: []runtime.Object{ Subscription().ReferencesResolved().PhysicalSubscriber(targetDNS).Reply(), }, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source with a reference to the From Channel @@ -894,6 +956,9 @@ var testCases = []controllertesting.TestCase{ Subscription().Renamed().ReferencesResolved().PhysicalSubscriber(targetDNS).Reply(), Subscription().DifferentChannel(), }, + WantEvent: []corev1.Event{ + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + }, Scheme: scheme.Scheme, Objects: []runtime.Object{ // Source with a reference to the From Channel @@ -988,7 +1053,7 @@ var testCases = []controllertesting.TestCase{ //getChannelWithOtherSubscription(), }, WantEvent: []corev1.Event{ - {Reason: "PhysicalChannelSyncFailed", Type: corev1.EventTypeWarning,}, + events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], }, Objects: []runtime.Object{ // Source channel diff --git a/pkg/controller/testing/mock_event_recorder.go b/pkg/controller/testing/mock_event_recorder.go index ae99207ee82..e823b69dfc7 100644 --- a/pkg/controller/testing/mock_event_recorder.go +++ b/pkg/controller/testing/mock_event_recorder.go @@ -20,11 +20,8 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/tools/record" ) -var _ record.EventRecorder = (*MockEventRecorder)(nil) - // mockEventRecorder is a recorder.EventRecorder that allows to save v1 Events emitted. type MockEventRecorder struct { events []corev1.Event @@ -35,23 +32,28 @@ func NewEventRecorder() *MockEventRecorder { } func (m *MockEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { - // Create an event with only the information that we need to verify in the test. - // Should include more information if we want to check other fields - event := corev1.Event{ - Reason: reason, - Type: eventtype, - } - m.events = append(m.events, event) + appendEvent(eventtype, reason, m) } func (m *MockEventRecorder) Event(object runtime.Object, eventtype, reason, message string) { - panic("not implemented") + appendEvent(eventtype, reason, m) } func (m *MockEventRecorder) PastEventf(object runtime.Object, timestamp metav1.Time, eventtype, reason, messageFmt string, args ...interface{}) { - panic("not implemented") + appendEvent(eventtype, reason, m) } func (m *MockEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { - panic("not implemented") + appendEvent(eventtype, reason, m) +} + +// Helper function to append an event type and reason to the MockEventRecorder +// Only interested in type and reason of Events for now. If we are planning on verifying other fields in +// the test cases, we need to include them here. +func appendEvent(eventtype, reason string, m *MockEventRecorder) { + event := corev1.Event{ + Reason: reason, + Type: eventtype, + } + m.events = append(m.events, event) } From 5f5dd4434f82a73d4bfefa096f47b98b674d1d00 Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Wed, 23 Jan 2019 15:16:47 -0800 Subject: [PATCH 4/9] Fixing UTs. Adding eventRecorder mock to tc.Runner invocations --- .../gcppubsub/controller/channel/reconcile_test.go | 6 +++--- .../controller/clusterchannelprovisioner/reconcile_test.go | 6 +++--- .../gcppubsub/dispatcher/dispatcher/reconcile_test.go | 6 +++--- pkg/provisioners/kafka/controller/channel/reconcile_test.go | 5 ++--- pkg/provisioners/kafka/controller/reconcile_test.go | 5 ++--- pkg/provisioners/natss/controller/channel/reconcile_test.go | 5 ++--- .../controller/clusterchannelprovisioner/reconcile_test.go | 5 ++--- 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/pkg/provisioners/gcppubsub/controller/channel/reconcile_test.go b/pkg/provisioners/gcppubsub/controller/channel/reconcile_test.go index 1435ac6534c..c50e8880dc2 100644 --- a/pkg/provisioners/gcppubsub/controller/channel/reconcile_test.go +++ b/pkg/provisioners/gcppubsub/controller/channel/reconcile_test.go @@ -43,7 +43,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -693,9 +692,10 @@ func TestReconcile(t *testing.T) { WantErrMsg: testErrorMessage, }, } - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) + for _, tc := range testCases { c := tc.GetClient() + recorder := tc.GetEventRecorder() r := &reconciler{ client: c, recorder: recorder, @@ -710,7 +710,7 @@ func TestReconcile(t *testing.T) { tc.ReconcileKey = fmt.Sprintf("/%s", cName) } tc.IgnoreTimes = true - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/provisioners/gcppubsub/controller/clusterchannelprovisioner/reconcile_test.go b/pkg/provisioners/gcppubsub/controller/clusterchannelprovisioner/reconcile_test.go index c8f867ca85d..fdb33a8d6e2 100644 --- a/pkg/provisioners/gcppubsub/controller/clusterchannelprovisioner/reconcile_test.go +++ b/pkg/provisioners/gcppubsub/controller/clusterchannelprovisioner/reconcile_test.go @@ -30,7 +30,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -184,9 +183,10 @@ func TestReconcile(t *testing.T) { WantErrMsg: testErrorMessage, }, } - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) + for _, tc := range testCases { c := tc.GetClient() + recorder := tc.GetEventRecorder() r := &reconciler{ client: c, recorder: recorder, @@ -196,7 +196,7 @@ func TestReconcile(t *testing.T) { tc.ReconcileKey = fmt.Sprintf("/%s", Name) } tc.IgnoreTimes = true - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/provisioners/gcppubsub/dispatcher/dispatcher/reconcile_test.go b/pkg/provisioners/gcppubsub/dispatcher/dispatcher/reconcile_test.go index 79420cd335e..491838d4367 100644 --- a/pkg/provisioners/gcppubsub/dispatcher/dispatcher/reconcile_test.go +++ b/pkg/provisioners/gcppubsub/dispatcher/dispatcher/reconcile_test.go @@ -45,7 +45,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -338,9 +337,10 @@ func TestReconcile(t *testing.T) { // Note - we do not test update status since this dispatcher only adds // finalizers to the channel } - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) + for _, tc := range testCases { c := tc.GetClient() + recorder := tc.GetEventRecorder() r := &reconciler{ client: c, recorder: recorder, @@ -383,7 +383,7 @@ func TestReconcile(t *testing.T) { } tc.AdditionalVerification = append(tc.AdditionalVerification, cc.verify) tc.IgnoreTimes = true - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/provisioners/kafka/controller/channel/reconcile_test.go b/pkg/provisioners/kafka/controller/channel/reconcile_test.go index a4b8d4c9c38..9dedb38fd62 100644 --- a/pkg/provisioners/kafka/controller/channel/reconcile_test.go +++ b/pkg/provisioners/kafka/controller/channel/reconcile_test.go @@ -37,7 +37,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -223,13 +222,13 @@ var testCases = []controllertesting.TestCase{ } func TestAllCases(t *testing.T) { - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) for _, tc := range testCases { tc.ReconcileKey = fmt.Sprintf("%s/%s", testNS, channelName) tc.IgnoreTimes = true c := tc.GetClient() + recorder := tc.GetEventRecorder() logger := provisioners.NewProvisionerLoggerFromConfig(provisioners.NewLoggingConfig()) r := &reconciler{ client: c, @@ -239,7 +238,7 @@ func TestAllCases(t *testing.T) { kafkaClusterAdmin: &mockClusterAdmin{}, } t.Logf("Running test %s", tc.Name) - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/provisioners/kafka/controller/reconcile_test.go b/pkg/provisioners/kafka/controller/reconcile_test.go index 3a4798b169e..b8f3d4772da 100644 --- a/pkg/provisioners/kafka/controller/reconcile_test.go +++ b/pkg/provisioners/kafka/controller/reconcile_test.go @@ -29,7 +29,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -132,10 +131,10 @@ var testCases = []controllertesting.TestCase{ } func TestAllCases(t *testing.T) { - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) for _, tc := range testCases { c := tc.GetClient() + recorder := tc.GetEventRecorder() logger := provisioners.NewProvisionerLoggerFromConfig(provisioners.NewLoggingConfig()) r := &reconciler{ client: c, @@ -144,7 +143,7 @@ func TestAllCases(t *testing.T) { config: getControllerConfig(), } t.Logf("Running test %s", tc.Name) - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/provisioners/natss/controller/channel/reconcile_test.go b/pkg/provisioners/natss/controller/channel/reconcile_test.go index c245e28ba24..0dc4b341e24 100644 --- a/pkg/provisioners/natss/controller/channel/reconcile_test.go +++ b/pkg/provisioners/natss/controller/channel/reconcile_test.go @@ -30,7 +30,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -115,10 +114,10 @@ var testCases = []controllertesting.TestCase{ } func TestAllCases(t *testing.T) { - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) for _, tc := range testCases { c := tc.GetClient() + recorder := tc.GetEventRecorder() logger := provisioners.NewProvisionerLoggerFromConfig(provisioners.NewLoggingConfig()) r := &reconciler{ client: c, @@ -126,7 +125,7 @@ func TestAllCases(t *testing.T) { logger: logger.Desugar(), } t.Logf("Running test %s", tc.Name) - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } diff --git a/pkg/provisioners/natss/controller/clusterchannelprovisioner/reconcile_test.go b/pkg/provisioners/natss/controller/clusterchannelprovisioner/reconcile_test.go index 95882ea60e4..fe6e4d4a8bd 100644 --- a/pkg/provisioners/natss/controller/clusterchannelprovisioner/reconcile_test.go +++ b/pkg/provisioners/natss/controller/clusterchannelprovisioner/reconcile_test.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -154,10 +153,10 @@ var testCases = []controllertesting.TestCase{ } func TestAllCases(t *testing.T) { - recorder := record.NewBroadcaster().NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) for _, tc := range testCases { c := tc.GetClient() + recorder := tc.GetEventRecorder() logger := provisioners.NewProvisionerLoggerFromConfig(provisioners.NewLoggingConfig()) r := &reconciler{ client: c, @@ -166,7 +165,7 @@ func TestAllCases(t *testing.T) { } tc.IgnoreTimes = true t.Logf("Running test %s", tc.Name) - t.Run(tc.Name, tc.Runner(t, r, c)) + t.Run(tc.Name, tc.Runner(t, r, c, recorder)) } } From 6c37b9891c8d064d50c658ab0636c231e02298e2 Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Wed, 23 Jan 2019 16:49:34 -0800 Subject: [PATCH 5/9] Changes after code review --- .../in-memory-channel/in-memory-channel.yaml | 5 --- hack/boilerplate/boilerplate.go.txt | 2 +- .../eventing/inmemory/channel/reconcile.go | 6 +-- .../inmemory/channel/reconcile_test.go | 19 ++++----- .../clusterchannelprovisioner/reconcile.go | 4 +- .../reconcile_test.go | 3 +- .../eventing/subscription/reconcile.go | 10 ++--- .../eventing/subscription/reconcile_test.go | 41 +++++++++---------- pkg/controller/testing/mock_event_recorder.go | 4 +- pkg/controller/testing/table.go | 7 ++-- 10 files changed, 44 insertions(+), 57 deletions(-) diff --git a/config/provisioners/in-memory-channel/in-memory-channel.yaml b/config/provisioners/in-memory-channel/in-memory-channel.yaml index bb153102f9b..765b43c7df4 100644 --- a/config/provisioners/in-memory-channel/in-memory-channel.yaml +++ b/config/provisioners/in-memory-channel/in-memory-channel.yaml @@ -97,12 +97,7 @@ rules: resources: - events verbs: - - get - - list - - watch - create - - update - - patch --- diff --git a/hack/boilerplate/boilerplate.go.txt b/hack/boilerplate/boilerplate.go.txt index 02c504e9302..1f43b023ad2 100644 --- a/hack/boilerplate/boilerplate.go.txt +++ b/hack/boilerplate/boilerplate.go.txt @@ -1,5 +1,5 @@ /* -Copyright 2018 The Knative Authors +Copyright 2019 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. diff --git a/pkg/controller/eventing/inmemory/channel/reconcile.go b/pkg/controller/eventing/inmemory/channel/reconcile.go index fe0c0521aea..079a3b62d38 100644 --- a/pkg/controller/eventing/inmemory/channel/reconcile.go +++ b/pkg/controller/eventing/inmemory/channel/reconcile.go @@ -42,7 +42,6 @@ const ( // Name of the corev1.Events emitted from the reconciliation process channelReconciled = "ChannelReconciled" - channelReconcileFailed = "ChannelReconcileFailed" channelUpdateStatusFailed = "ChannelUpdateStatusFailed" channelConfigSyncFailed = "ChannelConfigSyncFailed" k8sServiceCreateFailed = "K8sServiceCreateFailed" @@ -99,7 +98,6 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err err = r.reconcile(ctx, c) if err != nil { logger.Info("Error reconciling Channel", zap.Error(err)) - r.recorder.Eventf(c, corev1.EventTypeWarning, channelReconcileFailed, "Failed to reconcile Channel: %v", err) // Note that we do not return the error here, because we want to update the Status // regardless of the error. } else { @@ -154,7 +152,7 @@ func (r *reconciler) reconcile(ctx context.Context, c *eventingv1alpha1.Channel) svc, err := util.CreateK8sService(ctx, r.client, c) if err != nil { logger.Info("Error creating the Channel's K8s Service", zap.Error(err)) - r.recorder.Eventf(c, corev1.EventTypeWarning, k8sServiceCreateFailed, "Failed to create Channel's K8s Service: %v", err) + r.recorder.Eventf(c, corev1.EventTypeWarning, k8sServiceCreateFailed, "Failed to reconcile Channel's K8s Service: %v", err) return err } c.Status.SetAddress(controller.ServiceHostName(svc.Name, svc.Namespace)) @@ -162,7 +160,7 @@ func (r *reconciler) reconcile(ctx context.Context, c *eventingv1alpha1.Channel) _, err = util.CreateVirtualService(ctx, r.client, c, svc) if err != nil { logger.Info("Error creating the Virtual Service for the Channel", zap.Error(err)) - r.recorder.Eventf(c, corev1.EventTypeWarning, virtualServiceCreateFailed, "Failed to create Virtual Service for the Channel: %v", err) + r.recorder.Eventf(c, corev1.EventTypeWarning, virtualServiceCreateFailed, "Failed to reconcile Virtual Service for the Channel: %v", err) return err } diff --git a/pkg/controller/eventing/inmemory/channel/reconcile_test.go b/pkg/controller/eventing/inmemory/channel/reconcile_test.go index e592c4da857..7c698de1262 100644 --- a/pkg/controller/eventing/inmemory/channel/reconcile_test.go +++ b/pkg/controller/eventing/inmemory/channel/reconcile_test.go @@ -180,7 +180,6 @@ var ( // map of events to set test cases' expectations easier events = map[string]corev1.Event{ - channelReconcileFailed: {Reason: channelReconcileFailed, Type: corev1.EventTypeWarning}, channelReconciled: {Reason: channelReconciled, Type: corev1.EventTypeNormal}, channelUpdateStatusFailed: {Reason: channelUpdateStatusFailed, Type: corev1.EventTypeWarning}, channelConfigSyncFailed: {Reason: channelConfigSyncFailed, Type: corev1.EventTypeWarning}, @@ -262,7 +261,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[channelConfigSyncFailed], events[channelReconcileFailed], + events[channelConfigSyncFailed], }, }, { @@ -287,7 +286,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[channelConfigSyncFailed], events[channelReconcileFailed], + events[channelConfigSyncFailed], }, }, { @@ -300,7 +299,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[channelConfigSyncFailed], events[channelReconcileFailed], + events[channelConfigSyncFailed], }, }, { @@ -313,7 +312,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[channelConfigSyncFailed], events[channelReconcileFailed], + events[channelConfigSyncFailed], }, }, { @@ -327,7 +326,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[channelConfigSyncFailed], events[channelReconcileFailed], + events[channelConfigSyncFailed], }, }, { @@ -344,7 +343,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[k8sServiceCreateFailed], events[channelReconcileFailed], + events[k8sServiceCreateFailed], }, }, { @@ -362,7 +361,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[k8sServiceCreateFailed], events[channelReconcileFailed], + events[k8sServiceCreateFailed], }, }, { @@ -383,7 +382,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[virtualServiceCreateFailed], events[channelReconcileFailed], + events[virtualServiceCreateFailed], }, }, { @@ -403,7 +402,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[virtualServiceCreateFailed], events[channelReconcileFailed], + events[virtualServiceCreateFailed], }, }, { diff --git a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go index 6442e4f45c0..15e861f8896 100644 --- a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go +++ b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile.go @@ -43,7 +43,6 @@ const ( // Name of the corev1.Events emitted from the reconciliation process ccpReconciled = "CcpReconciled" - ccpReconcileFailed = "CcpReconcileFailed" ccpUpdateStatusFailed = "CcpUpdateStatusFailed" k8sServiceCreateFailed = "K8sServiceCreateFailed" k8sServiceDeleteFailed = "K8sServiceDeleteFailed" @@ -103,7 +102,6 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err err = r.reconcile(ctx, ccp) if err != nil { logger.Info("Error reconciling ClusterChannelProvisioner", zap.Error(err)) - r.recorder.Eventf(ccp, corev1.EventTypeWarning, ccpReconcileFailed, "Failed to reconcile ClusterChannelProvisioner: %v", err) // Note that we do not return the error here, because we want to update the Status // regardless of the error. } else { @@ -152,7 +150,7 @@ func (r *reconciler) reconcile(ctx context.Context, ccp *eventingv1alpha1.Cluste if err != nil { logger.Info("Error creating the ClusterChannelProvisioner's K8s Service", zap.Error(err)) - r.recorder.Eventf(ccp, corev1.EventTypeWarning, k8sServiceCreateFailed, "Failed to create ClusterChannelProvisioner's K8s Service: %v", err) + r.recorder.Eventf(ccp, corev1.EventTypeWarning, k8sServiceCreateFailed, "Failed to reconcile ClusterChannelProvisioner's K8s Service: %v", err) return err } diff --git a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile_test.go b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile_test.go index cbd4be6d720..4ad88ddd2de 100644 --- a/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile_test.go +++ b/pkg/controller/eventing/inmemory/clusterchannelprovisioner/reconcile_test.go @@ -54,7 +54,6 @@ var ( // map of events to set test cases' expectations easier events = map[string]corev1.Event{ ccpReconciled: {Reason: ccpReconciled, Type: corev1.EventTypeNormal}, - ccpReconcileFailed: {Reason: ccpReconcileFailed, Type: corev1.EventTypeWarning}, ccpUpdateStatusFailed: {Reason: ccpUpdateStatusFailed, Type: corev1.EventTypeWarning}, k8sServiceCreateFailed: {Reason: k8sServiceCreateFailed, Type: corev1.EventTypeWarning}, k8sServiceDeleteFailed: {Reason: k8sServiceDeleteFailed, Type: corev1.EventTypeWarning}, @@ -180,7 +179,7 @@ func TestReconcile(t *testing.T) { }, WantErrMsg: testErrorMessage, WantEvent: []corev1.Event{ - events[k8sServiceCreateFailed], events[ccpReconcileFailed], + events[k8sServiceCreateFailed], }, }, { diff --git a/pkg/controller/eventing/subscription/reconcile.go b/pkg/controller/eventing/subscription/reconcile.go index ec449291d0d..def95e2a1a0 100644 --- a/pkg/controller/eventing/subscription/reconcile.go +++ b/pkg/controller/eventing/subscription/reconcile.go @@ -45,10 +45,9 @@ const ( // Name of the corev1.Events emitted from the reconciliation process subscriptionReconciled = "SubscriptionReconciled" - subscriptionReconcileFailed = "SubscriptionReconcileFailed" subscriptionUpdateStatusFailed = "SubscriptionUpdateStatusFailed" physicalChannelSyncFailed = "PhysicalChannelSyncFailed" - objectReferenceFetchFailed = "ObjectReferenceFetchFailed" + channelReferenceFetchFailed = "ChannelReferenceFetchFailed" subscriberResolveFailed = "SubscriberResolveFailed" resultResolveFailed = "ResultResolveFailed" ) @@ -76,7 +75,6 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err err = r.reconcile(subscription) if err != nil { glog.Warningf("Error reconciling Subscription: %v", err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, subscriptionReconcileFailed, "Failed to reconcile Subscription: %v", err) } else { glog.Info("Subscription reconciled") r.recorder.Eventf(subscription, corev1.EventTypeNormal, subscriptionReconciled, "Subscription reconciled: %q", subscription.Name) @@ -123,13 +121,13 @@ func (r *reconciler) reconcile(subscription *v1alpha1.Subscription) error { _, err = r.fetchObjectReference(subscription.Namespace, &subscription.Spec.Channel) if err != nil { glog.Warningf("Failed to validate `channel` exists: %+v, %v", subscription.Spec.Channel, err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, objectReferenceFetchFailed, "Failed to validate Channel exists: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, channelReferenceFetchFailed, "Failed to validate spec.channel exists: %v", err) return err } if subscriberURI, err := r.resolveSubscriberSpec(subscription.Namespace, subscription.Spec.Subscriber); err != nil { glog.Warningf("Failed to resolve Subscriber %+v : %s", *subscription.Spec.Subscriber, err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, subscriberResolveFailed, "Failed to resolve Subscriber: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, subscriberResolveFailed, "Failed to resolve spec.subscriber: %v", err) return err } else { subscription.Status.PhysicalSubscription.SubscriberURI = subscriberURI @@ -138,7 +136,7 @@ func (r *reconciler) reconcile(subscription *v1alpha1.Subscription) error { if replyURI, err := r.resolveResult(subscription.Namespace, subscription.Spec.Reply); err != nil { glog.Warningf("Failed to resolve Result %v : %v", subscription.Spec.Reply, err) - r.recorder.Eventf(subscription, corev1.EventTypeWarning, resultResolveFailed, "Failed to resolve Result: %v", err) + r.recorder.Eventf(subscription, corev1.EventTypeWarning, resultResolveFailed, "Failed to resolve spec.reply: %v", err) return err } else { subscription.Status.PhysicalSubscription.ReplyURI = replyURI diff --git a/pkg/controller/eventing/subscription/reconcile_test.go b/pkg/controller/eventing/subscription/reconcile_test.go index 3f29c7b9fd8..d719fcccf4b 100644 --- a/pkg/controller/eventing/subscription/reconcile_test.go +++ b/pkg/controller/eventing/subscription/reconcile_test.go @@ -44,10 +44,9 @@ var ( // map of events to set test cases' expectations easier events = map[string]corev1.Event{ subscriptionReconciled: {Reason: subscriptionReconciled, Type: corev1.EventTypeNormal}, - subscriptionReconcileFailed: {Reason: subscriptionReconcileFailed, Type: corev1.EventTypeWarning}, subscriptionUpdateStatusFailed: {Reason: subscriptionUpdateStatusFailed, Type: corev1.EventTypeWarning}, physicalChannelSyncFailed: {Reason: physicalChannelSyncFailed, Type: corev1.EventTypeWarning}, - objectReferenceFetchFailed: {Reason: objectReferenceFetchFailed, Type: corev1.EventTypeWarning}, + channelReferenceFetchFailed: {Reason: channelReferenceFetchFailed, Type: corev1.EventTypeWarning}, subscriberResolveFailed: {Reason: subscriberResolveFailed, Type: corev1.EventTypeWarning}, resultResolveFailed: {Reason: resultResolveFailed, Type: corev1.EventTypeWarning}, } @@ -89,7 +88,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: `channels.eventing.knative.dev "fromchannel" not found`, WantEvent: []corev1.Event{ - events[objectReferenceFetchFailed], events[subscriptionReconcileFailed], + events[channelReferenceFetchFailed], }, }, { Name: "subscription, but From is not subscribable", @@ -102,7 +101,7 @@ var testCases = []controllertesting.TestCase{ // Spec.Subscribers field. WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -164,7 +163,7 @@ var testCases = []controllertesting.TestCase{ Subscription().UnknownConditions(), }, WantEvent: []corev1.Event{ - events[subscriberResolveFailed], events[subscriptionReconcileFailed], + events[subscriberResolveFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -193,7 +192,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "status does not contain address", WantEvent: []corev1.Event{ - events[subscriberResolveFailed], events[subscriptionReconcileFailed], + events[subscriberResolveFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -236,7 +235,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: `channels.eventing.knative.dev "resultchannel" not found`, WantEvent: []corev1.Event{ - events[resultResolveFailed], events[subscriptionReconcileFailed], + events[resultResolveFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -284,7 +283,7 @@ var testCases = []controllertesting.TestCase{ Subscription().UnknownConditions().PhysicalSubscriber(targetDNS), }, WantEvent: []corev1.Event{ - events[resultResolveFailed], events[subscriptionReconcileFailed], + events[resultResolveFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -347,7 +346,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -415,7 +414,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -464,7 +463,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -513,7 +512,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -561,7 +560,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -614,7 +613,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -661,7 +660,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -713,7 +712,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -762,7 +761,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "services \"testk8sservice\" not found", WantEvent: []corev1.Event{ - events[subscriberResolveFailed], events[subscriptionReconcileFailed], + events[subscriberResolveFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -796,7 +795,7 @@ var testCases = []controllertesting.TestCase{ }, WantErrMsg: "invalid JSON document", WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -859,7 +858,7 @@ var testCases = []controllertesting.TestCase{ Subscription().ReferencesResolved().PhysicalSubscriber(targetDNS).Reply(), }, WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -957,7 +956,7 @@ var testCases = []controllertesting.TestCase{ Subscription().DifferentChannel(), }, WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Scheme: scheme.Scheme, Objects: []runtime.Object{ @@ -1053,7 +1052,7 @@ var testCases = []controllertesting.TestCase{ //getChannelWithOtherSubscription(), }, WantEvent: []corev1.Event{ - events[physicalChannelSyncFailed], events[subscriptionReconcileFailed], + events[physicalChannelSyncFailed], }, Objects: []runtime.Object{ // Source channel diff --git a/pkg/controller/testing/mock_event_recorder.go b/pkg/controller/testing/mock_event_recorder.go index e823b69dfc7..2daee3f0fce 100644 --- a/pkg/controller/testing/mock_event_recorder.go +++ b/pkg/controller/testing/mock_event_recorder.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Knative Authors +Copyright 2019 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. @@ -22,7 +22,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) -// mockEventRecorder is a recorder.EventRecorder that allows to save v1 Events emitted. +// MockEventRecorder is a recorder.EventRecorder that saves emitted v1 Events. type MockEventRecorder struct { events []corev1.Event } diff --git a/pkg/controller/testing/table.go b/pkg/controller/testing/table.go index 1b22d65552a..99cafaa31e4 100644 --- a/pkg/controller/testing/table.go +++ b/pkg/controller/testing/table.go @@ -19,11 +19,12 @@ package testing import ( "context" "fmt" - corev1 "k8s.io/api/core/v1" "reflect" "strings" "testing" + corev1 "k8s.io/api/core/v1" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/knative/pkg/apis" @@ -281,7 +282,7 @@ func (tc *TestCase) VerifyWantAbsent(c client.Client) error { } // VerifyWantEvent verifies that the eventRecorder does contain the events -// expected to be emitted after reconciliation. +// expected in the same order as they were emitted after reconciliation. func (tc *TestCase) VerifyWantEvent(eventRecorder *MockEventRecorder) error { if !reflect.DeepEqual(tc.WantEvent, eventRecorder.events) { return fmt.Errorf("expected %s, got %s", getEventsAsString(tc.WantEvent), getEventsAsString(eventRecorder.events)) @@ -290,7 +291,7 @@ func (tc *TestCase) VerifyWantEvent(eventRecorder *MockEventRecorder) error { } func getEventsAsString(events []corev1.Event) []string { - eventsAsString := make([]string, 0) + eventsAsString := make([]string, len(events)) for _, event := range events { eventsAsString = append(eventsAsString, fmt.Sprintf("(%s,%s)", event.Reason, event.Type)) } From d8cf65fa6dba38dd4248e5f359cd784afc2323ae Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Wed, 23 Jan 2019 17:03:30 -0800 Subject: [PATCH 6/9] Boilerplate back to 2018. Should change it in another PR --- hack/boilerplate/boilerplate.go.txt | 2 +- pkg/controller/testing/mock_event_recorder.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/boilerplate/boilerplate.go.txt b/hack/boilerplate/boilerplate.go.txt index 1f43b023ad2..02c504e9302 100644 --- a/hack/boilerplate/boilerplate.go.txt +++ b/hack/boilerplate/boilerplate.go.txt @@ -1,5 +1,5 @@ /* -Copyright 2019 The Knative Authors +Copyright 2018 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. diff --git a/pkg/controller/testing/mock_event_recorder.go b/pkg/controller/testing/mock_event_recorder.go index 2daee3f0fce..793d8f56fe5 100644 --- a/pkg/controller/testing/mock_event_recorder.go +++ b/pkg/controller/testing/mock_event_recorder.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Knative Authors +Copyright 2018 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. From d6ce3669054438ddb5ffd488ddf3744c6e48f302 Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Thu, 24 Jan 2019 09:11:30 -0800 Subject: [PATCH 7/9] Setting initial capacity of slice --- pkg/controller/testing/table.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/testing/table.go b/pkg/controller/testing/table.go index 99cafaa31e4..8075d406a02 100644 --- a/pkg/controller/testing/table.go +++ b/pkg/controller/testing/table.go @@ -291,7 +291,7 @@ func (tc *TestCase) VerifyWantEvent(eventRecorder *MockEventRecorder) error { } func getEventsAsString(events []corev1.Event) []string { - eventsAsString := make([]string, len(events)) + eventsAsString := make([]string, 0, len(events)) for _, event := range events { eventsAsString = append(eventsAsString, fmt.Sprintf("(%s,%s)", event.Reason, event.Type)) } From d1138fe7dc379c5e917988396d3e87a3fcfca561 Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Thu, 24 Jan 2019 13:48:11 -0800 Subject: [PATCH 8/9] Adding path verb to in-memory-channel-controller --- config/provisioners/in-memory-channel/in-memory-channel.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/provisioners/in-memory-channel/in-memory-channel.yaml b/config/provisioners/in-memory-channel/in-memory-channel.yaml index 765b43c7df4..8bfdf03c405 100644 --- a/config/provisioners/in-memory-channel/in-memory-channel.yaml +++ b/config/provisioners/in-memory-channel/in-memory-channel.yaml @@ -98,6 +98,7 @@ rules: - events verbs: - create + - patch --- From e97f671ed86c7fd37ec6d69b11a080145e647933 Mon Sep 17 00:00:00 2001 From: Nacho Cano Date: Thu, 24 Jan 2019 15:28:08 -0800 Subject: [PATCH 9/9] Adding update verb for events to in-memory-channel --- config/provisioners/in-memory-channel/in-memory-channel.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/provisioners/in-memory-channel/in-memory-channel.yaml b/config/provisioners/in-memory-channel/in-memory-channel.yaml index 8bfdf03c405..0445e341e22 100644 --- a/config/provisioners/in-memory-channel/in-memory-channel.yaml +++ b/config/provisioners/in-memory-channel/in-memory-channel.yaml @@ -99,6 +99,7 @@ rules: verbs: - create - patch + - update ---