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

Task/jenkins 37667 Adopt Bismuth API to build pipeline node DAG #557

Merged
merged 19 commits into from Oct 21, 2016

Conversation

4 participants
@vivek
Copy link
Collaborator

commented Oct 7, 2016

Description

See https://issues.jenkins-ci.org/browse/JENKINS-37667

To test manually try different pipeline jobs and see if there is any regressions.

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description
  • Appropriate unit or acceptance tests or explanation to why this change has no tests
  • Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.
  • Ran Acceptance Test Harness against PR changes.

Tried running ATH locally but get this error:

2016-10-07 07:52:37,057 [Thread-4] ERROR (InputStreamHandler.java:60) com.github.eirslett.maven.plugins.frontend.lib.DefaultGulpRunner - Error retrieving a new session from the selenium server 2016-10-07 07:52:37,058 [Thread-4] ERROR (InputStreamHandler.java:60) com.github.eirslett.maven.plugins.frontend.lib.DefaultGulpRunner - 2016-10-07 07:52:37,058 [Thread-4] ERROR (InputStreamHandler.java:60) com.github.eirslett.maven.plugins.frontend.lib.DefaultGulpRunner - Connection refused! Is selenium server started? 2016-10-07 07:52:37,058 [Thread-4] ERROR (InputStreamHandler.java:60) com.github.eirslett.maven.plugins.frontend.lib.DefaultGulpRunner - { Error: connect ECONNREFUSED 192.168.1.104:4444

Reviewer checklist

  • Run the changes and verified the change matches the issue description
  • Reviewed the code
  • Verified that the appropriate tests have been written or valid explanation given

@jenkinsci/code-reviewers @reviewbybees

@reviewbybees

This comment has been minimized.

Copy link
Collaborator

commented Oct 7, 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.

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 7, 2016

cc: @svanoort will be great to have you review this PR.

@vivek vivek added the needs-review label Oct 7, 2016

@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 7, 2016

OOH! NICE! giving this a crack on: https://ci.blueocean.io/job/ATH-Jenkinsfile/job/master/119/console

And it passes!

@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 7, 2016

OK so not reviewed the code (I think Sam best for that) but it looks like this may introduce a few regressions in karaoke (probably things that were worked around with since it had problems in the past with the old approach) - this may require some testing with @scherler and coordination.

From my first pass of testing, some :bugs:

  1. running deprecated style stages results in stack trace:
    In one case, I was building https://github.com/kzantow/failure-project multibranch pipeline - re-running the "passing" branch, and following along (karoke).

Clicking back on the build stage results (or checkout) results in a stacktrace 🐛:
https://gist.github.com/michaelneale/ad65eb3329d893ff3a4c0b836e7bdf06

The steps in those stages are also not marked as completed (they show the inprogress icon) and no logs are visible. (you can see the logs as the build progresses).

So there is at least one thing not right. I worked out this is due to using deprecated stages. You can do this by running:

node {    
    stage "first"
        sh 'ping -c 4 www.apple.com'

    stage "second" 
        sh 'ping -c 4 www.apple.com'
}
  1. running new style block scoped stages results in earlier stages having steps shown as running (blue) and sometimes having a truncated log (this looks like a regressino, have seen this before):

screen shot 2016-10-07 at 6 23 59 pm

pipeline to see this:

node {

    stage("first") {
      sh "ping -c 3 www.apple.com"  
    }


    stage("second") {
      sh "ping -c 3 www.apple.com"  
    }

}
  1. using parallel results in odd behavior, which also may be regression:

pipeline:

node {
    stage("first") {
        sh 'ping -c 3 www.apple.com'    
    }

    stage("second") {
        parallel (
            "left" : {
                sh 'ping -c 3 www.apple.com'            
            }, 
            "right" : {
                sh 'ping -c 10 www.apple.com'    
            }
            )

    }

    stage("third") {
        sh 'ping -c 3 www.apple.com'    
    }

}

it seems to highlight second branch (used to highlight first) and things generally look odd (logs truncated all over the place).

So a few problems. #1 is clearly a new bug, the others seem to be re-regressing, so perhaps more testing is needed, and front end code may need to be adjusted (perhaps book some time with @scherler to go over these, as he had to do some "nasty code" to make things work the previous way, which may be able to be simplified now).

It would be good if there was a switch to turn it on/off to quickly compare with non bismuth.

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 10, 2016

@michaelneale Thanks for detailed report. #1 is clearly a bug. I am going to investigate and get back.

@svanoort

This comment has been minimized.

Copy link
Member

commented Oct 10, 2016

To update: I am working on the review here - bear with me, it needs some time to give proper review, and I want to make sure it is thorough.

Vivek Pandey Vivek Pandey
@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 10, 2016

@michaelneale @scherler

I fixed issue 1. Issue 2 is weird, I see log API returning correct logs for each step.

For log of step inside 'first':

GET http://localhost:8080/jenkins/blue/rest/organizations/jenkins/pipelines/dp1/runs/2/nodes/11/steps/12/log/

and for log inside step for 'second':

GET http://localhost:8080/jenkins/blue/rest/organizations/jenkins/pipelines/dp1/runs/2/nodes/11/steps/12/log/

@scherler Looks like UI code is fetching logs from overall log instead of calling log API for each step? In that process its truncating log of each step? I will dig bit more and send meeting invite for sometime tomorrow.

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 10, 2016

@michaelneale for issue 3, I can't reproduce, tried running multiple times. Take a look at, https://youtu.be/AuVUyindgZQ. This looks correct to me, not sure how I reproduce what you are seeing.

@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 11, 2016

awaiting review from Sam and discussion with @vivek and @scherler

@svanoort

This comment has been minimized.

Copy link
Member

commented Oct 11, 2016

@michaelneale I'm still working through it, I'm afraid -- I was expecting to be able to finish yesterday but had blockers come up. Currently at 15 review comments and counting (some really are just comments, not changes, but I may just submit the batch tonight and then keep going from there).

@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 11, 2016

@svanoort no worries, its worth it! and much appreciated.

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 12, 2016

@michaelneale @scherler issue#2 (truncated) logs might be due to this branch not merged with latest master (was bit dated sept 15). After merge with latest master don't see truncated log anymore. Fee free to git it try.

@svanoort
Copy link
Member

left a comment

I haven't been able to cover everything here in depth, but am submitting the current batch since there are some requested changes that can be worked on in the meantime.

* @author Vivek Pandey
*/
public class FlowNodeWrapper {
public enum NodeType {STAGE, PARALLEL, STEP, UNKNWON}

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

🐛 Er, UNKNOWN, not UNKWON

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

Also, why UNKNOWN?

This comment has been minimized.

Copy link
@vivek

vivek Oct 14, 2016

Author Collaborator

Thats to represent unknown state, as discussed in meeting I will get rid of it.

}

public @Nullable FlowNodeWrapper getFirstParent(){
return parents.size() > 0 ? parents.get(0): null;

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

Is there a reason this is implemented in this particular way?

this.node = node;
this.status = status;
this.timingInfo = timingInfo;
if(PipelineNodeUtil.isStage(node)){

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

Coming back to this: needs close reading, because it's very easy to hang yourself up on these tests.

Also seems very iffy to only mark parallel branches

This comment has been minimized.

Copy link
@vivek

vivek Oct 14, 2016

Author Collaborator

We only care about parallel branches, parallel blocks are ignored.

public final BlueRun.BlueRunResult result;
public final BlueRun.BlueRunState state;

public NodeRunStatus(FlowNode endNode) {

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

🐛 Duplicates logic in the pipeline graph analysis APIs - from experience I'll say it is very easy to end up with bugs in the handling. Better to just accept the GenericStatus constructor.

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

Ugh, okay, I see why we need it (legacy API) but I would ask if we can instead invoke the status and timing APIs without context info (or treat it as the first AND last node in flow).

They should handle this case: you'll get incorrect results like you would here due to not having all the necessary information to compute full status (contextual info), but are no worse off and are at least invoking more hardened APIs.

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 14, 2016

Member

Is okay, as it was covered by preexisting code -- but needs to be removed once the Bismuth based APIs are vetted and legacy impl is removed.

This comment has been minimized.

Copy link
@vivek

vivek Oct 14, 2016

Author Collaborator

Ignore this, NodeRunStatus(FlowNode endNode) is for legacy graph builder, its disabled by default and will be removed eventually soon.

public final BlueRun.BlueRunState state;

public NodeRunStatus(FlowNode endNode) {
if (endNode.getError() != null) {

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

Nuance: what about caught/handled errors? Try/catch, I mean.

My approach has been to treat those as successes (handled) by looking at the following node for a failure. One can look for just an error there by not including the following node in consideration though.


@Override
public void parallelBranchStart(@Nonnull FlowNode parallelStartNode, @Nonnull FlowNode branchStartNode, @Nonnull ForkScanner scanner) {
super.parallelBranchStart(parallelStartNode, branchStartNode, scanner);

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

Again, no-op super 🐜

This comment has been minimized.

Copy link
@vivek

vivek Oct 14, 2016

Author Collaborator

Done

/**
* @author Vivek Pandey
*/
public class PipelineNodeGraphVisitor extends StandardChunkVisitor implements NodeGraphBuilder{

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

🐛 I think this approach will be brittle and have odd misbehaviors if someone does inadvertently does unsupported nesting. Better to use an ArrayDeque, where containertype is a parallel or stage. This arraydeque can fairly naturally handle state tracking (and may have an internal list of branches in-progress and total for a parallel).

This comment has been minimized.

Copy link
@vivek

vivek Oct 14, 2016

Author Collaborator

I think we went over in our meeting, I will add docs on nested stages.

new NodeRunStatus(status), times);

if(nextStage!=null) {
branch.addEdge(nextStage.getId());

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

Probably ok, though I feel weird with splitting stage vs. parallel this way.

dump(String.format("handleChunkDone=> id: %s, name: %s, function: %s", chunk.getFirstNode().getId(),
chunk.getFirstNode().getDisplayName(), chunk.getFirstNode().getDisplayFunctionName()));
if(isNested()){
return;

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

again, ow

public List<BluePipelineStep> getPipelineNodeSteps(final String nodeId, Link parent) {
DepthFirstScanner depthFirstScanner = new DepthFirstScanner();
//If blocked scope, get the end node
FlowNode n = depthFirstScanner.findFirstMatch(run.getExecution().getCurrentHeads(), new Predicate<FlowNode>() {

This comment has been minimized.

Copy link
@svanoort

svanoort Oct 13, 2016

Member

Can we collect this while iterating and not run through nodes again?

This comment has been minimized.

Copy link
@vivek

vivek Oct 14, 2016

Author Collaborator

Eventually will have cache to optimize it, for now I will keep it this way.

@svanoort

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

@vivek Replied to your email with my thoughts about how to approach it. End of day the problem requires keeping a bit more info around (you can't simply throw away chunks when you hit their start if you care about nesting, due to incompletes). That's why Bismuth level 2 is SAX-like, and I didn't end up implementing Level 3 (DOM tree-like APIs) -- needed to decide on an appropriate way to store the representation and didn't hit an API & data structure combo that covered the cases well enough.

@svanoort

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

Also @vivek CC @michaelneale RE sorting branches: as of latest workflow-api (make sure you're consuming that), they return parallel branches from last -> first, in order of declaration. They do get executed in order of declaration.

I suspect the bug here has to do with the ordering of map entries in your internal storage, since iteration order is locked. Which one hashes before each other, the order can swap.

I do however have an in-flight change that I'm working on which assures that the ChunkVisitor API uses the temporally last branch (last node run) when returning block/parallel end events. This ensures you don't get wacky timing/status when parallel branches complete in a different order than begun.

For display: I remember now why I used the name-based branch APIs: users are doing dynamic parallels, where the actual branches change conditionally between executions. Best practice would be to do a sorting by branch name to handle that case and it will incidentally work around the likely map-key-ordering issue.

@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

@svanoort @vivek so in summary: the best practice for all cases is to sort by name for display as that assures consistency (at least once they have started) including in dynamic case - is that right?

@svanoort

This comment has been minimized.

Copy link
Member

commented Oct 17, 2016

@michaelneale That's about the size of it, yeah.

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 17, 2016

@svanoort Ah, good catch, I should be using LinkedHashMap, not HashMap as I do in existing non-bismuth implementation. Out of order might happen for other nodes too. I am fixing that.

For parallel branch ordering, I am going to implement ordering by name to ensure its going to work in all cases.

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 19, 2016

@svanoort @michaelneale I think its functionally complete, has more features in terms of parity with existing implementation. these functionalities have been implemented:

  • branches are returned sorted by name
  • nested stages within stage is ignored in DAG
  • Added tests to test above

Who knows there might be edge cases, since its better than parity with existing implementation, probably we can merge if all looks good and if there are edge cases identified we open separate ticket for those?

@michaelneale

This comment has been minimized.

@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 19, 2016

Still not sure it is right.

I am running kzantow/failure-project, the "passing" branch, with karoke, and there are 500 errors when returning steps for the build stage when it is running.

I think this has issues with classic stages still...

http://localhost:8080/jenkins/blue/rest/organizations/jenkins/pipelines/keith/branches/passing/runs/37/nodes/5/steps/

yields a large stacktrace:






  <!DOCTYPE html><html><head resURL="/jenkins/static/3af10c65" data-rooturl="/jenkins" data-resurl="/jenkins/static/3af10c65">


    <title>Jenkins [Jenkins]</title><link rel="stylesheet" href="/jenkins/static/3af10c65/css/layout-common.css" type="text/css" /><link rel="stylesheet" href="/jenkins/static/3af10c65/css/style.css" type="text/css" /><link rel="stylesheet" href="/jenkins/static/3af10c65/css/color.css" type="text/css" /><link rel="stylesheet" href="/jenkins/static/3af10c65/css/responsive-grid.css" type="text/css" /><link rel="shortcut icon" href="/jenkins/static/3af10c65/favicon.ico" type="image/vnd.microsoft.icon" /><link color="black" rel="mask-icon" href="/jenkins/images/mask-icon.svg" /><script>var isRunAsTest=false; var rootURL="/jenkins"; var resURL="/jenkins/static/3af10c65";</script><script src="/jenkins/static/3af10c65/scripts/prototype.js" type="text/javascript"></script><script src="/jenkins/static/3af10c65/scripts/behavior.js" type="text/javascript"></script><script src='/jenkins/adjuncts/3af10c65/org/kohsuke/stapler/bind.js' type='text/javascript'></script><script src="/jenkins/static/3af10c65/scripts/yui/yahoo/yahoo-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/dom/dom-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/event/event-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/animation/animation-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/dragdrop/dragdrop-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/container/container-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/connection/connection-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/datasource/datasource-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/autocomplete/autocomplete-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/menu/menu-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/element/element-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/button/button-min.js"></script><script src="/jenkins/static/3af10c65/scripts/yui/storage/storage-min.js"></script><script src="/jenkins/static/3af10c65/scripts/hudson-behavior.js" type="text/javascript"></script><script src="/jenkins/static/3af10c65/scripts/sortable.js" type="text/javascript"></script><script>crumb.init("Jenkins-Crumb", "89e5c33705504bd95069d98f6286447a");</script><link rel="stylesheet" href="/jenkins/static/3af10c65/scripts/yui/container/assets/container.css" type="text/css" /><link rel="stylesheet" href="/jenkins/static/3af10c65/scripts/yui/assets/skins/sam/skin.css" type="text/css" /><link rel="stylesheet" href="/jenkins/static/3af10c65/scripts/yui/container/assets/skins/sam/container.css" type="text/css" /><link rel="stylesheet" href="/jenkins/static/3af10c65/scripts/yui/button/assets/skins/sam/button.css" type="text/css" /><link rel="stylesheet" href="/jenkins/static/3af10c65/scripts/yui/menu/assets/skins/sam/menu.css" type="text/css" /><meta name="ROBOTS" content="INDEX,NOFOLLOW" /><link rel='stylesheet' href='/jenkins/adjuncts/3af10c65/io/jenkins/blueocean/try.css' type='text/css' /><script src='/jenkins/adjuncts/3af10c65/io/jenkins/blueocean/try.js' type='text/javascript'></script><script src="/jenkins/static/3af10c65/jsbundles/page-init.js" type="text/javascript"></script></head><body data-model-type="hudson.model.Hudson" id="jenkins" class="yui-skin-sam jenkins-2.7.1 two-column" data-version="2.7.1"><a href="#skip2content" class="skiplink">Skip to content</a><div id="page-head"><div id="header"><div class="logo"><a id="jenkins-home-link" href="/jenkins/"><img src="/jenkins/static/3af10c65/images/headshot.png" alt="title" id="jenkins-head-icon" /><img src="/jenkins/static/3af10c65/images/title.png" alt="title" width="139" id="jenkins-name-icon" height="34" /></a></div><div class="login"> <a href="/jenkins/login?from=%2Fjenkins%2Fblue%2Frest%2Forganizations%2Fjenkins%2Fpipelines%2Fkeith%2Fbranches%2Fpassing%2Fruns%2F37%2Fnodes%2F5%2Fsteps%2F"><b>log in</b></a></div><div class="searchbox hidden-xs"><form method="get" name="search" action="/jenkins/search/" style="position:relative;" class="no-json"><div id="search-box-minWidth"></div><div id="search-box-sizer"></div><div id="searchform"><input name="q" placeholder="search" id="search-box" class="has-default-text" /> <a href="http://wiki.jenkins-ci.org/display/JENKINS/Search+Box"><img src="/jenkins/static/3af10c65/images/16x16/help.png" style="width: 16px; height: 16px; " class="icon-help icon-sm" /></a><div id="search-box-completion"></div><script>createSearchBox("/jenkins/search/");</script></div></form></div></div><div id="breadcrumbBar"><tr id="top-nav"><td id="left-top-nav" colspan="2"><link rel='stylesheet' href='/jenkins/adjuncts/3af10c65/lib/layout/breadcrumbs.css' type='text/css' /><script src='/jenkins/adjuncts/3af10c65/lib/layout/breadcrumbs.js' type='text/javascript'></script><div class="top-sticker noedge"><div class="top-sticker-inner"><div id="right-top-nav"></div><ul id="breadcrumbs"><li class="item"><a href="/jenkins/" class="model-link inside">Jenkins</a></li><li href="/jenkins/" class="children"></li></ul><div id="breadcrumb-menu-target"></div></div></div></td></tr></div></div><div id="page-body" class="clear"><div id="side-panel"><div class="task"><a href="http://jenkins-ci.org/" class="task-icon-link"><img src="/jenkins/static/3af10c65/images/24x24/next.png" style="width: 24px; height: 24px; width: 24px; height: 24px; margin: 2px;" class="icon-next icon-md" /></a> <a href="http://jenkins-ci.org/" class="task-link">Jenkins project</a></div><div class="task"><a href="http://issues.jenkins-ci.org/" class="task-icon-link"><img src="/jenkins/static/3af10c65/images/24x24/gear2.png" style="width: 24px; height: 24px; width: 24px; height: 24px; margin: 2px;" class="icon-gear2 icon-md" /></a> <a href="http://issues.jenkins-ci.org/" class="task-link">Bug tracker</a></div><div class="task"><a href="http://jenkins-ci.org/content/mailing-lists" class="task-icon-link"><img src="/jenkins/static/3af10c65/images/24x24/search.png" style="width: 24px; height: 24px; width: 24px; height: 24px; margin: 2px;" class="icon-search icon-md" /></a> <a href="http://jenkins-ci.org/content/mailing-lists" class="task-link">Mailing Lists</a></div><div class="task"><a href="https://twitter.com/jenkinsci" class="task-icon-link"><img src="/jenkins/static/3af10c65/images/24x24/user.png" style="width: 24px; height: 24px; width: 24px; height: 24px; margin: 2px;" class="icon-user icon-md" /></a> <a href="https://twitter.com/jenkinsci" class="task-link">Twitter: @jenkinsci</a></div></div><div id="main-panel"><a name="skip2content"></a><h1 style="text-align: center"><img src="/jenkins/static/3af10c65/images/rage.png" width="154" height="179" /><span style="font-size:50px"> Oops!</span></h1><div id="error-description"><p>A problem occurred while processing the request.
        Please check <a href="https://issues.jenkins-ci.org/">our bug tracker</a> to see if a similar problem has already been reported.
        If it is already reported, please vote and put a comment on it to let us gauge the impact of the problem.
        If you think this is a new issue, please file a new issue.
        When you file an issue, make sure to add the entire stack trace, along with the version of Jenkins and relevant plugins.
        <a href="http://jenkins-ci.org/content/mailing-lists">The users list</a> might be also useful in understanding what has happened.</p><h2>Stack trace</h2><pre style="margin:2em; clear:both">java.lang.NullPointerException
    at io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeGraphVisitor.getPipelineNodeSteps(PipelineNodeGraphVisitor.java:274)
    at io.jenkins.blueocean.rest.impl.pipeline.PipelineStepContainerImpl.iterator(PipelineStepContainerImpl.java:43)
    at io.jenkins.blueocean.rest.model.Container.iterator(Container.java:39)
    at io.jenkins.blueocean.rest.pageable.PagedResponse$Processor$1.generateResponse(PagedResponse.java:57)
    at org.kohsuke.stapler.HttpResponseRenderer$Default.handleHttpResponse(HttpResponseRenderer.java:124)
    at org.kohsuke.stapler.HttpResponseRenderer$Default.generateResponse(HttpResponseRenderer.java:69)
    at org.kohsuke.stapler.Function.renderResponse(Function.java:119)
    at org.kohsuke.stapler.Function.bindAndInvokeAndServeResponse(Function.java:102)
    at org.kohsuke.stapler.IndexDispatcher.dispatch(IndexDispatcher.java:26)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$3.doDispatch(MetaClass.java:196)
    at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$11.dispatch(MetaClass.java:380)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$3.doDispatch(MetaClass.java:196)
    at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$11.dispatch(MetaClass.java:380)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$3.doDispatch(MetaClass.java:196)
    at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$11.dispatch(MetaClass.java:380)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$3.doDispatch(MetaClass.java:196)
    at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$11.dispatch(MetaClass.java:380)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$3.doDispatch(MetaClass.java:196)
    at org.kohsuke.stapler.NameBasedDispatcher.dispatch(NameBasedDispatcher.java:58)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$11.dispatch(MetaClass.java:380)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$11.dispatch(MetaClass.java:380)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$11.dispatch(MetaClass.java:380)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:686)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.MetaClass$11.dispatch(MetaClass.java:380)
    at org.kohsuke.stapler.Stapler.tryInvoke(Stapler.java:746)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:876)
    at org.kohsuke.stapler.Stapler.invoke(Stapler.java:649)
    at org.kohsuke.stapler.Stapler.service(Stapler.java:238)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
    at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:812)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1669)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:135)
    at org.jenkinsci.plugins.ssegateway.Endpoint$SSEListenChannelFilter.doFilter(Endpoint.java:206)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:132)
    at jenkins.metrics.impl.MetricsFilter.doFilter(MetricsFilter.java:117)
    at hudson.util.PluginServletFilter$1.doFilter(PluginServletFilter.java:132)
    at hudson.util.PluginServletFilter.doFilter(PluginServletFilter.java:126)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
    at hudson.security.csrf.CrumbFilter.doFilter(CrumbFilter.java:86)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:84)
    at hudson.security.UnwrapSecurityExceptionFilter.doFilter(UnwrapSecurityExceptionFilter.java:51)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
    at jenkins.security.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:117)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
    at org.acegisecurity.providers.anonymous.AnonymousProcessingFilter.doFilter(AnonymousProcessingFilter.java:125)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
    at org.acegisecurity.ui.rememberme.RememberMeProcessingFilter.doFilter(RememberMeProcessingFilter.java:142)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
    at org.acegisecurity.ui.AbstractProcessingFilter.doFilter(AbstractProcessingFilter.java:271)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
    at jenkins.security.BasicHeaderProcessor.doFilter(BasicHeaderProcessor.java:93)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
    at org.acegisecurity.context.HttpSessionContextIntegrationFilter.doFilter(HttpSessionContextIntegrationFilter.java:249)
    at hudson.security.HttpSessionContextIntegrationFilter2.doFilter(HttpSessionContextIntegrationFilter2.java:67)
    at hudson.security.ChainedServletFilter$1.doFilter(ChainedServletFilter.java:87)
    at hudson.security.ChainedServletFilter.doFilter(ChainedServletFilter.java:76)
    at hudson.security.HudsonFilter.doFilter(HudsonFilter.java:171)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
    at org.kohsuke.stapler.compression.CompressionFilter.doFilter(CompressionFilter.java:49)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
    at hudson.util.CharacterEncodingFilter.doFilter(CharacterEncodingFilter.java:82)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
    at org.kohsuke.stapler.DiagnosticThreadNameFilter.doFilter(DiagnosticThreadNameFilter.java:30)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:585)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:553)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:223)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1127)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
    at org.eclipse.jetty.server.handler.ContextHandlerCollection.handle(ContextHandlerCollection.java:215)
    at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:110)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97)
    at org.eclipse.jetty.server.Server.handle(Server.java:499)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:311)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257)
    at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:544)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555)
    at java.lang.Thread.run(Thread.java:745)
