Adjust log level and fix log format

Signed-off-by: Yanjun Zhou <yanjun.zhou@broadcom.com>

Author: yanjunz97

pkg/config/config.go

@@ -390,7 +390,7 @@ func (nsxConfig *NsxConfig) validateCert() error {
 		configLog.Infof("Validate thumbprint: %d", tpCount)
 		if tpCount > 1 && tpCount != mCount {
 			err := errors.New("thumbprint count not match manager count")
-			configLog.Error(err, "validate NsxConfig failed", "thumbprint count", tpCount, "manager count", mCount)
+			configLog.Error(err, "Validate NsxConfig failed", "thumbprint count", tpCount, "manager count", mCount)
 			return err
 		}
 	}

pkg/controllers/common/utils.go

@@ -49,9 +49,8 @@ func AllocateSubnetFromSubnetSet(subnetSet *v1alpha1.SubnetSet, vpcService servi
 	log.Info("The existing subnets are not available, creating new subnet", "subnetList", subnetList, "subnetSet.Name", subnetSet.Name, "subnetSet.Namespace", subnetSet.Namespace)
 	vpcInfoList := vpcService.ListVPCInfo(subnetSet.Namespace)
 	if len(vpcInfoList) == 0 {
-		err := errors.New("no VPC found")
-		log.Error(err, "Failed to allocate Subnet")
-		return "", err
+		log.Warn("No VPC found for SubnetSet, will retry later", "Namespace", subnetSet.Namespace)
+		return "", errors.New("no VPC found, will retry later")
 	}
 	nsxSubnet, err := subnetService.CreateOrUpdateSubnet(subnetSet, vpcInfoList[0], tags)
 	if err != nil {

pkg/controllers/ipaddressallocation/ipaddressallocation_controller.go

@@ -5,7 +5,6 @@ package ipaddressallocation
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	v1 "k8s.io/api/core/v1"
@@ -82,7 +81,9 @@ func setReadyStatusTrue(client client.Client, ctx context.Context, obj client.Ob
 	e := client.Status().Update(ctx, ipaddressallocation)
 	if e != nil {
 		log.Error(e, "Unable to update IPAddressAllocation status", "IPAddressAllocation", ipaddressallocation)
+		return
 	}
+	log.Debug("Updated IPAddressAllocation", "Name", ipaddressallocation.Name, "Namespace", ipaddressallocation.Namespace, "Conditions", conditions)
 }
 
 func (r *IPAddressAllocationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
@@ -189,7 +190,7 @@ func (r *IPAddressAllocationReconciler) RestoreReconcile() error {
 		}
 	}
 	if len(errorList) > 0 {
-		return errors.Join(errorList...)
+		return fmt.Errorf("errors found in IPAddressAllocation restore: %v", errorList)
 	}
 	return nil
 }
@@ -213,7 +214,7 @@ func (r *IPAddressAllocationReconciler) getRestoreList() ([]types.NamespacedName
 
 func (r *IPAddressAllocationReconciler) StartController(mgr ctrl.Manager, hookServer webhook.Server) error {
 	if err := r.setupWithManager(mgr); err != nil {
-		log.Error(err, "Failed to create ipaddressallocation controller")
+		log.Error(err, "Failed to create IPAddressAllocation controller")
 		return err
 	}
 	if hookServer != nil {

pkg/controllers/ipaddressallocation/ipaddressallocation_webhook.go

@@ -39,14 +39,14 @@ func (v *IPAddressAllocationValidator) Handle(ctx context.Context, req admission
 		err = v.decoder.Decode(req, ipAddressAllocation)
 	}
 	if err != nil {
-		log.Error(err, "error while decoding IPAddressAllocation", "IPAddressAllocation", req.Namespace+"/"+req.Name)
+		log.Error(err, "Error while decoding IPAddressAllocation", "IPAddressAllocation", req.Namespace+"/"+req.Name)
 		return admission.Errored(http.StatusBadRequest, err)
 	}
 	switch req.Operation {
 	case admissionv1.Delete:
 		existingAddressBindingList := &v1alpha1.AddressBindingList{}
 		if err := v.Client.List(context.TODO(), existingAddressBindingList, client.InNamespace(ipAddressAllocation.Namespace), client.MatchingFields{util.AddressBindingIPAddressAllocationNameIndexKey: ipAddressAllocation.Name}); err != nil {
-			log.Error(err, "failed to list AddressBindings", "Namespace", ipAddressAllocation.Namespace)
+			log.Error(err, "Failed to list AddressBindings", "Namespace", ipAddressAllocation.Namespace)
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 		if len(existingAddressBindingList.Items) > 0 {

pkg/controllers/namespace/namespace_controller.go

@@ -177,8 +177,11 @@ func (r *NamespaceReconciler) deleteDefaultSubnetSet(ns string) error {
 }
 
 func (r *NamespaceReconciler) namespaceError(ctx context.Context, k8sObj client.Object, msg string, err error) {
-	logErr := util.If(err == nil, errors.New(msg), err).(error)
-	log.Error(logErr, msg)
+	if err != nil {
+		log.Error(err, msg)
+	} else {
+		log.Warn(msg)
+	}
 	changes := map[string]string{common.AnnotationNamespaceVPCError: msg}
 	util.UpdateK8sResourceAnnotation(r.Client, ctx, k8sObj, changes)
 }
@@ -229,14 +232,16 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			return common.ResultRequeue, nil
 		}
 		if !ncExist {
+			// This is expected when the Namespace is just created
+			// as NetworkConfig is created after Namespace creation
 			message := fmt.Sprintf("missing NetworkConfig %s for Namespace %s", ncName, ns)
 			r.namespaceError(ctx, obj, message, nil)
-			return common.ResultRequeueAfter10sec, errors.New(message)
+			return common.ResultRequeueAfter10sec, nil
 		}
 		if err = r.VPCService.ValidateNetworkConfig(nc); err != nil {
 			// if network config is not valid, no need to retry, skip processing
 			message := fmt.Sprintf("invalid NetworkConfig %s for Namespace %s, error: %v", ncName, ns, err)
-			r.namespaceError(ctx, obj, message, nil)
+			r.namespaceError(ctx, obj, message, err)
 			return common.ResultRequeueAfter10sec, errors.New(message)
 		}
 

pkg/controllers/namespace/namespace_controller_test.go

@@ -203,8 +203,7 @@ func TestNamespaceReconciler_Reconcile(t *testing.T) {
 				})
 				return patches
 			},
-			expectErrStr: "missing NetworkConfig",
-			expectRes:    ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second},
+			expectRes: ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second},
 			existingNamespaceCR: &v1.Namespace{
 				TypeMeta:   metav1.TypeMeta{},
 				ObjectMeta: metav1.ObjectMeta{Name: "test-ns"},

pkg/controllers/networkinfo/networkinfo_controller.go

@@ -5,7 +5,6 @@ package networkinfo
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"time"
 
@@ -666,7 +665,7 @@ func (r *NetworkInfoReconciler) RestoreReconcile() error {
 		}
 	}
 	if len(errorList) > 0 {
-		return errors.Join(errorList...)
+		return fmt.Errorf("errors found in NetworkInfo restore: %v", errorList)
 	}
 	return nil
 }

pkg/controllers/networkinfo/networkinfo_utils.go

@@ -30,7 +30,9 @@ func setNetworkInfoVPCStatus(client client.Client, ctx context.Context, obj clie
 	if args[0] == nil {
 		// if createdVPC is empty, remove the VPC from networkInfo
 		networkInfo.VPCs = []v1alpha1.VPCState{}
-		client.Update(ctx, networkInfo)
+		if err := client.Update(ctx, networkInfo); err != nil {
+			log.Error(err, "Failed to update NetworkInfo CR", "NetworkInfo", networkInfo)
+		}
 		return
 	} else {
 		createdVPC = args[0].(*v1alpha1.VPCState)
@@ -45,7 +47,9 @@ func setNetworkInfoVPCStatus(client client.Client, ctx context.Context, obj clie
 		return
 	}
 	networkInfo.VPCs = []v1alpha1.VPCState{*createdVPC}
-	client.Update(ctx, networkInfo)
+	if err := client.Update(ctx, networkInfo); err != nil {
+		log.Error(err, "Failed to update NetworkInfo CR", "NetworkInfo", networkInfo)
+	}
 	return
 }
 
@@ -71,7 +75,7 @@ func setVPCNetworkConfigurationStatusWithLBS(ctx context.Context, client client.
 		log.Error(err, "Update VPCNetworkConfiguration status failed", "ncName", ncName, "vpcName", vpcName, "nc.Status.VPCs", nc.Status.VPCs)
 		return
 	}
-	log.Info("Updated VPCNetworkConfiguration status", "ncName", ncName, "vpcName", vpcName, "nc.Status.VPCs", nc.Status.VPCs)
+	log.Debug("Updated VPCNetworkConfiguration status", "ncName", ncName, "vpcName", vpcName, "nc.Status.VPCs", nc.Status.VPCs)
 }
 
 func setVPCNetworkConfigurationStatusWithGatewayConnection(ctx context.Context, client client.Client, nc *v1alpha1.VPCNetworkConfiguration, connectionStatus *common.VPCConnectionStatus) {
@@ -102,8 +106,12 @@ func setVPCNetworkConfigurationStatusWithGatewayConnection(ctx context.Context,
 		}
 	}
 	if conditionsUpdated {
-		client.Status().Update(ctx, nc)
-		log.Info("Set VPCNetworkConfiguration status", "ncName", nc.Name, "condition", newConditions)
+		err := client.Status().Update(ctx, nc)
+		if err != nil {
+			log.Error(err, "Failed to update VPCNetworkConfiguration status", "Name", nc.Name)
+			return
+		}
+		log.Debug("Updated VPCNetworkConfiguration", "Name", nc.Name, "New Conditions", newConditions)
 	}
 }
 
