Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[JENKINS-39596,JENKINS-39617] - hudsonUrl in Remoting Engine was always null since 3.0 #131

Merged
merged 2 commits into from
Nov 9, 2016
Merged

[JENKINS-39596,JENKINS-39617] - hudsonUrl in Remoting Engine was always null since 3.0 #131

merged 2 commits into from
Nov 9, 2016

Conversation

oleg-nenashev
Copy link
Member

Starting from Jenkins 2.16 (remoting 3.0) JNLP agents cannot be connected via Java Web Start JENKINS-39596. It happens, because Slave Installer Module in Jenkins core relies on the hudsonUrl in hudson.remoting.Engine, but starting from remoting 3.0 this field was alway null. Hence we were getting crappy exceptions in Jenkins, which were blocking the agent connection:

java.net.MalformedURLException: no protocol: jnlpJars/slave.jar
	at java.net.URL.<init>(URL.java:593)
	at java.net.URL.<init>(URL.java:490)
	at org.jenkinsci.modules.slave_installer.impl.InstallerGui.call(InstallerGui.java:65)
	at org.jenkinsci.modules.slave_installer.impl.InstallerGui.call(InstallerGui.java:34)
	at hudson.remoting.UserRequest.perform(UserRequest.java:153)
	at hudson.remoting.UserRequest.perform(UserRequest.java:50)

Changes:

  • JnlpAgentEndpointResolver now records service URL to the discovered Endpoint
  • Remoting Engine assigns the discovered service URL as hudsonUrl and passes it to API users
  • Remoting Engine does not fail the connection if one of the passed parameters is invalid (JENKINS-39617). We needed such check for URL parsing for this issue in any case

@oleg-nenashev
Copy link
Member Author

@reviewbybees, esp @stephenc

@oleg-nenashev oleg-nenashev changed the title [JENKINS-39596,JENKINS-39617] [JENKINS-39596,JENKINS-39617] - hudsonUrl in Remoting Engine was always null since 3.0 Nov 9, 2016
oleg-nenashev
oleg-nenashev commented Nov 9, 2016
View reviewed changes
src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpoint.java
@@ -96,6 +117,15 @@ public InetSocketAddress getAddress() {
}

/**
* Retrieves URL of the web service providing the remoting endpoint.
* @return Service URL if available. {@code null} otherwise.
*/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing @since. Will fix during the merge if there is no other bugs

@ghost
Copy link

ghost commented Nov 9, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@stephenc
Copy link
Member

stephenc commented Nov 9, 2016

🐝

1 similar comment
@jtnord
Copy link
Member

jtnord commented Nov 9, 2016

🐝

jglick
jglick approved these changes Nov 9, 2016
View reviewed changes
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐝

@oleg-nenashev
Copy link
Member Author

@reviewbybees done

@ghost
Copy link

ghost commented Nov 9, 2016

This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging.

@oleg-nenashev oleg-nenashev merged commit e0cfb18 into jenkinsci:master Nov 9, 2016
oleg-nenashev added a commit that referenced this pull request Nov 9, 2016
@oleg-nenashev
Changelog: Noting #131
f5abb07
oleg-nenashev added a commit to oleg-nenashev/jenkins that referenced this pull request Nov 9, 2016
@oleg-nenashev
Update remoting to 2.31 in the Jenkins core.
3095ca4 
The change introduces one serious bugfix (JENKINS-39596) and a bunch of various diagnostics improvements.

Bugfixes:

* [JENKINS-39596](https://issues.jenkins-ci.org/browse/JENKINS-39596) -
Jenkins URL in `hudson.remoting.Engine` was always `null` since `3.0`.
It was causing connection failures of Jenkins JNLP agents when using Java Web Start.
([PR jenkinsci#131](jenkinsci/remoting#131))
* [JENKINS-39617](https://issues.jenkins-ci.org/browse/JENKINS-39617) -
`hudson.remoting.Engine` was failing to establish connection if one of the URLs parameter in parameters was malformed.
([PR jenkinsci#131](jenkinsci/remoting#131))

Improvements:

* [JENKINS-39150](https://issues.jenkins-ci.org/browse/JENKINS-39150) -
Add logic for dumping diagnostics across all the channels.
([PR jenkinsci#122](jenkinsci/remoting#122), [PR jenkinsci#125](jenkinsci/remoting#125))
* [JENKINS-39543](https://issues.jenkins-ci.org/browse/JENKINS-39543) -
Improve the caller/callee correlation diagnostics in thread dumps.
([PR jenkinsci#119](jenkinsci/remoting#119))
* [JENKINS-39290](https://issues.jenkins-ci.org/browse/JENKINS-39290) -
Add the `org.jenkinsci.remoting.nio.NioChannelHub.disabled` flag for disabling NIO (mostly for debugging purposes).
([PR jenkinsci#123](jenkinsci/remoting#123))
* [JENKINS-38692](https://issues.jenkins-ci.org/browse/JENKINS-38692) -
Add extra logging to help diagnosing `IOHub` Thread spikes.
([PR #116](jenkinsci/remoting#116))
* [JENKINS-39289](https://issues.jenkins-ci.org/browse/JENKINS-39289) -
 When a proxy fails, report what caused the channel to go down.
([PR jenkinsci#128](jenkinsci/remoting#128))
@oleg-nenashev oleg-nenashev mentioned this pull request Nov 9, 2016
Merged
oleg-nenashev added a commit to jenkinsci/jenkins that referenced this pull request Nov 10, 2016
@oleg-nenashev
Update remoting to 2.31 in the Jenkins core. (#2628)
7a948d3 
The change introduces one serious bugfix (JENKINS-39596) and a bunch of various diagnostics improvements.

Bugfixes:

* [JENKINS-39596](https://issues.jenkins-ci.org/browse/JENKINS-39596) -
Jenkins URL in `hudson.remoting.Engine` was always `null` since `3.0`.
It was causing connection failures of Jenkins JNLP agents when using Java Web Start.
([PR #131](jenkinsci/remoting#131))
* [JENKINS-39617](https://issues.jenkins-ci.org/browse/JENKINS-39617) -
`hudson.remoting.Engine` was failing to establish connection if one of the URLs parameter in parameters was malformed.
([PR #131](jenkinsci/remoting#131))

Improvements:

* [JENKINS-39150](https://issues.jenkins-ci.org/browse/JENKINS-39150) -
Add logic for dumping diagnostics across all the channels.
([PR #122](jenkinsci/remoting#122), [PR #125](jenkinsci/remoting#125))
* [JENKINS-39543](https://issues.jenkins-ci.org/browse/JENKINS-39543) -
Improve the caller/callee correlation diagnostics in thread dumps.
([PR #119](jenkinsci/remoting#119))
* [JENKINS-39290](https://issues.jenkins-ci.org/browse/JENKINS-39290) -
Add the `org.jenkinsci.remoting.nio.NioChannelHub.disabled` flag for disabling NIO (mostly for debugging purposes).
([PR #123](jenkinsci/remoting#123))
* [JENKINS-38692](https://issues.jenkins-ci.org/browse/JENKINS-38692) -
Add extra logging to help diagnosing `IOHub` Thread spikes.
([PR #116](jenkinsci/remoting#116))
* [JENKINS-39289](https://issues.jenkins-ci.org/browse/JENKINS-39289) -
 When a proxy fails, report what caused the channel to go down.
([PR #128](jenkinsci/remoting#128))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jglick jglick jglick approved these changes

Assignees
No one assigned
Labels
needs-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@oleg-nenashev @stephenc @jtnord @jglick