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

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

    XMLWordPrintable

    Details

    • Similar Issues:

      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 Jesse Glick there.

        Attachments

          Issue Links

            Activity

            batmat Baptiste Mathus created issue -
            jglick Jesse Glick made changes -
            Field Original Value New Value
            Link This issue relates to JENKINS-38389 [ JENKINS-38389 ]
            Hide
            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.

            Show
            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.
            Hide
            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.

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

            That sounds right to me.

            Show
            abayer Andrew Bayer added a comment - That sounds right to me.
            oleg_nenashev Oleg Nenashev made changes -
            Assignee Oleg Nenashev [ oleg_nenashev ]
            oleg_nenashev Oleg Nenashev made changes -
            Labels robustness newbie-friendly robustness
            oleg_nenashev Oleg Nenashev made changes -
            Issue Type Improvement [ 4 ] Bug [ 1 ]
            abayer Andrew Bayer made changes -
            Assignee Oleg Nenashev [ oleg_nenashev ] Andrew Bayer [ abayer ]
            abayer Andrew Bayer made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            abayer Andrew Bayer made changes -
            Status In Progress [ 3 ] In Review [ 10005 ]
            Show
            abayer Andrew Bayer added a comment - PR up at  https://github.com/jenkinsci/jenkins/pull/2836
            abayer Andrew Bayer made changes -
            Remote Link This issue links to "PR #2836 (Web Link)" [ 16000 ]
            Hide
            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.

            Show
            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.
            abayer Andrew Bayer made changes -
            Resolution Fixed [ 1 ]
            Status In Review [ 10005 ] Resolved [ 5 ]
            oleg_nenashev Oleg Nenashev made changes -
            Labels newbie-friendly robustness 2.46.2-rejected lts-candidate newbie-friendly robustness
            olivergondza Oliver Gondža made changes -
            Labels 2.46.2-rejected lts-candidate newbie-friendly robustness 2.46.2-rejected 2.46.3-fixed newbie-friendly robustness
            Hide
            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)

            Show
            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)
            cloudbees CloudBees Inc. made changes -
            Remote Link This issue links to "CloudBees Internal OSS-1998 (Web Link)" [ 18471 ]

              People

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

                Dates

                Created:
                Updated:
                Resolved: