Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Demo updates #11

Merged
merged 2 commits into from Sep 22, 2016
Merged

Demo updates #11

merged 2 commits into from Sep 22, 2016

Conversation

jglick
Copy link
Member

@jglick jglick commented Sep 21, 2016

@reviewbybees esp. @amuniz

@ghost
Copy link

ghost commented Sep 21, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@@ -22,6 +22,7 @@ stage('QA') {
milestone 1
stage('Staging') {
lock(resource: 'staging-server', inversePrecedence: true) {
milestone 2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amuniz I found a problem in the following scenario:

  • start build 1, wait for it to pause in input
  • start build 2, wait for it to pause in lock
  • start build 3, wait for it to pause in lock
  • approve build 1, wait for it to finish
  • approve build 3, wait for it to finish

Build 2 ought to have been aborted cleanly, but shows as failed since it started trying to deploy into staging, and was aborted in the middle of a step:

[Pipeline] milestone
Trying to pass milestone 1
[Pipeline] stage
[Pipeline] { (Staging)
[Pipeline] lock
Trying to acquire lock on [staging-server]
[staging-server] is locked, waiting...
Lock acquired on [staging-server]
[Pipeline] {
[Pipeline] node
Running on mock-agent-2 in /var/jenkins_home/mock-agents/mock-agent-2/workspace/cd/master
[Pipeline] {
[Pipeline] unstash
Superseded by cd/master#3
[Pipeline] }
[Pipeline] // node
[Pipeline] }
Lock released on resource [staging-server]
[Pipeline] // lock
[Pipeline] }
[Pipeline] // stage
[Pipeline] End of Pipeline
java.lang.InterruptedException
    at java.lang.Object.wait(Native Method)
    at hudson.remoting.Request.call(Request.java:147)
    at hudson.remoting.Channel.call(Channel.java:780)
    at hudson.FilePath.act(FilePath.java:1007)
    at hudson.FilePath.act(FilePath.java:996)
    at hudson.FilePath.untarFrom(FilePath.java:728)
    at org.jenkinsci.plugins.workflow.flow.StashManager.unstash(StashManager.java:124)
    at org.jenkinsci.plugins.workflow.support.steps.stash.UnstashStep$Execution.run(UnstashStep.java:64)
    at org.jenkinsci.plugins.workflow.support.steps.stash.UnstashStep$Execution.run(UnstashStep.java:54)
    at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1$1.call(AbstractSynchronousNonBlockingStepExecution.java:52)
    at hudson.security.ACL.impersonate(ACL.java:213)
    at org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution$1.run(AbstractSynchronousNonBlockingStepExecution.java:49)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
Finished: NOT_BUILT

After this fix, as soon as build 1 proceeds into production, build 2 is canceled (since 3 is already waiting), which is what I think we want:

[Pipeline] milestone
Trying to pass milestone 1
[Pipeline] stage
[Pipeline] { (Staging)
[Pipeline] lock
Trying to acquire lock on [staging-server]
[staging-server] is locked, waiting...
Superseded by cd/master#3
[Pipeline] // lock
[Pipeline] }
[Pipeline] // stage
[Pipeline] End of Pipeline
Finished: NOT_BUILT

Copy link
Member

@amuniz amuniz Sep 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, good catch.

I actually think more natural would be to add the milestone after input so once a build passes it any older build waiting for lock is aborted (without even trying to acquire the lock).

milestone 1
stage('Staging') {
    lock(resource: 'staging-server', inversePrecedence: true) {
        node {
            servers.deploy 'staging'
        }
        input message: "Does ${jettyUrl}staging/ look good?"
        milestone 2
    }
    try {
        checkpoint('Before production')
    } catch (NoSuchMethodError _) {
        echo 'Checkpoint feature available in CloudBees Jenkins Enterprise.'
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any older build waiting for lock is aborted

That is exactly what my patch accomplishes.

without even trying to acquire the lock

Well, it was waiting to acquire the lock already.

Your variant does not work as well. In that case, after approving build 1, build 3 shows the prompt, but build 2 remains running, even though we can see at this point that it is obsolete. If you approve build 3, it runs to completion, but build 2 proceeds to input for reasons TBD (maybe a bug in LockStepExecution.stop?), yet even if you approve it, it will abort:

[Pipeline] milestone
Trying to pass milestone 1
[Pipeline] stage
[Pipeline] { (Staging)
[Pipeline] lock
Trying to acquire lock on [staging-server]
[staging-server] is locked, waiting...
Superseded by cd/master#3
Lock acquired on [staging-server]
[Pipeline] {
[Pipeline] node
Running on mock-agent-4 in /var/jenkins_home/mock-agents/mock-agent-4/workspace/cd/master
[Pipeline] {
[Pipeline] unstash
[Pipeline] sh
[master] Running shell script
+ cp x.war /tmp/webapps/staging.war
[Pipeline] }
[Pipeline] // node
[Pipeline] input
Does http://localhost:8081/staging/ look good?
Proceed or Abort
Click here to forcibly terminate running steps
[Pipeline] milestone
Trying to pass milestone 2
Canceled since build #3 already got here
[Pipeline] }
Lock released on resource [staging-server]
[Pipeline] // lock
[Pipeline] }
[Pipeline] // stage
[Pipeline] End of Pipeline
Finished: NOT_BUILT

You can see that cancelOldersInSight has been called, but the interrupt did not actually halt the lock step (hence the suggestion to escalate to doTerm).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it runs to completion, but build 2 proceeds to input for reasons TBD (maybe a bug in LockStepExecution.stop?)

Yes, certainly a bug, to be investigated.

@amuniz
Copy link
Member

amuniz commented Sep 22, 2016

🐝

@jglick jglick merged commit 8a69bb4 into jenkinsci:master Sep 22, 2016
@jglick jglick deleted the demo branch September 22, 2016 14:15
rhutchison added a commit to rhutchison/workflow-aggregator-plugin that referenced this pull request Feb 22, 2017
@rhutchison rhutchison mentioned this pull request Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants