Repository navigation
atecontroller: deregister workers when their WorkerPool is deleted - #2093
Conversation
|
Scott Funkenhauser (@sfunkenhauser) Lior Lieberman (@LiorLieberman) Addressed the review, PTAL when you get a chance. Thank you! |
Maya Wang (mayawang)
added
kind/bug
|
Max Smythe (@maxsmythe) Bowei Du (@bowei) PTAL when you get a chance, the review feedback is addressed. Thank you! |
|
Luiz Oliveira (@laoj2) Jeff Luo (@JeffLuoo) PTAL when you get a chance. Thank you! |
|
Merged main and resolved the conflicts. PTAL. |
Scott Funkenhauser (sfunkenhauser)
left a comment
Scott Funkenhauser (sfunkenhauser)
left a comment
There was a problem hiding this comment.
Sorry for the delay. One more small comment, but otherwise LGTM.
| } | ||
| if !exists { | ||
| return fmt.Errorf("getting WorkerPool %s/%s: not found", key.namespace, poolName) | ||
| return s.reconcileMissingPool(ctx, key, pod, poolName) |
There was a problem hiding this comment.
We should raise this up higher in the reconcile flow, before the isWorkerEligible check. If a pod fails the isWorkerEligible check (because the pod isn't ready), createOrUpdateWorker never gets called, and reconcileMissingPool is never triggered.
There was a problem hiding this comment.
Done in 3b88e5b. The pool check now runs before the eligibility gate, with a test for a not-Ready orphan.
There was a problem hiding this comment.
Main dropped the gate; the pool check still runs first.
|
Scott Funkenhauser (@sfunkenhauser) Max Smythe (@maxsmythe) The last note is addressed (pool check now runs before the eligibility gate), PTAL when you get a chance. Thank you! |
|
Looks like one of the github workflows is stuck (e2e-test (envoy, preview)). I think this is caused by changes in |
Orphaned pods outlive their pool, so the syncer retried them forever and left their Workers schedulable.
75d4b13 to
fdfa838
Compare
|
Rebased on main. |
The syncer now deregisters the Worker of a pod whose WorkerPool is gone, and reconciles a pool's pods when the pool is deleted. It waits for the pool cache to sync first.
Fixes #2054