Merge pull request #540 from weaveworks/events-fix

logs: reduce log verbosity

Author: stefanprodan

pkg/canary/daemonset_controller.go

@@ -83,7 +83,7 @@ func (c *DaemonSetController) Initialize(cd *flaggerv1.Canary) (err error) {
 	if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing {
 		if !cd.SkipAnalysis() {
 			if err := c.IsPrimaryReady(cd); err != nil {
-				return fmt.Errorf("IsPrimaryReady failed: %w", err)
+				return fmt.Errorf("%w", err)
 			}
 		}
 

pkg/canary/deployment_controller.go

@@ -37,7 +37,7 @@ func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (err error) {
 	if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing {
 		if !cd.SkipAnalysis() {
 			if err := c.IsPrimaryReady(cd); err != nil {
-				return fmt.Errorf("IsPrimaryReady failed: %w", err)
+				return fmt.Errorf("%w", err)
 			}
 		}
 

pkg/canary/deployment_ready.go

@@ -12,7 +12,7 @@ import (
 
 // IsPrimaryReady checks the primary deployment status and returns an error if
 // the deployment is in the middle of a rolling update or if the pods are unhealthy
-// it will return a non retriable error if the rolling update is stuck
+// it will return a non retryable error if the rolling update is stuck
 func (c *DeploymentController) IsPrimaryReady(cd *flaggerv1.Canary) error {
 	primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
 	primary, err := c.kubeClient.AppsV1().Deployments(cd.Namespace).Get(primaryName, metav1.GetOptions{})
@@ -22,7 +22,7 @@ func (c *DeploymentController) IsPrimaryReady(cd *flaggerv1.Canary) error {
 
 	_, err = c.isDeploymentReady(primary, cd.GetProgressDeadlineSeconds())
 	if err != nil {
-		return fmt.Errorf("primary deployment %s.%s not ready: %w", primaryName, cd.Namespace, err)
+		return fmt.Errorf("%s.%s not ready: %w", primaryName, cd.Namespace, err)
 	}
 
 	if primary.Spec.Replicas == int32p(0) {
@@ -45,8 +45,8 @@ func (c *DeploymentController) IsCanaryReady(cd *flaggerv1.Canary) (bool, error)
 	retryable, err := c.isDeploymentReady(canary, cd.GetProgressDeadlineSeconds())
 	if err != nil {
 		return retryable, fmt.Errorf(
-			"canary deployment %s.%s not ready with retryable %v: %w",
-			targetName, cd.Namespace, retryable, err,
+			"canary deployment %s.%s not ready: %w",
+			targetName, cd.Namespace, err,
 		)
 	}
 	return true, nil

pkg/controller/controller.go

@@ -202,7 +202,7 @@ func (c *Controller) processNextWorkItem() bool {
 			return nil
 		}
 		// Run the syncHandler, passing it the namespace/name string of the
-		// Foo resource to be synced.
+		// Canary resource to be synced.
 		if err := c.syncHandler(key); err != nil {
 			return fmt.Errorf("error syncing '%s': %w", key, err)
 		}
@@ -234,7 +234,6 @@ func (c *Controller) syncHandler(key string) error {
 
 	// Finalize if canary has been marked for deletion and revert is desired
 	if cd.Spec.RevertOnDeletion && cd.ObjectMeta.DeletionTimestamp != nil {
-
 		// If finalizers have been previously removed proceed
 		if !hasFinalizer(cd) {
 			c.logger.Infof("Canary %s.%s has been finalized", cd.Name, cd.Namespace)
@@ -243,12 +242,16 @@ func (c *Controller) syncHandler(key string) error {
 
 		if cd.Status.Phase != flaggerv1.CanaryPhaseTerminated {
 			if err := c.finalize(cd); err != nil {
+				c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).
+					Errorf("Unable to finalize canary: %v", err)
 				return fmt.Errorf("unable to finalize to canary %s.%s error: %w", cd.Name, cd.Namespace, err)
 			}
 		}
 
 		// Remove finalizer from Canary
 		if err := c.removeFinalizer(cd); err != nil {
+			c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).
+				Errorf("Unable to remove finalizer for canary %s.%s error: %v", cd.Name, cd.Namespace, err)
 			return fmt.Errorf("unable to remove finalizer for canary %s.%s: %w", cd.Name, cd.Namespace, err)
 		}
 
@@ -302,7 +305,7 @@ func checkCustomResourceType(obj interface{}, logger *zap.SugaredLogger) (flagge
 	var roll *flaggerv1.Canary
 	var ok bool
 	if roll, ok = obj.(*flaggerv1.Canary); !ok {
-		logger.Errorf("Event Watch received an invalid object: %#v", obj)
+		logger.Errorf("Event watch received an invalid object: %#v", obj)
 		return flaggerv1.Canary{}, false
 	}
 	return *roll, true

pkg/controller/finalizer.go

@@ -12,63 +12,61 @@ import (
 const finalizer = "finalizer.flagger.app"
 
 func (c *Controller) finalize(old interface{}) error {
-
-	r, ok := old.(*flaggerv1.Canary)
+	canary, ok := old.(*flaggerv1.Canary)
 	if !ok {
 		return fmt.Errorf("received unexpected object: %v", old)
 	}
 
-	_, err := c.flaggerClient.FlaggerV1beta1().Canaries(r.Namespace).Get(r.Name, metav1.GetOptions{})
+	_, err := c.flaggerClient.FlaggerV1beta1().Canaries(canary.Namespace).Get(canary.Name, metav1.GetOptions{})
 	if err != nil {
 		return fmt.Errorf("get query error: %w", err)
 	}
 
 	// Retrieve a controller
-	canaryController := c.canaryFactory.Controller(r.Spec.TargetRef.Kind)
+	canaryController := c.canaryFactory.Controller(canary.Spec.TargetRef.Kind)
 
 	// Set the status to terminating if not already in that state
-	if r.Status.Phase != flaggerv1.CanaryPhaseTerminating {
-		if err := canaryController.SetStatusPhase(r, flaggerv1.CanaryPhaseTerminating); err != nil {
+	if canary.Status.Phase != flaggerv1.CanaryPhaseTerminating {
+		if err := canaryController.SetStatusPhase(canary, flaggerv1.CanaryPhaseTerminating); err != nil {
 			return fmt.Errorf("failed to update status: %w", err)
 		}
 
 		// record event
-		c.recordEventInfof(r, "Terminating canary %s.%s", r.Name, r.Namespace)
+		c.recordEventInfof(canary, "Terminating canary %s.%s", canary.Name, canary.Namespace)
 	}
 
-	err = canaryController.Finalize(r)
+	// Revert the Kubernetes deployment or daemonset
+	err = canaryController.Finalize(canary)
 	if err != nil {
 		return fmt.Errorf("failed to revert target: %w", err)
 	}
-	c.logger.Infof("%s.%s kind %s reverted", r.Name, r.Namespace, r.Spec.TargetRef.Kind)
+	c.logger.Infof("%s.%s kind %s reverted", canary.Name, canary.Namespace, canary.Spec.TargetRef.Kind)
 
 	// Ensure that targetRef has met a ready state
-	c.logger.Infof("Checking is canary is ready %s.%s", r.Name, r.Namespace)
-	_, err = canaryController.IsCanaryReady(r)
+	c.logger.Infof("Checking is canary is ready %s.%s", canary.Name, canary.Namespace)
+	_, err = canaryController.IsCanaryReady(canary)
 	if err != nil {
 		return fmt.Errorf("canary not ready during finalizing: %w", err)
 	}
 
-	c.logger.Infof("%s.%s moving forward with router finalizing", r.Name, r.Namespace)
-	labelSelector, ports, err := canaryController.GetMetadata(r)
+	labelSelector, ports, err := canaryController.GetMetadata(canary)
 	if err != nil {
 		return fmt.Errorf("failed to get metadata for router finalizing: %w", err)
 	}
 
-	// Revert the router
-	router := c.routerFactory.KubernetesRouter(r.Spec.TargetRef.Kind, labelSelector, map[string]string{}, ports)
-	if err := router.Finalize(r); err != nil {
+	// Revert the Kubernetes service
+	router := c.routerFactory.KubernetesRouter(canary.Spec.TargetRef.Kind, labelSelector, map[string]string{}, ports)
+	if err := router.Finalize(canary); err != nil {
 		return fmt.Errorf("failed revert router: %w", err)
 	}
-	c.logger.Infof("%s.%s router reverted", r.Name, r.Namespace)
+	c.logger.Infof("%s.%s router reverted", canary.Name, canary.Namespace)
 
-	// TODO if I can't revert the mesh continue on?
-	// Revert the Mesh
-	if err := c.revertMesh(r); err != nil {
+	// Revert the mesh objects
+	if err := c.revertMesh(canary); err != nil {
 		return fmt.Errorf("failed to revert mesh: %w", err)
 	}
 
-	c.logger.Infof("Finalization complete for %s.%s", r.Name, r.Namespace)
+	c.logger.Infof("Finalization complete for %s.%s", canary.Name, canary.Namespace)
 	return nil
 }