Merge pull request #6571 from greenmoon55/cronfederatedhpa-logs

structured logging for cronfederatedhpa controllers

Author: karmada-bot

pkg/controllers/cronfederatedhpa/cronfederatedhpa_controller.go

@@ -56,17 +56,17 @@ type CronFHPAController struct {
 // The Controller will requeue the Request to be processed again if an error is non-nil or
 // Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
 func (c *CronFHPAController) Reconcile(ctx context.Context, req controllerruntime.Request) (controllerruntime.Result, error) {
-	klog.V(4).Infof("Reconciling CronFederatedHPA %s", req.NamespacedName)
+	klog.V(4).InfoS("Reconciling CronFederatedHPA", "namespace", req.Namespace, "name", req.Name)
 
 	cronFHPA := &autoscalingv1alpha1.CronFederatedHPA{}
 	if err := c.Client.Get(ctx, req.NamespacedName, cronFHPA); err != nil {
 		if apierrors.IsNotFound(err) {
-			klog.V(4).Infof("Begin to cleanup the cron jobs for CronFederatedHPA:%s", req.NamespacedName)
+			klog.V(4).InfoS("Begin to cleanup the cron jobs for CronFederatedHPA", "namespace", req.Namespace, "name", req.Name)
 			c.CronHandler.StopCronFHPAExecutor(req.NamespacedName.String())
 			return controllerruntime.Result{}, nil
 		}
 
-		klog.Errorf("Fail to get CronFederatedHPA(%s):%v", req.NamespacedName, err)
+		klog.ErrorS(err, "Fail to get CronFederatedHPA", "namespace", req.Namespace, "name", req.Name)
 		return controllerruntime.Result{}, err
 	}
 
@@ -140,7 +140,7 @@ func (c *CronFHPAController) processCronRule(ctx context.Context, cronFHPA *auto
 	if !helper.IsCronFederatedHPARuleSuspend(rule) {
 		if err := c.CronHandler.CreateCronJobForExecutor(cronFHPA, rule); err != nil {
 			c.EventRecorder.Event(cronFHPA, corev1.EventTypeWarning, "StartRuleFailed", err.Error())
-			klog.Errorf("Fail to start cron for CronFederatedHPA(%s) rule(%s):%v", cronFHPAKey, rule.Name, err)
+			klog.ErrorS(err, "Fail to start cron for CronFederatedHPA rule", "cronFederatedHPA", cronFHPAKey, "rule", rule.Name)
 			return err
 		}
 	}
@@ -159,8 +159,7 @@ func (c *CronFHPAController) updateRuleHistory(ctx context.Context, cronFHPA *au
 		// If rule is not suspended, we should set the nextExecutionTime filed, or the nextExecutionTime will be nil
 		next, err := c.CronHandler.GetRuleNextExecuteTime(cronFHPA, rule.Name)
 		if err != nil {
-			klog.Errorf("Fail to get next execution time for CronFederatedHPA(%s/%s) rule(%s):%v",
-				cronFHPA.Namespace, cronFHPA.Name, rule.Name, err)
+			klog.ErrorS(err, "Fail to get next execution time for CronFederatedHPA rule", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name, "rule", rule.Name)
 			return err
 		}
 		nextExecutionTime = &metav1.Time{Time: next}
@@ -185,8 +184,7 @@ func (c *CronFHPAController) updateRuleHistory(ctx context.Context, cronFHPA *au
 	}
 
 	if err := c.Client.Status().Update(ctx, cronFHPA); err != nil {
-		klog.Errorf("Fail to update CronFederatedHPA(%s/%s) rule(%s)'s next execution time:%v",
-			cronFHPA.Namespace, cronFHPA.Name, rule.Name, err)
+		klog.ErrorS(err, "Fail to update CronFederatedHPA rule's next execution time", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name, "rule", rule.Name)
 		return err
 	}
 
@@ -210,7 +208,7 @@ func (c *CronFHPAController) removeCronFHPAHistory(ctx context.Context, cronFHPA
 	}
 	if err := c.Client.Status().Update(ctx, cronFHPA); err != nil {
 		c.EventRecorder.Event(cronFHPA, corev1.EventTypeWarning, "UpdateCronFederatedHPAFailed", err.Error())
-		klog.Errorf("Fail to remove CronFederatedHPA(%s/%s) rule(%s) history:%v", cronFHPA.Namespace, cronFHPA.Name, ruleName, err)
+		klog.ErrorS(err, "Fail to remove CronFederatedHPA rule history", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name, "rule", ruleName)
 		return err
 	}
 

pkg/controllers/cronfederatedhpa/cronfederatedhpa_handler.go

@@ -142,17 +142,15 @@ func (c *CronHandler) CreateCronJobForExecutor(cronFHPA *autoscalingv1alpha1.Cro
 		timeZone, err = time.LoadLocation(*rule.TimeZone)
 		if err != nil {
 			// This should not happen because there is validation in webhook
-			klog.Errorf("Invalid CronFederatedHPA(%s/%s) rule(%s) time zone(%s):%v",
-				cronFHPA.Namespace, cronFHPA.Namespace, rule.Name, *rule.TimeZone, err)
+			klog.ErrorS(err, "Invalid CronFederatedHPA rule time zone", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name, "rule", rule.Name, "timeZone", *rule.TimeZone)
 			return err
 		}
 	}
 
 	scheduler := gocron.NewScheduler(timeZone)
 	cronJob := NewCronFederatedHPAJob(c.client, c.eventRecorder, scheduler, cronFHPA, rule)
 	if _, err := scheduler.Cron(rule.Schedule).Do(RunCronFederatedHPARule, cronJob); err != nil {
-		klog.Errorf("Create cron job for CronFederatedHPA(%s/%s) rule(%s) error:%v",
-			cronFHPA.Namespace, cronFHPA.Name, rule.Name, err)
+		klog.ErrorS(err, "Create cron job for CronFederatedHPA rule error", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name, "rule", rule.Name)
 		return err
 	}
 	scheduler.StartAsync()

pkg/controllers/cronfederatedhpa/cronfederatedhpa_job.go

@@ -76,18 +76,18 @@ func RunCronFederatedHPARule(c *ScalingJob) {
 	err = c.client.Get(context.TODO(), c.namespaceName, cronFHPA)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
-			klog.Infof("CronFederatedHPA(%s) not found", c.namespaceName)
+			klog.InfoS("CronFederatedHPA not found", "cronFederatedHPA", c.namespaceName)
 		} else {
 			// TODO: This may happen when the the network is down, we should do something here
 			// But we are not sure what to do(retry not solve the problem)
-			klog.Errorf("Get CronFederatedHPA(%s) failed: %v", c.namespaceName, err)
+			klog.ErrorS(err, "Get CronFederatedHPA failed", "cronFederatedHPA", c.namespaceName)
 		}
 		return
 	}
 
 	if helper.IsCronFederatedHPARuleSuspend(c.rule) {
 		// If the rule is suspended, this job will be stopped soon
-		klog.V(4).Infof("CronFederatedHPA(%s) Rule(%s) is suspended, skip it", c.namespaceName, c.rule.Name)
+		klog.V(4).InfoS("CronFederatedHPA rule is suspended, skip it", "cronFederatedHPA", c.namespaceName, "rule", c.rule.Name)
 		return
 	}
 
@@ -153,17 +153,14 @@ func (c *ScalingJob) ScaleFHPA(cronFHPA *autoscalingv1alpha1.CronFederatedHPA) e
 	if update {
 		err := c.client.Update(context.TODO(), fhpa)
 		if err != nil {
-			klog.Errorf("CronFederatedHPA(%s) updates FederatedHPA(%s/%s) failed: %v",
-				c.namespaceName, fhpa.Namespace, fhpa.Name, err)
+			klog.ErrorS(err, "CronFederatedHPA updates FederatedHPA failed", "namespace", c.namespaceName.Namespace, "cronFederatedHPA", c.namespaceName.Name, "federatedHPA", fhpa.Name)
 			return err
 		}
-		klog.V(4).Infof("CronFederatedHPA(%s) scales FederatedHPA(%s/%s) successfully",
-			c.namespaceName, fhpa.Namespace, fhpa.Name)
+		klog.V(4).InfoS("CronFederatedHPA scales FederatedHPA successfully", "namespace", c.namespaceName.Namespace, "cronFederatedHPA", c.namespaceName.Name, "federatedHPA", fhpa.Name)
 		return nil
 	}
 
-	klog.V(4).Infof("CronFederatedHPA(%s) find nothing updated for FederatedHPA(%s/%s), skip it",
-		c.namespaceName, fhpa.Namespace, fhpa.Name)
+	klog.V(4).InfoS("CronFederatedHPA find nothing updated for FederatedHPA, skip it", "namespace", c.namespaceName.Namespace, "cronFederatedHPA", c.namespaceName.Name, "federatedHPA", fhpa.Name)
 	return nil
 }
 
@@ -175,8 +172,7 @@ func (c *ScalingJob) ScaleWorkloads(cronFHPA *autoscalingv1alpha1.CronFederatedH
 
 	targetGV, err := schema.ParseGroupVersion(cronFHPA.Spec.ScaleTargetRef.APIVersion)
 	if err != nil {
-		klog.Errorf("CronFederatedHPA(%s) parses GroupVersion(%s) failed: %v",
-			c.namespaceName, cronFHPA.Spec.ScaleTargetRef.APIVersion, err)
+		klog.ErrorS(err, "CronFederatedHPA parses GroupVersion failed", "cronFederatedHPA", c.namespaceName, "groupVersion", cronFHPA.Spec.ScaleTargetRef.APIVersion)
 		return err
 	}
 	targetGVK := schema.GroupVersionKind{
@@ -188,37 +184,35 @@ func (c *ScalingJob) ScaleWorkloads(cronFHPA *autoscalingv1alpha1.CronFederatedH
 	targetResource.SetGroupVersionKind(targetGVK)
 	err = c.client.Get(ctx, types.NamespacedName{Namespace: cronFHPA.Namespace, Name: cronFHPA.Spec.ScaleTargetRef.Name}, targetResource)
 	if err != nil {
-		klog.Errorf("Get Resource(%s/%s) failed: %v", cronFHPA.Namespace, cronFHPA.Spec.ScaleTargetRef.Name, err)
+		klog.ErrorS(err, "Get resource failed", "namespace", cronFHPA.Namespace, "name", cronFHPA.Spec.ScaleTargetRef.Name)
 		return err
 	}
 
 	scaleObj := &unstructured.Unstructured{}
 	err = scaleClient.Get(ctx, targetResource, scaleObj)
 	if err != nil {
-		klog.Errorf("Get Scale for resource(%s/%s) failed: %v", cronFHPA.Namespace, cronFHPA.Spec.ScaleTargetRef.Name, err)
+		klog.ErrorS(err, "Get Scale for resource failed", "namespace", cronFHPA.Namespace, "name", cronFHPA.Spec.ScaleTargetRef.Name)
 		return err
 	}
 
 	scale := &autoscalingv1.Scale{}
 	err = helper.ConvertToTypedObject(scaleObj, scale)
 	if err != nil {
-		klog.Errorf("Convert Scale failed: %v", err)
+		klog.ErrorS(err, "Convert Scale failed", "namespace", cronFHPA.Namespace, "name", cronFHPA.Spec.ScaleTargetRef.Name)
 		return err
 	}
 
 	if scale.Spec.Replicas != *c.rule.TargetReplicas {
 		if err := helper.ApplyReplica(scaleObj, int64(*c.rule.TargetReplicas), util.ReplicasField); err != nil {
-			klog.Errorf("CronFederatedHPA(%s) applies Replicas for %s/%s failed: %v",
-				c.namespaceName, cronFHPA.Namespace, cronFHPA.Spec.ScaleTargetRef.Name, err)
+			klog.ErrorS(err, "CronFederatedHPA applies Replicas failed", "cronFederatedHPA", c.namespaceName, "namespace", cronFHPA.Namespace, "name", cronFHPA.Spec.ScaleTargetRef.Name)
 			return err
 		}
 		err := scaleClient.Update(ctx, targetResource, client.WithSubResourceBody(scaleObj))
 		if err != nil {
-			klog.Errorf("CronFederatedHPA(%s) updates scale resource failed: %v", c.namespaceName, err)
+			klog.ErrorS(err, "CronFederatedHPA updates scale resource failed", "cronFederatedHPA", c.namespaceName, "namespace", cronFHPA.Namespace, "name", cronFHPA.Spec.ScaleTargetRef.Name)
 			return err
 		}
-		klog.V(4).Infof("CronFederatedHPA(%s) scales resource(%s/%s) successfully",
-			c.namespaceName, cronFHPA.Namespace, cronFHPA.Spec.ScaleTargetRef.Name)
+		klog.V(4).InfoS("CronFederatedHPA scales resource successfully", "cronFederatedHPA", c.namespaceName, "namespace", cronFHPA.Namespace, "name", cronFHPA.Spec.ScaleTargetRef.Name)
 		return nil
 	}
 	return nil
@@ -258,12 +252,12 @@ func (c *ScalingJob) addFailedExecutionHistory(
 		})
 		return err
 	}); err != nil {
-		klog.Errorf("Failed to add failed history record to CronFederatedHPA(%s/%s): %v", cronFHPA.Namespace, cronFHPA.Name, err)
+		klog.ErrorS(err, "Failed to add failed history record to CronFederatedHPA", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name)
 		return err
 	}
 
 	if operationResult == controllerutil.OperationResultUpdatedStatusOnly {
-		klog.V(4).Infof("CronFederatedHPA(%s/%s) status has been updated successfully", cronFHPA.Namespace, cronFHPA.Name)
+		klog.V(4).InfoS("CronFederatedHPA status has been updated successfully", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name)
 	}
 
 	return nil
@@ -316,12 +310,12 @@ func (c *ScalingJob) addSuccessExecutionHistory(
 		})
 		return err
 	}); err != nil {
-		klog.Errorf("Failed to add success history record to CronFederatedHPA(%s/%s): %v", cronFHPA.Namespace, cronFHPA.Name, err)
+		klog.ErrorS(err, "Failed to add success history record to CronFederatedHPA", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name)
 		return err
 	}
 
 	if operationResult == controllerutil.OperationResultUpdatedStatusOnly {
-		klog.V(4).Infof("CronFederatedHPA(%s/%s) status has been updated successfully", cronFHPA.Namespace, cronFHPA.Name)
+		klog.V(4).InfoS("CronFederatedHPA status has been updated successfully", "namespace", cronFHPA.Namespace, "name", cronFHPA.Name)
 	}
 
 	return nil