Use passthrough loadbalancing in activator state keeping #11172
Conversation
Codecov Report
@@ Coverage Diff @@
## main #11172 +/- ##
==========================================
- Coverage 87.69% 87.65% -0.05%
==========================================
Files 191 191
Lines 9195 9209 +14
==========================================
+ Hits 8064 8072 +8
- Misses 877 883 +6
Partials 254 254
Continue to review full report at Codecov.
|
| @@ -264,7 +280,7 @@ func (rw *revisionWatcher) checkDests(curDests, prevDests dests) { | |||
|
|
|||
| // If we have discovered that this revision cannot be probed directly | |||
| // do not spend time trying. | |||
| if rw.podsAddressable { | |||
| if rw.podsAddressable || rw.usePassthroughLb { | |||
There was a problem hiding this comment.
I think this isn't possible since if usePassthroughLb is set we will never try service scraping, and therefore would never mark podAddressable false.
There was a problem hiding this comment.
Fair enough, I still feel like it's worth having it here for added clarity?
There was a problem hiding this comment.
Maybe, fwiw it was more confusing to me on first reading: ie even if usePassthroughLb is set it wouldnt make much sense to try to directly access pods if we already determined they're not accessible. Then I realised this never happens anyway. Also if we do want to keep it we should probably update the comment to match.
There was a problem hiding this comment.
I'll drop it then.
| @@ -301,6 +317,12 @@ func (rw *revisionWatcher) checkDests(curDests, prevDests dests) { | |||
| } | |||
| } | |||
|
|
|||
| if rw.usePassthroughLb { | |||
There was a problem hiding this comment.
👍️ I like that we'll get a clear indication here if the flag was set and doesnt work, rather than silently falling back and succeeding.
b71930a to
7a0f271
Compare
|
Rebased and added usage of the naming helper. |
| // We cannot set these headers unconditionally as the Host header will cause the | ||
| // request to be loadbalanced by ingress "silently" if passthrough LB is not | ||
| // configured, which will cause the request to "pass" but doesn't guarantee it | ||
| // actually lands on the correct pod, which breaks our state keeping. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: julz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Proposed Changes
Ref #10751
This connects the revision watchers in the activator with the networking configmap and makes it sensitive to the EnableMeshPodAddressability setting.
Release Note
/assign @vagababov @julz