refactor(client): remove duplicate processing

[wafuwafu13]

Signed-off-by: wafuwafu13 mariobaske@i.softbank.jp

• as this comment say, c.authTokenBundle != nil is equal to c.Username != "" && c.Password != "", and c.getToken just return nil if either c.Username or c.Password is "". so if c.authTokenBundle != nil is unnecessary.
• getToken returns nil if err == rpctypes.ErrAuthNotEnabled, and rpctypes.Error(err) != rpctypes.ErrAuthNotEnabled is always true, so unnecessary.

ref: // TODO(cfc4n): keep this code block, remove codes about getToken in client.go after pr #12165 merged. https://github.com/etcd-io/etcd/pull/12165/files

[ahrtr]

LGTM

THank you @wafuwafu13

[ptabor]

I'm a little confused. Are we getting a new token for each creation of a stream... and getting the token requires a separate call to etcd ? I thought we use token stored in the authTokenBundle... as long as we don't get error suggesting shouldRefreshToken...

I know it's not change in this PR, but still if you know, please let me know.

[wafuwafu13]

I didn't understand why getToken needed to be called, but since it was mentioned in the comment, I didn't remove it.

But I think that getToken is not reqired before RecvMsg because getToken is called at client initialization and token is stored in the authTokenBundle.

[ahrtr]

Per my understanding streamClientInterceptor is just a client side stream Interceptor, but etcd only uses server side streaming (watch stream and lease keep alive), so streamClientInterceptor is only executed once on the very beginning of a stream. Note that for server-side streaming, the client side only calls SendMsg once.

So actually the token will never be refreshed during a watch stream, and it's exactly the reason of #12385, which was resolved in #14322. But unfortunately, we rollbacked the fix due to some concern on K8s side. FYI. #14992

[ahrtr]

Update: lease keep alive should be a bi-directional stream. The client side keeps sending LeaseKeepAliveRequest, and server side responds with a LeaseKeepAliveResponse each time.

[wafuwafu13]

Thank you for letting me know. I added comment.

[wafuwafu13]

@ahrtr @ptabor CI passed and I have nothing to change in this PR :)

[ahrtr]

@wafuwafu13 Please squash the commits and rebase this PR.

[codecov-commenter]

Codecov Report

Merging #15032 (9df449b) into main (ff71968) will decrease coverage by 0.04%.
The diff coverage is 25.00%.

@@            Coverage Diff             @@
##             main   #15032      +/-   ##
==========================================
- Coverage   74.73%   74.69%   -0.05%     
==========================================
  Files         415      415              
  Lines       34276    34275       -1     
==========================================
- Hits        25617    25601      -16     
- Misses       7017     7025       +8     
- Partials     1642     1649       +7     

Flag	Coverage Δ
all	74.69% <25.00%> (-0.05%)	⬇️

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

Impacted Files	Coverage Δ
client/v3/retry_interceptor.go	67.11% <25.00%> (-0.15%)	⬇️
client/v3/leasing/util.go	91.66% <0.00%> (-6.67%)	⬇️
server/etcdserver/api/v3rpc/util.go	70.96% <0.00%> (-3.23%)	⬇️
client/v3/leasing/txn.go	88.09% <0.00%> (-3.18%)	⬇️
server/proxy/grpcproxy/watch.go	92.48% <0.00%> (-2.90%)	⬇️
server/etcdserver/api/v3election/election.go	66.66% <0.00%> (-2.78%)	⬇️
client/v3/experimental/recipes/double_barrier.go	68.83% <0.00%> (-2.60%)	⬇️
client/v3/client.go	82.33% <0.00%> (-1.27%)	⬇️
client/v3/op.go	74.71% <0.00%> (-1.15%)	⬇️
server/etcdserver/raft.go	88.63% <0.00%> (-1.14%)	⬇️
... and 10 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ahrtr]

The second commit b606721 doesn't make any sense. Please rebase this PR instead of merging main

[wafuwafu13]

I made mistakes and will remake it.