@@ -125,7 +133,12 @@ func setVPCNetworkConfigurationStatusWithSnatEnabled(ctx context.Context, client
 		}
 	}
 	if conditionsUpdated {
-		client.Status().Update(ctx, nc)
+		err := client.Status().Update(ctx, nc)
+		if err != nil {
+			log.Error(err, "Failed to update VPCNetworkConfiguration status", "Name", nc.Name)
+			return
+		}
+		log.Debug("Updated VPCNetworkConfiguration", "Name", nc.Name, "New Conditions", newConditions)
 	}
 }
 
@@ -147,7 +160,7 @@ func setVPCNetworkConfigurationStatusWithNoExternalIPBlock(ctx context.Context,
 			return
 		}
 	}
-	log.Info("Updated VPCNetworkConfiguration status", "VPCNetworkConfiguration", nc.Name, "status", newCondition)
+	log.Debug("Updated VPCNetworkConfiguration status", "VPCNetworkConfiguration", nc.Name, "status", newCondition)
 }
 
 // TODO: abstract the logic of merging condition for common, which can be used by the other controller, e.g. security policy
@@ -252,7 +265,7 @@ func updateVPCNetworkConfigurationStatusWithAliveVPCs(ctx context.Context, clien
 			log.Error(err, "Failed to update VPCNetworkConfiguration status", "Name", ncName, "nc.Status.VPCs", nc.Status.VPCs)
 			return
 		}
