Skip to content

fix: avoid misleading wait hook change logs#573

Merged
furykerry merged 2 commits into
openkruise:masterfrom
Jayant-kernel:fix/wait-reconciler-hook-log
Jun 25, 2026
Merged

fix: avoid misleading wait hook change logs#573
furykerry merged 2 commits into
openkruise:masterfrom
Jayant-kernel:fix/wait-reconciler-hook-log

Conversation

@Jayant-kernel

Copy link
Copy Markdown
Contributor

Ⅰ. Describe what this PR does

This PR avoids a misleading debug log in the generic WaitReconciler.

Previously, WaitReconciler.Reconcile logged object with wait hook changed for every observed object update, even when no wait hook existed for that object. The log now happens only after the matching wait hook is found, immediately before the hook check runs.

A regression test covers both cases:

  • no matching wait hook: no object with wait hook changed log is emitted;
  • matching wait hook: the log is emitted once.

Ⅱ. Does this pull request fix one issue?

Fixes #571

Ⅲ. Describe how to verify it

The focused test is:

go test -count=1 ./pkg/cache/controllers -run 'TestCacheSandboxWaitReconciler_(Reconcile|LogsObjectChangedOnlyWithWaitHook)$'

I could not run the Go test locally because the local Windows environment blocks the available Go binary from executing. The change is limited to the wait-hook log path and its regression test; GitHub CI should run the package tests.

Ⅳ. Special notes for reviews

No API, CRD, generated code, or behavior outside debug logging is changed.

Signed-off-by: Jayant <212013719+Jayant-kernel@users.noreply.github.com>
@kruise-bot

Copy link
Copy Markdown

[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 furykerry for approval by writing /assign @furykerry in a comment. For more information see:The Kubernetes 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:

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

@Jayant-kernel

Copy link
Copy Markdown
Contributor Author

@AiRanthem review my pr

@Jayant-kernel Jayant-kernel marked this pull request as ready for review June 24, 2026 15:25
@kruise-bot kruise-bot requested a review from furykerry June 24, 2026 15:25
@AiRanthem

Copy link
Copy Markdown
Member

@Jayant-kernel plz fix your UT

@AiRanthem

Copy link
Copy Markdown
Member

Your unit tests are too messy. Please keep AI-generated code concise and high-quality.

Signed-off-by: Jayant <212013719+Jayant-kernel@users.noreply.github.com>
@kruise-bot kruise-bot added size/M and removed size/L labels Jun 25, 2026
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.78%. Comparing base (901921b) to head (1903b36).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #573   +/-   ##
=======================================
  Coverage   79.78%   79.78%           
=======================================
  Files         202      202           
  Lines       14688    14689    +1     
=======================================
+ Hits        11719    11720    +1     
  Misses       2544     2544           
  Partials      425      425           
Flag Coverage Δ
unittests 79.78% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Jayant-kernel

Copy link
Copy Markdown
Contributor Author

@AiRanthem
Sorry for the mess , i cleaned the unit test

@furykerry furykerry merged commit f4f0a0b into openkruise:master Jun 25, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Misleading "object with wait hook changed" log in WaitReconciler

4 participants