</pre></div></div></div><footer><div class="container-fluid"><div class="row"><div class="col-md-6" id="footer"></div><div class="col-md-18"><span class="page_generated">Page generated: Oct 19, 2016 5:19:31 PM EST</span><span class="rest_api"><a href="api/">REST API</a></span><span class="jenkins_ver"><a href="http://jenkins-ci.org/">Jenkins ver. 2.7.1</a></span></div></div></div></footer></body></html>

so a bit of a 🐛 to look into IMO

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 19, 2016

@michaelneale in this case run.getExecution() is null, code assumed that it will be always null since run object is resolved, but apparently run.getExecution() can be null and null check should have been there. I am going to return empty steps in this case.

Vivek Pandey Vivek Pandey
NPE fix
In some cases WorkfloRun's execution object may be null.
@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

@vivek right no more stack trace, but getting no steps now with https://github.com/kzantow/failure-project project - try it. Look at the passing branch. Even looking at a completed run - there are no steps for the build stage (there should be at least one)

so still a 🐛 for classic stages at least. (and not related to karaoke)

@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

also an additional 🐛 is that a completed branch is not shown as completed, still. According to @svanoort's comment on this: https://issues.jenkins-ci.org/browse/JENKINS-38223 - it should be possible using some hacky code (which hopefully will be replaced) to actually know abranch has completed.

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 20, 2016

@michaelneale

@vivek right no more stack trace, but getting no steps now with https://github.com/kzantow/failure-project project - try it. Look at the passing branch. Even looking at a completed run - there are no steps for the build stage (there should be at least one)

Ack. bug fixed, tests added.

also an additional 🐛 is that a completed branch is not shown as completed, still. According to @svanoort's comment on this: https://issues.jenkins-ci.org/browse/JENKINS-38223 - it should be possible using some hacky code (which hopefully will be replaced) to actually know abranch has completed.

Hmm, with bismuth it should not happen as it doesn't use FlowNode.isRunning(). I tried various parallel jobs and see the completed branch shown correctly. I will ping you this afternoon to get to the bottom of it.

Vivek Pandey Vivek Pandey
@michaelneale

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

🐝 (as long as looks good with ATH)

Edit: ATH is good. Go for it.

@vivek vivek merged commit 2cbc261 into master Oct 21, 2016

1 check passed

continuous-integration/jenkins/branch This commit looks good
Details

@vivek vivek deleted the task/JENKINS-37667 branch Oct 21, 2016

@svanoort

This comment has been minimized.

Copy link
Member

commented Oct 22, 2016

Hmm, with bismuth it should not happen as it doesn't use FlowNode.isRunning(). I tried various parallel jobs and see the completed branch shown correctly. I will ping you this afternoon to get to the bottom of it.

Any chance it's related to JENKINS-38536

@svanoort

This comment has been minimized.

Copy link
Member

commented Oct 22, 2016

Belated 🐝 now that I've come up for air after putting out fires (not literal ones)

@vivek

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 23, 2016

@svanoort It turned out to be a UI issue where it doesn't reflect finished branch accurately. Thanks for this pointer, please drop a note when JENKINS-38536 gets released so that we plan on integrating it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.