Adding milestone, lock, and block-stage steps to the demo #3

amuniz · 2016-04-19T11:13:43Z

Downstream of

@reviewbybees

amuniz · 2016-04-19T11:14:37Z

demo/Dockerfile

@@ -29,7 +29,7 @@ USER jenkins
 RUN echo '<settings><mirrors><mirror><id>central</id><url>http://repo.jenkins-ci.org/simple/repo1-cache/</url><mirrorOf>central</mirrorOf></mirror></mirrors><localRepository>/usr/share/jenkins/ref/.m2/repository</localRepository></settings>' > settings.xml
 RUN /usr/local/maven/bin/mvn -s settings.xml -f repo-wc install && /usr/local/maven/bin/mvn -s settings.xml -f repo-wc/sometests -Dmaven.test.failure.ignore clean install

-COPY plugins.txt .
+COPY plugins.txt ./


Not sure how it worked before. Without the slash I get this.

What version of Docker are you using? I've seen a couple quirks around this where behaviors changed between docker versions.

ghost · 2016-04-19T11:18:19Z

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.

jglick · 2016-04-19T13:13:51Z

demo/plugins.txt

@@ -9,7 +9,10 @@ git-server:1.6
 handlebars:1.1.1
 icon-shim:2.0.3
 jquery-detached:1.2.1
+lockable-resources:1.8
+mailer:1.5


Transitive dependencies of lockable-resources. All dependencies must be explicitly set as there is no dependencies resolution here, no?

there is no dependencies resolution here

Correct.

svanoort · 2016-04-25T14:20:31Z

demo/repo/Jenkinsfile

