[JENKINS-64510] Issues with TCP agent reconnects #429
Conversation
Define a consistently used cookie name. Provide a way to attach the cookie to the channel.
| @@ -66,7 +66,7 @@ | |||
| /** | |||
| * The proprty name for the cookie name key. | |||
| */ | |||
| public static final String COOKIE_KEY = "Cookie"; | |||
| public static final String COOKIE_KEY = "JnlpAgentProtocol.cookie"; | |||
There was a problem hiding this comment.
Use a more interesting, common name between agent and controller.
| @@ -152,7 +152,7 @@ public Jnlp4ConnectionState createConnectionState(@Nonnull Socket socket, | |||
| .filter(new ConnectionHeadersFilterLayer(headers, handler)) | |||
| .named(String.format("%s connection from %s", getName(), socket.getRemoteSocketAddress())) | |||
| .listener(handler) | |||
| .build(new ChannelApplicationLayer(threadPool, handler)) | |||
| .build(new ChannelApplicationLayer(threadPool, handler, headers.get(JnlpConnectionState.COOKIE_KEY))) | |||
There was a problem hiding this comment.
This part and the corresponding additions to ChannelApplicationLayer are part of attaching the cookie to the channel properties on the controller side.
|
|
|
Yes, I certainly don't want to break that. Is that something I can easily run in a command-line or CI environment? Or otherwise, could you help test it when I have everything prepared. |
|
It's a standard integration test which you can run with |
|
Hmm ... I tried updating the Jenkins version in swarm-plugin to something recent (for example, 2.363) and it gets a compilation error in |
Presumably because jenkinsci/jenkins#4027 removed a public API from Jenkins core without updating pre-existing consumers. CC @Wadeck |
|
See jenkinsci/jenkins#5138 for the corresponding core changes needed to tie these together. |
|
Maybe a rebuild will help. |
|
Incrementals worked this time. Currently at 4.7-rc2860.f4a9a135655d |
|
I ran the swarm build and all the tests passed based on the updated core PR so this should be safe there. |
|
I'm marking this as ready-for-merge. Any further additional reviews are welcome. I'll try to merge it in a day or two. |
See JENKINS-64510
The reconnect mechanism for TCP inbound agents hasn't been working properly for some period of time, most likely a long time. It's not a serious loss, but getting it working again can make some minor improvements in clarity and robustness.
This is a necessary, but insufficient, change to get it working. A corresponding change is also necessary in Jenkins core. Both are required to get it working correctly, but they're loosely coupled otherwise. Individually, they don't make the situation any worse, so it's fine to only have one of them.