Skip to content

Commit 99583d7

Browse files
CaesarTYCaesarTY
committed
refactor: enable JSON logging in mcs controller
Signed-off-by: yteng35 <[email protected]> nit Signed-off-by: yteng35 <[email protected]> address comments Signed-off-by: yteng35 <[email protected]> add separate log keys for the name and namespace Signed-off-by: yteng35 <[email protected]> add workName key to reduce confusion Signed-off-by: yteng35 <[email protected]>
1 parent f4919ab commit 99583d7

File tree

3 files changed

+44
-43
lines changed

3 files changed

+44
-43
lines changed

‎pkg/controllers/mcs/endpointslice_controller.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ type EndpointSliceController struct {
5454

5555
// Reconcile performs a full reconciliation for the object referred to by the Request.
5656
func (c *EndpointSliceController) Reconcile(ctx context.Context, req controllerruntime.Request) (controllerruntime.Result, error) {
57-
klog.V(4).Infof("Reconciling Work %s.", req.NamespacedName.String())
57+
klog.V(4).InfoS("Reconciling Work", "namespace", req.NamespacedName.Namespace, "name", req.NamespacedName.Name)
5858

5959
work := &workv1alpha1.Work{}
6060
if err := c.Client.Get(ctx, req.NamespacedName, work); err != nil {
@@ -69,7 +69,8 @@ func (c *EndpointSliceController) Reconcile(ctx context.Context, req controllerr
6969
if err := helper.DeleteEndpointSlice(ctx, c.Client, labels.Set{
7070
workv1alpha2.WorkPermanentIDLabel: work.Labels[workv1alpha2.WorkPermanentIDLabel],
7171
}); err != nil {
72-
klog.Errorf("Failed to delete endpointslice of the work(%s/%s) when deleting the work, err is %v", work.Namespace, work.Name, err)
72+
klog.ErrorS(err, "Failed to delete endpointslice when deleting work", "namespace",
73+
work.Namespace, "workName", work.Name)
7374
return controllerruntime.Result{}, err
7475
}
7576
return controllerruntime.Result{}, c.removeFinalizer(ctx, work.DeepCopy())
@@ -83,7 +84,7 @@ func (c *EndpointSliceController) Reconcile(ctx context.Context, req controllerr
8384
workv1alpha2.WorkPermanentIDLabel: work.Labels[workv1alpha2.WorkPermanentIDLabel],
8485
})
8586
if err != nil {
86-
klog.Errorf("Failed to delete endpointslice of the work(%s/%s) when the serviceexport is deleted, err is %v", work.Namespace, work.Name, err)
87+
klog.ErrorS(err, "Failed to delete endpointslice when serviceexport is deleted", "namespace", work.Namespace, "workName", work.Name)
8788
return controllerruntime.Result{}, err
8889
}
8990
return controllerruntime.Result{}, c.removeFinalizer(ctx, work.DeepCopy())
@@ -130,21 +131,21 @@ func (c *EndpointSliceController) SetupWithManager(mgr controllerruntime.Manager
130131
func (c *EndpointSliceController) collectEndpointSliceFromWork(ctx context.Context, work *workv1alpha1.Work) error {
131132
clusterName, err := names.GetClusterName(work.Namespace)
132133
if err != nil {
133-
klog.Errorf("Failed to get cluster name for work %s/%s", work.Namespace, work.Name)
134+
klog.ErrorS(err, "Failed to get cluster name for work", "namespace", work.Namespace, "workName", work.Name)
134135
return err
135136
}
136137

137138
for _, manifest := range work.Spec.Workload.Manifests {
138139
unstructObj := &unstructured.Unstructured{}
139140
if err := unstructObj.UnmarshalJSON(manifest.Raw); err != nil {
140-
klog.Errorf("Failed to unmarshal workload, error is: %v", err)
141+
klog.ErrorS(err, "Failed to unmarshal workload")
141142
return err
142143
}
143144

144145
endpointSlice := &discoveryv1.EndpointSlice{}
145146
err = helper.ConvertToTypedObject(unstructObj, endpointSlice)
146147
if err != nil {
147-
klog.Errorf("Failed to convert unstructured to typed object: %v", err)
148+
klog.ErrorS(err, "Failed to convert unstructured to typed object")
148149
return err
149150
}
150151

@@ -159,6 +160,7 @@ func (c *EndpointSliceController) collectEndpointSliceFromWork(ctx context.Conte
159160
})
160161

161162
if err = helper.CreateOrUpdateEndpointSlice(ctx, c.Client, desiredEndpointSlice); err != nil {
163+
klog.ErrorS(err, "Failed to create or update endpointslice", "namespace", desiredEndpointSlice.Namespace, "name", desiredEndpointSlice.Name)
162164
return err
163165
}
164166
}

‎pkg/controllers/mcs/service_export_controller.go‎

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ var (
9494

9595
// Reconcile performs a full reconciliation for the object referred to by the Request.
9696
func (c *ServiceExportController) Reconcile(ctx context.Context, req controllerruntime.Request) (controllerruntime.Result, error) {
97-
klog.V(4).Infof("Reconciling Work %s", req.NamespacedName.String())
97+
klog.V(4).InfoS("Reconciling Work", "namespace", req.NamespacedName.Namespace, "name", req.NamespacedName.Name)
9898

9999
work := &workv1alpha1.Work{}
100100
if err := c.Client.Get(ctx, req.NamespacedName, work); err != nil {
@@ -118,19 +118,20 @@ func (c *ServiceExportController) Reconcile(ctx context.Context, req controllerr
118118

119119
clusterName, err := names.GetClusterName(work.Namespace)
120120
if err != nil {
121-
klog.Errorf("Failed to get member cluster name for work %s/%s", work.Namespace, work.Name)
121+
klog.ErrorS(err, "Failed to get member cluster name for work", "namespace", work.Namespace, "workName", work.Name)
122122
return controllerruntime.Result{}, err
123123
}
124124

125125
cluster, err := util.GetCluster(c.Client, clusterName)
126126
if err != nil {
127-
klog.Errorf("Failed to get the given member cluster %s", clusterName)
127+
klog.ErrorS(err, "Failed to get the given member cluster", "cluster", clusterName)
128128
return controllerruntime.Result{}, err
129129
}
130130

131131
if !util.IsClusterReady(&cluster.Status) {
132-
klog.Errorf("Stop sync work(%s/%s) for cluster(%s) as cluster not ready.", work.Namespace, work.Name, cluster.Name)
133-
return controllerruntime.Result{}, fmt.Errorf("cluster(%s) not ready", cluster.Name)
132+
err := fmt.Errorf("cluster(%s) not ready", cluster.Name)
133+
klog.ErrorS(err, "Stop sync work for cluster as cluster not ready.", "namespace", work.Namespace, "workName", work.Name, "cluster", cluster.Name)
134+
return controllerruntime.Result{}, err
134135
}
135136

136137
return controllerruntime.Result{}, c.buildResourceInformers(cluster)
@@ -165,7 +166,7 @@ func (c *ServiceExportController) enqueueReportedEpsServiceExport() {
165166
indexregistry.WorkIndexByFieldSuspendDispatching: "true",
166167
})
167168
if err != nil {
168-
klog.Errorf("Failed to list collected EndpointSlices Work from member clusters: %v", err)
169+
klog.ErrorS(err, "Failed to list collected EndpointSlices Work from member clusters")
169170
return false, nil
170171
}
171172
return true, nil
@@ -208,23 +209,22 @@ func (c *ServiceExportController) syncServiceExportOrEndpointSlice(key util.Queu
208209
ctx := context.Background()
209210
fedKey, ok := key.(keys.FederatedKey)
210211
if !ok {
211-
klog.Errorf("Failed to sync serviceExport as invalid key: %v", key)
212-
return fmt.Errorf("invalid key")
212+
err := fmt.Errorf("invalid key")
213+
klog.ErrorS(err, "Failed to sync serviceExport as invalid key", "key", key)
214+
return err
213215
}
214216

215-
klog.V(4).Infof("Begin to sync %s", fedKey)
217+
klog.V(4).InfoS("Begin to sync", "key", fedKey)
216218

217219
switch fedKey.Kind {
218220
case util.ServiceExportKind:
219221
if err := c.handleServiceExportEvent(ctx, fedKey); err != nil {
220-
klog.Errorf("Failed to handle serviceExport(%s) event, Error: %v",
221-
fedKey.NamespaceKey(), err)
222+
klog.ErrorS(err, "Failed to handle serviceExport event", "serviceExport", fedKey.NamespaceKey())
222223
return err
223224
}
224225
case util.EndpointSliceKind:
225226
if err := c.handleEndpointSliceEvent(ctx, fedKey); err != nil {
226-
klog.Errorf("Failed to handle endpointSlice(%s) event, Error: %v",
227-
fedKey.NamespaceKey(), err)
227+
klog.ErrorS(err, "Failed to handle endpointSlice event", "endpointSlice", fedKey.NamespaceKey())
228228
return err
229229
}
230230
}
@@ -235,7 +235,7 @@ func (c *ServiceExportController) syncServiceExportOrEndpointSlice(key util.Queu
235235
func (c *ServiceExportController) buildResourceInformers(cluster *clusterv1alpha1.Cluster) error {
236236
err := c.registerInformersAndStart(cluster)
237237
if err != nil {
238-
klog.Errorf("Failed to register informer for Cluster %s. Error: %v.", cluster.Name, err)
238+
klog.ErrorS(err, "Failed to register informer for Cluster", "cluster", cluster.Name)
239239
return err
240240
}
241241
return nil
@@ -248,7 +248,7 @@ func (c *ServiceExportController) registerInformersAndStart(cluster *clusterv1al
248248
if singleClusterInformerManager == nil {
249249
dynamicClusterClient, err := c.ClusterDynamicClientSetFunc(cluster.Name, c.Client, c.ClusterClientOption)
250250
if err != nil {
251-
klog.Errorf("Failed to build dynamic cluster client for cluster %s.", cluster.Name)
251+
klog.ErrorS(err, "Failed to build dynamic cluster client for cluster.", "cluster", cluster.Name)
252252
return err
253253
}
254254
singleClusterInformerManager = c.InformerManager.ForCluster(dynamicClusterClient.ClusterName, dynamicClusterClient.DynamicClientSet, 0)
@@ -284,7 +284,7 @@ func (c *ServiceExportController) registerInformersAndStart(cluster *clusterv1al
284284
}
285285
return nil
286286
}(); err != nil {
287-
klog.Errorf("Failed to sync cache for cluster: %s, error: %v", cluster.Name, err)
287+
klog.ErrorS(err, "Failed to sync cache for cluster", "cluster", cluster.Name)
288288
c.InformerManager.Stop(cluster.Name)
289289
return err
290290
}
@@ -309,7 +309,7 @@ func (c *ServiceExportController) genHandlerAddFunc(clusterName string) func(obj
309309
curObj := obj.(runtime.Object)
310310
key, err := keys.FederatedKeyFunc(clusterName, curObj)
311311
if err != nil {
312-
klog.Warningf("Failed to generate key for obj: %s", curObj.GetObjectKind().GroupVersionKind())
312+
klog.ErrorS(err, "Failed to generate key for obj", "gvk", curObj.GetObjectKind().GroupVersionKind())
313313
return
314314
}
315315
c.worker.Add(key)
@@ -322,7 +322,7 @@ func (c *ServiceExportController) genHandlerUpdateFunc(clusterName string) func(
322322
if !reflect.DeepEqual(oldObj, newObj) {
323323
key, err := keys.FederatedKeyFunc(clusterName, curObj)
324324
if err != nil {
325-
klog.Warningf("Failed to generate key for obj: %s", curObj.GetObjectKind().GroupVersionKind())
325+
klog.ErrorS(err, "Failed to generate key for obj", "gvk", curObj.GetObjectKind().GroupVersionKind())
326326
return
327327
}
328328
c.worker.Add(key)
@@ -342,7 +342,7 @@ func (c *ServiceExportController) genHandlerDeleteFunc(clusterName string) func(
342342
oldObj := obj.(runtime.Object)
343343
key, err := keys.FederatedKeyFunc(clusterName, oldObj)
344344
if err != nil {
345-
klog.Warningf("Failed to generate key for obj: %s", oldObj.GetObjectKind().GroupVersionKind())
345+
klog.ErrorS(err, "Failed to generate key for obj", "gvk", oldObj.GetObjectKind().GroupVersionKind())
346346
return
347347
}
348348
c.worker.Add(key)
@@ -366,8 +366,7 @@ func (c *ServiceExportController) handleServiceExportEvent(ctx context.Context,
366366
// If skip report here, after ServiceExport deletion and re-creation, if no EndpointSlice changes, we didn't get a
367367
// change to sync.
368368
if err = c.reportEndpointSliceWithServiceExportCreate(ctx, serviceExportKey); err != nil {
369-
klog.Errorf("Failed to handle ServiceExport(%s) event, Error: %v",
370-
serviceExportKey.NamespaceKey(), err)
369+
klog.ErrorS(err, "Failed to handle ServiceExport event", "namespace", serviceExportKey.Namespace, "name", serviceExportKey.Name)
371370
return err
372371
}
373372

@@ -387,8 +386,7 @@ func (c *ServiceExportController) handleEndpointSliceEvent(ctx context.Context,
387386
}
388387

389388
if err = c.reportEndpointSliceWithEndpointSliceCreateOrUpdate(ctx, endpointSliceKey.Cluster, endpointSliceObj); err != nil {
390-
klog.Errorf("Failed to handle endpointSlice(%s) event, Error: %v",
391-
endpointSliceKey.NamespaceKey(), err)
389+
klog.ErrorS(err, "Failed to handle endpointSlice event", "namespace", endpointSliceKey.Namespace, "name", endpointSliceKey.Name)
392390
return err
393391
}
394392

@@ -445,8 +443,9 @@ func (c *ServiceExportController) removeOrphanWork(ctx context.Context, endpoint
445443
}),
446444
FieldSelector: fields.OneTermEqualSelector(indexregistry.WorkIndexByFieldSuspendDispatching, "true"),
447445
}); err != nil {
448-
klog.Errorf("Failed to list endpointslice work with serviceExport(%s/%s) under namespace %s: %v",
449-
serviceExportKey.Namespace, serviceExportKey.Name, names.GenerateExecutionSpaceName(serviceExportKey.Cluster), err)
446+
klog.ErrorS(err, "Failed to list workList reported by ServiceExport in executionSpace",
447+
"namespace", serviceExportKey.Namespace, "name", serviceExportKey.Name, "executionSpace",
448+
names.GenerateExecutionSpaceName(serviceExportKey.Cluster))
450449
return err
451450
}
452451

@@ -490,7 +489,7 @@ func (c *ServiceExportController) reportEndpointSliceWithEndpointSliceCreateOrUp
490489
return nil
491490
}
492491

493-
klog.Errorf("Failed to get ServiceExport object %s/%s. error: %v.", relatedServiceName, endpointSlice.GetNamespace(), err)
492+
klog.ErrorS(err, "Failed to get ServiceExport object", "namespace", endpointSlice.GetNamespace(), "name", relatedServiceName)
494493
return err
495494
}
496495

@@ -522,7 +521,7 @@ func getEndpointSliceWorkMeta(ctx context.Context, c client.Client, ns string, w
522521
Namespace: ns,
523522
Name: workName,
524523
}, existWork); err != nil && !apierrors.IsNotFound(err) {
525-
klog.Errorf("Get EndpointSlice work(%s/%s) error:%v", ns, workName, err)
524+
klog.ErrorS(err, "Failed to get EndpointSlice work", "namespace", ns, "name", workName)
526525
return metav1.ObjectMeta{}, err
527526
}
528527

@@ -562,8 +561,8 @@ func cleanupWorkWithServiceExportDelete(ctx context.Context, c client.Client, se
562561
util.ServiceNameLabel: serviceExportKey.Name,
563562
}),
564563
}); err != nil {
565-
klog.Errorf("Failed to list workList reported by ServiceExport(%s) in executionSpace(%s), Error: %v",
566-
serviceExportKey.NamespaceKey(), executionSpace, err)
564+
klog.ErrorS(err, "Failed to list workList reported by ServiceExport in executionSpace",
565+
"namespace", serviceExportKey.Namespace, "name", serviceExportKey.Name, "executionSpace", executionSpace)
567566
return err
568567
}
569568

@@ -589,7 +588,7 @@ func cleanupWorkWithEndpointSliceDelete(ctx context.Context, c client.Client, en
589588
return nil
590589
}
591590

592-
klog.Errorf("Failed to get work(%s) in executionSpace(%s), Error: %v", workNamespaceKey.String(), executionSpace, err)
591+
klog.ErrorS(err, "Failed to get work in executionSpace", "work", workNamespaceKey.String(), "executionSpace", executionSpace)
593592
return err
594593
}
595594

@@ -611,14 +610,14 @@ func cleanEndpointSliceWork(ctx context.Context, c client.Client, work *workv1al
611610
work.Labels[util.EndpointSliceWorkManagedByLabel] = strings.Join(controllerSet.UnsortedList(), ".")
612611

613612
if err := c.Update(ctx, work); err != nil {
614-
klog.Errorf("Failed to update work(%s/%s): %v", work.Namespace, work.Name, err)
613+
klog.ErrorS(err, "Failed to update work", "namespace", work.Namespace, "workName", work.Name)
615614
return err
616615
}
617616
return nil
618617
}
619618

620619
if err := c.Delete(ctx, work); err != nil {
621-
klog.Errorf("Failed to delete work(%s/%s), Error: %v", work.Namespace, work.Name, err)
620+
klog.ErrorS(err, "Failed to delete work", "namespace", work.Namespace, "workName", work.Name)
622621
return err
623622
}
624623

‎pkg/controllers/mcs/service_import_controller.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ type ServiceImportController struct {
4949

5050
// Reconcile performs a full reconciliation for the object referred to by the Request.
5151
func (c *ServiceImportController) Reconcile(ctx context.Context, req controllerruntime.Request) (controllerruntime.Result, error) {
52-
klog.V(4).Infof("Reconciling ServiceImport %s.", req.NamespacedName.String())
52+
klog.V(4).InfoS("Reconciling ServiceImport", "name", req.NamespacedName.String())
5353

5454
svcImport := &mcsv1alpha1.ServiceImport{}
5555
if err := c.Client.Get(ctx, req.NamespacedName, svcImport); err != nil {
@@ -98,7 +98,7 @@ func (c *ServiceImportController) deleteDerivedService(ctx context.Context, svcI
9898

9999
err = c.Client.Delete(ctx, derivedSvc)
100100
if err != nil && !apierrors.IsNotFound(err) {
101-
klog.Errorf("Delete derived service(%s) failed, Error: %v", derivedSvcNamespacedName, err)
101+
klog.ErrorS(err, "Delete derived service failed", "name", derivedSvcNamespacedName)
102102
return controllerruntime.Result{}, err
103103
}
104104

@@ -125,7 +125,7 @@ func (c *ServiceImportController) deriveServiceFromServiceImport(ctx context.Con
125125
if err != nil {
126126
if apierrors.IsNotFound(err) {
127127
if err = c.Client.Create(ctx, newDerivedService); err != nil {
128-
klog.Errorf("Create derived service(%s/%s) failed, Error: %v", newDerivedService.Namespace, newDerivedService.Name, err)
128+
klog.ErrorS(err, "Create derived service failed", "namespace", newDerivedService.Namespace, "name", newDerivedService.Name)
129129
return err
130130
}
131131

@@ -140,7 +140,7 @@ func (c *ServiceImportController) deriveServiceFromServiceImport(ctx context.Con
140140

141141
err = c.Client.Update(ctx, newDerivedService)
142142
if err != nil {
143-
klog.Errorf("Update derived service(%s/%s) failed, Error: %v", newDerivedService.Namespace, newDerivedService.Name, err)
143+
klog.ErrorS(err, "Update derived service failed", "namespace", newDerivedService.Namespace, "name", newDerivedService.Name)
144144
return err
145145
}
146146

@@ -169,7 +169,7 @@ func (c *ServiceImportController) updateServiceStatus(ctx context.Context, svcIm
169169
})
170170

171171
if err != nil {
172-
klog.Errorf("Update derived service(%s/%s) status failed, Error: %v", derivedService.Namespace, derivedService.Name, err)
172+
klog.ErrorS(err, "Update derived service status failed", "namespace", derivedService.Namespace, "name", derivedService.Name)
173173
return err
174174
}
175175
return nil

0 commit comments

Comments
 (0)