Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-42043

A NPE in SlaveComputer.setNode shouldn't prevent Jenkins from starting

      Currently, an exception thrown in SlaveComputer.setNode() prevents Jenkins from starting (JENKINS-38389).

      It would be nice to catch RuntimeException there for robustness, as advised by jglick there.

          [JENKINS-42043] A NPE in SlaveComputer.setNode shouldn't prevent Jenkins from starting

          rsandell added a comment -

          But what should happen if the operation fails?
          The stacktrace shows that Jenkins is currently updating all "computers" to see which ones to reuse/what executors to remove. If the node failed to update should it then be considered for reuse or for removal?
          The error in ec2 plugin suggests that it was a "harmless" mistake and we should probably keep it around, while I would guess in most cases since it is in error it should be removed from the list.

          rsandell added a comment - But what should happen if the operation fails? The stacktrace shows that Jenkins is currently updating all "computers" to see which ones to reuse/what executors to remove. If the node failed to update should it then be considered for reuse or for removal? The error in ec2 plugin suggests that it was a "harmless" mistake and we should probably keep it around, while I would guess in most cases since it is in error it should be removed from the list.

          My $0.02 - I think it should keep the node and act as if it is a disconnected node instead of removing the node (i.e. ignore it and move on with the startup). We should never be removing nodes from the list that fail to connect. We should only remove them if the node is manually removed by an administrator, or the node is removed by the plugin via a timeout.

          Steven Christou added a comment - My $0.02 - I think it should keep the node and act as if it is a disconnected node instead of removing the node (i.e. ignore it and move on with the startup). We should never be removing nodes from the list that fail to connect. We should only remove them if the node is manually removed by an administrator, or the node is removed by the plugin via a timeout.

          Andrew Bayer added a comment -

          That sounds right to me.

          Andrew Bayer added a comment - That sounds right to me.

          Andrew Bayer added a comment -

          Andrew Bayer added a comment - PR up at  https://github.com/jenkinsci/jenkins/pull/2836

          Code changed in jenkins
          User: Andrew Bayer
          Path:
          core/src/main/java/hudson/model/AbstractCIBase.java
          http://jenkins-ci.org/commit/jenkins/23b0085f453454462542ae6e0fd67915b760ee4e
          Log:
          [FIXED JENKINS-42043] Catch and log RuntimeException in setNode

          Also make sure we don't mark the Computer as used so that we kill any
          executors that may be related to it somehow.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andrew Bayer Path: core/src/main/java/hudson/model/AbstractCIBase.java http://jenkins-ci.org/commit/jenkins/23b0085f453454462542ae6e0fd67915b760ee4e Log: [FIXED JENKINS-42043] Catch and log RuntimeException in setNode Also make sure we don't mark the Computer as used so that we kill any executors that may be related to it somehow.

          Code changed in jenkins
          User: Andrew Bayer
          Path:
          core/src/main/java/hudson/model/AbstractCIBase.java
          http://jenkins-ci.org/commit/jenkins/6128459dd39a7a1722894cd0a5a69a6c8c767abb
          Log:
          [FIXED JENKINS-42043] Catch and log RuntimeException in setNode

          Also make sure we don't mark the Computer as used so that we kill any
          executors that may be related to it somehow.

          (cherry picked from commit 23b0085f453454462542ae6e0fd67915b760ee4e)

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andrew Bayer Path: core/src/main/java/hudson/model/AbstractCIBase.java http://jenkins-ci.org/commit/jenkins/6128459dd39a7a1722894cd0a5a69a6c8c767abb Log: [FIXED JENKINS-42043] Catch and log RuntimeException in setNode Also make sure we don't mark the Computer as used so that we kill any executors that may be related to it somehow. (cherry picked from commit 23b0085f453454462542ae6e0fd67915b760ee4e)

            abayer Andrew Bayer
            batmat Baptiste Mathus
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: