Skip to content

fix: log correct error from Prepare Data Plugins#2512

Merged
k8s-ci-robot merged 1 commit into
kubernetes-sigs:mainfrom
varad-ahirwadkar:fix-log
Mar 7, 2026
Merged

fix: log correct error from Prepare Data Plugins#2512
k8s-ci-robot merged 1 commit into
kubernetes-sigs:mainfrom
varad-ahirwadkar:fix-log

Conversation

@varad-ahirwadkar

@varad-ahirwadkar varad-ahirwadkar commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?
/kind bug

What this PR does / why we need it:
HandleRequest() ignored the error returned by runPrepareDataPlugins and logged a stale err variable, causing misleading logs.
Capture and log the actual returned error to improve observability.

Which issue(s) this PR fixes:

Fixes #2508

Does this PR introduce a user-facing change?:
NONE

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 7, 2026
@netlify

netlify Bot commented Mar 7, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for gateway-api-inference-extension ready!

Name Link
🔨 Latest commit d0fc625
🔍 Latest deploy log https://app.netlify.com/projects/gateway-api-inference-extension/deploys/69ac78a8f0fca60008e0b3db
😎 Deploy Preview https://deploy-preview-2512--gateway-api-inference-extension.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@k8s-ci-robot
k8s-ci-robot requested review from danehans and shmuelk March 7, 2026 11:57
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 7, 2026
@k8s-ci-robot

k8s-ci-robot commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

Hi @varad-ahirwadkar. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 7, 2026
nirrozenbaum
nirrozenbaum reviewed Mar 7, 2026
View reviewed changes
Comment thread pkg/epp/requestcontrol/director.go Outdated

// Prepare per request data by running PrepareData plugins.
if d.runPrepareDataPlugins(ctx, reqCtx.SchedulingRequest, snapshotOfCandidatePods) != nil {
prepErr := d.runPrepareDataPlugins(ctx, reqCtx.SchedulingRequest, snapshotOfCandidatePods)

@nirrozenbaum nirrozenbaum Mar 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you reuse err instead of introducing a new var?

@varad-ahirwadkar varad-ahirwadkar Mar 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!

@nirrozenbaum

nirrozenbaum commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 7, 2026
@varad-ahirwadkar
log correct error from Prepare Data Plugins
d0fc625 
Signed-off-by: Varad <varad.ahirwadkar1@ibm.com>
nirrozenbaum
nirrozenbaum approved these changes Mar 7, 2026
View reviewed changes

@nirrozenbaum nirrozenbaum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 7, 2026
@k8s-ci-robot

k8s-ci-robot commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nirrozenbaum, varad-ahirwadkar

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

The pull request process is described 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 7, 2026
@k8s-ci-robot
k8s-ci-robot merged commit 2389218 into kubernetes-sigs:main Mar 7, 2026
11 checks passed
RyanRosario pushed a commit to RyanRosario/gateway-api-inference-extension that referenced this pull request Mar 9, 2026
@varad-ahirwadkar @RyanRosario
log correct error from Prepare Data Plugins (kubernetes-sigs#2512)
3a8b086 
Signed-off-by: Varad <varad.ahirwadkar1@ibm.com>
BizerNotNull pushed a commit to BizerNotNull/gateway-api-inference-extension that referenced this pull request Mar 15, 2026
@varad-ahirwadkar @BizerNotNull
log correct error from Prepare Data Plugins (kubernetes-sigs#2512)
63dfc30 
Signed-off-by: Varad <varad.ahirwadkar1@ibm.com>
elevran pushed a commit to llm-d/llm-d-router that referenced this pull request Apr 23, 2026
@varad-ahirwadkar
log correct error from Prepare Data Plugins (kubernetes-sigs/gateway-…
2f74ab0 
…api-inference-extension#2512)

Signed-off-by: Varad <varad.ahirwadkar1@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nirrozenbaum nirrozenbaum nirrozenbaum approved these changes
@danehans danehans Awaiting requested review from danehans
@shmuelk shmuelk Awaiting requested review from shmuelk

Assignees

@nirrozenbaum nirrozenbaum

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Director: Prepare-plugin error logged incorrectly

3 participants

@varad-ahirwadkar @k8s-ci-robot @nirrozenbaum