-    servers.deploy 'production'
-    echo "Deployed to ${jettyUrl}production/"
+milestone()
+lock('production-server') {


🐜 Similar to previous comment: one consistent style is helpful, alternately an explaining comment like "this is the same as specifying resource: 'production-server' "

…k-demo

amuniz · 2016-05-12T17:58:29Z

@jglick updated.

jglick · 2016-05-12T18:14:33Z

Needs to be updated to include jenkinsci/pipeline-stage-step-plugin#4.

amuniz · 2016-05-17T18:28:44Z

@jglick I found some cases where the blocked stage can not cover what non-blocked stage covered. For example:

stage 'QA'
echo 'Hi!'
lock(resource: 'staging-server') {
    stage name: 'Staging'
    echo 'In staging'
}

The action to acquire staging-server is in QA stage, but I see no way to do that with blocked stages. Or maybe stage name: 'Staging' should be conceptually before lock?

jglick · 2016-05-17T19:06:46Z

@amuniz what is wrong with (roughly)

stage('Staging') {
  lock('staging-server') {
    node {
      servers.deploy 'staging'
    }
    input "Does ${jettyUrl}staging/ look good?"
  }
}
checkpoint 'Before production'

?

amuniz · 2016-05-18T08:30:08Z

Nothing wrong, but not equivalent. The build is in Staging stage while waiting to acquire the lock, but in the original script it is in QA stage.

jglick · 2016-05-19T14:12:25Z

Well then we have fixed a buglet: logically it makes no sense for the build to be considered in QA when we have finished QA and are now just waiting to stage it.

jglick · 2016-05-19T14:14:49Z

I mean, depending on how the stage view visualizes things, this would also be valid:

lock('staging-server') {
  stage('Staging') {
    node {
      servers.deploy 'staging'
    }
    input "Does ${jettyUrl}staging/ look good?"
  }
}
checkpoint 'Before production'

which would mean that if we have to wait for the server, there will be a PauseAction (lock needs to add this BTW if you have not done so already!) which will be between stages rather than inside some stage. I have no strong opinion about which is “better” from a UI rendering standpoint. Keeping stage outside lock would minimize the need for refinements to the existing visualization scheme.

jglick · 2016-05-19T19:35:10Z

demo/repo/Jenkinsfile

-    echo "Deployed to ${jettyUrl}production/"
+milestone 2
+stage ('Production') {
+    lock('production-server') {


Is there a reason inversePrecedence is not being specified here too?

jglick · 2016-05-19T20:07:22Z

So. One obvious problem is that @svanoort has not yet patched pipeline-stage-view to display blocky stages, so you are back to textual console log visualization (or Pipeline Steps).

More to the point, the demo as written does not solve the problem originally stated in JENKINS-27039. If you decline to approve the first build of cd/master

…
[Pipeline] }
[Pipeline] // stage
[Pipeline] milestone
Trying to pass milestone 1
[Pipeline] stage
[Pipeline] { (Staging)
[Pipeline] lock
Resource [staging-server] did not exist. Created.
Trying to acquire lock on [staging-server]
Lock acquired on [staging-server]
[Pipeline] {
[Pipeline] node
Running on mock-slave-4 in /var/jenkins_home/mock-slaves/mock-slave-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

and then trigger a second build, it hangs:

…
[Pipeline] }
[Pipeline] // stage
[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...

I thought the idea was that once the second build passed milestone 1, if the first build was still between 1 and 2 it would be canceled. But this does not seem to happen.

amuniz · 2016-05-20T10:36:35Z

So. One obvious problem is that @svanoort has not yet patched pipeline-stage-view to display blocky stages, so you are back to textual console log visualization (or Pipeline Steps).

Yeah, I see this in the logs, maybe unrelated:

May 20, 2016 10:08:30 AM org.kohsuke.stapler.framework.adjunct.AdjunctsInPage findNeeded
WARNING: No such adjunct found: org.jenkinsci.pipeline.stageview_adjunct
org.kohsuke.stapler.framework.adjunct.NoSuchAdjunctException: Neither org.jenkinsci.pipeline.stageview_adjunct.css, .js, .html, nor .jelly were found

I thought the idea was that once the second build passed milestone 1, if the first build was still between 1 and 2 it would be canceled. But this does not seem to happen.

Yes, that's what milestone is supposed to do, but input inside lock is preventing the second build to go ahead. So in this usage example, input should go outside the locked block.

Fixed in last commit.

jglick · 2016-05-23T14:59:57Z

maybe unrelated

I think unrelated.

input inside lock is preventing the second build to go ahead

Build 1 is past milestone 1 and holding the lock and paused in input. Build 2 is also past milestone 1. OK, rereading the description of the milestone step (BTW the third point is not grammatically correct English and I am not sure what is means), I see that the fourth point says that build 1 will only be aborted once build 2 hits the second milestone, whereas I thought it would be aborted once build 2 hits the first milestone.

But now there may be a problem in that the lock is not held while we prompt for input, so someone could come to this prompt while the server is being updated to some newer build, which would be misleading. What we would really want (I think) is that only one build at a time can be in staging, and if build 1 is waiting in this stage and build 2 arrives, build 1 is aborted then.

At any rate, I should play with the behavior as of the revised demo.

amuniz · 2016-05-23T16:49:43Z

I see that the fourth point says that build 1 will only be aborted once build 2 hits the second milestone, whereas I thought it would be aborted once build 2 hits the first milestone.

Right, that way the never deploy syndrome (newer builds always canceling the previous one in a loaded pipeline) is avoided. And this feature is what fixes JENKINS-27039.

But now there may be a problem in that the lock is not held while we prompt for input, so someone could come to this prompt while the server is being updated to some newer build, which would be misleading.

Yes, I think I misread your previous comment about the hanging build. The previous version of the Jenkinsfile was ok and works as expected (locking the stage-server and unlocking on build abort/proceed). I'm not able to reproduce the hanging on lock you pointed before.

I'm going back to that version.

jglick · 2016-05-23T17:05:49Z

I'm not able to reproduce the hanging on lock you pointed before.

No? I thought this was the designed behavior when the input was inside lock. Build 1 would wait for something to happen, and build 2 would just sit waiting for the lock. Perhaps that is not wrong; it just means that if you do nothing about build 1 but build 3 comes along, 2 will be canceled, and 3 will be waiting for 1. Eventually you either approve or cancel 1; either way, 3 begins staging.

amuniz · 2016-05-23T17:23:04Z

No?

No. Maybe we are talking about different things. Correct me if I'm wrong, the use case you exposed in this comment was:

Run build 1
Decline it while waiting on input
Run build 2
It hangs waiting for lock staging-server

That's not reproducible for me, and if it were, it would be a bug.

Build 1 would wait for something to happen, and build 2 would just sit waiting for the lock

Yes, this is right.

it just means that if you do nothing about build 1 but build 3 comes along, 2 will be canceled, and 3 will be waiting for 1

Not exactly. In that situation build 2 and 3 will keep queued until build 1 releases the lock (allowed to proceed or not), at that point the newer build waiting for lock will be allowed to go in (as inversePrecedence: true is configured in lock), so build 3 acquires the lock. Build 2 will sit waiting for lock. Only when build 3 reaches the next milestone then build 2 is cancelled (and it's fine, as build 3 could fail before reaching the next milestone so build 2 would become the newest build which is potentially fine to go to production).

jglick · 2016-05-23T17:53:16Z

No, my use case was

run build 1
wait for build 1 to ask for input (do not accept nor decline)
run build 2
build 2 will wait for the lock

Which I suppose is OK.

Only when build 3 reaches the next milestone then build 2 is cancelled

Note that in the meantime build 2 would probably have acquired the staging lock and started staging, so it will get canceled in the middle of that somewhere.

amuniz · 2016-05-23T18:57:31Z

Then yes, that's ok, build 2 has to wait for the lock.

Note that in the meantime build 2 would probably have acquired the staging lock

Right. I think it doesn't matter (in this example), otherwise the script needs to be modified as:

milestone 1
stage('Staging') {
    lock(resource: 'staging-server', inversePrecedence: true) {
        node {
            servers.deploy 'staging'
        }
        input message: "Does ${jettyUrl}staging/ look good?"
        milestone 2
    }
    ...
}

So any older build waiting in lock will be cancelled before the lock is released.
I can change it if you feel strong about it.

jglick · 2016-05-23T19:02:17Z

So any older build waiting in lock will be cancelled before the lock is released.

Yeah, worth exploring.

Did you check behavior with checkpoints by the way?

amuniz · 2016-05-24T09:34:47Z

Did you check behavior with checkpoints by the way?

Yes, basic tests though, it seems to work. Running a checkpoint inside a lock is not reliable (as it was before with stage concurrency: 1 I believe).

Adding milestone and lock steps to the demo

de9dfeb

amuniz reviewed Apr 19, 2016
View reviewed changes

jglick reviewed Apr 19, 2016
View reviewed changes

jglick added the work-in-progress label Apr 19, 2016

amuniz added 2 commits April 19, 2016 19:02

@jglick's comments addressed

a349288

Revert. The parameter name is required as both parameters are optional

592d0ef

svanoort reviewed Apr 25, 2016
View reviewed changes

amuniz added 3 commits May 12, 2016 19:26

Merge remote-tracking branch 'upstream/master' into milestone-and-loc…

6ed125b

…k-demo

Let ordinals to be autogenerated

14130f3

Revert to the original behavior about loading snapshot plugins

80a65fb

Use the recommended syntax: 'milestone 1'

19f54ca

Add the blocked stage step to the demo

9fb810e

jglick changed the title ~~Adding milestone and lock steps to the demo~~ Adding milestone, lock, and block-stage steps to the demo May 19, 2016

jglick reviewed May 19, 2016
View reviewed changes

Place input outside lock

881fbea

Revert the previous commit, as the input step must be inside the lock

b12cd65

jglick removed the work-in-progress label May 23, 2016

amuniz mentioned this pull request Jun 2, 2016

Make RequireResourcesProperty work with workflows jenkinsci/lockable-resources-plugin#32

Closed

jglick mentioned this pull request Jul 1, 2016

[JENKINS-27039] New step: milestone jenkinsci/pipeline-milestone-step-plugin#1

Merged

4 tasks

jglick mentioned this pull request Aug 30, 2016

[JENKINS-26107] Using blocky stages #9

Merged

jglick merged commit b12cd65 into jenkinsci:master Aug 31, 2016

jglick mentioned this pull request Sep 21, 2016

Demo updates #11

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding milestone, lock, and block-stage steps to the demo #3

Adding milestone, lock, and block-stage steps to the demo #3

amuniz commented Apr 19, 2016 •

edited

amuniz Apr 19, 2016

svanoort Apr 25, 2016

ghost commented Apr 19, 2016

jglick Apr 19, 2016

amuniz Apr 19, 2016

jglick Apr 19, 2016

svanoort Apr 25, 2016

amuniz commented May 12, 2016

jglick commented May 12, 2016

amuniz commented May 17, 2016 •

edited

jglick commented May 17, 2016

amuniz commented May 18, 2016

jglick commented May 19, 2016

jglick commented May 19, 2016

jglick May 19, 2016

jglick commented May 19, 2016

amuniz commented May 20, 2016

jglick commented May 23, 2016

amuniz commented May 23, 2016

jglick commented May 23, 2016

amuniz commented May 23, 2016

jglick commented May 23, 2016

amuniz commented May 23, 2016 •

edited

jglick commented May 23, 2016

amuniz commented May 24, 2016

Adding milestone, lock, and block-stage steps to the demo #3

Adding milestone, lock, and block-stage steps to the demo #3

Conversation

amuniz commented Apr 19, 2016 • edited

amuniz Apr 19, 2016

Choose a reason for hiding this comment

svanoort Apr 25, 2016

Choose a reason for hiding this comment

ghost commented Apr 19, 2016

jglick Apr 19, 2016

Choose a reason for hiding this comment

amuniz Apr 19, 2016

Choose a reason for hiding this comment

jglick Apr 19, 2016

Choose a reason for hiding this comment

svanoort Apr 25, 2016

Choose a reason for hiding this comment

amuniz commented May 12, 2016

jglick commented May 12, 2016

amuniz commented May 17, 2016 • edited

jglick commented May 17, 2016

amuniz commented May 18, 2016

jglick commented May 19, 2016

jglick commented May 19, 2016

jglick May 19, 2016

Choose a reason for hiding this comment

jglick commented May 19, 2016

amuniz commented May 20, 2016

jglick commented May 23, 2016

amuniz commented May 23, 2016

jglick commented May 23, 2016

amuniz commented May 23, 2016

jglick commented May 23, 2016

amuniz commented May 23, 2016 • edited

jglick commented May 23, 2016

amuniz commented May 24, 2016

amuniz commented Apr 19, 2016 •

edited

amuniz commented May 17, 2016 •

edited

amuniz commented May 23, 2016 •

edited