Set default Client.callOpts in NewCtxClient

[brandond]

clientv3.New sets Client.callOpts to defaultCallOpts in clientv3.newClient, but clientv3.NewCtxClient does not, and there is no other way to modify call options - so the default GRPC message sizes are used, which is not correct and makes it impossible to perform operations that require payloads larger than the default 4MB grpc message size.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: brandond
Once this PR has been reviewed and has the lgtm label, please assign spzala 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

[k8s-ci-robot]

Hi @brandond. 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.

[silentred]

It seems on purpose.

etcd/client/v3/client.go

Lines 95 to 97 in 8ad92a0

	// NewCtxClient creates a client with a context but no underlying grpc
	// connection. This is useful for embedded cases that override the
	// service interface implementations and do not need connection management.

[brandond]

It says no underlying grpc connection. I don't WANT a GRPC connection. That's not what I'm asking for. I am asking to set callOpts.

[brandond]

I am constructing my client like this:

  cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(insecure.NewCredentials()))
  conn, err := grpc.NewClient(endpoint, cfg.DialOptions...)
  if err != nil {
    return nil, nil, err
  }

  client := clientv3.NewCtxClient(ctx, clientv3.WithZapLogger(logger.Named("my-client")))
  client.Cluster = clientv3.NewClusterFromClusterClient(etcdserverpb.NewClusterClient(conn), client)
  client.KV = clientv3.NewKVFromKVClient(etcdserverpb.NewKVClient(conn), client)
  client.Maintenance = clientv3.NewMaintenanceFromMaintenanceClient(etcdserverpb.NewMaintenanceClient(conn), client)
  client.Watcher = clientv3.NewWatchFromWatchClient(etcdserverpb.NewWatchClient(conn), client)

• NewKVFromClient copies callOptions from the client:
  

  etcd/client/v3/kv.go

  Lines 151 to 157 in 8ad92a0

  	func NewKVFromKVClient(remote pb.KVClient, c *Client) KV {
  	api := &kv{remote: remote}
  	if c != nil {
  	api.callOpts = c.callOpts
  	}
  	return api
  	}

• These options are later used when KV makes GRPC calls:
  

  etcd/client/v3/kv.go

  Line 171 in 8ad92a0

  c, err := kv.remote.RangeStream(ctx, op.toRangeRequest(), kv.callOpts...)

As far as I can tell there is no way to override the callOpts in either Client or kv, so if they are left empty then large GRPC messages cannot be sent or received.

[silentred]

/ok-to-test

[k8s-ci-robot]

@brandond: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-etcd-grpcproxy-integration-amd64	e009726	link	true	/test pull-etcd-grpcproxy-integration-amd64

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[silentred]

@brandond I was wondering, is there a specific reason for not invoking clientv3.New() directly?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.07%. Comparing base (23db69a) to head (e009726).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

Files with missing lines	Coverage Δ
client/v3/client.go	85.22% <100.00%> (ø)

... and 22 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #21809      +/-   ##
==========================================
- Coverage   70.23%   70.07%   -0.17%     
==========================================
  Files         426      427       +1     
  Lines       35245    35304      +59     
==========================================
- Hits        24755    24739      -16     
- Misses       9100     9165      +65     
- Partials     1390     1400      +10     

---

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 23db69a...e009726. Read the comment docs.

🚀 New features to boost your workflow:

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

[brandond]

I want more direct control over the grpc connection, endpoint dialing, and error handling/retry behavior. Which is what NewCtxClient is for, I believe?