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.

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

Define hudsonUrl in WebSocket mode to fix agent installers #454

Merged
merged 1 commit into from
May 26, 2021

Conversation

jglick
Copy link
Member

@jglick jglick commented May 18, 2021

I noticed in CloudBees CI trying to launch a shared agent in WebSocket mode from the Java Web Start button an error indicating that the equivalent of https:/jenkinsci/slave-installer-module/blob/8ab32a0616d147adef0d27fb51a359796e069eeb/src/main/java/org/jenkinsci/modules/slave_installer/impl/InstallerGui.java#L78 was throwing an NPE, despite the Jenkins root URL being configured. I could not reproduce a similar stack trace using an OSS agent connection, though the installer menu was not created either, so maybe the exception was just swallowed somewhere, or printed to the agent log which I forgot to check. Anyway, it seems clear from code that this getHudsonUrl method would have worked in TCP but not WebSocket mode.

Also cleaning up a trailing slash check introduced in #391.

Tested in context by building Jenkins core with this, then building CBCI operations center & connected controller with that, running both, hooking them up, and going through two scenarios:

  • static agent on the CC
  • shared agent on the OC

In both cases, used the JWS launch button and got a File from the agent installer (systemd in my case) as expected.

@jglick jglick added the bug For changelog: Fixes a bug. label May 18, 2021
@jglick jglick changed the title Define hudsonUrl in WebSocket mode Define hudsonUrl in WebSocket mode to fix agent installers May 18, 2021
@jglick
Copy link
Member Author

jglick commented May 19, 2021

Note that the agent installer system only appears in GUI mode, which is only enabled when launched via JWS, which is not something we generally encourage. Discussion in JEP-230

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jglick !

We may merge it in 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process

@oleg-nenashev oleg-nenashev merged commit 20e4994 into jenkinsci:master May 26, 2021
@jglick jglick deleted the hudsonUrl branch May 26, 2021 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Fixes a bug. ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants