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

[JENKINS-33596, JENKINS-60913] Remove network discovery services #4460

Merged
merged 14 commits into from
Feb 7, 2020
Merged

[JENKINS-33596, JENKINS-60913] Remove network discovery services #4460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9afc63f
Remove multicast discovery services.
jeffret-b Jan 29, 2020
61d9c67
Refine clean up with DNSMultiCast.
jeffret-b Jan 29, 2020
43096a9
Deprecate remaining class fragment.
jeffret-b Jan 30, 2020
684fbbd
Remove unneeded dependency.
jeffret-b Jan 30, 2020
c7afc8a
Improve code comments for external usages.
jeffret-b Jan 30, 2020
4274291
Remove multicast discovery services.
jeffret-b Jan 29, 2020
746d004
Refine clean up with DNSMultiCast.
jeffret-b Jan 29, 2020
d18c7b5
Deprecate remaining class fragment.
jeffret-b Jan 30, 2020
5afcead
Remove unneeded dependency.
jeffret-b Jan 30, 2020
8fec924
Add log warning messages.
jeffret-b Feb 5, 2020
21bc110
Don't need to implement the interface anymore.
jeffret-b Feb 5, 2020
8394ae1
Merge remote-tracking branch 'origin/removeMulticast' into removeMult…
jeffret-b Feb 5, 2020
6b118ce
Fix initializer.
jeffret-b Feb 5, 2020
4b471a1
Add URLs to log messages.
jeffret-b Feb 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,11 +533,6 @@ THE SOFTWARE.
<artifactId>wstx-asl</artifactId>
<version>3.2.9</version>
</dependency>
<dependency>
<groupId>org.jmdns</groupId>
<artifactId>jmdns</artifactId>
<version>3.5.5</version>
</dependency>
<dependency>
<groupId>net.java.dev.jna</groupId>
<artifactId>jna</artifactId>
Expand Down
203 changes: 15 additions & 188 deletions core/src/main/java/hudson/DNSMultiCast.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,205 +1,32 @@
package hudson;

import hudson.init.Initializer;
import jenkins.util.SystemProperties;
import jenkins.model.Jenkins;
import jenkins.model.Jenkins.MasterComputer;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

import javax.jmdns.ServiceInfo;
import javax.jmdns.impl.JmDNSImpl;
import java.io.Closeable;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.InetAddress;
import java.net.URL;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.logging.Level;
import java.util.logging.Logger;

import static hudson.init.InitMilestone.COMPLETED;