-		log.Info("Updated VPCNetworkConfiguration status", "Name", ncName, "nc.Status.VPCs", nc.Status.VPCs)
+		log.Debug("Updated VPCNetworkConfiguration status", "Name", ncName, "nc.Status.VPCs", nc.Status.VPCs)
 	}
 }
 
@@ -301,7 +314,7 @@ func setNSNetworkReadyCondition(ctx context.Context, kubeClient client.Client, n
 		log.Error(err, "Failed to update Namespace status", "Namespace", nsName)
 		return
 	}
-	log.Info("Updated Namespace network condition", "Namespace", nsName, "status", condition.Status, "reason", condition.Reason, "message", condition.Message)
+	log.Debug("Updated Namespace network condition", "Namespace", nsName, "status", condition.Status, "reason", condition.Reason, "message", condition.Message)
 }
 
 // nsConditionEquals compares the old and new Namespace condition. The compare ignores the differences in field

pkg/controllers/networkinfo/vpcnetworkconfig_handler.go

@@ -39,7 +39,7 @@ func (h *VPCNetworkConfigurationHandler) Create(ctx context.Context, e event.Cre
 func (h *VPCNetworkConfigurationHandler) Delete(ctx context.Context, e event.DeleteEvent, _ workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	vpcConfigCR := e.Object.(*v1alpha1.VPCNetworkConfiguration)
 	if err := h.ipBlocksInfoService.SyncIPBlocksInfo(ctx); err != nil {
-		log.Error(err, "failed to synchronize IPBlocksInfo when deleting %s", vpcConfigCR.Name)
+		log.Error(err, "Failed to synchronize IPBlocksInfo when deleting %s", vpcConfigCR.Name)
 	} else {
 		h.ipBlocksInfoService.ResetPeriodicSync()
 	}

pkg/controllers/node/node_controller.go

@@ -5,7 +5,6 @@ package node
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"os"
 	"reflect"
@@ -41,35 +40,35 @@ type NodeReconciler struct {
 func (r *NodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	node := &v1.Node{}
 	deleted := false
-	log.Info("reconciling node", "node", req.NamespacedName)
+	log.Info("Reconciling node", "node", req.NamespacedName)
 
 	metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerSyncTotal, MetricResTypeNode)
 
 	if err := r.Client.Get(ctx, req.NamespacedName, node); err != nil {
 		if apierrors.IsNotFound(err) {
-			log.Info("node not found", "req", req.NamespacedName)
+			log.Info("Node not found", "req", req.NamespacedName)
 			deleted = true
 			if err := r.Service.SyncNodeStore(req.NamespacedName.Name, deleted); err != nil {
-				log.Error(err, "failed to sync node store", "req", req.NamespacedName)
+				log.Error(err, "Failed to sync node store", "req", req.NamespacedName)
 				return common.ResultNormal, err
 			}
 		} else {
-			log.Error(err, "unable to fetch node", "req", req.NamespacedName)
+			log.Error(err, "Unable to fetch node", "req", req.NamespacedName)
 		}
 		return common.ResultNormal, client.IgnoreNotFound(err)
 	}
 	if common.NodeIsMaster(node) {
 		// For WCP supervisor cluster, the master node isn't a transport node.
-		log.Info("skipping handling master node", "node", req.NamespacedName)
+		log.Info("Skipping handling master node", "node", req.NamespacedName)
 		return common.ResultNormal, nil
 	}
 	if !node.ObjectMeta.DeletionTimestamp.IsZero() {
-		log.Info("node is being deleted", "node", req.NamespacedName)
+		log.Info("Node is being deleted", "node", req.NamespacedName)
 		deleted = true
 	}
 
 	if err := r.Service.SyncNodeStore(node.Name, deleted); err != nil {
-		log.Error(err, "failed to sync node store", "req", req.NamespacedName)
+		log.Error(err, "Failed to sync node store", "req", req.NamespacedName)
 		return common.ResultNormal, err
 	}
 	return common.ResultNormal, nil
@@ -92,7 +91,7 @@ func StartNodeController(mgr ctrl.Manager, nodeService *node.NodeService) {
 	nodePortReconciler.Service = nodeService
 
 	if err := nodePortReconciler.Start(mgr); err != nil {
-		log.Error(err, "failed to create controller", "controller", "Node")
+		log.Error(err, "Failed to create controller", "controller", "Node")
 		os.Exit(1)
 	}
 }
@@ -113,7 +112,7 @@ func (r *NodeReconciler) RestoreReconcile() error {
 		}
 	}
 	if len(errorList) > 0 {
-		return errors.Join(errorList...)
+		return fmt.Errorf("errors found in Node restore: %v", errorList)
 	}
 	return nil
 }
@@ -124,7 +123,7 @@ func (r *NodeReconciler) CollectGarbage(_ context.Context) error {
 
 func (r *NodeReconciler) StartController(mgr ctrl.Manager, _ webhook.Server) error {
 	if err := r.Start(mgr); err != nil {
-		log.Error(err, "failed to create controller", "controller", "Node")
+		log.Error(err, "Failed to create controller", "controller", "Node")
 		return err
 	}
 	return nil