HYPERFLEET-630 - fix: consolidate duplicate resource operation logs

[xueli181114]

Summary

• Remove redundant pre/post operation logs from k8s_client and maestro_client CRUD methods (25 log lines removed)
• Consolidate to a single authoritative INFO log per resource operation at the executor layer
• Retain apply-layer decision log (operation + reason) at DEBUG level for troubleshooting
• Handle "already exists" errors from concurrent creates gracefully as a skip instead of ERROR

Test plan

• make test-all passes (lint + unit + integration tests)
• Deploy to dev and verify log output shows single INFO line per resource operation
• Verify DEBUG level still shows apply decision details when enabled
• Verify concurrent create race condition produces skip instead of ERROR

Relates to: HYPERFLEET-630

Summary by CodeRabbit

• Bug Fixes

  • Concurrent resource creation attempts are now detected and treated as already-existing, allowing operations to complete successfully instead of failing.

• Chores

  • Reduced internal logging verbosity for resource and manifest operations to cut noise.

• Tests

  • Added unit tests covering apply/create behavior, concurrent-create handling, and nil-manifest validation.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tirthct for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bdcd00b0-4824-4dbc-8bb4-f51c7347c8e9

📥 Commits

Reviewing files that changed from the base of the PR and between ba4011f and b24c590.

📒 Files selected for processing (5)

• internal/k8sclient/apply.go
• internal/k8sclient/apply_test.go
• internal/k8sclient/client.go
• internal/maestroclient/client.go
• internal/maestroclient/operations.go

💤 Files with no reviewable changes (3)

• internal/maestroclient/client.go
• internal/k8sclient/client.go
• internal/maestroclient/operations.go

---

Walkthrough

The pull request updates manifest apply behavior to treat a resource creation failure with an "already exists" error as a concurrent-create race: the apply result is changed to OperationSkip with reason "already exists (concurrent create)" and the error is cleared. It also removes numerous informational/debug log statements across k8sclient and maestroclient CRUD and apply helper methods. Unit tests were added for the updated ApplyManifest behavior.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant k8sclient
    participant K8sAPI
    participant Logger

    Caller->>k8sclient: ApplyManifest(manifest, existing=nil)
    k8sclient->>K8sAPI: CreateResource(manifest)
    alt API returns Created
        K8sAPI-->>k8sclient: Created (no error)
        k8sclient-->>Caller: ApplyResult(OperationCreate)
    else API returns AlreadyExists error
        K8sAPI-->>k8sclient: Error: AlreadyExists
        k8sclient->>Logger: Debug("concurrent create detected")
        k8sclient-->>Caller: ApplyResult(OperationSkip, reason="already exists (concurrent create)")
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: consolidating duplicate resource operation logs. It directly summarizes the primary objective of removing redundant logging statements across CRUD methods.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pnguyen44]

Missing test coverage for this change

[xueli181114]

Added test coverage in apply_test.go. TestApplyManifest_CreateAlreadyExists verifies the concurrent create race condition: first ApplyManifest creates the resource, second ApplyManifest with existing=nil hits AlreadyExists and returns OperationSkip with reason "already exists (concurrent create)". Also added tests for create success, same-generation skip, and nil manifest validation.