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

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

    XMLWordPrintable

Details

    Description

      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.

      Attachments

        Issue Links

          Activity

            rsandell 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 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.

            schristou 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.
            abayer Andrew Bayer added a comment -

            That sounds right to me.

            abayer Andrew Bayer added a comment - That sounds right to me.
            abayer 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_issue_link 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_issue_link 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)

            People

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

              Dates

                Created:
                Updated:
                Resolved: