Configuration for placing the xauthority file in node fs root

Added a configuration option to place the temporary xauthority file on the fs root of the node instead of the in the build's workspace. This can be used as a workaround for JENKINS-19139 if the path to the fs root doesn't contain any spaces but for example the folder to the job does.
jenkinsci · Aug 25, 2015 · 1ce6699 · 1ce6699
1 parent 967b674
commit 1ce6699
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 10 deletions.
diff --git a/src/main/java/hudson/plugins/xvnc/Xvnc.java b/src/main/java/hudson/plugins/xvnc/Xvnc.java
@@ -87,30 +87,40 @@ public void setUp(Context context, Run<?, ?> build, FilePath workspace, Launcher
 
         workspace.mkdirs();
         doSetUp(context, build, workspace, node, launcher, logger, cmd, 10, DESCRIPTOR.minDisplayNumber,
-                DESCRIPTOR.maxDisplayNumber);
+                DESCRIPTOR.maxDisplayNumber, DESCRIPTOR.xauthorityInFsRoot);
     }
 
     private void doSetUp(Context context, Run<?,?> build, FilePath workspace, Node node, final Launcher launcher, final PrintStream logger,
-            String cmd, int retries, int minDisplayNumber, int maxDisplayNumber)
+            String cmd, int retries, int minDisplayNumber, int maxDisplayNumber, boolean xauthorityInFsRoot)
                     throws IOException, InterruptedException {
 
         final DisplayAllocator allocator = getAllocator(node);
 
         final int displayNumber = allocator.allocate(minDisplayNumber, maxDisplayNumber);
-        final String actualCmd = Util.replaceMacro(cmd, Collections.singletonMap("DISPLAY_NUMBER",String.valueOf(displayNumber)));
+        final String actualCmd = Util.replaceMacro(cmd, Collections.singletonMap("DISPLAY_NUMBER", String.valueOf(displayNumber)));
 
         logger.println(Messages.Xvnc_STARTING());
 
         String[] cmds = Util.tokenize(actualCmd);
 
-        final FilePath xauthority = workspace.createTempFile(".Xauthority-", "");
+        final FilePath xRoot;
+
+        if (xauthorityInFsRoot) {
+            xRoot = workspace.toComputer().getNode().getRootPath();
+        } else {
+            xRoot = workspace;
+        }
+
+        final FilePath xauthority;
         final Map<String,String> xauthorityEnv = new HashMap<String, String>();
         if (useXauthority) {
+            xauthority = xRoot.createTempFile(".Xauthority-", "");
             xauthorityEnv.put("XAUTHORITY", xauthority.getRemote());
             context.env("XAUTHORITY", xauthority.getRemote());
         } else {
             // Need something to identify it by for Launcher.kill in DisposerImpl.
             xauthorityEnv.put("XVNC_COOKIE", UUID.randomUUID().toString());
+            xauthority = null;
         }
 
         final Proc proc = launcher.launch().cmds(cmds).envs(xauthorityEnv).stdout(logger).pwd(workspace).start();
@@ -128,7 +138,7 @@ private void doSetUp(Context context, Run<?,?> build, FilePath workspace, Node n
                 allocator.blacklist(displayNumber);
                 if (retries > 0) {
                     doSetUp(context, build, workspace, node, launcher, logger, cmd, retries - 1,
-                            minDisplayNumber, maxDisplayNumber);
+                            minDisplayNumber, maxDisplayNumber, xauthorityInFsRoot);
                     return;
                 } else {
                     throw new IOException(message);
@@ -139,7 +149,7 @@ private void doSetUp(Context context, Run<?,?> build, FilePath workspace, Node n
         }
 
         context.env("DISPLAY", ":" + displayNumber);
-        context.setDisposer(new DisposerImpl(displayNumber, xauthorityEnv, vncserverCommand, takeScreenshot, xauthority.getRemote()));
+        context.setDisposer(new DisposerImpl(displayNumber, xauthorityEnv, vncserverCommand, takeScreenshot, xauthority != null ? xauthority.getRemote() : null));
     }
 
     private static class DisposerImpl extends Disposer {
@@ -148,11 +158,13 @@ private static class DisposerImpl extends Disposer {
 
         private final int displayNumber;
         private final Map<String,String> xauthorityEnv;
-        private final @CheckForNull String vncserverCommand;
+        @CheckForNull
+        private final String vncserverCommand;
         private final boolean takeScreenshot;
+        @CheckForNull
         private final String xauthorityPath;
 
-        DisposerImpl(int displayNumber, Map<String,String> xauthorityEnv, @CheckForNull String vncserverCommand, boolean takeScreenshot, String xauthorityPath) {
+        DisposerImpl(int displayNumber, Map<String,String> xauthorityEnv, @CheckForNull String vncserverCommand, boolean takeScreenshot, @CheckForNull String xauthorityPath) {
             this.displayNumber = displayNumber;
             this.xauthorityEnv = xauthorityEnv;
             this.vncserverCommand = vncserverCommand;
@@ -188,7 +200,12 @@ private static class DisposerImpl extends Disposer {
                 throw new AbortException("No node recognized for " + workspace);
             }
             getAllocator(node).free(displayNumber);
-            workspace.child(xauthorityPath).delete();
+            if (xauthorityPath != null) {
+                FilePath path = new FilePath(workspace.toComputer().getChannel(), xauthorityPath);
+                if (path.exists()) {
+                    path.delete();
+                }
+            }
         }
     }
 
@@ -250,6 +267,11 @@ public static final class DescriptorImpl extends BuildWrapperDescriptor {
          */
         public boolean skipOnWindows = true;
 
+        /**
+         * If true the XAuthority file will be placed on the slave's FS root instead of in the build's workspace.
+         */
+        public boolean xauthorityInFsRoot = false;
+
         /**
          * If true, try to clean up old processes and locks when first run.
          */

diff --git a/src/main/resources/hudson/plugins/xvnc/Xvnc/global.jelly b/src/main/resources/hudson/plugins/xvnc/Xvnc/global.jelly
@@ -17,5 +17,9 @@
       <f:checkbox/>
       <label class="attach-previous">${%Clean up before start}</label>
     </f:entry>
+    <f:entry field="xauthorityInFsRoot">
+      <f:checkbox/>
+      <label class="attach-previous">${%XAuthority file in slave FS root}</label>
+    </f:entry>
   </f:section>
 </j:jelly>
diff --git a/src/main/resources/hudson/plugins/xvnc/Xvnc/help-xauthorityInFsRoot.html b/src/main/resources/hudson/plugins/xvnc/Xvnc/help-xauthorityInFsRoot.html
@@ -0,0 +1,9 @@
+<div>
+    <p>
+        When using dedicated Xauthority file per build;
+        instead of placing the file in the build's workspace it should be placed in the FS root of the node.
+    </p>
+    <p>
+        This can be useful if for example the workspaces can contain white space characters, but you know the FS root on the nodes doesn't.
+    </p>
+</div>
diff --git a/src/test/java/hudson/plugins/xvnc/XvncTest.java b/src/test/java/hudson/plugins/xvnc/XvncTest.java
@@ -23,6 +23,7 @@
  */
 package hudson.plugins.xvnc;
 
+import hudson.FilePath;
 import hudson.Launcher;
 import hudson.model.BuildListener;
 import hudson.model.FreeStyleBuild;
@@ -38,12 +39,17 @@
 
 import java.io.IOException;
 import java.util.concurrent.Future;
+
+import static org.hamcrest.collection.IsCollectionWithSize.hasSize;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.TestBuilder;
 
 public class XvncTest {
 
@@ -134,6 +140,75 @@ public void avoidNpeAfterDeserialiation() throws Exception {
         j.buildAndAssertSuccess(p);
     }
 
+    @Test
+    public void testXauthorityInFsRootFalse() throws Exception {
+        DumbSlave slaveA = j.createOnlineSlave();
+
+        FreeStyleProject jobA = j.jenkins.createProject(FreeStyleProject.class, "jobA");
+        jobA.setAssignedNode(slaveA);
+        runXvnc(jobA, true).xauthorityInFsRoot = false;
+
+        jobA.getBuildersList().add(new TestBuilder() {
+            @Override
+            public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
+                FilePath[] list = build.getWorkspace().list(".Xauthority-*");
+                assertEquals("There should be one Xauthority file", 1, list.length);
+                assertEquals(build.getWorkspace().getRemote(), list[0].getParent().getRemote());
+                return true;
+            }
+        });
+        j.buildAndAssertSuccess(jobA);
+
+        FilePath[] list = jobA.getLastBuild().getWorkspace().list(".Xauthority-*");
+        assertEquals("The Xauthority file should be removed", 0, list.length);
+    }
+
+    @Test
+    public void testXauthorityInFsRootTrue() throws Exception {
+        DumbSlave slaveA = j.createOnlineSlave();
+
+        FreeStyleProject jobA = j.jenkins.createProject(FreeStyleProject.class, "jobA");
+        jobA.setAssignedNode(slaveA);
+        runXvnc(jobA, true).xauthorityInFsRoot = true;
+
+        jobA.getBuildersList().add(new TestBuilder() {
+            @Override
+            public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
+                FilePath rootPath = build.getWorkspace().toComputer().getNode().getRootPath();
+                FilePath[] list = rootPath.list(".Xauthority-*");
+                assertEquals("There should be one Xauthority file", 1, list.length);
+                assertEquals(rootPath.getRemote(), list[0].getParent().getRemote());
+
+                list = build.getWorkspace().list(".Xauthority-*");
+                assertEquals("There should be no Xauthority files in the workspace", 0, list.length);
+
+                return true;
+            }
+        });
+        j.buildAndAssertSuccess(jobA);
+
+        FilePath[] list = slaveA.getRootPath().list(".Xauthority-*");
+        assertEquals("The Xauthority file should be removed", 0, list.length);
+    }
+
+    @Test
+    public void testGlobalConfigRoundtrip() throws Exception {
+        DescriptorImpl descriptor = j.jenkins.getDescriptorByType(DescriptorImpl.class);
+        descriptor.maxDisplayNumber = descriptor.minDisplayNumber = 42;
+        descriptor.xvnc = "true";
+        descriptor.xauthorityInFsRoot = true;
+        descriptor.skipOnWindows = true;
+
+        j.configRoundtrip();
+
+        descriptor = j.jenkins.getDescriptorByType(DescriptorImpl.class);
+
+        assertEquals(42, descriptor.maxDisplayNumber);
+        assertEquals("true", descriptor.xvnc);
+        assertTrue(descriptor.xauthorityInFsRoot);
+        assertTrue(descriptor.skipOnWindows);
+    }
+
     private Xvnc fakeXvncRun(FreeStyleProject p) throws Exception {
         final Xvnc xvnc = new Xvnc(false, false);
         p.getBuildWrappersList().add(xvnc);
@@ -145,7 +220,11 @@ private Xvnc fakeXvncRun(FreeStyleProject p) throws Exception {
     }
 
     private Xvnc.DescriptorImpl runXvnc(FreeStyleProject p) throws Exception {
-        final Xvnc xvnc = new Xvnc(false, false);
+        return runXvnc(p, false);
+    }
+
+    private Xvnc.DescriptorImpl runXvnc(FreeStyleProject p, boolean useXauthority) throws Exception {
+        final Xvnc xvnc = new Xvnc(false, useXauthority);
         p.getBuildWrappersList().add(xvnc);
         DescriptorImpl descriptor = Hudson.getInstance().getDescriptorByType(DescriptorImpl.class);
         descriptor.maxDisplayNumber = descriptor.minDisplayNumber = 42;