More events!

pkg/reconciler/v1alpha1/autoscaling/autoscaling.go

@@ -23,6 +23,13 @@ import (
 
 	"github.com/knative/pkg/controller"
 	"github.com/knative/pkg/logging"
+	"github.com/knative/serving/pkg/apis/autoscaling"
+	kpa "github.com/knative/serving/pkg/apis/autoscaling/v1alpha1"
+	"github.com/knative/serving/pkg/apis/serving"
+	"github.com/knative/serving/pkg/autoscaler"
+	informers "github.com/knative/serving/pkg/client/informers/externalversions/autoscaling/v1alpha1"
+	listers "github.com/knative/serving/pkg/client/listers/autoscaling/v1alpha1"
+	"github.com/knative/serving/pkg/reconciler"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -31,14 +38,6 @@ import (
 	corev1informers "k8s.io/client-go/informers/core/v1"
 	corev1listers "k8s.io/client-go/listers/core/v1"
 	"k8s.io/client-go/tools/cache"
-
-	"github.com/knative/serving/pkg/apis/autoscaling"
-	kpa "github.com/knative/serving/pkg/apis/autoscaling/v1alpha1"
-	"github.com/knative/serving/pkg/apis/serving"
-	"github.com/knative/serving/pkg/autoscaler"
-	informers "github.com/knative/serving/pkg/client/informers/externalversions/autoscaling/v1alpha1"
-	listers "github.com/knative/serving/pkg/client/listers/autoscaling/v1alpha1"
-	"github.com/knative/serving/pkg/reconciler"
 )
 
 const (
@@ -158,12 +157,11 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		// This is important because the copy we loaded from the informer's
 		// cache may be stale and we don't want to overwrite a prior update
 		// to status with this stale state.
-	} else {
-		// logger.Infof("Updating Status (-old, +new): %v", cmp.Diff(original, kpa))
-		if _, err := c.updateStatus(kpa); err != nil {
-			logger.Warn("Failed to update kpa status", zap.Error(err))
-			return err
-		}
+	} else if _, err := c.updateStatus(kpa); err != nil {
+		logger.Warn("Failed to update kpa status", zap.Error(err))
+		c.Recorder.Eventf(kpa, corev1.EventTypeWarning, "UpdateFailed",
+			"Failed to update status for KPA %q: %v", kpa.Name, err)
+		return err
 	}
 	return err
 }
