Repository navigation
ateapi: reach an atelet again after its node restarts - #2118
Luiz Oliveira (laoj2) merged 2 commits into
Conversation
A node restart keeps the atelet pod UID but changes its IP, and the conn cache kept the old one.
Maya Wang (mayawang)
added
kind/bug
|
Tested this PR ( Setup: kind with 1 control-plane node and 2 worker nodes, gVisor sandbox class. The services ran on the control plane. Both workers of the pool ran on one worker node (the "victim"). I used Each round:
In one round on this PR, I compared timestamps. The last call to the old IP was 32 s before the new atelet container started. After that, the only log lines with the old IP were Notes:
|
| func newAteletConnCache(size int) *lru.Cache { | ||
| return lru.NewWithEvictionFunc(size, func(_ lru.Key, value interface{}) { | ||
| value.(*grpc.ClientConn).Close() | ||
| value.(*ateletConn).conn.Close() |
There was a problem hiding this comment.
Linter is failing with
Error: cmd/ateapi/internal/controlapi/dialer.go:109:33: Error return value of `.Close` is not checked (errcheck)
value.(*ateletConn).conn.Close()
^
1 issues:
| ateletKey := string(selectedAtelet.ObjectMeta.UID) | ||
| ateletIP := selectedAtelet.Status.PodIPs[0].IP | ||
|
|
||
| if cached, ok := d.ateletConns.Get(ateletKey); ok { |
There was a problem hiding this comment.
There's a race condition now when we call DialForAteletOnNode for many actors, because Get and Remove are not under the same lock.
For example, let's say that A and B are actors
Node just restarted, then:
- A and B both Get(ateletKey) a stale entry.
- A calls Remove(ateletKey), which closes the stale connection.
- A dials connA, calls Add(ateletKey, connA), and returns connA.
- B calls Remove(ateletKey). That removes connA, and the eviction function closes it.
B just removed the good connection.
There was a problem hiding this comment.
Good catch. Fixed in 086ccfc: a mutex now covers the lookup, evict and insert. Added a concurrent test that fails without it.
Fixes #2116
Redial when an atelet pod keeps its UID but gets a new IP.