Enable direct connections to TCP agent listener #602

alxsap · 2019-09-24T15:58:42Z

Remoting#3.34 introduced direct inbound TCP agent connections (to masters without Web UI/HTTP port). The feature is documented here.
Oleg released jnlp-slave 3.35-5 which contains the required changes. Thanks, @oleg-nenashev!

This Pull Request passes the required parameters to the JNLP container. In particular this change is needed to enable pipeline runs in Jenkinsfile Runner on Kubernetes (without exposed HTTP port).

On Windows test `org.csanchez.jenkins.plugins.kubernetes.KubernetesFactoryAdapterTest.autoConfig()` fails because `/src/test/resources/kubenamespace` contains a "\n" line separator but `io.fabric8.kubernetes.client.Config.tryNamespaceFromPath(Config)` uses `System.lineSeparator()` for replacement which is "\r\n" on Windows. ``` config.setNamespace(namespace.replace(System.lineSeparator(), "")); ```

[Remoting#3.34](https://github.com/jenkinsci/remoting/releases/tag/remoting-3.34) introduced [direct inbound TCP agent connections](jenkinsci/remoting#338) (to masters without Web UI/HTTP port). The [feature is documented here](https://github.com/jenkinsci/remoting/blob/master/docs/tcpAgent.md#connect-directly-to-tcp-port).

jglick

Sounds like it should work. Probably @Vlatombe or I need to try it in a few contexts.

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java

jglick · 2019-09-24T16:12:11Z

...resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/help-directConnection.html

@@ -0,0 +1,6 @@
+<div>
+  When this option is enabled a <a href="https://github.com/jenkinsci/remoting/blob/master/docs/tcpAgent.md#connect-directly-to-tcp-port">direct connection</a>


Use target="_blank", and never link to a GitHub branch head reference from the product, as the link could break at any moment. You could use a tagged version, but better is to create a redirect on jenkins.io (example: jenkins-infra/jenkins.io#2290), or simply make sure that everything the user needs to know is listed here.

jglick · 2019-09-24T16:12:43Z

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly

@@ -27,6 +27,10 @@

    <f:validateButton title="${%Test Connection}" progress="${%Testing...}" method="testConnection" with="name,serverUrl,credentialsId,serverCertificate,skipTlsVerify,namespace" />

+    <f:entry title="${%Direct Connection}" field="directConnection">
+      <f:checkbox default="true"/>


You are optimistic!

Shall I switch to false? Assuming the feature works, from my perspective this is much more straight forward than configuring and using the HTTP(S) URL.

The main drawback I see with this is that it won't work if the Jenkins instance is set to use a random port for inbound TCP. In case the master restarts, the agent won't be able to reconnect.

I removed the default.

For our (Jenkinsfile Runner in Kubernetes) scenario Jenkins restarts are not relevant, but I'll try to test the behaviour.

How important is this topic for you? I mean the agents created by the Kubernetes plugin are typically not living very long.
I can also add some notes to the help.

the agents created by the Kubernetes plugin are typically not living very long

But they can survive a restart of the Jenkins master, which might occur within a matter of minutes after the agent pod has started. It is expected to reconnect to the restarted master, even with a random TCP port (Jenkins.slaveAgentPort == 0).

The existing tests passed, but I suspect this was only because you made this field enabled by default in the GUI form whereas KubernetesTestUtil.setupCloud uses KubernetesCloud.<init>(String) which leaves it off.

Vlatombe

It would be interesting to validate against cases where Jenkins restarts to make sure the behaviour is correct.

pom.xml

Vlatombe · 2019-09-25T09:23:02Z

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly

@@ -27,6 +27,10 @@

    <f:validateButton title="${%Test Connection}" progress="${%Testing...}" method="testConnection" with="name,serverUrl,credentialsId,serverCertificate,skipTlsVerify,namespace" />

+    <f:entry title="${%Direct Connection}" field="directConnection">
+      <f:checkbox default="true"/>


The main drawback I see with this is that it won't work if the Jenkins instance is set to use a random port for inbound TCP. In case the master restarts, the agent won't be able to reconnect.

alxsap · 2019-09-25T11:30:51Z

It would be interesting to validate against cases where Jenkins restarts to make sure the behaviour is correct.

@Vlatombe I tested the behaviour. With fixed port the agent has been able to reconnect to the master. With random port, as expected, this is not possible.

Below you see the jnlp container log with fixed port. The reconnect works after some time.
With random port the IOException is repeated until timeout.

Sep 25, 2019 11:09:20 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Agent discovery successful
  Agent address: xx.xx.xx.xx
  Agent port:    12345
  Identity:      d7:69:f9:1b:67:ba:f3:10:7b:0b:b5:b5:7c:7e:b6:ef
Sep 25, 2019 11:09:20 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Handshaking
Sep 25, 2019 11:09:20 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Connecting to xx.xx.xx.xx:12345
Sep 25, 2019 11:09:20 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Trying protocol: JNLP4-connect
Sep 25, 2019 11:09:24 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Remote identity confirmed: d7:69:f9:1b:67:ba:f3:10:7b:0b:b5:b5:7c:7e:b6:ef
Sep 25, 2019 11:09:25 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Connected

Sep 25, 2019 11:09:40 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Terminated
Sep 25, 2019 11:09:40 AM org.jenkinsci.remoting.engine.JnlpAgentEndpointConfigurator waitForReady
INFO: Sleeping 10s before reconnect.
Sep 25, 2019 11:09:40 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Performing onReconnect operation.
Sep 25, 2019 11:09:40 AM jenkins.slaves.restarter.JnlpSlaveRestarterInstaller$FindEffectiveRestarters$1 onReconnect
INFO: Restarting agent via jenkins.slaves.restarter.UnixSlaveRestarter@56cb564
Sep 25, 2019 11:09:42 AM hudson.remoting.jnlp.Main createEngine
INFO: Setting up agent: testpodtemplate-zlfv5-sgf4c
Sep 25, 2019 11:09:42 AM hudson.remoting.jnlp.Main$CuiListener <init>
INFO: Jenkins agent is running in headless mode.
Sep 25, 2019 11:09:42 AM hudson.remoting.Engine startEngine
INFO: Using Remoting version: 3.35
Sep 25, 2019 11:09:42 AM org.jenkinsci.remoting.engine.WorkDirManager initializeWorkDir
INFO: Using /home/jenkins/agent/remoting as a remoting work directory
Sep 25, 2019 11:09:42 AM org.jenkinsci.remoting.engine.WorkDirManager setupLogging
INFO: Both error and output logs will be printed to /home/jenkins/agent/remoting
Sep 25, 2019 11:09:42 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Locating server among []
Sep 25, 2019 11:09:42 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Agent discovery successful
  Agent address: xx.xx.xx.xx
  Agent port:    12345
  Identity:      d7:69:f9:1b:67:ba:f3:10:7b:0b:b5:b5:7c:7e:b6:ef
Sep 25, 2019 11:09:42 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Handshaking
Sep 25, 2019 11:09:42 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Connecting to xx.xx.xx.xx:12345

Sep 25, 2019 11:09:52 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Connecting to xx.xx.xx.xx:12345 (retrying:2)
java.io.IOException: Failed to connect to xx.xx.xx.xx:12345
        at org.jenkinsci.remoting.engine.JnlpAgentEndpoint.open(JnlpAgentEndpoint.java:242)
        at hudson.remoting.Engine.connectTcp(Engine.java:678)
        at hudson.remoting.Engine.innerRun(Engine.java:556)
        at hudson.remoting.Engine.run(Engine.java:488)
Caused by: java.net.ConnectException: Connection refused
        at sun.nio.ch.Net.connect0(Native Method)
        at sun.nio.ch.Net.connect(Net.java:454)
        at sun.nio.ch.Net.connect(Net.java:446)
        at sun.nio.ch.SocketChannelImpl.connect(SocketChannelImpl.java:648)
        at java.nio.channels.SocketChannel.open(SocketChannel.java:189)
        at org.jenkinsci.remoting.engine.JnlpAgentEndpoint.open(JnlpAgentEndpoint.java:203)
        ... 3 more

Sep 25, 2019 11:10:22 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Trying protocol: JNLP4-connect
Sep 25, 2019 11:10:27 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Remote identity confirmed: d7:69:f9:1b:67:ba:f3:10:7b:0b:b5:b5:7c:7e:b6:ef
Sep 25, 2019 11:10:28 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Connected

The pipeline log (fixed port) looks like this:

Execute container content in Kubernetes pod
[Pipeline] container
[Pipeline] {
[Pipeline] sh
+ echo Hello 1
Hello 1
+ sleep 120

Resuming build at Wed Sep 25 13:10:18 CEST 2019 after Jenkins restart
Waiting to resume part of PipelineK8sPlugin jenkinsci/docker-inbound-agent#10: Finished waiting
Waiting to resume part of PipelineK8sPlugin jenkinsci/docker-inbound-agent#10: Finished waiting
Still trying to load Looking for path named ‘/home/jenkins/agent/workspace/PipelineK8sPlugin’ on computer named ‘testpodtemplate-zlfv5-sgf4c’
Waiting to resume part of PipelineK8sPlugin jenkinsci/docker-inbound-agent#10: ‘testpodtemplate-zlfv5-sgf4c’ is offline
Still trying to load Looking for path named ‘/home/jenkins/agent/workspace/PipelineK8sPlugin’ on computer named ‘testpodtemplate-zlfv5-sgf4c’
Waiting to resume part of PipelineK8sPlugin jenkinsci/docker-inbound-agent#10: ‘testpodtemplate-zlfv5-sgf4c’ is offline
Still trying to load Looking for path named ‘/home/jenkins/agent/workspace/PipelineK8sPlugin’ on computer named ‘testpodtemplate-zlfv5-sgf4c’
Waiting to resume part of PipelineK8sPlugin jenkinsci/docker-inbound-agent#10: ‘testpodtemplate-zlfv5-sgf4c’ is offline
Still trying to load Looking for path named ‘/home/jenkins/agent/workspace/PipelineK8sPlugin’ on computer named ‘testpodtemplate-zlfv5-sgf4c’
Waiting to resume part of PipelineK8sPlugin jenkinsci/docker-inbound-agent#10: ‘Jenkins’ doesn’t have label ‘testpodtemplate-zlfv5-sgf4c’
‘testpodtemplate-zlfv5-sgf4c’ is offline
Ready to run at Wed Sep 25 13:10:35 CEST 2019
Agent testpodtemplate-zlfv5-sgf4c is provisioned from template Kubernetes Pod Template

+ echo Hello 2
Hello 2

Vlatombe · 2019-09-25T13:08:18Z

So based on observation, one thing that could be done is to enable the new way by default unless the incoming TCP port is random. Is it even needed to have a checkbox ?

jglick · 2019-09-25T14:23:02Z

enable the new way by default unless the incoming TCP port is random

Or, if we still have an explicit option for the new mode, add form validation with a warning if you turn it while using a random port that your builds will not survive a master restart.

alxsap · 2019-09-25T16:54:14Z

Or, if we still have an explicit option for the new mode, add form validation with a warning if you turn it while using a random port that your builds will not survive a master restart.

I implemented Jesses proposal. I would keep the mode configurable, at least in the beginning, to have the possibility to use the old behaviour in case there are side effects in some scenarios.

...resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/help-directConnection.html

alxsap · 2019-09-27T11:35:38Z

@jglick @Vlatombe
jnlp-slave 3.35-5 which contains the required changes is released now.
The Pull Request is ready for further review and merging. Thanks a lot.

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly

alxsap · 2019-09-27T15:01:06Z

@oleg-nenashev thanks for the review. I pushed the changes.

Vlatombe · 2019-09-27T15:55:46Z

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java

+            if (!cloud.isDirectConnection()) {
+                env.put("JENKINS_URL", cloud.getJenkinsUrlOrDie());
+            } else {
+                Jenkins jenkins = Jenkins.getInstanceOrNull();


Use Jenkins.get()

Vlatombe · 2019-09-27T15:57:42Z

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

@@ -107,6 +110,7 @@
    private boolean capOnlyOnAlivePods;

    private String namespace;
+    private boolean directConnection;


To enable it by default for new instances (and also for unit tests)

Suggested change

private boolean directConnection;

private boolean directConnection = true;

Done. I also had to adjust/fix the tests.

Missed the tests running only in K8s clusters.
Is Jenkinsfile-kubernetes the easiest way to test this "locally"?

yes, that's the one

The PodTemplateBuilderTests now test both, with directConnection=true and directConnection=false (old test behaviour). JenkinsRule is active now since some Jenkins APIs are accessed by the new functionalities.

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java

switch to TcpSlaveAgentListener.getAdvertisedHost() Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

alxsap · 2019-10-07T07:11:30Z

@Vlatombe @jglick Tests are successful now. Only KubernetesPipelineTest#cascadingDelete is skipped.
I had to change some tests to test both, enabled directConnection and old behaviour. It would be nice if you could review again, to finish this pull request.
Thanks a lot!

Vlatombe · 2019-10-07T08:16:52Z

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly

@@ -27,6 +27,10 @@

    <f:validateButton title="${%Test Connection}" progress="${%Testing...}" method="testConnection" with="name,serverUrl,credentialsId,serverCertificate,skipTlsVerify,namespace" />

+    <f:entry title="${%Direct Connection}" field="directConnection">


Could use an optional block to wrap the jenkinsUrl in, since the two fields exclude each other.

Something like

Disable direct connection
then display the Jenkins URL field if it is ticked.

Hmm, I don't like the reverse logic. And for those already having a jenkinsUrl configured we would need to activate this flag ("if jenkinsUrl configured, set disableDirectConnection to true"). Sounds unnecessary complicated.

What do you think? If you still prefer it this way, I can implement it.

Vlatombe · 2019-10-07T08:19:56Z

src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilderTest.java

@@ -94,23 +102,43 @@ public void testParseLivenessProbe() {
    }

    @Test
-    public void testBuildWithoutSlave() throws Exception {
+    public void testBuildWithoutSlave_directConnection() throws Exception {


Sounds like something that could be written using https://github.com/junit-team/junit4/wiki/parameterized-tests

I first implemented org.junit.runners.Parameterized but then switched to junitparams.JUnitParamsRunner since Parameterized runs all test methods with the configured Parameters, not only the ones where it is wanted.

org.junit.runners.Parameterized does not allow to run only specific test methods with parameters.

alxsap · 2019-10-10T06:47:18Z

@oleg-nenashev @Vlatombe @jglick how shall we continue?

Vlatombe

lgtm

alxsap · 2019-10-15T06:36:22Z

@oleg-nenashev @Vlatombe any chance to review (Oleg) and release this?

alxsap · 2019-10-18T09:16:59Z

I can't reproduce the merge voter issue. All 27 KubernetesPipelineTests are successful in my cluster for the merge commit of d64a8b2 and 3386474.

Vlatombe · 2019-10-18T12:22:23Z

Looks like an infra issue.

siwyd · 2019-10-24T15:24:21Z

I believe this feature broke my existing Jenkins k8s setup. Rolling back to 1.20.1 fixed things. JNLP container image kept on crashing when Jenkins provisioned pods. Tried updating the jnlp-slave 3.35-5 image (no alpine available yet it seems?), but that did not fix it either it seems.

alxsap · 2019-10-25T09:24:27Z

Hello Simon,

Do you have an error message and stack trace of your crashing JNLP container?
I assume you still have the Jenkins URL configured in the Kubernetes configuration? Did you try direct Connection?
Do you have some special configuration, like Jenkins tunnel, etc.?
What's your Jenkins version?

Btw. jnlp-slave 3.35-5-alpine is available now (see https://github.com/jenkinsci/docker-jnlp-slave/issues/125).

bergemalm · 2019-10-28T15:12:59Z

This setting of direct connection set to true by default breaks helm chart deployment of stable/Jenkins. Since Jenkins URL and Jenkins tunnel is configured the jnlp fails in doing a connection because of:

option "-direct (-directConnection)" cannot be used with the option(s) [-url, -tunnel]

I would guess that is also what @siwyd might have run into?

Yes, you see the warning in the ui but it seems it should be an error with red letters? :)

If this is the way forward I can do a pr disabling direct connect for the stable/jenkins helm chart.

@alxsap

wgj · 2019-10-28T19:51:54Z

This went poorly for us using Helm chart stable/jenkins with existing JCasC yaml defining jenkinsUrl and jenkinsTunnel, and we are unable to use directConnection: true without url and tunnel set as, and JNLP tries to use the ingress, instead of the service:

Warning: JnlpProtocol3 is disabled by default, use JNLP_PROTOCOL_OPTS to alter the behavior
Oct 28, 2019 7:17:26 PM hudson.remoting.jnlp.Main createEngine
INFO: Setting up agent: jenkins-wgj-notifications-cicd-test-github-notifications--vcqpc
...
Oct 28, 2019 7:17:26 PM hudson.remoting.jnlp.Main$CuiListener status
INFO: Connecting to tribar.dev.example.com:50000

 ~  kubectl -n cicd get ingress              
NAME             HOSTS                        ADDRESS                                                                           PORTS     AGE
tribar-jenkins   tribar.dev.example.com   internal-a26e5115ccce11e8b42602604e068ad-27643652.us-west-2.elb.amazonaws.com   80, 443   98m
 ~  kubectl -n cicd get services
NAME                   TYPE        CLUSTER-IP       EXTERNAL-IP   PORT(S)     AGE
tribar-jenkins         ClusterIP   100.67.38.74     <none>        8080/TCP    102m
tribar-jenkins-agent   ClusterIP   100.67.160.163   <none>        50000/TCP   102m

edit:
Using Docker Hub image jenkins/jenkins:lts.

bergemalm · 2019-10-29T08:51:59Z

Fyi; created PR here helm/charts#18410

alxsap · 2019-10-29T09:45:53Z

@bergemalm You PR should solve your problem from my perspective.
On plugin side I think we should allow -direct together -tunnel to set the agent host:port explicitly.

I tried to reproduce the issue.

A scratch stable/jenkins helm installation has jenkinsTunnel configured to <name>-jenkins-agent:50000 and jenkinsUrl to http://<name>-jenkins:8080.
Note the different host! The agent listener is a separate service.
The installation has an outdated Kubernetes plugin and the configured JNLP image is also too old.
After updating both to the latest version the Kubernetes plugin has the new direct connection flag, but it is disabled due to the existing configuration.
A test pipeline worked with disabled direct connection and configured Jenkins Tunnel and Jenkins URL.
When enabling directConnection and removing Jenkins tunnel and Jenkins URL the JNLP container tries to connect, but times out. The used host is the wrong one.
The reason is how direct connection tries to identify the correct host automatically.

JNLP Log:

Oct 29, 2019 8:58:17 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Agent discovery successful
  Agent address: womping-hydra-jenkins
  Agent port:    50000
  Identity:      33:52:38:6b:73:bf:cf:be:ea:13:7d:b2:b4:5d:89:06
Oct 29, 2019 8:58:17 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Handshaking
Oct 29, 2019 8:58:17 AM hudson.remoting.jnlp.Main$CuiListener status
INFO: Connecting to womping-hydra-jenkins:50000

When configuring Jenkins tunnel again remoting inside the JNLP container fails with this error.

Warning: JnlpProtocol3 is disabled by default, use JNLP_PROTOCOL_OPTS to alter the behavior
option "-direct (-directConnection)" cannot be used with the option(s) [-url, -tunnel]

Next steps

I think we have to allow -tunnel together with -direct to override the agent host:port.

@Vlatombe @jglick @oleg-nenashev

siwyd · 2019-10-29T20:10:39Z

@alxsap The log of the crashing JNLP container. This is using 3.35-5-alpine.

❯ k logs -c jnlp python-oci-image-master-20-r0xg3-k64jw-9dmp3
Warning: JnlpProtocol3 is disabled by default, use JNLP_PROTOCOL_OPTS to alter the behavior
Warning: SECRET is defined twice in command-line arguments and the environment variable
Warning: AGENT_NAME is defined twice in command-line arguments and the environment variable
At least one -url option is required.
java -jar agent.jar [options...] <secret key> <agent name>

I'm using JCasC to provision the Kubernetes cloud:

        jenkins:
          clouds:
          - kubernetes:
              jenkinsTunnel: jenkins-agent:50000
              jenkinsUrl: http://jenkins:8080
              name: kubernetes
              namespace: cicd
              waitForPodSec: 300
              defaultsProviderTemplate: default
              templates:
              - name: default
                containers:
                - name: jnlp
                  image: jenkins/jnlp-slave:3.35-5-alpine
                  args: "^${computer.jnlpmac} ^${computer.name}"
                showRawYaml: true
                nodeUsageMode: NORMAL

When looking in the settings, seems like direct connection is set by default then:

bergemalm · 2019-10-29T20:50:34Z

After updating both to the latest version the Kubernetes plugin has the new direct connection flag, but it is disabled due to the existing configuration.

Not sure how you updated but setting kubernetes plugin to version 1.20.2 (via helm) sets direct connection to the default "true". Not disabled. (also each pod start will set it to true)

alxsap · 2019-10-30T07:59:10Z

@bergemalm @siwyd I think this is related to how the casc plugin creates/injects the config. It seems when it does not set a specific parameter (direct connection in this case) its default is used.
I'll try to check this if I find some time.

Vlatombe · 2019-10-30T14:13:41Z

Maybe we should just change back the default value for that checkbox, since this causing so much trouble.

alxsap · 2019-11-06T08:02:28Z

@siwyd @bergemalm @wgj Release 1.21.1 switched the direct connection default to false. Do you still experience the described issues with casc?

bergemalm · 2019-11-06T08:09:42Z

I did a fix for it which created some issues. Since it defaults to false now a pr removing the fix was merged this morning. Shouldn't be an issue anymore. Thx.
helm/charts#18490

siwyd · 2019-11-06T19:47:43Z

@alxsap I just reprovisioned with 1.21.1 and things seem to be working!

alxsap added 2 commits September 23, 2019 12:35

jglick reviewed Sep 24, 2019

View reviewed changes

Vlatombe reviewed Sep 25, 2019

View reviewed changes

alxsap added 2 commits September 25, 2019 12:14

Review feedback

9cc5841

Review feedback

51c5672

Added random port warning

f220a38

Vlatombe added the enhancement Improvements label Sep 25, 2019

Added warnings when agent listener port random or disabled

f41b08a

Improved getAdvertisedHost() + referenced jenkins core pull request

91b7d13

MRamonLeon mentioned this pull request Sep 26, 2019

Introduce new hudson.TcpSlaveAgentListener.getAdvertisedHost() method jenkinsci/jenkins#4227

Merged

4 tasks

alxsap commented Sep 27, 2019

View reviewed changes

...resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/help-directConnection.html Show resolved Hide resolved

Added supported jnlp-slave version to help

6ecf80d

alxsap changed the title ~~WIP: Enable direct connections to TCP agent listener~~ Enable direct connections to TCP agent listener Sep 27, 2019

oleg-nenashev self-requested a review September 27, 2019 11:51

jglick requested a review from Vlatombe September 27, 2019 12:31

oleg-nenashev reviewed Sep 27, 2019

View reviewed changes

Review feedback

d7d3655

Merge branch 'master' into PR_directConnection

18b7f72

Vlatombe reviewed Sep 27, 2019

View reviewed changes

alxsap added 2 commits October 1, 2019 11:31

Jenkins.get() instead of .getInstanceOrNull()

64cfe5f

directConnection=true default now + fixed tests

cd5f83f

The PodTemplateBuilderTests now test both, with directConnection=true and directConnection=false (old test behaviour). JenkinsRule is active now since some Jenkins APIs are accessed by the new functionalities.

jglick reviewed Oct 1, 2019

View reviewed changes

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java Outdated Show resolved Hide resolved

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java Show resolved Hide resolved

Adjusted TODO

7e4c412

switch to TcpSlaveAgentListener.getAdvertisedHost() Co-Authored-By: Jesse Glick <jglick@cloudbees.com>

This comment has been minimized.

Sign in to view

minor import cleanup

18fa342

Vlatombe reviewed Oct 7, 2019

View reviewed changes

alxsap added 2 commits October 7, 2019 11:08

Parameterized test

1b90613

Switch to JUnitParams

5925a51

org.junit.runners.Parameterized does not allow to run only specific test methods with parameters.

oleg-nenashev self-requested a review October 7, 2019 11:06

minor change: rename parameterized @TestCaseNames

d64a8b2

Vlatombe approved these changes Oct 10, 2019

View reviewed changes

Vlatombe merged commit 342166c into jenkinsci:master Oct 18, 2019

alxsap deleted the PR_directConnection branch October 18, 2019 12:55

torstenwalter mentioned this pull request Oct 29, 2019

[stable/jenkins] Fix: Disable direct connection option default helm/charts#18410

Merged

alxsap mentioned this pull request Oct 31, 2019

Switch directConnection default to false #633

Merged

		@@ -0,0 +1,6 @@
		<div>
		When this option is enabled a <a href="https://github.com/jenkinsci/remoting/blob/master/docs/tcpAgent.md#connect-directly-to-tcp-port">direct connection</a>

	private boolean directConnection;
	private boolean directConnection = true;

		@@ -27,6 +27,10 @@

		<f:validateButton title="${%Test Connection}" progress="${%Testing...}" method="testConnection" with="name,serverUrl,credentialsId,serverCertificate,skipTlsVerify,namespace" />

		<f:entry title="${%Direct Connection}" field="directConnection">

Enable direct connections to TCP agent listener #602

Enable direct connections to TCP agent listener #602

Conversation

alxsap commented Sep 24, 2019 • edited

jglick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vlatombe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alxsap commented Sep 25, 2019

Vlatombe commented Sep 25, 2019

jglick commented Sep 25, 2019

alxsap commented Sep 25, 2019

alxsap commented Sep 27, 2019

alxsap commented Sep 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

alxsap commented Oct 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alxsap commented Oct 10, 2019

Vlatombe left a comment

Choose a reason for hiding this comment

alxsap commented Oct 15, 2019

alxsap commented Oct 18, 2019

Vlatombe commented Oct 18, 2019

siwyd commented Oct 24, 2019

alxsap commented Oct 25, 2019

bergemalm commented Oct 28, 2019 • edited

wgj commented Oct 28, 2019 • edited

bergemalm commented Oct 29, 2019

alxsap commented Oct 29, 2019

siwyd commented Oct 29, 2019

bergemalm commented Oct 29, 2019 • edited

alxsap commented Oct 30, 2019

Vlatombe commented Oct 30, 2019

alxsap commented Nov 6, 2019

bergemalm commented Nov 6, 2019 • edited

siwyd commented Nov 6, 2019

alxsap commented Sep 24, 2019 •

edited

bergemalm commented Oct 28, 2019 •

edited

wgj commented Oct 28, 2019 •

edited

bergemalm commented Oct 29, 2019 •

edited

bergemalm commented Nov 6, 2019 •

edited