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

When clause shouldn't require node to evaluate

    XMLWordPrintable

Details

    • Pipeline - December

    Description

      In previous versions of Jenkins Pipeline, each stage's "where" clause would evaluate quickly, not requiring an allocated node to see if its stage needed to be run.  Suddenly, we see Jenkins  allocating the stage's node to evaluate the "where" clause for each stage.  Allocating nodes for us is fairly expensive and we want to avoid this if the where clause stipulates the stage won't run.

      Attachments

        Issue Links

          Activity

            chrisleck Christopher Eck created issue -
            abayer Andrew Bayer added a comment -

            Hrm, I'll need to look into this - I could have sworn I'd specifically made sure that when is evaluated before the agent is handled.

            abayer Andrew Bayer added a comment - Hrm, I'll need to look into this - I could have sworn I'd specifically made sure that when is evaluated before the agent is handled.

            Hi there, the same thing should apply to docker agent.

            The docker agent container should not be started before the when condition is evaluated.

            sdurrheimer Steve Durrheimer added a comment - Hi there, the same thing should apply to docker agent. The docker agent container should not be started before the when condition is evaluated.
            jamesdumay James Dumay made changes -
            Field Original Value New Value
            Epic Link JENKINS-45426 [ 183594 ]
            abayer Andrew Bayer added a comment -

            Since there are some cases where users may need to have the agent allocated before when is evaluated, we have to do it in this order. Sorry.

            abayer Andrew Bayer added a comment - Since there are some cases where users may need to have the agent allocated before when is evaluated, we have to do it in this order. Sorry.
            abayer Andrew Bayer made changes -
            Resolution Won't Fix [ 2 ]
            Status Open [ 1 ] Resolved [ 5 ]

            This is a really huge pain for our use of Jenkins - it can extend the time for checkin processing by minutes.  If you can't do this for the standard "when" clause, could you generate a new "quick when" clause that behaves like the old version?  I'm happy to use a restricted version that can only look at flags set for the overall run.

            chrisleck Christopher Eck added a comment - This is a really huge pain for our use of Jenkins - it can extend the time for checkin processing by minutes.  If you can't do this for the standard "when" clause, could you generate a new "quick when" clause that behaves like the old version?  I'm happy to use a restricted version that can only look at flags set for the overall run.
            chrisleck Christopher Eck made changes -
            Resolution Won't Fix [ 2 ]
            Status Resolved [ 5 ] Reopened [ 4 ]
            abayer Andrew Bayer made changes -
            Link This issue is duplicated by JENKINS-46693 [ JENKINS-46693 ]
            wsaxon Will Saxon added a comment -

            Yeah this is pretty disappointing. It would be interesting to know how often 'when' clauses require node processing. It seems like this would be the exception.

            How does this get processed? Could there be something like a "requireNodeAllocation: true" in the when {} block, or has the ship sailed by the time that would get evaluated?

            wsaxon Will Saxon added a comment - Yeah this is pretty disappointing. It would be interesting to know how often 'when' clauses require node processing. It seems like this would be the exception. How does this get processed? Could there be something like a "requireNodeAllocation: true" in the when {} block, or has the ship sailed by the time that would get evaluated?
            abayer Andrew Bayer added a comment -

            So I think I may be able to make it so that when types are marked internally as to whether they require an agent. Off the top of my head, the only one that I'd mark as requiring an agent is expression, since I can't tell what you might be doing in there. So if you used any when condition but expression, it wouldn't go into an agent first, but if you used expression (including inside a nested anyOf, allOf, or not), it would go into an agent first. How's that sound?

            abayer Andrew Bayer added a comment - So I think I may be able to make it so that when types are marked internally as to whether they require an agent . Off the top of my head, the only one that I'd mark as requiring an agent is expression , since I can't tell what you might be doing in there. So if you used any when condition but expression , it wouldn't go into an agent first, but if you used expression (including inside a nested anyOf , allOf , or not ), it would go into an agent first. How's that sound?
            wsaxon Will Saxon added a comment -

            Actually a common use case of ours is to have expression inside the when; we have stages which toggle based on an environment variable set on the pipeline job itself, so that we can have a single Jenkinsfile handling both pre- and post-merge workflows.

            So for us this wouldn't work, but we can deal with it by having separate Jenkinsfiles and/or using shared libraries instead.

            Example pipeline:

            ciBuild = env.CI == null ? true : env.CI.toLowerCase() == 'true'
            
            pipeline {
              agent none
              stages {
                stage('Build') {
                  when { expression { return !ciBuild } }
                  agent { label 'someLabel' }
                  steps {
                    sh "echo Build"
                  }
                }
                stage('Test') {
                  when { expression { return ciBuild } }
                  agent { label 'someOtherLabel' }
                  steps {
                    sh "echo Test"
                  }
                }
              }
            }
            

            What specifically brought me here is that someOtherLabel points to a huge Docker image which takes a while to spin up; if it's not already cached it can take 20 minutes to pull down. Once I noticed this I got a little scared because our pipelines actually have several more stages, most of which are skipped in the ciBuild scenario. The extra time spent allocating nodes would generate complaints.

            Subsequent to commenting I noticed that I can put a when inside a parallel, and if I have {{ agent none }} as above, no node will be allocated. Can you have parallel with a single substage? Even if you have to have a dummy placeholder stage, people might be fine trading the extra logging and UI noise for reduced node allocations.

            wsaxon Will Saxon added a comment - Actually a common use case of ours is to have expression inside the when ; we have stages which toggle based on an environment variable set on the pipeline job itself, so that we can have a single Jenkinsfile handling both pre- and post-merge workflows. So for us this wouldn't work, but we can deal with it by having separate Jenkinsfiles and/or using shared libraries instead. Example pipeline: ciBuild = env.CI == null ? true : env.CI.toLowerCase() == ' true ' pipeline { agent none stages { stage( 'Build' ) { when { expression { return !ciBuild } } agent { label 'someLabel' } steps { sh "echo Build" } } stage( 'Test' ) { when { expression { return ciBuild } } agent { label 'someOtherLabel' } steps { sh "echo Test" } } } } What specifically brought me here is that someOtherLabel points to a huge Docker image which takes a while to spin up; if it's not already cached it can take 20 minutes to pull down. Once I noticed this I got a little scared because our pipelines actually have several more stages, most of which are skipped in the ciBuild scenario. The extra time spent allocating nodes would generate complaints. Subsequent to commenting I noticed that I can put a when inside a parallel , and if I have {{ agent none }} as above, no node will be allocated. Can you have parallel with a single substage? Even if you have to have a dummy placeholder stage, people might be fine trading the extra logging and UI noise for reduced node allocations.
            abayer Andrew Bayer added a comment -

            So it turns out that it's easier to just add a beforeAgent field for when - the default will be to require the agent first, but if beforeAgent true is specified, it'll enter the agent after the when evaluation.

            PR incoming.

            abayer Andrew Bayer added a comment - So it turns out that it's easier to just add a beforeAgent field for when - the default will be to require the agent first, but if beforeAgent true is specified, it'll enter the agent after the when evaluation. PR incoming.
            abayer Andrew Bayer made changes -
            Status Reopened [ 4 ] In Progress [ 3 ]
            abayer Andrew Bayer made changes -
            Status In Progress [ 3 ] In Review [ 10005 ]
            abayer Andrew Bayer added a comment - PR up at https://github.com/jenkinsci/pipeline-model-definition-plugin/pull/232 , with docs PR up at https://github.com/jenkins-infra/jenkins.io/pull/1290
            abayer Andrew Bayer made changes -
            Remote Link This issue links to "PR #232 (Web Link)" [ 19510 ]
            abayer Andrew Bayer made changes -
            Sprint Pipeline - December [ 446 ]

            Code changed in jenkins
            User: Andrew Bayer
            Path:
            pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWhen.java
            pipeline-model-api/src/main/resources/ast-schema.json
            pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/StageConditionals.groovy
            pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/JSONParser.groovy
            pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy
            pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy
            pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/Messages.properties
            pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ModelInterpreter.groovy
            pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java
            pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java
            pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/WhenStageTest.java
            pipeline-model-definition/src/test/resources/json/whenBeforeAgentTrue.json
            pipeline-model-definition/src/test/resources/whenBeforeAgentFalse.groovy
            pipeline-model-definition/src/test/resources/whenBeforeAgentTrue.groovy
            pipeline-model-definition/src/test/resources/whenBeforeAgentUnspecified.groovy
            http://jenkins-ci.org/commit/pipeline-model-definition-plugin/ae2406d833ccb9b199e9c4865d302ef882ed1dae
            Log:
            [FIXED JENKINS-44461] Add beforeAgent flag for when

            If true, the when condition will be evaluated before entering the
            stage's agent (if defined). The default is false - better safe than
            sorry there.

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andrew Bayer Path: pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWhen.java pipeline-model-api/src/main/resources/ast-schema.json pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/StageConditionals.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/JSONParser.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/Messages.properties pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ModelInterpreter.groovy pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/WhenStageTest.java pipeline-model-definition/src/test/resources/json/whenBeforeAgentTrue.json pipeline-model-definition/src/test/resources/whenBeforeAgentFalse.groovy pipeline-model-definition/src/test/resources/whenBeforeAgentTrue.groovy pipeline-model-definition/src/test/resources/whenBeforeAgentUnspecified.groovy http://jenkins-ci.org/commit/pipeline-model-definition-plugin/ae2406d833ccb9b199e9c4865d302ef882ed1dae Log: [FIXED JENKINS-44461] Add beforeAgent flag for when If true, the when condition will be evaluated before entering the stage's agent (if defined). The default is false - better safe than sorry there.
            abayer Andrew Bayer added a comment -

            Merged, will be in Declarative 1.2.6.

            abayer Andrew Bayer added a comment - Merged, will be in Declarative 1.2.6.
            abayer Andrew Bayer made changes -
            Resolution Fixed [ 1 ]
            Status In Review [ 10005 ] Resolved [ 5 ]

            Code changed in jenkins
            User: Andrew Bayer
            Path:
            content/doc/book/pipeline/syntax.adoc
            http://jenkins-ci.org/commit/jenkins.io/09ee78185f68c7212613202ef20d7523504acf1c
            Log:
            JENKINS-44461 Add documentation for new when->beforeAgent field

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andrew Bayer Path: content/doc/book/pipeline/syntax.adoc http://jenkins-ci.org/commit/jenkins.io/09ee78185f68c7212613202ef20d7523504acf1c Log: JENKINS-44461 Add documentation for new when->beforeAgent field

            Code changed in jenkins
            User: Liam Newman
            Path:
            content/doc/book/pipeline/syntax.adoc
            http://jenkins-ci.org/commit/jenkins.io/003b9ead4337beba1b4a835a2496f2d7ce1ad365
            Log:
            Merge pull request #1290 from abayer/jenkins-44461

            JENKINS-44461 Add documentation for new when->beforeAgent field

            Compare: https://github.com/jenkins-infra/jenkins.io/compare/ff40d07092a3...003b9ead4337

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Liam Newman Path: content/doc/book/pipeline/syntax.adoc http://jenkins-ci.org/commit/jenkins.io/003b9ead4337beba1b4a835a2496f2d7ce1ad365 Log: Merge pull request #1290 from abayer/jenkins-44461 JENKINS-44461 Add documentation for new when->beforeAgent field Compare: https://github.com/jenkins-infra/jenkins.io/compare/ff40d07092a3...003b9ead4337
            bitwiseman Liam Newman added a comment -

            Bulk closing resolved issues.

            bitwiseman Liam Newman added a comment - Bulk closing resolved issues.
            bitwiseman Liam Newman made changes -
            Status Resolved [ 5 ] Closed [ 6 ]
            kon Kalle Niemitalo made changes -
            Link This issue is duplicated by JENKINS-66796 [ JENKINS-66796 ]

            People

              abayer Andrew Bayer
              chrisleck Christopher Eck
              Votes:
              10 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: