Skip to content

Commit 1dbb531

Browse files
xueli181114xueli181114
committed
HYPERFLEET-630 - fix: consolidate duplicate resource operation logs
Remove redundant pre/post operation logs from k8s_client and maestro_client CRUD methods. The executor layer provides a single authoritative INFO log per resource operation. The apply-layer decision log (operation + reason) is retained at DEBUG level. Handle "already exists" errors from concurrent creates gracefully by treating them as a successful skip instead of an ERROR.
1 parent 333054e commit 1dbb531

4 files changed

Lines changed: 8 additions & 55 deletions

File tree

‎internal/k8s_client/apply.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ func (c *Client) ApplyManifest(
106106
switch result.Operation {
107107
case manifest.OperationCreate:
108108
_, applyErr = c.CreateResource(ctx, newManifest)
109+
if applyErr != nil && apierrors.IsAlreadyExists(applyErr) {
110+
// Resource was created by a concurrent process between our Get and Create.
111+
// Treat as a successful no-op rather than an error.
112+
c.log.Debugf(ctx, "Resource %s/%s already exists (concurrent create), treating as skip", gvk.Kind, name)
113+
result.Operation = manifest.OperationSkip
114+
result.Reason = "already exists (concurrent create)"
115+
applyErr = nil
116+
}
109117

110118
case manifest.OperationUpdate:
111119
// Preserve resourceVersion and UID from existing for update

‎internal/k8s_client/client.go‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,6 @@ func (c *Client) CreateResource(ctx context.Context, obj *unstructured.Unstructu
120120
namespace := obj.GetNamespace()
121121
name := obj.GetName()
122122

123-
c.log.Infof(ctx, "Creating resource: %s/%s (namespace: %s)", gvk.Kind, name, namespace)
124-
125123
err := c.client.Create(ctx, obj)
126124
if err != nil {
127125
if apierrors.IsAlreadyExists(err) {
@@ -136,15 +134,11 @@ func (c *Client) CreateResource(ctx context.Context, obj *unstructured.Unstructu
136134
Err: err,
137135
}
138136
}
139-
140-
c.log.Infof(ctx, "Successfully created resource: %s/%s", gvk.Kind, name)
141137
return obj, nil
142138
}
143139

144140
// GetResource retrieves a specific Kubernetes resource by GVK, namespace, and name
145141
func (c *Client) GetResource(ctx context.Context, gvk schema.GroupVersionKind, namespace, name string, _ transport_client.TransportContext) (*unstructured.Unstructured, error) {
146-
c.log.Infof(ctx, "Getting resource: %s/%s (namespace: %s)", gvk.Kind, name, namespace)
147-
148142
obj := &unstructured.Unstructured{}
149143
obj.SetGroupVersionKind(gvk)
150144

@@ -168,8 +162,6 @@ func (c *Client) GetResource(ctx context.Context, gvk schema.GroupVersionKind, n
168162
Err: err,
169163
}
170164
}
171-
172-
c.log.Infof(ctx, "Successfully retrieved resource: %s/%s", gvk.Kind, name)
173165
return obj, nil
174166
}
175167

@@ -182,8 +174,6 @@ func (c *Client) GetResource(ctx context.Context, gvk schema.GroupVersionKind, n
182174
//
183175
// For more flexible discovery (including by-name lookup), use DiscoverResources() instead.
184176
func (c *Client) ListResources(ctx context.Context, gvk schema.GroupVersionKind, namespace string, labelSelector string) (*unstructured.UnstructuredList, error) {
185-
c.log.Infof(ctx, "Listing resources: %s (namespace: %s, labelSelector: %s)", gvk.Kind, namespace, labelSelector)
186-
187177
list := &unstructured.UnstructuredList{}
188178
list.SetGroupVersionKind(gvk)
189179

@@ -215,7 +205,6 @@ func (c *Client) ListResources(ctx context.Context, gvk schema.GroupVersionKind,
215205
}
216206
}
217207

218-
c.log.Infof(ctx, "Successfully listed resources: %s (found %d items)", gvk.Kind, len(list.Items))
219208
return list, nil
220209
}
221210

@@ -245,8 +234,6 @@ func (c *Client) UpdateResource(ctx context.Context, obj *unstructured.Unstructu
245234
namespace := obj.GetNamespace()
246235
name := obj.GetName()
247236

248-
c.log.Infof(ctx, "Updating resource: %s/%s (namespace: %s)", gvk.Kind, name, namespace)
249-
250237
err := c.client.Update(ctx, obj)
251238
if err != nil {
252239
if apierrors.IsConflict(err) {
@@ -261,15 +248,11 @@ func (c *Client) UpdateResource(ctx context.Context, obj *unstructured.Unstructu
261248
Err: err,
262249
}
263250
}
264-
265-
c.log.Infof(ctx, "Successfully updated resource: %s/%s", gvk.Kind, name)
266251
return obj, nil
267252
}
268253

269254
// DeleteResource deletes a Kubernetes resource
270255
func (c *Client) DeleteResource(ctx context.Context, gvk schema.GroupVersionKind, namespace, name string) error {
271-
c.log.Infof(ctx, "Deleting resource: %s/%s (namespace: %s)", gvk.Kind, name, namespace)
272-
273256
obj := &unstructured.Unstructured{}
274257
obj.SetGroupVersionKind(gvk)
275258
obj.SetNamespace(namespace)
@@ -278,7 +261,6 @@ func (c *Client) DeleteResource(ctx context.Context, gvk schema.GroupVersionKind
278261
err := c.client.Delete(ctx, obj)
279262
if err != nil {
280263
if apierrors.IsNotFound(err) {
281-
c.log.Infof(ctx, "Resource already deleted: %s/%s", gvk.Kind, name)
282264
return nil
283265
}
284266
return &apperrors.K8sOperationError{
@@ -290,8 +272,6 @@ func (c *Client) DeleteResource(ctx context.Context, gvk schema.GroupVersionKind
290272
Err: err,
291273
}
292274
}
293-
294-
c.log.Infof(ctx, "Successfully deleted resource: %s/%s", gvk.Kind, name)
295275
return nil
296276
}
297277

@@ -321,8 +301,6 @@ func (c *Client) DeleteResource(ctx context.Context, gvk schema.GroupVersionKind
321301
// patchData := []byte(`{"metadata":{"labels":{"new-label":"value"}}}`)
322302
// patched, err := client.PatchResource(ctx, gvk, "default", "my-cm", patchData)
323303
func (c *Client) PatchResource(ctx context.Context, gvk schema.GroupVersionKind, namespace, name string, patchData []byte) (*unstructured.Unstructured, error) {
324-
c.log.Infof(ctx, "Patching resource: %s/%s (namespace: %s)", gvk.Kind, name, namespace)
325-
326304
// Parse patch data to validate JSON
327305
var patchObj map[string]interface{}
328306
if err := json.Unmarshal(patchData, &patchObj); err != nil {
@@ -355,8 +333,6 @@ func (c *Client) PatchResource(ctx context.Context, gvk schema.GroupVersionKind,
355333
}
356334
}
357335

358-
c.log.Infof(ctx, "Successfully patched resource: %s/%s", gvk.Kind, name)
359-
360336
// Get the updated resource to return
361337
return c.GetResource(ctx, gvk, namespace, name, nil)
362338
}

‎internal/maestro_client/client.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -469,8 +469,6 @@ func (c *Client) ApplyResource(
469469
// Set namespace to consumer name
470470
work.Namespace = consumerName
471471

472-
c.log.Infof(ctx, "Applying ManifestWork %s/%s", consumerName, work.Name)
473-
474472
// Apply the ManifestWork (create or update with generation comparison)
475473
result, err := c.ApplyManifestWork(ctx, consumerName, work)
476474
if err != nil {

‎internal/maestro_client/operations.go‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,6 @@ func (c *Client) CreateManifestWork(
5151
ctx = logger.WithLogField(ctx, "manifestwork", work.Name)
5252
ctx = logger.WithObservedGeneration(ctx, manifest.GetGeneration(work.ObjectMeta))
5353

54-
c.log.WithFields(map[string]interface{}{
55-
"manifests": len(work.Spec.Workload.Manifests),
56-
}).Debug(ctx, "Creating ManifestWork")
57-
5854
// Set namespace to consumer name (required by Maestro)
5955
work.Namespace = consumerName
6056

@@ -68,7 +64,6 @@ func (c *Client) CreateManifestWork(
6864
consumerName, work.Name, err)
6965
}
7066

71-
c.log.Info(ctx, "Created ManifestWork")
7267
return created, nil
7368
}
7469

@@ -81,8 +76,6 @@ func (c *Client) GetManifestWork(
8176
ctx = logger.WithMaestroConsumer(ctx, consumerName)
8277
ctx = logger.WithLogField(ctx, "manifestwork", workName)
8378

84-
c.log.Debug(ctx, "Getting ManifestWork")
85-
8679
work, err := c.workClient.ManifestWorks(consumerName).Get(ctx, workName, metav1.GetOptions{})
8780
if err != nil {
8881
// Return not found error without wrapping for callers to check
@@ -106,8 +99,6 @@ func (c *Client) PatchManifestWork(
10699
ctx = logger.WithMaestroConsumer(ctx, consumerName)
107100
ctx = logger.WithLogField(ctx, "manifestwork", workName)
108101

109-
c.log.Debug(ctx, "Patching ManifestWork")
110-
111102
patched, err := c.workClient.ManifestWorks(consumerName).Patch(
112103
ctx,
113104
workName,
@@ -120,7 +111,6 @@ func (c *Client) PatchManifestWork(
120111
consumerName, workName, err)
121112
}
122113

123-
c.log.Info(ctx, "Patched ManifestWork")
124114
return patched, nil
125115
}
126116

@@ -133,20 +123,16 @@ func (c *Client) DeleteManifestWork(
133123
ctx = logger.WithMaestroConsumer(ctx, consumerName)
134124
ctx = logger.WithLogField(ctx, "manifestwork", workName)
135125

136-
c.log.Debug(ctx, "Deleting ManifestWork")
137-
138126
err := c.workClient.ManifestWorks(consumerName).Delete(ctx, workName, metav1.DeleteOptions{})
139127
if err != nil {
140128
// Ignore not found errors (already deleted)
141129
if apierrors.IsNotFound(err) {
142-
c.log.Debug(ctx, "ManifestWork already deleted")
143130
return nil
144131
}
145132
return apperrors.MaestroError("failed to delete ManifestWork %s/%s: %v",
146133
consumerName, workName, err)
147134
}
148135

149-
c.log.Info(ctx, "Deleted ManifestWork")
150136
return nil
151137
}
152138

@@ -158,10 +144,6 @@ func (c *Client) ListManifestWorks(
158144
) (*workv1.ManifestWorkList, error) {
159145
ctx = logger.WithMaestroConsumer(ctx, consumerName)
160146

161-
c.log.WithFields(map[string]interface{}{
162-
"labelSelector": labelSelector,
163-
}).Debug(ctx, "Listing ManifestWorks")
164-
165147
opts := metav1.ListOptions{}
166148
if labelSelector != "" {
167149
opts.LabelSelector = labelSelector
@@ -173,9 +155,6 @@ func (c *Client) ListManifestWorks(
173155
consumerName, err)
174156
}
175157

176-
c.log.WithFields(map[string]interface{}{
177-
"count": len(list.Items),
178-
}).Debug(ctx, "Listed ManifestWorks")
179158
return list, nil
180159
}
181160

@@ -217,8 +196,6 @@ func (c *Client) ApplyManifestWork(
217196
ctx = logger.WithLogField(ctx, "manifestwork", manifestWork.Name)
218197
ctx = logger.WithObservedGeneration(ctx, newGeneration)
219198

220-
c.log.Debug(ctx, "Applying ManifestWork")
221-
222199
// Check if ManifestWork exists
223200
existing, err := c.GetManifestWork(ctx, consumerName, manifestWork.Name)
224201
exists := err == nil
@@ -307,8 +284,6 @@ func (c *Client) DiscoverManifest(
307284
ctx = logger.WithMaestroConsumer(ctx, consumerName)
308285
ctx = logger.WithLogField(ctx, "manifestwork", workName)
309286

310-
c.log.Debug(ctx, "Discovering manifests in ManifestWork")
311-
312287
// Get the ManifestWork
313288
work, err := c.GetManifestWork(ctx, consumerName, workName)
314289
if err != nil {
@@ -329,10 +304,6 @@ func (c *Client) DiscoverManifest(
329304
consumerName, workName, err)
330305
}
331306

332-
c.log.WithFields(map[string]interface{}{
333-
"found": len(list.Items),
334-
}).Debug(ctx, "Discovered manifests in ManifestWork")
335-
336307
return list, nil
337308
}
338309

0 commit comments

Comments
 (0)