/**
* Registers a DNS multi-cast service-discovery support.
*
* @author Kohsuke Kawaguchi
* @deprecated No longer does anything.
*/
public class DNSMultiCast implements Closeable {
private JenkinsJmDNS jmdns;

public DNSMultiCast(final Jenkins jenkins) {
jeffret-b marked this conversation as resolved.
Show resolved Hide resolved
if (disabled) return; // escape hatch

// the registerService call can be slow. run these asynchronously
MasterComputer.threadPoolForRemoting.submit(new Callable<Object>() {
public Object call() {
try {
jmdns = new JenkinsJmDNS(null, null);

Map<String,String> props = new HashMap<>();
String rootURL = jenkins.getRootUrl();
if (rootURL==null) return null;

props.put("url", rootURL);
try {
props.put("version",String.valueOf(Jenkins.getVersion()));
} catch (IllegalArgumentException e) {
// failed to parse the version number
}

TcpSlaveAgentListener tal = jenkins.getTcpSlaveAgentListener();
if (tal!=null)
props.put("slave-port",String.valueOf(tal.getPort()));

props.put("server-id", jenkins.getLegacyInstanceId());

URL jenkins_url = new URL(rootURL);
int jenkins_port = jenkins_url.getPort();
if (jenkins_port == -1) {
jenkins_port = 80;
}
if (jenkins_url.getPath().length() > 0) {
props.put("path", jenkins_url.getPath());
}

jmdns.registerService(ServiceInfo.create("_hudson._tcp.local.","jenkins",
jenkins_port,0,0,props)); // for backward compatibility
jmdns.registerService(ServiceInfo.create("_jenkins._tcp.local.","jenkins",
jenkins_port,0,0,props));

// Make Jenkins appear in Safari's Bonjour bookmarks
jmdns.registerService(ServiceInfo.create("_http._tcp.local.","Jenkins",
jenkins_port,0,0,props));
} catch (IOException e) {
LOGGER.log(Level.INFO, "Cannot advertise service to DNS multi-cast, skipping: {0}", e);
LOGGER.log(Level.FINE, null, e);
}
return null;
}
});
}

public void close() {
if (jmdns!=null) {
try {
jmdns.abort();
jmdns = null;
} catch (final IOException e) {
LOGGER.log(Level.WARNING,"Failed to close down JmDNS instance!",e);
}
}
}

@Deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Add Restricted(DoNotUse) to fail builds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't get it to work without failing the Jenkins build. We could use NoExternalUse, though.

Copy link
Member

Choose a reason for hiding this comment

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

jenkinsci/lib-access-modifier#17

@Restricted(NoExternalUse.class)
public class DNSMultiCast {
private static final Logger LOGGER = Logger.getLogger(DNSMultiCast.class.getName());

/**
* Class that extends {@link JmDNSImpl} to add an abort method. Since {@link javax.jmdns.JmDNS#close()} might
* make the instance hang during the shutdown, the abort method terminate uncleanly, but rapidly and
* without blocking.
*
* Initially it was part of the jenkinsci/jmdns forked library, but now this class is responsible for aborting,
* allowing to have a direct and clean dependency to the original library.
*
* The abort() method is pretty similar to close() method. To access private methods and fields uses
* reflection.
*
* @since 2.178
*
* See JENKINS-25369 for further details
*/
private static class JenkinsJmDNS extends JmDNSImpl {
private static Logger logger = Logger.getLogger(JmDNSImpl.class.getName());
private final Class parent;

/**
* Create an instance of JmDNS and bind it to a specific network interface given its IP-address.
*
* @param address IP address to bind to.
* @param name name of the newly created JmDNS
* @throws IOException
*/
public JenkinsJmDNS(InetAddress address, String name) throws IOException {
super(address, name);
this.parent = this.getClass().getSuperclass();
}

/**
* Works like {@link #close()} but terminate uncleanly, but rapidly and without blocking.
*/
public void abort() throws IOException {
if (this.isClosing()) {
return;
}

if (logger.isLoggable(Level.FINER)) {
logger.finer("Aborting JmDNS: " + this);
}
// Stop JmDNS
// This protects against recursive calls
if (this.closeState()) {
// We got the tie break now clean up

// Stop the timer
logger.finer("Canceling the timer");
this.cancelTimer();

// Cancel all services
// KK: this is a blocking call that doesn't fit 'abort'
// this.unregisterAllServices();
executePrivateParentMethod("disposeServiceCollectors");

// KK: another blocking call
// if (logger.isLoggable(Level.FINER)) {
// logger.finer("Wait for JmDNS cancel: " + this);
// }
// this.waitForCanceled(DNSConstants.CLOSE_TIMEOUT);

// Stop the canceler timer
logger.finer("Canceling the state timer");
this.cancelStateTimer();

// Stop the executor
shutdown();

// close socket
executePrivateParentMethod("closeMulticastSocket");

// remove the shutdown hook
if (_shutdown != null) {
Runtime.getRuntime().removeShutdownHook(_shutdown);
}

if (logger.isLoggable(Level.FINER)) {
logger.finer("JmDNS closed.");
}
}
advanceState(null);
}

private void shutdown() throws IOException {
try {
Field executor = this.parent.getDeclaredField("_executor");
executor.setAccessible(true);
ExecutorService _executor = (ExecutorService) executor.get(this);
_executor.shutdown();
} catch (NoSuchFieldException | IllegalAccessException e) {
logger.log(Level.SEVERE, "Error trying to abort JmDNS", e);
throw new IOException(e);
}
}
public static boolean disabled = SystemProperties.getBoolean(DNSMultiCast.class.getName()+".disabled", true);
jeffret-b marked this conversation as resolved.
Show resolved Hide resolved
jeffret-b marked this conversation as resolved.
Show resolved Hide resolved

private void executePrivateParentMethod(String method) throws IOException {
try {
Method m = this.parent.getDeclaredMethod(method);
m.setAccessible(true);
m.invoke(this);
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
logger.log(Level.SEVERE, "Error trying to abort JmDNS", e);
throw new IOException(e);
}
@Initializer(before=COMPLETED)
public static void warn() {
if (!disabled) {
LOGGER.warning("DNS multicast capability has been removed from Jenkins. More information: https://jenkins.io/redirect/dns-multicast");
}

}
public static boolean disabled = SystemProperties.getBoolean(DNSMultiCast.class.getName()+".disabled", true);

}
58 changes: 0 additions & 58 deletions core/src/main/java/hudson/UDPBroadcastFragment.java
View file
Open in desktop

This file was deleted.

Loading