-
Notifications
You must be signed in to change notification settings - Fork 51
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
Improve job hostname detection on Windows #448
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,49 +26,9 @@ of this software and associated documentation files (the "Software"), to deal | |
package org.datadog.jenkins.plugins.datadog; | ||
|
||
import edu.umd.cs.findbugs.annotations.NonNull; | ||
import hudson.EnvVars; | ||
import hudson.ExtensionList; | ||
import hudson.FilePath; | ||
import hudson.PluginManager; | ||
import hudson.PluginWrapper; | ||
import hudson.model.Actionable; | ||
import hudson.model.Computer; | ||
import hudson.model.Item; | ||
import hudson.model.Node; | ||
import hudson.model.Result; | ||
import hudson.model.Run; | ||
import hudson.model.TaskListener; | ||
import hudson.model.User; | ||
import hudson.*; | ||
import hudson.model.*; | ||
import hudson.model.labels.LabelAtom; | ||
import java.io.BufferedReader; | ||
import java.io.File; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.io.InputStreamReader; | ||
import java.net.Inet4Address; | ||
import java.net.UnknownHostException; | ||
import java.nio.charset.Charset; | ||
import java.text.ParseException; | ||
import java.text.SimpleDateFormat; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Collection; | ||
import java.util.Collections; | ||
import java.util.Date; | ||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.TimeZone; | ||
import java.util.function.Function; | ||
import java.util.logging.Level; | ||
import java.util.logging.Logger; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
import java.util.stream.Collectors; | ||
import javax.annotation.Nonnull; | ||
import javax.annotation.Nullable; | ||
import jenkins.model.Jenkins; | ||
import jenkins.security.MasterToSlaveCallable; | ||
import org.apache.commons.lang.StringEscapeUtils; | ||
|
@@ -82,22 +42,30 @@ of this software and associated documentation files (the "Software"), to deal | |
import org.datadog.jenkins.plugins.datadog.util.SuppressFBWarnings; | ||
import org.datadog.jenkins.plugins.datadog.util.TagsUtil; | ||
import org.jenkinsci.plugins.pipeline.StageStatus; | ||
import org.jenkinsci.plugins.workflow.actions.ArgumentsAction; | ||
import org.jenkinsci.plugins.workflow.actions.ErrorAction; | ||
import org.jenkinsci.plugins.workflow.actions.LabelAction; | ||
import org.jenkinsci.plugins.workflow.actions.NotExecutedNodeAction; | ||
import org.jenkinsci.plugins.workflow.actions.QueueItemAction; | ||
import org.jenkinsci.plugins.workflow.actions.StageAction; | ||
import org.jenkinsci.plugins.workflow.actions.ThreadNameAction; | ||
import org.jenkinsci.plugins.workflow.actions.TimingAction; | ||
import org.jenkinsci.plugins.workflow.actions.WarningAction; | ||
import org.jenkinsci.plugins.workflow.actions.*; | ||
import org.jenkinsci.plugins.workflow.flow.FlowExecution; | ||
import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner; | ||
import org.jenkinsci.plugins.workflow.graph.BlockEndNode; | ||
import org.jenkinsci.plugins.workflow.graph.BlockStartNode; | ||
import org.jenkinsci.plugins.workflow.graph.FlowNode; | ||
import org.jenkinsci.plugins.workflow.job.WorkflowRun; | ||
|
||
import javax.annotation.Nonnull; | ||
import javax.annotation.Nullable; | ||
import java.io.*; | ||
import java.net.Inet4Address; | ||
import java.net.UnknownHostException; | ||
import java.nio.charset.Charset; | ||
import java.text.ParseException; | ||
import java.text.SimpleDateFormat; | ||
import java.util.*; | ||
import java.util.function.Function; | ||
import java.util.logging.Level; | ||
import java.util.logging.Logger; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
import java.util.stream.Collectors; | ||
|
||
public class DatadogUtilities { | ||
|
||
private static final Logger logger = Logger.getLogger(DatadogUtilities.class.getName()); | ||
|
@@ -743,19 +711,29 @@ public static boolean isMainNode(String nodeName) { | |
} | ||
|
||
public static String getNodeHostname(@Nullable EnvVars envVars, @Nullable Computer computer) { | ||
if (envVars != null) { | ||
String ddHostname = envVars.get(DatadogGlobalConfiguration.DD_CI_HOSTNAME); | ||
if (DatadogUtilities.isValidHostname(ddHostname)) { | ||
return ddHostname; | ||
} | ||
String hostname = envVars.get("HOSTNAME"); | ||
if (DatadogUtilities.isValidHostname(hostname)) { | ||
return hostname; | ||
if (computer != null) { | ||
try { | ||
EnvVars computerEnvironment = computer.getEnvironment(); | ||
String computerEnvVarsHostname = getNodeHostname(computerEnvironment); | ||
if (isValidHostname(computerEnvVarsHostname)) { | ||
return computerEnvVarsHostname; | ||
} | ||
|
||
} catch (InterruptedException e) { | ||
Thread.currentThread().interrupt(); | ||
logException(logger, Level.FINE, "Interrupted while trying to get computer env vars", e); | ||
} catch (Exception e) { | ||
logException(logger, Level.FINE, "Error getting computer env vars", e); | ||
} | ||
} | ||
|
||
String envVarsHostname = getNodeHostname(envVars); | ||
if (isValidHostname(envVarsHostname)) { | ||
return envVarsHostname; | ||
} | ||
|
||
Comment on lines
+730
to
+734
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems we're duplicating this piece of logic with computer EnvVars and other EnvVars. Have we considered the following approach to avoid this duplicated code?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to examine both, as I couldn't find a clear definition of what the "job-level" vars are: I suspect they might include whatever's being set in the Job config UI or in the pipeline DSL. The computer-level ones, however, include variables set externally on the worker node that executes the job. |
||
try { | ||
if(computer != null) { | ||
if (computer != null) { | ||
String computerNodeName = DatadogUtilities.getNodeName(computer); | ||
if (DatadogUtilities.isMainNode(computerNodeName)) { | ||
String masterHostname = DatadogUtilities.getHostname(null); | ||
|
@@ -772,26 +750,46 @@ public static String getNodeHostname(@Nullable EnvVars envVars, @Nullable Comput | |
Node node = computer.getNode(); | ||
if (node != null) { | ||
FilePath rootPath = node.getRootPath(); | ||
if (isUnix(rootPath)) { | ||
ShellCommandCallable hostnameCommand = new ShellCommandCallable( | ||
Collections.emptyMap(), HOSTNAME_CMD_TIMEOUT_MILLIS, "hostname", "-f"); | ||
if (rootPath != null) { | ||
String[] command; | ||
if (isUnix(rootPath)) { | ||
command = new String[]{"hostname", "-f"}; | ||
} else { | ||
command = new String[]{"hostname"}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this command work in all Windows environments? If not, what would happen in that case? Would it throw an exception? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure about every possible version, but it does seem to be a standard command. If it doesn't work an exception will be thrown, then caught in this method and logged. |
||
} | ||
|
||
ShellCommandCallable hostnameCommand = new ShellCommandCallable(Collections.emptyMap(), HOSTNAME_CMD_TIMEOUT_MILLIS, command); | ||
String shellHostname = rootPath.act(hostnameCommand).trim(); | ||
if (DatadogUtilities.isValidHostname(shellHostname)) { | ||
return shellHostname; | ||
} | ||
} | ||
} | ||
} | ||
} catch (InterruptedException e){ | ||
} catch (InterruptedException e) { | ||
Thread.currentThread().interrupt(); | ||
logException(logger, Level.FINE, "Interrupted while trying to extract hostname from StepContext.", e); | ||
|
||
} catch (Exception e){ | ||
} catch (Exception e) { | ||
logException(logger, Level.FINE, "Unable to get hostname for node " + computer.getName(), e); | ||
} | ||
return null; | ||
} | ||
|
||
private static String getNodeHostname(@Nullable EnvVars envVars) { | ||
if (envVars != null) { | ||
String ddHostname = envVars.get(DatadogGlobalConfiguration.DD_CI_HOSTNAME); | ||
if (DatadogUtilities.isValidHostname(ddHostname)) { | ||
return ddHostname; | ||
} | ||
String hostname = envVars.get("HOSTNAME"); | ||
if (DatadogUtilities.isValidHostname(hostname)) { | ||
return hostname; | ||
} | ||
} | ||
return null; | ||
} | ||
|
||
private static boolean isUnix(FilePath filePath) throws IOException, InterruptedException { | ||
return filePath != null && filePath.act(new IsUnix()); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between
EnvVars
coming as the method parameter and theEnvVars
coming from theComputer
object? Are those not the same?Also, how do we know the "computer" envvars have prevalence over "global?" envvars?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ones coming from the computer are relevant for that specific node (could be either the controller or a worker node). The ones coming as method parameter are the ones associated with the job. Debugging shows they're different.