@@ -247,15 +245,14 @@ func (c *Reconciler) updateStatus(desired *kpa.PodAutoscaler) (*kpa.PodAutoscale
 	if err != nil {
 		return nil, err
 	}
-	// Check if there is anything to update.
-	if !reflect.DeepEqual(kpa.Status, desired.Status) {
-		// Don't modify the informers copy
-		existing := kpa.DeepCopy()
-		existing.Status = desired.Status
-
-		// TODO: for CRD there's no updatestatus, so use normal update
-		return c.ServingClientSet.AutoscalingV1alpha1().PodAutoscalers(kpa.Namespace).Update(existing)
-		//	return prClient.UpdateStatus(newKPA)
+	// If there's nothing to update, just return.
+	if reflect.DeepEqual(kpa.Status, desired.Status) {
+		return kpa, nil
 	}
-	return kpa, nil
+	// Don't modify the informers copy
+	existing := kpa.DeepCopy()
+	existing.Status = desired.Status
+
+	// TODO: for CRD there's no updatestatus, so use normal update
+	return c.ServingClientSet.AutoscalingV1alpha1().PodAutoscalers(kpa.Namespace).Update(existing)
 }

pkg/reconciler/v1alpha1/clusteringress/clusteringress.go

@@ -20,12 +20,6 @@ import (
 	"context"
 	"reflect"
 
-	"go.uber.org/zap"
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
-	apierrs "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/client-go/tools/cache"
-
 	"github.com/knative/pkg/apis/istio/v1alpha3"
 	istioinformers "github.com/knative/pkg/client/informers/externalversions/istio/v1alpha3"
 	istiolisters "github.com/knative/pkg/client/listers/istio/v1alpha3"
@@ -38,6 +32,11 @@ import (
 	"github.com/knative/serving/pkg/reconciler"
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/clusteringress/resources"
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/clusteringress/resources/names"
+	"go.uber.org/zap"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/client-go/tools/cache"
 )
 
 const controllerAgentName = "clusteringress-controller"
@@ -117,7 +116,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		// This is important because the copy we loaded from the informer's
 		// cache may be stale and we don't want to overwrite a prior update
 		// to status with this stale state.
-	} else if _, err := c.updateStatus(ctx, ci); err != nil {
+	} else if _, err := c.updateStatus(ci); err != nil {
 		logger.Warn("Failed to update clusterIngress status", zap.Error(err))
 		c.Recorder.Eventf(ci, corev1.EventTypeWarning, "UpdateFailed",
 			"Failed to update status for ClusterIngress %q: %v", ci.Name, err)
@@ -128,7 +127,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 
 // Update the Status of the ClusterIngress.  Caller is responsible for checking
 // for semantic differences before calling.
-func (c *Reconciler) updateStatus(ctx context.Context, desired *v1alpha1.ClusterIngress) (*v1alpha1.ClusterIngress, error) {
+func (c *Reconciler) updateStatus(desired *v1alpha1.ClusterIngress) (*v1alpha1.ClusterIngress, error) {
 	ci, err := c.clusterIngressLister.Get(desired.Name)
 	if err != nil {
 		return nil, err
@@ -141,13 +140,7 @@ func (c *Reconciler) updateStatus(ctx context.Context, desired *v1alpha1.Cluster
 	existing := ci.DeepCopy()
 	existing.Status = desired.Status
 	// TODO: for CRD there's no updatestatus, so use normal update.
-	updated, err := c.ServingClientSet.NetworkingV1alpha1().ClusterIngresses().Update(existing)
-	if err != nil {
-		return nil, err
-	}
-
-	c.Recorder.Eventf(desired, corev1.EventTypeNormal, "Updated", "Updated status for clusterIngress %q", desired.Name)
-	return updated, nil
+	return c.ServingClientSet.NetworkingV1alpha1().ClusterIngresses().Update(existing)
 }
 
 func (c *Reconciler) reconcile(ctx context.Context, ci *v1alpha1.ClusterIngress) error {

pkg/reconciler/v1alpha1/configuration/configuration.go

@@ -25,6 +25,14 @@ import (
 	"github.com/knative/pkg/configmap"
 	"github.com/knative/pkg/controller"
 	"github.com/knative/pkg/logging"
+	"github.com/knative/serving/pkg/apis/serving"
+	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
+	servinginformers "github.com/knative/serving/pkg/client/informers/externalversions/serving/v1alpha1"
+	listers "github.com/knative/serving/pkg/client/listers/serving/v1alpha1"
+	"github.com/knative/serving/pkg/reconciler"
+	configns "github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/config"
+	"github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/resources"
+	resourcenames "github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/resources/names"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -34,15 +42,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/tools/cache"
-
-	"github.com/knative/serving/pkg/apis/serving"
-	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
-	servinginformers "github.com/knative/serving/pkg/client/informers/externalversions/serving/v1alpha1"
-	listers "github.com/knative/serving/pkg/client/listers/serving/v1alpha1"
-	"github.com/knative/serving/pkg/reconciler"
-	configns "github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/config"
-	"github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/resources"
-	resourcenames "github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/resources/names"
 )
 
 const controllerAgentName = "configuration-controller"
@@ -139,6 +138,8 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		// to status with this stale state.
 	} else if _, err := c.updateStatus(config); err != nil {
 		logger.Warn("Failed to update configuration status", zap.Error(err))
+		c.Recorder.Eventf(config, corev1.EventTypeWarning, "UpdateFailed",
+			"Failed to update status for Configuration %q: %v", config.Name, err)
 		return err
 	}
 	return err
@@ -266,19 +267,19 @@ func (c *Reconciler) createRevision(ctx context.Context, config *v1alpha1.Config
 }
 
 func (c *Reconciler) updateStatus(desired *v1alpha1.Configuration) (*v1alpha1.Configuration, error) {
-	u, err := c.configurationLister.Configurations(desired.Namespace).Get(desired.Name)
+	config, err := c.configurationLister.Configurations(desired.Namespace).Get(desired.Name)
 	if err != nil {
 		return nil, err
 	}
-	if !reflect.DeepEqual(u.Status, desired.Status) {
-		// Don't modify the informers copy
-		existing := u.DeepCopy()
-		existing.Status = desired.Status
-		// TODO: for CRD there's no updatestatus, so use normal update
-		return c.ServingClientSet.ServingV1alpha1().Configurations(desired.Namespace).Update(existing)
-		//	return configClient.UpdateStatus(newu)
+	// If there's nothing to update, just return.
+	if reflect.DeepEqual(config.Status, desired.Status) {
+		return config, nil
 	}
-	return u, nil
+	// Don't modify the informers copy
+	existing := config.DeepCopy()
+	existing.Status = desired.Status
+	// TODO: for CRD there's no updatestatus, so use normal update
+	return c.ServingClientSet.ServingV1alpha1().Configurations(desired.Namespace).Update(existing)
 }
 
 func (c *Reconciler) gcRevisions(ctx context.Context, config *v1alpha1.Configuration) error {

pkg/reconciler/v1alpha1/revision/revision.go

@@ -249,12 +249,11 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		// This is important because the copy we loaded from the informer's
 		// cache may be stale and we don't want to overwrite a prior update
 		// to status with this stale state.
-	} else {
-		// logger.Infof("Updating Status (-old, +new): %v", cmp.Diff(original, rev))
-		if _, err := c.updateStatus(rev); err != nil {
-			logger.Warn("Failed to update revision status", zap.Error(err))
-			return err
-		}
+	} else if _, err := c.updateStatus(rev); err != nil {
+		logger.Warn("Failed to update revision status", zap.Error(err))
+		c.Recorder.Eventf(rev, corev1.EventTypeWarning, "UpdateFailed",
+			"Failed to update status for Revision %q: %v", rev.Name, err)
+		return err
 	}
 	return err
 }
@@ -411,15 +410,13 @@ func (c *Reconciler) updateStatus(desired *v1alpha1.Revision) (*v1alpha1.Revisio
 	if err != nil {
 		return nil, err
 	}
-	// Check if there is anything to update.
-	if !reflect.DeepEqual(rev.Status, desired.Status) {
-		// Don't modify the informers copy
-		existing := rev.DeepCopy()
-		existing.Status = desired.Status
-
-		// TODO: for CRD there's no updatestatus, so use normal update
-		return c.ServingClientSet.ServingV1alpha1().Revisions(desired.Namespace).Update(existing)
-		//	return prClient.UpdateStatus(newRev)
+	// If there's nothing to update, just return.
+	if reflect.DeepEqual(rev.Status, desired.Status) {
+		return rev, nil
 	}
-	return rev, nil
+	// Don't modify the informers copy
+	existing := rev.DeepCopy()
+	existing.Status = desired.Status
+	// TODO: for CRD there's no updatestatus, so use normal update
+	return c.ServingClientSet.ServingV1alpha1().Revisions(desired.Namespace).Update(existing)
 }

pkg/reconciler/v1alpha1/route/reconcile_resources.go

@@ -141,25 +141,20 @@ func (c *Reconciler) reconcilePlaceholderService(ctx context.Context, route *v1a
 
 // Update the Status of the route.  Caller is responsible for checking
 // for semantic differences before calling.
-func (c *Reconciler) updateStatus(ctx context.Context, route *v1alpha1.Route) (*v1alpha1.Route, error) {
-	existing, err := c.routeLister.Routes(route.Namespace).Get(route.Name)
+func (c *Reconciler) updateStatus(desired *v1alpha1.Route) (*v1alpha1.Route, error) {
+	route, err := c.routeLister.Routes(desired.Namespace).Get(desired.Name)
 	if err != nil {
 		return nil, err
 	}
-	existing = existing.DeepCopy()
 	// If there's nothing to update, just return.
-	if reflect.DeepEqual(existing.Status, route.Status) {
-		return existing, nil
+	if reflect.DeepEqual(route.Status, desired.Status) {
+		return route, nil
 	}
-	existing.Status = route.Status
+	// Don't modify the informers copy
+	existing := route.DeepCopy()
+	existing.Status = desired.Status
 	// TODO: for CRD there's no updatestatus, so use normal update.
-	updated, err := c.ServingClientSet.ServingV1alpha1().Routes(route.Namespace).Update(existing)
-	if err != nil {
-		return nil, err
-	}
-
-	c.Recorder.Eventf(route, corev1.EventTypeNormal, "Updated", "Updated status for route %q", route.Name)
-	return updated, nil
+	return c.ServingClientSet.ServingV1alpha1().Routes(desired.Namespace).Update(existing)
 }
 
 // Update the lastPinned annotation on revisions we target so they don't get GC'd.

pkg/reconciler/v1alpha1/route/route.go

@@ -200,10 +200,10 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		// This is important because the copy we loaded from the informer's
 		// cache may be stale and we don't want to overwrite a prior update
 		// to status with this stale state.
-	} else if _, err := c.updateStatus(ctx, route); err != nil {
+	} else if _, err := c.updateStatus(route); err != nil {
 		logger.Warn("Failed to update route status", zap.Error(err))
 		c.Recorder.Eventf(route, corev1.EventTypeWarning, "UpdateFailed",
-			"Failed to update status for route %q: %v", route.Name, err)
+			"Failed to update status for Route %q: %v", route.Name, err)
 		return err
 	}
 	return err

pkg/reconciler/v1alpha1/route/route_test.go

@@ -38,6 +38,7 @@ import (
 	"github.com/knative/serving/pkg/gc"
 	rclr "github.com/knative/serving/pkg/reconciler"
 	"github.com/knative/serving/pkg/reconciler/v1alpha1/route/config"
+	. "github.com/knative/serving/pkg/reconciler/v1alpha1/testing"
 	"github.com/knative/serving/pkg/system"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -46,8 +47,6 @@ import (
 	"k8s.io/apimachinery/pkg/watch"
 	kubeinformers "k8s.io/client-go/informers"
 	fakekubeclientset "k8s.io/client-go/kubernetes/fake"
-
-	. "github.com/knative/serving/pkg/reconciler/v1alpha1/testing"
 )
 
 const (
@@ -273,7 +272,6 @@ func TestCreateRouteForOneReserveRevision(t *testing.T) {
 	// hooks here. Each hook tests for a specific event.
 	h.OnCreate(&kubeClient.Fake, "events", ExpectNormalEventDelivery(t, `Created service "test-route"`))
 	h.OnCreate(&kubeClient.Fake, "events", ExpectNormalEventDelivery(t, "^Created ClusterIngress.*" /*ingress name is unset in test*/))
-	h.OnCreate(&kubeClient.Fake, "events", ExpectNormalEventDelivery(t, `Updated status for route "test-route"`))
 
 	// An inactive revision
 	rev := getTestRevisionWithCondition("test-rev",

pkg/reconciler/v1alpha1/service/resources/route.go

@@ -28,7 +28,6 @@ import (
 
 // MakeRoute creates a Route from a Service object.
 func MakeRoute(service *v1alpha1.Service) (*v1alpha1.Route, error) {
-
 	c := &v1alpha1.Route{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      names.Route(service),