Chore/wrpc inline version negotiation #8775
Conversation
Update `.proto` files
Remove the version negotiation url and immediate handlers
First try on the wrpc url. if not there, try with old protocol.
Replicate version negotiation data in wrpc call and handler
Add version negotiation service handler to the connection.
Split data plane connection into a separate step. Add the initial negotiation call in the wrpc data plane.
javierguerragiraldez
force-pushed
the
chore/wrpc_inline_version_negotiation
branch
from
26e6ec5
to
6789286
Compare
|
Still things to do, but it actually works and the error flows are in place.
|
kong/clustering/init.lua
Outdated
| end | ||
|
|
||
| if not ws_conn then | ||
| ngx_log(ngx_DEBUG, _log_prefix, "ugh retrying from the start") |
kong/clustering/init.lua
Outdated
| local ws_conn, err = implementation.open_connection(self) | ||
| ngx_log(ngx_DEBUG, _log_prefix, "got conn: ", tostring(ws_conn), ", err: ", tostring(err)) | ||
|
|
||
| -- TODO: find how to detect a 404 response |
There was a problem hiding this comment.
the websocket library doesn't return the http result code, instead, returns the whole header line if it doesn't match the expected pattern. this, added to some inherited fallbacks in the specific server context in nginx config, makes reliably detecting a 404 response a bit more involved than expected. i left the specifics to after i get the negotiation itself working. (that is, now).
the "got conn:" log messages are just to see the exact returned values. they won't stay in the final code
There was a problem hiding this comment.
as it turns out, any HTTP result is "good enough" to the library, so the user never sees a 404 or any other error.
I've just hacked a peeking socket to work around it.
| @@ -60,6 +61,7 @@ end | |||
| local function get_config_service(self) | |||
| if not wrpc_config_service then | |||
| wrpc_config_service = wrpc.new_service() | |||
| version_negotiation.add_negotiation_service(wrpc_config_service, self.conf) | |||
| wrpc_config_service:add("kong.services.config.v1.config") | |||
There was a problem hiding this comment.
What prevents a CP node to execute a ConfigService.SyncConfig before the negotiation is finished?
There was a problem hiding this comment.
nothing yet. i could put a "negotiated" flag in the peer object and just drop (or reject) everything else until then.
Apply suggestions from review
switches to old websocket protocol if the wrpc connection returns a 404 error. any other error does the usual "retry in X seconds"
Remove http-specific code from version negotiation.
Overrides are allow-list, not block-list.
Protocol itself is a special case of service `protocol.json`/`protocol .wrpc`. Default is to try both. If only one is mentioned, the other is skipped.
Use services override in tests to specify the protocol.
close all pending semaphores when the connection is closed
|
The refactoring to wRPC causes too much conflict to this PR. We will create another PR and take this as a reference. |
| } | ||
|
|
||
| message DPNodeDescription { | ||
| string id = 1; |
There was a problem hiding this comment.
The id should be already known from URI parameters. Do we need to include it here?
| } | ||
|
|
||
| message CPNodeDescription { | ||
| string id = 1; |
There was a problem hiding this comment.
Currently, CP's ID(cluster ID) is synced with the config.
There was a problem hiding this comment.
Maybe a more proper way to do that is to restructure config and get ID from negotiation.
|
Due to we have merged another PR (#8926), I think we should close this. |
Move the services/versions negotiation to the wRPC connection.
Because of limitations in the lua-resty-websocket library, the HTTP-based version negotiation and the websocket connection used separate TCP connections instead of reusing (via keep-alive). So, in the (very common) case of a load balancer in front of a cluster of control plane nodes, it could easily happen to negotiate against one node and then connect to another. To avoid that situation, this PR changes the implemenation of the version negotiation into a wRPC service with a single RPC method. The DP is responsible to make this call as soon as connected, so the agreed result of the negotiation is stored in both sides in the context of the peer connection.