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

Nodejs plugin installs wrong platform package when using parallel pipeline

    Details

    • Type: Bug
    • Status: Fixed but Unreleased (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: nodejs-plugin
    • Labels:
      None
    • Environment:
      Jenkins 2.162
      NodeJS plugin 1.2.7
      Latest pipeline plugins as of 04/02/2019

      Jenkins Tool config:
      NodeJs version 8.15.0 - Install automatically - Global packages = "yarn"
    • Similar Issues:

      Description

      Hey,

      We have a Pipeline job that runs steps in parallel across three nodes - one Ubuntu, one Mac, one Windows. Some time recently (we're not exactly sure when), it started downloading the wrong node package to the machines, e.g. downloading "node-v8.15.0-darwin-x64.tar.gz" to the Windows machine.

      Even when it does (randomly?) select the correct package, we're unable to build, since the windows logic in NodeJSInstaller doesn't fire, so it uses an incorrect command.

      I've cut everything down to a minimal Jenkinsfile that reproduces the issue for us. See screenshots for the console output.

       

      nodeLinux = 'ubuntu64-normal-amazon-1'
      nodeWin = 'windows-office-devops'
      nodeMac = 'devops-osx-slave'
      
      nodejsInstallation = 'node8'
      
      pipeline {
          agent { label 'master' }
          stages {
              stage('Test') {
                  parallel {
                      stage('Linux') {
                          agent { label nodeLinux }
                          steps {
                              nodejs(nodeJSInstallationName: nodejsInstallation) {
                                  sh 'yarn install'
                              }
                          }
                      }
                      stage('Win') {
                          agent { label nodeWin }
                          steps {
                              nodejs(nodeJSInstallationName: nodejsInstallation) {
                                  bat 'yarn install'
                              }
                          }
                      }
                      stage('MacOS') {
                          agent { label nodeMac }
                          steps {
                              nodejs(nodeJSInstallationName: nodejsInstallation) {
                                  sh 'yarn install'
                              }
                          }
                      }
                  }
              }
          }
      }

       
      It works fine if not run in parallel. I've also tried the OS detection logic in the Jenkins console and it works fine there, too:

       

      import jenkins.plugins.nodejs.tools.*;
      
      println('Linux:')
      println(ToolsUtils.getPlatform(Jenkins.instance.getNode('ubuntu64-normal-amazon-1')));
      println('Mac:')
      println(ToolsUtils.getPlatform(Jenkins.instance.getNode('devops-osx-slave')));
      println('Windows:')
      println(ToolsUtils.getPlatform(Jenkins.instance.getNode('windows-office-devops')));
      
      ---
      
      Linux:
      LINUX
      Mac:
      OSX
      Windows:
      WINDOWS
      

        Attachments

          Activity

          Hide
          adaphi Philip Adams added a comment -

          I've spent some time with a stripped-down Jenkins instance and managed to track down the exact plugin upgrade that caused this. It is related to version 2.14 of the workflow-basic-steps plugin.

          When running Jenkins v2.163 with nodeJS plugin v1.2.7 and workflow-basic-steps 2.13, everything is fine. When using the same but with workflow-basic-steps 2.14, the error happens as described. Reverting back to 2.13 makes the install work again.

          From looking at the logs, it appears that with the older version the node installation doesn't actually happen in parallel - it installs node on one machine at a time, then goes on to execute the rest of the script. With the newer version, it attempts to install node in parallel, and the machines interfere with each other, so the result of the getPlatform call is applied to all of the machines, and you end up with the wrong package.

          Please let me know if this is an issue for you, or if I should raise it with the maintainers of the other plugin.

          Show
          adaphi Philip Adams added a comment - I've spent some time with a stripped-down Jenkins instance and managed to track down the exact plugin upgrade that caused this. It is related to version 2.14 of the workflow-basic-steps plugin. When running Jenkins v2.163 with nodeJS plugin v1.2.7 and workflow-basic-steps 2.13, everything is fine. When using the same but with workflow-basic-steps 2.14, the error happens as described. Reverting back to 2.13 makes the install work again. From looking at the logs, it appears that with the older version the node installation doesn't  actually happen in parallel - it installs node on one machine at a time, then goes on to execute the rest of the script. With the newer version, it attempts to install node in parallel, and the machines interfere with each other, so the result of the getPlatform call is applied to all of the machines, and you end up with the wrong package. Please let me know if this is an issue for you, or if I should raise it with the maintainers of the other plugin.
          Hide
          nfalco Nikolas Falco added a comment -

          Philip Adams your analysis was very very good. Thank a lot. The platform is taken from system property gather by the EnvVars for that node (JENKINS-14807). I should check if NodeJSInstall is instantiated just one time but run in parallel by the parallel task (so I have to check it must be thread safe) or there is an issue in the EnvVars given by the node.

          Show
          nfalco Nikolas Falco added a comment - Philip Adams your analysis was very very good. Thank a lot. The platform is taken from system property gather by the EnvVars for that node ( JENKINS-14807 ). I should check if NodeJSInstall is instantiated just one time but run in parallel by the parallel task (so I have to check it must be thread safe) or there is an issue in the EnvVars given by the node.
          Hide
          paulomart Paulomart added a comment -

          Its there any update on this issue? We have a lot of parallel pipelines and this fails a lot.

           

          Thanks, Paul

          Show
          paulomart Paulomart added a comment - Its there any update on this issue? We have a lot of parallel pipelines and this fails a lot.   Thanks, Paul
          Hide
          achicu Alexandru Chiculita added a comment - - edited

          I had the same issue and I solved it by adding different NodeJS installers in my configuration. It doesn't have to use different node versions, just different labels for the tool.

          stage('Linux') {
              agent { label nodeLinux }
              steps {
                  nodejs(nodeJSInstallationName: nodejsInstallation + '-linux') {
                      sh 'yarn install'
                  }
              }
          }
          stage('Win') {
              agent { label nodeWin }
              steps {
                  nodejs(nodeJSInstallationName: nodejsInstallation + '-windows') {
                      bat 'yarn install'
                  }
              }
          }
          stage('MacOS') {
              agent { label nodeMac }
              steps {
                  nodejs(nodeJSInstallationName: nodejsInstallation + '-macos') {
                      sh 'yarn install'
                  }
              }
          }
          

           

          The NodeJSInstaller sets `this.platform` on the object itself and then calls a different method to compute the URL. That is not thread safe, so any other node installing at the same, will use the wrong platform. The NodeJSInstaller is used by InstallerTranslator (linked below) on multiple nodes simultaneously.

          https://github.com/jenkinsci/nodejs-plugin/blob/master/src/main/java/jenkins/plugins/nodejs/tools/NodeJSInstaller.java#L127

           

          Notice how the semaphore used in InstallerTranslator is per node & per tool. That means the tool can install at most once at a time on a specific node. However, that doesn't prevent the same tool instance from installing on different nodes simultaneously.

          https://github.com/jenkinsci/jenkins/blob/22aa2e6e766074d11249893e3f35e0b99e20d3d0/core/src/main/java/hudson/tools/InstallerTranslator.java#L72

           

          Show
          achicu Alexandru Chiculita added a comment - - edited I had the same issue and I solved it by adding different NodeJS installers in my configuration. It doesn't have to use different node versions, just different labels for the tool. stage( 'Linux' ) { agent { label nodeLinux } steps { nodejs(nodeJSInstallationName: nodejsInstallation + '-linux' ) { sh 'yarn install' } } } stage( 'Win' ) { agent { label nodeWin } steps { nodejs(nodeJSInstallationName: nodejsInstallation + '-windows' ) { bat 'yarn install' } } } stage( 'MacOS' ) { agent { label nodeMac } steps { nodejs(nodeJSInstallationName: nodejsInstallation + '-macos' ) { sh 'yarn install' } } }   The NodeJSInstaller sets `this.platform` on the object itself and then calls a different method to compute the URL. That is not thread safe, so any other node installing at the same, will use the wrong platform. The NodeJSInstaller is used by InstallerTranslator (linked below) on multiple nodes simultaneously. https://github.com/jenkinsci/nodejs-plugin/blob/master/src/main/java/jenkins/plugins/nodejs/tools/NodeJSInstaller.java#L127   Notice how the semaphore used in InstallerTranslator is per node & per tool. That means the tool can install at most once at a time on a specific node. However, that doesn't prevent the same tool instance from installing on different nodes simultaneously. https://github.com/jenkinsci/jenkins/blob/22aa2e6e766074d11249893e3f35e0b99e20d3d0/core/src/main/java/hudson/tools/InstallerTranslator.java#L72  
          Hide
          paulomart Paulomart added a comment - - edited

          To fix this `this.platform` and `this.cpu` would need to be stored for each node correct?

           

          I created this a PR proposing the changes: https://github.com/jenkinsci/nodejs-plugin/pull/22

           (I didnt test the changes yet)

          Show
          paulomart Paulomart added a comment - - edited To fix this `this.platform` and `this.cpu` would need to be stored for each node correct?   I created this a PR proposing the changes: https://github.com/jenkinsci/nodejs-plugin/pull/22  (I didnt test the changes yet)
          Hide
          nfalco Nikolas Falco added a comment -

          Alexandru Chiculita thanks a lot to had found the root cause.

          Show
          nfalco Nikolas Falco added a comment - Alexandru Chiculita thanks a lot to had found the root cause.

            People

            • Assignee:
              nfalco Nikolas Falco
              Reporter:
              adaphi Philip Adams
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: