Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DRA: fix failing test #126805

Merged
merged 1 commit into from
Aug 20, 2024
Merged

DRA: fix failing test #126805

merged 1 commit into from
Aug 20, 2024

Conversation

bart0sh
Copy link
Contributor

@bart0sh bart0sh commented Aug 20, 2024

What type of PR is this?

/kind cleanup
/kind failing-test

What this PR does / why we need it:

TestGRPCConnIsReused test case silently fails NodePrepareResources call with "rpc error: code = DeadlineExceeded desc = context deadline exceeded". Fixed by explicitly setting clientCallTimeout for the plugin.

Does this PR introduce a user-facing change?

NONE

@bart0sh
DRA: fix failing test
9d893c8 
Added error assertion for NodePrepareResources call unveiled
"rpc error: code = DeadlineExceeded desc = context deadline exceeded"
failure in the TestGRPCConnIsReused test.

Setting clientCallTimeout field when creating plugin should fix it.
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 20, 2024
@bart0sh
Copy link
Contributor Author

bart0sh commented Aug 20, 2024

/sig node
/assign @pohly
/triage accepted
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. area/kubelet and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 20, 2024
@bart0sh bart0sh mentioned this pull request Aug 20, 2024
Merged
@pohly
Copy link
Contributor

pohly commented Aug 20, 2024

TestGRPCConnIsReused test case silently fails NodePrepareResources call with "rpc error: code = DeadlineExceeded desc = context deadline exceeded".

What does "fails silently" mean? Shouldn't the test fail "loudly" if it gets that error?

pohly
pohly reviewed Aug 20, 2024
View reviewed changes
pkg/kubelet/cm/dra/plugin/client_test.go
@@ -148,7 +149,8 @@ func TestGRPCConnIsReused(t *testing.T) {
},
},
}
client.NodePrepareResources(context.TODO(), req)
_, err = client.NodePrepareResources(context.TODO(), req)
assert.NoError(t, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the test fail "loudly" if it gets that error?

Ah, error checking was missing. Never mind.

/lgtm
/assign @klueska

For approval.

No need to backport because the test isn't flaking in previous releases.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 20, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: b69fb6569ef9df8674d1c6120e3aa9272f7d9490

@bart0sh
Copy link
Contributor Author

bart0sh commented Aug 20, 2024

/assign @SergeyKanzhelev @klueska @mrunalp
for approval

SergeyKanzhelev
SergeyKanzhelev approved these changes Aug 20, 2024
View reviewed changes
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

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
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bart0sh, SergeyKanzhelev

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

The pull request process is described here

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 Aug 20, 2024
@k8s-ci-robot k8s-ci-robot merged commit f6c88ab into kubernetes:master Aug 20, 2024
19 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.32 milestone Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pohly pohly pohly left review comments

@SergeyKanzhelev SergeyKanzhelev SergeyKanzhelev approved these changes

@mtaufen mtaufen Awaiting requested review from mtaufen

@wzshiming wzshiming Awaiting requested review from wzshiming

Assignees

@pohly pohly

@mrunalp mrunalp

@klueska klueska

@SergeyKanzhelev SergeyKanzhelev

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node CI/Test Board
Status: Done
Milestone
v1.32
Development

Successfully merging this pull request may close these issues.
6 participants
@bart0sh @pohly @k8s-ci-robot @SergeyKanzhelev @mrunalp @klueska