From 435efcc52ab23df03675399a4f5495dc139ba217 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Thu, 30 Nov 2017 10:42:22 +0100 Subject: [PATCH 1/8] [JENKINS-45892] IllegalStateException at hudson.XmlFile.replaceIfNotAtTopLevel JENKINS-45892 introduced a change that is causing Nov 29, 2017 6:58:52 PM hudson.XmlFile replaceIfNotAtTopLevel WARNING: JENKINS-45892: reference to org.jenkinsci.plugins.workflow.job.WorkflowJob@488f6bf6[cloudbees/test/branch] being saved from unexpected /var/jenkins_home/jobs/test/branches/branch.njvm0k/builds/7/build.xml java.lang.IllegalStateException at hudson.XmlFile.replaceIfNotAtTopLevel(XmlFile.java:210) --- pom.xml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7b725db323..1ed4aadf8c 100644 --- a/pom.xml +++ b/pom.xml @@ -46,7 +46,7 @@ 4.5.1 2.5.0 - 2.32.1 + 2.89 2.6.1 @@ -208,6 +208,11 @@ ssh-credentials 1.12 + + org.apache.sshd + sshd-core + 1.6.0 + From d55cfce0603902d89b15cd89b0caac8ffdece2fd Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Thu, 30 Nov 2017 12:32:21 +0100 Subject: [PATCH 2/8] [JENKINS-45892] Run should be transient in actions --- .../pipeline/AbstractInvisibleRunAction2.java | 58 +++++++++++++++++++ .../kubernetes/pipeline/NamespaceAction.java | 44 +++++++------- .../pipeline/PodTemplateAction.java | 42 +++++++------- 3 files changed, 98 insertions(+), 46 deletions(-) create mode 100644 src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java new file mode 100644 index 0000000000..f7266df3ae --- /dev/null +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java @@ -0,0 +1,58 @@ +/* + * 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 hudson.model.InvisibleAction; +import hudson.model.Run; +import jenkins.model.RunAction2; + +/** + * @author Carlos Sanchez + * @since 1.1.1 + * + */ +public class AbstractInvisibleRunAction2 extends InvisibleAction implements RunAction2 { + + private transient Run run; + + public Run getRun() { + return run; + } + + protected void setRun(Run run) { + this.run = run; + } + + @Override + public void onAttached(Run r) { + setRun(r); + } + + @Override + public void onLoad(Run r) { + setRun(r); + } + +} diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java index 4ea6fa8d47..0412f2986b 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java @@ -1,40 +1,36 @@ 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 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 namespaces = new Stack<>(); - private final Run run; - - public NamespaceAction(Run run) { - this.run = run; + public NamespaceAction(Run run) { + setRun(run); } public void push(String namespace) throws IOException { - if (run == null) { + if (getRun() == null) { LOGGER.warning("run is null, cannot push"); return; } - synchronized (run) { - BulkChange bc = new BulkChange(run); + synchronized (getRun()) { + BulkChange bc = new BulkChange(getRun()); try { - NamespaceAction action = run.getAction(NamespaceAction.class); + NamespaceAction action = getRun().getAction(NamespaceAction.class); if (action == null) { - action = new NamespaceAction(run); - run.addAction(action); + action = new NamespaceAction(getRun()); + getRun().addAction(action); } action.namespaces.push(namespace); bc.commit(); @@ -45,17 +41,17 @@ public void push(String namespace) throws IOException { } public String pop() throws IOException { - if (run == null) { + if (getRun() == null) { LOGGER.warning("run is null, cannot pop"); return null; } - synchronized (run) { - BulkChange bc = new BulkChange(run); + synchronized (getRun()) { + BulkChange bc = new BulkChange(getRun()); try { - NamespaceAction action = run.getAction(NamespaceAction.class); + NamespaceAction action = getRun().getAction(NamespaceAction.class); if (action == null) { - action = new NamespaceAction(run); - run.addAction(action); + action = new NamespaceAction(getRun()); + getRun().addAction(action); } String namespace = action.namespaces.pop(); bc.commit(); @@ -68,11 +64,11 @@ public String pop() throws IOException { } public String getNamespace() { - synchronized (run) { - NamespaceAction action = run.getAction(NamespaceAction.class); + synchronized (getRun()) { + NamespaceAction action = getRun().getAction(NamespaceAction.class); if (action == null) { - action = new NamespaceAction(run); - run.addAction(action); + action = new NamespaceAction(getRun()); + getRun().addAction(action); } try { return action.namespaces.peek(); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java index 77c721f69a..72644bf315 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java @@ -7,33 +7,31 @@ import java.util.logging.Logger; 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 names = new Stack<>(); - private final Run run; - - PodTemplateAction(Run run) { - this.run = run; + PodTemplateAction(Run run) { + setRun(run); } public void push(String template) throws IOException { - if (run == null) { + if (getRun() == null) { LOGGER.warning("run is null, cannot push"); return; } - synchronized (run) { - BulkChange bc = new BulkChange(run); + synchronized (getRun()) { + BulkChange bc = new BulkChange(getRun()); try { - PodTemplateAction action = run.getAction(PodTemplateAction.class); + PodTemplateAction action = getRun().getAction(PodTemplateAction.class); if (action == null) { - action = new PodTemplateAction(run); - run.addAction(action); + action = new PodTemplateAction(getRun()); + getRun().addAction(action); } action.names.push(template); bc.commit(); @@ -44,17 +42,17 @@ public void push(String template) throws IOException { } public String pop() throws IOException { - if (run == null) { + if (getRun() == null) { LOGGER.warning("run is null, cannot pop"); return null; } - synchronized (run) { - BulkChange bc = new BulkChange(run); + synchronized (getRun()) { + BulkChange bc = new BulkChange(getRun()); try { - PodTemplateAction action = run.getAction(PodTemplateAction.class); + PodTemplateAction action = getRun().getAction(PodTemplateAction.class); if (action == null) { - action = new PodTemplateAction(run); - run.addAction(action); + action = new PodTemplateAction(getRun()); + getRun().addAction(action); } String template = action.names.pop(); bc.commit(); @@ -67,11 +65,11 @@ public String pop() throws IOException { } public List getParentTemplateList() { - synchronized (run) { - PodTemplateAction action = run.getAction(PodTemplateAction.class); + synchronized (getRun()) { + PodTemplateAction action = getRun().getAction(PodTemplateAction.class); if (action == null) { - action = new PodTemplateAction(run); - run.addAction(action); + action = new PodTemplateAction(getRun()); + getRun().addAction(action); } return new ArrayList<>(action.names); } From 02b946df6b11b36d68802fda835931539d9ba6ef Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Thu, 30 Nov 2017 12:43:51 +0100 Subject: [PATCH 3/8] Remove duplication --- .../pipeline/AbstractInvisibleRunAction2.java | 57 +++++++++++++++++++ .../kubernetes/pipeline/NamespaceAction.java | 56 +----------------- .../pipeline/PodTemplateAction.java | 56 +----------------- 3 files changed, 61 insertions(+), 108 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java index f7266df3ae..d2315324af 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java @@ -24,6 +24,11 @@ package org.csanchez.jenkins.plugins.kubernetes.pipeline; +import java.io.IOException; +import java.util.Stack; +import java.util.logging.Logger; + +import hudson.BulkChange; import hudson.model.InvisibleAction; import hudson.model.Run; import jenkins.model.RunAction2; @@ -35,8 +40,16 @@ */ public class AbstractInvisibleRunAction2 extends InvisibleAction implements RunAction2 { + private static final Logger LOGGER = Logger.getLogger(AbstractInvisibleRunAction2.class.getName()); + + protected final Stack stack = new Stack<>(); + private transient Run run; + public AbstractInvisibleRunAction2(Run run) { + setRun(run); + } + public Run getRun() { return run; } @@ -45,6 +58,50 @@ protected void setRun(Run run) { this.run = run; } + public void push(String item) throws IOException { + if (getRun() == null) { + LOGGER.warning("run is null, cannot push"); + return; + } + synchronized (getRun()) { + BulkChange bc = new BulkChange(getRun()); + try { + AbstractInvisibleRunAction2 action = getRun().getAction(AbstractInvisibleRunAction2.class); + if (action == null) { + action = new AbstractInvisibleRunAction2(getRun()); + getRun().addAction(action); + } + action.stack.push(item); + bc.commit(); + } finally { + bc.abort(); + } + } + } + + public String pop() throws IOException { + if (getRun() == null) { + LOGGER.warning("run is null, cannot pop"); + return null; + } + synchronized (getRun()) { + BulkChange bc = new BulkChange(getRun()); + try { + AbstractInvisibleRunAction2 action = getRun().getAction(AbstractInvisibleRunAction2.class); + if (action == null) { + action = new AbstractInvisibleRunAction2(getRun()); + getRun().addAction(action); + } + String template = action.stack.pop(); + bc.commit(); + return template; + } finally { + bc.abort(); + return null; + } + } + } + @Override public void onAttached(Run r) { setRun(r); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java index 0412f2986b..2a5e892904 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java @@ -1,66 +1,14 @@ package org.csanchez.jenkins.plugins.kubernetes.pipeline; -import java.io.IOException; import java.util.EmptyStackException; -import java.util.Stack; -import java.util.logging.Logger; -import hudson.BulkChange; import hudson.model.Run; import jenkins.model.RunAction2; public class NamespaceAction extends AbstractInvisibleRunAction2 implements RunAction2 { - private static final Logger LOGGER = Logger.getLogger(NamespaceAction.class.getName()); - - private final Stack namespaces = new Stack<>(); - public NamespaceAction(Run run) { - setRun(run); - } - - public void push(String namespace) throws IOException { - if (getRun() == null) { - LOGGER.warning("run is null, cannot push"); - return; - } - synchronized (getRun()) { - BulkChange bc = new BulkChange(getRun()); - try { - NamespaceAction action = getRun().getAction(NamespaceAction.class); - if (action == null) { - action = new NamespaceAction(getRun()); - getRun().addAction(action); - } - action.namespaces.push(namespace); - bc.commit(); - } finally { - bc.abort(); - } - } - } - - public String pop() throws IOException { - if (getRun() == null) { - LOGGER.warning("run is null, cannot pop"); - return null; - } - synchronized (getRun()) { - BulkChange bc = new BulkChange(getRun()); - try { - NamespaceAction action = getRun().getAction(NamespaceAction.class); - if (action == null) { - action = new NamespaceAction(getRun()); - getRun().addAction(action); - } - String namespace = action.namespaces.pop(); - bc.commit(); - return namespace; - } finally { - bc.abort(); - return null; - } - } + super(run); } public String getNamespace() { @@ -71,7 +19,7 @@ public String getNamespace() { getRun().addAction(action); } try { - return action.namespaces.peek(); + return action.stack.peek(); } catch (EmptyStackException e) { return null; } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java index 72644bf315..7aef12c23d 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java @@ -1,67 +1,15 @@ package org.csanchez.jenkins.plugins.kubernetes.pipeline; -import java.io.IOException; import java.util.ArrayList; import java.util.List; -import java.util.Stack; -import java.util.logging.Logger; -import hudson.BulkChange; import hudson.model.Run; import jenkins.model.RunAction2; public class PodTemplateAction extends AbstractInvisibleRunAction2 implements RunAction2 { - private static final Logger LOGGER = Logger.getLogger(PodTemplateAction.class.getName()); - - private final Stack names = new Stack<>(); - PodTemplateAction(Run run) { - setRun(run); - } - - public void push(String template) throws IOException { - if (getRun() == null) { - LOGGER.warning("run is null, cannot push"); - return; - } - synchronized (getRun()) { - BulkChange bc = new BulkChange(getRun()); - try { - PodTemplateAction action = getRun().getAction(PodTemplateAction.class); - if (action == null) { - action = new PodTemplateAction(getRun()); - getRun().addAction(action); - } - action.names.push(template); - bc.commit(); - } finally { - bc.abort(); - } - } - } - - public String pop() throws IOException { - if (getRun() == null) { - LOGGER.warning("run is null, cannot pop"); - return null; - } - synchronized (getRun()) { - BulkChange bc = new BulkChange(getRun()); - try { - PodTemplateAction action = getRun().getAction(PodTemplateAction.class); - if (action == null) { - action = new PodTemplateAction(getRun()); - getRun().addAction(action); - } - String template = action.names.pop(); - bc.commit(); - return template; - } finally { - bc.abort(); - return null; - } - } + super(run); } public List getParentTemplateList() { @@ -71,7 +19,7 @@ public List getParentTemplateList() { action = new PodTemplateAction(getRun()); getRun().addAction(action); } - return new ArrayList<>(action.names); + return new ArrayList<>(action.stack); } } From 86a15bc02298eb569186ba65eff7da8d927bb956 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Thu, 30 Nov 2017 12:48:34 +0100 Subject: [PATCH 4/8] Fix warning: finally block does not complete normally --- .../plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java index d2315324af..d937160da8 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java @@ -97,7 +97,6 @@ public String pop() throws IOException { return template; } finally { bc.abort(); - return null; } } } From f49f8ef90df7fe350a72b43ee018e13583da6777 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Thu, 30 Nov 2017 14:34:24 +0100 Subject: [PATCH 5/8] [JENKINS-45892] Use the correct classes when looking for actions in a Run --- .../pipeline/AbstractInvisibleRunAction2.java | 30 +++++++++++-------- .../kubernetes/pipeline/NamespaceAction.java | 5 ++++ .../pipeline/PodTemplateAction.java | 5 ++++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java index d937160da8..d4471fb952 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.Stack; +import java.util.logging.Level; import java.util.logging.Logger; import hudson.BulkChange; @@ -38,7 +39,7 @@ * @since 1.1.1 * */ -public class AbstractInvisibleRunAction2 extends InvisibleAction implements RunAction2 { +public abstract class AbstractInvisibleRunAction2 extends InvisibleAction implements RunAction2 { private static final Logger LOGGER = Logger.getLogger(AbstractInvisibleRunAction2.class.getName()); @@ -50,6 +51,8 @@ public AbstractInvisibleRunAction2(Run run) { setRun(run); } + protected abstract AbstractInvisibleRunAction2 createAction(Run run); + public Run getRun() { return run; } @@ -59,18 +62,19 @@ protected void setRun(Run run) { } public void push(String item) throws IOException { - if (getRun() == null) { + if (run == null) { LOGGER.warning("run is null, cannot push"); return; } - synchronized (getRun()) { - BulkChange bc = new BulkChange(getRun()); + synchronized (run) { + BulkChange bc = new BulkChange(run); try { - AbstractInvisibleRunAction2 action = getRun().getAction(AbstractInvisibleRunAction2.class); + AbstractInvisibleRunAction2 action = run.getAction(this.getClass()); if (action == null) { - action = new AbstractInvisibleRunAction2(getRun()); - getRun().addAction(action); + action = createAction(run); + run.addAction(action); } + LOGGER.log(Level.INFO, "Pushing item {0} to action {1} in run {2}", new Object[] { item, action, run }); action.stack.push(item); bc.commit(); } finally { @@ -80,21 +84,23 @@ public void push(String item) throws IOException { } public String pop() throws IOException { - if (getRun() == null) { + if (run == null) { LOGGER.warning("run is null, cannot pop"); return null; } synchronized (getRun()) { BulkChange bc = new BulkChange(getRun()); try { - AbstractInvisibleRunAction2 action = getRun().getAction(AbstractInvisibleRunAction2.class); + AbstractInvisibleRunAction2 action = getRun().getAction(this.getClass()); if (action == null) { - action = new AbstractInvisibleRunAction2(getRun()); + action = createAction(getRun()); getRun().addAction(action); } - String template = action.stack.pop(); + String item = action.stack.pop(); + LOGGER.log(Level.INFO, "Popped item {0} from action {1} in run {2}", + new Object[] { item, action, run }); bc.commit(); - return template; + return item; } finally { bc.abort(); } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java index 2a5e892904..f00bb8bc6c 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java @@ -11,6 +11,11 @@ public NamespaceAction(Run run) { super(run); } + @Override + protected AbstractInvisibleRunAction2 createAction(Run run) { + return new NamespaceAction(run); + } + public String getNamespace() { synchronized (getRun()) { NamespaceAction action = getRun().getAction(NamespaceAction.class); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java index 7aef12c23d..a04d0e77d2 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java @@ -12,6 +12,11 @@ public class PodTemplateAction extends AbstractInvisibleRunAction2 implements Ru super(run); } + @Override + protected AbstractInvisibleRunAction2 createAction(Run run) { + return new PodTemplateAction(run); + } + public List getParentTemplateList() { synchronized (getRun()) { PodTemplateAction action = getRun().getAction(PodTemplateAction.class); From 7728d9a6f04c14fe0b1966d853516901ee96cb4d Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Thu, 30 Nov 2017 14:36:57 +0100 Subject: [PATCH 6/8] Lower the logging level --- .../kubernetes/pipeline/AbstractInvisibleRunAction2.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java index d4471fb952..84576b98fe 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java @@ -74,7 +74,8 @@ public void push(String item) throws IOException { action = createAction(run); run.addAction(action); } - LOGGER.log(Level.INFO, "Pushing item {0} to action {1} in run {2}", new Object[] { item, action, run }); + LOGGER.log(Level.FINEST, "Pushing item {0} to action {1} in run {2}", + new Object[] { item, action, run }); action.stack.push(item); bc.commit(); } finally { @@ -97,7 +98,7 @@ public String pop() throws IOException { getRun().addAction(action); } String item = action.stack.pop(); - LOGGER.log(Level.INFO, "Popped item {0} from action {1} in run {2}", + LOGGER.log(Level.FINEST, "Popped item {0} from action {1} in run {2}", new Object[] { item, action, run }); bc.commit(); return item; From 4782429ef5b0ac84a40ba4504120bdf4260742ae Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Fri, 1 Dec 2017 11:59:28 +0100 Subject: [PATCH 7/8] Refactor PodTemplateAction and NamespaceAction to make things simpler and more object oriented --- .../pipeline/AbstractInvisibleRunAction2.java | 41 ++++++++++++++++--- .../kubernetes/pipeline/NamespaceAction.java | 27 +++++++----- .../pipeline/PodTemplateAction.java | 21 ++++++---- .../pipeline/PodTemplateStepExecution.java | 18 ++++---- .../pipeline/KubernetesPipelineTest.java | 6 +-- 5 files changed, 77 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java index 84576b98fe..3d5025d792 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java @@ -29,6 +29,7 @@ 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; @@ -47,10 +48,16 @@ public abstract class AbstractInvisibleRunAction2 extends InvisibleAction implem private transient Run run; + public AbstractInvisibleRunAction2() { + super(); + } + + @Deprecated public AbstractInvisibleRunAction2(Run run) { setRun(run); } + @Deprecated protected abstract AbstractInvisibleRunAction2 createAction(Run run); public Run getRun() { @@ -61,6 +68,29 @@ protected void setRun(Run run) { this.run = run; } + protected static void push(@NonNull Run run, @NonNull Class 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(); + } + } + } + + @Deprecated public void push(String item) throws IOException { if (run == null) { LOGGER.warning("run is null, cannot push"); @@ -84,18 +114,19 @@ public void push(String item) throws IOException { } } + @Deprecated public String pop() throws IOException { if (run == null) { LOGGER.warning("run is null, cannot pop"); return null; } - synchronized (getRun()) { - BulkChange bc = new BulkChange(getRun()); + synchronized (run) { + BulkChange bc = new BulkChange(run); try { - AbstractInvisibleRunAction2 action = getRun().getAction(this.getClass()); + AbstractInvisibleRunAction2 action = run.getAction(this.getClass()); if (action == null) { - action = createAction(getRun()); - getRun().addAction(action); + action = createAction(run); + run.addAction(action); } String item = action.stack.pop(); LOGGER.log(Level.FINEST, "Popped item {0} from action {1} in run {2}", diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java index f00bb8bc6c..ce98cf6425 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java @@ -1,33 +1,38 @@ package org.csanchez.jenkins.plugins.kubernetes.pipeline; +import java.io.IOException; import java.util.EmptyStackException; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.model.Run; import jenkins.model.RunAction2; public class NamespaceAction extends AbstractInvisibleRunAction2 implements RunAction2 { + NamespaceAction() { + super(); + } + + @Deprecated public NamespaceAction(Run run) { super(run); } @Override + @Deprecated protected AbstractInvisibleRunAction2 createAction(Run run) { return new NamespaceAction(run); } + protected static void push(@NonNull Run run, @NonNull String item) throws IOException { + AbstractInvisibleRunAction2.push(run, NamespaceAction.class, item); + } + public String getNamespace() { - synchronized (getRun()) { - NamespaceAction action = getRun().getAction(NamespaceAction.class); - if (action == null) { - action = new NamespaceAction(getRun()); - getRun().addAction(action); - } - try { - return action.stack.peek(); - } catch (EmptyStackException e) { - return null; - } + try { + return stack.peek(); + } catch (EmptyStackException e) { + return null; } } } diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java index a04d0e77d2..207b499a7a 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java @@ -1,31 +1,36 @@ package org.csanchez.jenkins.plugins.kubernetes.pipeline; +import java.io.IOException; import java.util.ArrayList; import java.util.List; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.model.Run; import jenkins.model.RunAction2; public class PodTemplateAction extends AbstractInvisibleRunAction2 implements RunAction2 { + PodTemplateAction() { + super(); + } + + @Deprecated PodTemplateAction(Run run) { super(run); } @Override + @Deprecated protected AbstractInvisibleRunAction2 createAction(Run run) { return new PodTemplateAction(run); } + protected static void push(@NonNull Run run, @NonNull String item) throws IOException { + AbstractInvisibleRunAction2.push(run, PodTemplateAction.class, item); + } + public List getParentTemplateList() { - synchronized (getRun()) { - PodTemplateAction action = getRun().getAction(PodTemplateAction.class); - if (action == null) { - action = new PodTemplateAction(getRun()); - getRun().addAction(action); - } - return new ArrayList<>(action.stack); - } + return new ArrayList<>(stack); } public String getParentTemplates() { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java index c50f5afb9b..2527fe5214 100755 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStepExecution.java +++ b/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(); diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java index b5cbbc9109..4a87adaa28 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java +++ b/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); From 70e834691ca26b998a56019b1f075ca5d6e165c2 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez Date: Fri, 1 Dec 2017 12:13:33 +0100 Subject: [PATCH 8/8] Refactor PodTemplateAction and NamespaceAction Leave deprecated methods as they were --- .../pipeline/AbstractInvisibleRunAction2.java | 63 +------------------ .../kubernetes/pipeline/NamespaceAction.java | 58 ++++++++++++++--- .../pipeline/PodTemplateAction.java | 58 ++++++++++++++--- 3 files changed, 103 insertions(+), 76 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java index 3d5025d792..143bc881d4 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/AbstractInvisibleRunAction2.java @@ -46,19 +46,7 @@ public abstract class AbstractInvisibleRunAction2 extends InvisibleAction implem protected final Stack stack = new Stack<>(); - private transient Run run; - - public AbstractInvisibleRunAction2() { - super(); - } - - @Deprecated - public AbstractInvisibleRunAction2(Run run) { - setRun(run); - } - - @Deprecated - protected abstract AbstractInvisibleRunAction2 createAction(Run run); + protected transient Run run; public Run getRun() { return run; @@ -90,55 +78,6 @@ protected static void push(@NonNull Run run, @NonNull Class r) { setRun(r); diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java index ce98cf6425..5d17a206eb 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/NamespaceAction.java @@ -2,30 +2,74 @@ import java.io.IOException; import java.util.EmptyStackException; +import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.BulkChange; import hudson.model.Run; import jenkins.model.RunAction2; public class NamespaceAction extends AbstractInvisibleRunAction2 implements RunAction2 { + private static final Logger LOGGER = Logger.getLogger(NamespaceAction.class.getName()); + NamespaceAction() { super(); } @Deprecated - public NamespaceAction(Run run) { - super(run); + public NamespaceAction(Run run) { + setRun(run); + } + + protected static void push(@NonNull Run run, @NonNull String item) throws IOException { + AbstractInvisibleRunAction2.push(run, NamespaceAction.class, item); } - @Override @Deprecated - protected AbstractInvisibleRunAction2 createAction(Run run) { - return new NamespaceAction(run); + public void push(String namespace) throws IOException { + if (run == null) { + LOGGER.warning("run is null, cannot push"); + return; + } + synchronized (run) { + BulkChange bc = new BulkChange(run); + try { + NamespaceAction action = run.getAction(NamespaceAction.class); + if (action == null) { + action = new NamespaceAction(run); + run.addAction(action); + } + action.stack.push(namespace); + bc.commit(); + } finally { + bc.abort(); + } + } } - protected static void push(@NonNull Run run, @NonNull String item) throws IOException { - AbstractInvisibleRunAction2.push(run, NamespaceAction.class, item); + @Deprecated + public String pop() throws IOException { + if (run == null) { + LOGGER.warning("run is null, cannot pop"); + return null; + } + synchronized (run) { + BulkChange bc = new BulkChange(run); + try { + NamespaceAction action = run.getAction(NamespaceAction.class); + if (action == null) { + action = new NamespaceAction(run); + run.addAction(action); + } + String namespace = action.stack.pop(); + bc.commit(); + return namespace; + } finally { + bc.abort(); + return null; + } + } } public String getNamespace() { diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java index 207b499a7a..591c733161 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateAction.java @@ -3,30 +3,74 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.logging.Logger; import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.BulkChange; import hudson.model.Run; import jenkins.model.RunAction2; public class PodTemplateAction extends AbstractInvisibleRunAction2 implements RunAction2 { + private static final Logger LOGGER = Logger.getLogger(PodTemplateAction.class.getName()); + PodTemplateAction() { super(); } @Deprecated - PodTemplateAction(Run run) { - super(run); + PodTemplateAction(Run run) { + setRun(run); + } + + protected static void push(@NonNull Run run, @NonNull String item) throws IOException { + AbstractInvisibleRunAction2.push(run, PodTemplateAction.class, item); } - @Override @Deprecated - protected AbstractInvisibleRunAction2 createAction(Run run) { - return new PodTemplateAction(run); + public void push(String template) throws IOException { + if (run == null) { + LOGGER.warning("run is null, cannot push"); + return; + } + synchronized (run) { + BulkChange bc = new BulkChange(run); + try { + PodTemplateAction action = run.getAction(PodTemplateAction.class); + if (action == null) { + action = new PodTemplateAction(run); + run.addAction(action); + } + action.stack.push(template); + bc.commit(); + } finally { + bc.abort(); + } + } } - protected static void push(@NonNull Run run, @NonNull String item) throws IOException { - AbstractInvisibleRunAction2.push(run, PodTemplateAction.class, item); + @Deprecated + public String pop() throws IOException { + if (run == null) { + LOGGER.warning("run is null, cannot pop"); + return null; + } + synchronized (run) { + BulkChange bc = new BulkChange(run); + try { + PodTemplateAction action = run.getAction(PodTemplateAction.class); + if (action == null) { + action = new PodTemplateAction(run); + run.addAction(action); + } + String template = action.stack.pop(); + bc.commit(); + return template; + } finally { + bc.abort(); + return null; + } + } } public List getParentTemplateList() {