[JENKINS-45892] IllegalStateException at hudson.XmlFile.replaceIfNotA…

pom.xml

@@ -46,7 +46,7 @@
     <!-- dependency versions -->
     <httpclient.version>4.5.1</httpclient.version>
     <jackson.version>2.5.0</jackson.version>
-    <jenkins.version>2.32.1</jenkins.version>
+    <jenkins.version>2.89</jenkins.version>
 
     <kubernetes-client.version>2.6.1</kubernetes-client.version>
 
@@ -208,6 +208,11 @@
         <artifactId>ssh-credentials</artifactId>
         <version>1.12</version>
       </dependency>
+      <dependency>
+        <groupId>org.apache.sshd</groupId>
+        <artifactId>sshd-core</artifactId>
+        <version>1.6.0</version>
+      </dependency>
     </dependencies>
   </dependencyManagement>
 

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java

@@ -0,0 +1,91 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package org.csanchez.jenkins.plugins.kubernetes.pipeline;
+
+import java.io.IOException;
+import java.util.Stack;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.BulkChange;
+import hudson.model.InvisibleAction;
+import hudson.model.Run;
+import jenkins.model.RunAction2;
+
+/**
+ * @author Carlos Sanchez
+ * @since 1.1.1
+ *
+ */
+public abstract class AbstractInvisibleRunAction2 extends InvisibleAction implements RunAction2 {
+
+    private static final Logger LOGGER = Logger.getLogger(AbstractInvisibleRunAction2.class.getName());
+
+    protected final Stack<String> stack = new Stack<>();
+
+    protected transient Run<?, ?> run;
+
+    public Run<?, ?> getRun() {
+        return run;
+    }
+
+    protected void setRun(Run<?, ?> run) {
+        this.run = run;
+    }
+
+    protected static void push(@NonNull Run<?, ?> run, @NonNull Class<? extends AbstractInvisibleRunAction2> clazz,
+            @NonNull String item) throws IOException {
+        synchronized (run) {
+            BulkChange bc = new BulkChange(run);
+            try {
+                AbstractInvisibleRunAction2 action = run.getAction(clazz);
+                if (action == null) {
+                    action = clazz.newInstance();
+                    run.addAction(action);
+                }
+                LOGGER.log(Level.FINEST, "Pushing item {0} to action {1} in run {2}",
+                        new Object[] { item, action, run });
+                action.stack.push(item);
+                bc.commit();
+            } catch (InstantiationException | IllegalAccessException e) {
+                throw new RuntimeException("Can not instantiate class " + clazz, e);
+            } finally {
+                bc.abort();
+            }
+        }
+    }
+
+    @Override
+    public void onAttached(Run<?, ?> r) {
+        setRun(r);
+    }
+
+    @Override
+    public void onLoad(Run<?, ?> r) {
+        setRun(r);
+    }
+
+}

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java

@@ -1,28 +1,32 @@
 package org.csanchez.jenkins.plugins.kubernetes.pipeline;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.EmptyStackException;
-import java.util.List;
-import java.util.Stack;
 import java.util.logging.Logger;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.BulkChange;
-import hudson.model.InvisibleAction;
 import hudson.model.Run;
+import jenkins.model.RunAction2;
 
-public class NamespaceAction extends InvisibleAction {
+public class NamespaceAction extends AbstractInvisibleRunAction2 implements RunAction2 {
 
     private static final Logger LOGGER = Logger.getLogger(NamespaceAction.class.getName());
 
-    private final Stack<String> namespaces = new Stack<>();
-    private final Run run;
-
+    NamespaceAction() {
+        super();
+    }
 
+    @Deprecated
     public NamespaceAction(Run run) {
-        this.run = run;
+        setRun(run);
     }
 
+    protected static void push(@NonNull Run<?, ?> run, @NonNull String item) throws IOException {
+        AbstractInvisibleRunAction2.push(run, NamespaceAction.class, item);
+    }
+
+    @Deprecated
     public void push(String namespace) throws IOException {
         if (run == null) {
             LOGGER.warning("run is null, cannot push");
@@ -36,14 +40,15 @@ public void push(String namespace) throws IOException {
                     action = new NamespaceAction(run);
                     run.addAction(action);
                 }
-                action.namespaces.push(namespace);
+                action.stack.push(namespace);
                 bc.commit();
             } finally {
                 bc.abort();
             }
         }
     }
 
+    @Deprecated
     public String pop() throws IOException {
         if (run == null) {
             LOGGER.warning("run is null, cannot pop");
@@ -57,7 +62,7 @@ public String pop() throws IOException {
                     action = new NamespaceAction(run);
                     run.addAction(action);
                 }
-                String namespace = action.namespaces.pop();
+                String namespace = action.stack.pop();
                 bc.commit();
                 return namespace;
             } finally {
@@ -68,17 +73,10 @@ public String pop() throws IOException {
     }
 
     public String getNamespace() {
-        synchronized (run) {
-            NamespaceAction action = run.getAction(NamespaceAction.class);
-            if (action == null) {
-                action = new NamespaceAction(run);
-                run.addAction(action);
-            }
-            try {
-                return action.namespaces.peek();
-            } catch (EmptyStackException e) {
-                return null;
-            }
+        try {
+            return stack.peek();
+        } catch (EmptyStackException e) {
+            return null;
         }
     }
 }

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java

@@ -3,25 +3,31 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Stack;
 import java.util.logging.Logger;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.BulkChange;
-import hudson.model.InvisibleAction;
 import hudson.model.Run;
+import jenkins.model.RunAction2;
 
-public class PodTemplateAction extends InvisibleAction {
+public class PodTemplateAction extends AbstractInvisibleRunAction2 implements RunAction2 {
 
     private static final Logger LOGGER = Logger.getLogger(PodTemplateAction.class.getName());
 
-    private final Stack<String> names = new Stack<>();
-    private final Run run;
-
+    PodTemplateAction() {
+        super();
+    }
 
+    @Deprecated
     PodTemplateAction(Run run) {
-        this.run = run;
+        setRun(run);
+    }
+
+    protected static void push(@NonNull Run<?, ?> run, @NonNull String item) throws IOException {
+        AbstractInvisibleRunAction2.push(run, PodTemplateAction.class, item);
     }
 
+    @Deprecated
     public void push(String template) throws IOException {
         if (run == null) {
             LOGGER.warning("run is null, cannot push");
@@ -35,14 +41,15 @@ public void push(String template) throws IOException {
                     action = new PodTemplateAction(run);
                     run.addAction(action);
                 }
-                action.names.push(template);
+                action.stack.push(template);
                 bc.commit();
             } finally {
                 bc.abort();
             }
         }
     }
 
+    @Deprecated
     public String pop() throws IOException {
         if (run == null) {
             LOGGER.warning("run is null, cannot pop");
@@ -56,7 +63,7 @@ public String pop() throws IOException {
                     action = new PodTemplateAction(run);
                     run.addAction(action);
                 }
-                String template = action.names.pop();
+                String template = action.stack.pop();
                 bc.commit();
                 return template;
             } finally {
@@ -67,14 +74,7 @@ public String pop() throws IOException {
     }
 
     public List<String> getParentTemplateList() {
-        synchronized (run) {
-            PodTemplateAction action = run.getAction(PodTemplateAction.class);
-            if (action == null) {
-                action = new PodTemplateAction(run);
-                run.addAction(action);
-            }
-            return new ArrayList<>(action.names);
-        }
+        return new ArrayList<>(stack);
     }
 
     public String getParentTemplates() {

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java

@@ -5,6 +5,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.commons.lang.RandomStringUtils;
 import org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud;
@@ -55,8 +56,10 @@ public boolean start() throws Exception {
         }
         KubernetesCloud kubernetesCloud = (KubernetesCloud) cloud;
 
-        PodTemplateAction podTemplateAction = new PodTemplateAction(getContext().get(Run.class));
-        NamespaceAction namespaceAction = new NamespaceAction(getContext().get(Run.class));
+        Run<?, ?> run = getContext().get(Run.class);
+        PodTemplateAction podTemplateAction = run.getAction(PodTemplateAction.class);
+        NamespaceAction namespaceAction = run.getAction(NamespaceAction.class);
+        String parentTemplates = podTemplateAction != null ? podTemplateAction.getParentTemplates() : null;
 
         //Let's generate a random name based on the user specified to make sure that we don't have
         //issues with concurrent builds, or messing with pre-existing configuration
@@ -67,7 +70,7 @@ public boolean start() throws Exception {
         newTemplate = new PodTemplate();
         newTemplate.setName(name);
         newTemplate.setNamespace(namespace);
-        newTemplate.setInheritFrom(!Strings.isNullOrEmpty( podTemplateAction.getParentTemplates()) ? podTemplateAction.getParentTemplates() : step.getInheritFrom());
+        newTemplate.setInheritFrom(!Strings.isNullOrEmpty(parentTemplates) ? parentTemplates : step.getInheritFrom());
         newTemplate.setInstanceCap(step.getInstanceCap());
         newTemplate.setIdleMinutes(step.getIdleMinutes());
         newTemplate.setSlaveConnectTimeout(step.getSlaveConnectTimeout());
@@ -91,8 +94,8 @@ public boolean start() throws Exception {
         kubernetesCloud.addTemplate(newTemplate);
         getContext().newBodyInvoker().withContext(step).withCallback(new PodTemplateCallback(newTemplate)).start();
 
-        podTemplateAction.push(name);
-        namespaceAction.push(namespace);
+        PodTemplateAction.push(run, name);
+        NamespaceAction.push(run, namespace);
         return false;
     }
 
@@ -101,12 +104,11 @@ public void stop(Throwable cause) throws Exception {
         new PodTemplateAction(getContext().get(Run.class)).pop();
     }
 
-
-    private String checkNamespace(KubernetesCloud kubernetesCloud, NamespaceAction namespaceAction) {
+    private String checkNamespace(KubernetesCloud kubernetesCloud, @CheckForNull NamespaceAction namespaceAction) {
         String namespace = null;
         if (!Strings.isNullOrEmpty(step.getNamespace())) {
             namespace = step.getNamespace();
-        } else if (!Strings.isNullOrEmpty(namespaceAction.getNamespace())) {
+        } else if ((namespaceAction != null) && (!Strings.isNullOrEmpty(namespaceAction.getNamespace()))) {
             namespace = namespaceAction.getNamespace();
         } else {
             namespace = kubernetesCloud.getNamespace();

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java

@@ -206,8 +206,7 @@ public void runWithOverriddenNamespace() throws Exception {
 
         WorkflowRun b = p.scheduleBuild2(0).waitForStart();
         assertNotNull(b);
-        NamespaceAction namespaceAction = new NamespaceAction(b);
-        namespaceAction.push(overriddenNamespace);
+        NamespaceAction.push(b, overriddenNamespace);
 
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
         r.assertLogContains(overriddenNamespace, b);
@@ -232,8 +231,7 @@ public void runWithStepOverriddenNamespace() throws Exception {
 
         WorkflowRun b = p.scheduleBuild2(0).waitForStart();
         assertNotNull(b);
-        NamespaceAction namespaceAction = new NamespaceAction(b);
-        namespaceAction.push(overriddenNamespace);
+        NamespaceAction.push(b, overriddenNamespace);
 
         r.assertBuildStatusSuccess(r.waitForCompletion(b));
         r.assertLogContains(stepNamespace, b);