Fix broken eqErrGRPC helper function in integration tests

[zhijun42]

The test helper function eqErrGRPC has always been flawed since introduced in 2016.

func eqErrGRPC(err1 error, err2 error) bool {
	return !(err1 == nil && err2 != nil) || err1.Error() == err2.Error()
}

When the first argument was non-nil (usually the case in tests), it will always return true. This has made all error assertions using this helper effectively no-ops.

Changes:

• Replace broken implementation with correct gRPC status comparison using status.FromError()
• Handle rpctypes.EtcdError so eqErrGRPC works in grpcproxy mode where ToGRPC adapters bypass the gRPC wire
• Handle nil errors (both nil → equal, one nil → not equal)
• Add unit tests covering code/message permutations and edge cases

Pre-existing test bugs exposed by the fix:

• TestV3PutIgnoreValue: sent IgnoreValue=true with a non-empty Value field, causing ErrGRPCValueProvided instead of the expected ErrGRPCKeyNotFound. Fixed by clearing Value.
• TestV3TooLargeRequest: sent a 2MB value which exceeded the gRPC MaxRecvMsgSize (MaxRequestBytes 1.5MB + 512KB overhead = 2MB), causing gRPC ResourceExhausted instead of etcd's ErrGRPCRequestTooLarge. Fixed by reducing the value size to stay under the gRPC transport limit.
• TestV3LeaseRecoverKeyWithMultipleLease: after server restart, leaseExist() used the old client which hadn't reconnected yet. The broken eqErrGRPC masked the connection error by returning true. Fixed by adding WaitStarted to ensure client connectivity before checking leases.
• TestV3LargeRequests: skipped in grpcproxy mode as the proxy does not propagate custom MaxRequestBytes to its internal gRPC client connection.

[k8s-ci-robot]

Hi @zhijun42. Thanks for your PR.

I'm waiting for a etcd-io 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. Regular contributors should join the org to skip this step.

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.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

[serathius]

/ok-to-test

[serathius]

Please fix tests

[serathius]

--- FAIL: TestV3CorruptAlarm (4.32s)
panic: eqErrGRPC: not a gRPC status error: rpctypes.EtcdError etcdserver: corrupt cluster [recovered, repanicked]

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.25%. Comparing base (053fc70) to head (dbf8c2a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

see 20 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #21131      +/-   ##
==========================================
- Coverage   70.27%   70.25%   -0.02%     
==========================================
  Files         426      426              
  Lines       35215    35215              
==========================================
- Hits        24747    24740       -7     
- Misses       9072     9082      +10     
+ Partials     1396     1393       -3     

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 053fc70...dbf8c2a. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[zhijun42]

/retest-required

[serathius]

/retest

[zhijun42]

@serathius All tests pass now. As I was fixing the CI, I found many more pre-existing bugs masked by the broken eqErrGRPC helper. I fixed them all and recorded in the PR description.

[serathius]

Can you resolve merge conflict?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: serathius, zhijun42

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:

• OWNERS [serathius]

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

[serathius]

PTAL @ahrtr @fuweid

[ahrtr]

not sure why change it? errors.Is might be doing almost the same thing as the new implementation of eqErrGRPC, but not sure if there are any differences.

[zhijun42]

Yeah good catch! You're right, it's not really needed. Cleaned these up.