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

First stage is always showing master as run node

      First stage is always showing master as the node where the stage was executed.

      Reproducible with this script (and having a node tagged as my-tag):

      def n = 'my-tag'
      stage 'Checkout'
      node(n) {
          echo 'checkout'
      }
      
      stage 'Build'
      node(n) {
          echo 'build'
      }
      

          [JENKINS-33290] First stage is always showing master as run node

          In my case all views have the same name. Before i updated the pipelienn plugin today it was always :master: but now it's always one of the slave names eventhough the job was executed on another slave

          Pavel Georgiev added a comment - In my case all views have the same name. Before i updated the pipelienn plugin today it was always :master: but now it's always one of the slave names eventhough the job was executed on another slave

          Sam Van Oort added a comment -

          This is an outcome of how the node labels are generated. Currently the label for a FlowNode comes from the nearest parent with a WorkspaceAction defined (tied to a node). It is not looking at child nodes, nor is it evaluating block scoping to match a node to its blocks.

          To fix this, block scoping must be tracked as the flow graph is walked, and a stack of all current scopes must be maintained.

          I am looking at how to implement that, but handling this correctly is rather complex and tends to recurse, so I am unsure on ETA.

          Sam Van Oort added a comment - This is an outcome of how the node labels are generated. Currently the label for a FlowNode comes from the nearest parent with a WorkspaceAction defined (tied to a node). It is not looking at child nodes, nor is it evaluating block scoping to match a node to its blocks. To fix this, block scoping must be tracked as the flow graph is walked, and a stack of all current scopes must be maintained. I am looking at how to implement that, but handling this correctly is rather complex and tends to recurse, so I am unsure on ETA.

          Marcus Philip added a comment -

          I solved this by putting the stage after the {{node { }} line.

          But this is so crappy. All the examples has the stage before the node, which is also what visually and logically makes sense.

          Did someone actually test this functionality before releasing it?

          Consider removing the node label form the view as it's broken and misleading!

          Marcus Philip added a comment - I solved this by putting the stage after the {{node { }} line. But this is so crappy. All the examples has the stage before the node, which is also what visually and logically makes sense. Did someone actually test this functionality before releasing it? Consider removing the node label form the view as it's broken and misleading!

          Jesse Glick added a comment -

          stage will soon be block-scoped, with the current syntax deprecated. Thus given

          def n = 'my-tag'
          stage('Checkout') {
            node(n) {
              echo 'checkout'
            }
          }
          stage('Build') {
            node(n) {
              echo 'build'
            }
          }
          

          we need to ensure that the stages correctly display the respective nodes. It seems most intuitive to me that all WorkspaceAction encountered within the block should be collected, and all such nodes (if any) displayed.

          I would agree that the current node display should just be removed until the logic is fixed.

          Jesse Glick added a comment - stage will soon be block-scoped, with the current syntax deprecated. Thus given def n = 'my-tag' stage( 'Checkout' ) { node(n) { echo 'checkout' } } stage( 'Build' ) { node(n) { echo 'build' } } we need to ensure that the stages correctly display the respective nodes. It seems most intuitive to me that all WorkspaceAction encountered within the block should be collected, and all such nodes (if any) displayed. I would agree that the current node display should just be removed until the logic is fixed.

          Sam Van Oort added a comment -

          It's a race between the fast fix (removing this) and the better fix (doing it correctly) which is getting very close.

          Sam Van Oort added a comment - It's a race between the fast fix (removing this) and the better fix (doing it correctly) which is getting very close.

          Sam Van Oort added a comment -

          Node labels are removed temporarily with version 1.5 – CC jglick hrmpw and marcus_phi

          This ensures that inaccurate labels are not displayed, but restoring labels correctly requires the new flow analyzer from https://issues.jenkins-ci.org/browse/JENKINS-34038

          Sam Van Oort added a comment - Node labels are removed temporarily with version 1.5 – CC jglick hrmpw and marcus_phi This ensures that inaccurate labels are not displayed, but restoring labels correctly requires the new flow analyzer from https://issues.jenkins-ci.org/browse/JENKINS-34038

          Sam Van Oort added a comment -

          I've got the new flow analyzer core in here which can support this, but the complexity of doing it right is still prohibitive.

          • Within a stage, they may include one or more node blocks (which can theoretically nest), or may be in a node block themself.
          • Correctly assigning which nodes were used for every flownode (and stage) is thus excessively complex and requires creating a full in-memory representation of the entire flow graph (yuck).

          At this point, better to permanently remove the labelling.

          Sam Van Oort added a comment - I've got the new flow analyzer core in here which can support this, but the complexity of doing it right is still prohibitive. Within a stage, they may include one or more node blocks (which can theoretically nest), or may be in a node block themself. Correctly assigning which nodes were used for every flownode (and stage) is thus excessively complex and requires creating a full in-memory representation of the entire flow graph (yuck). At this point, better to permanently remove the labelling.

          Sam Van Oort added a comment -

          Resolved by removing the functionality, since a correct implementation imposes unacceptable complexity and overhead.

          Sam Van Oort added a comment - Resolved by removing the functionality, since a correct implementation imposes unacceptable complexity and overhead.

            svanoort Sam Van Oort
            amuniz Antonio Muñiz
            Votes:
            3 Vote for this issue
            Watchers:
            9 Start watching this issue

              Created:
              Updated:
              Resolved: