[WIP] Add Jenkins CI integration

Review Request #9507 - Created Jan. 27, 2018 and updated

James Shephard
rbintegrations
master
ee3da30...
rbintegrations, students

Jenkins is a continuous integration environment, widely used in industry
for running automated test suites, performing deployments, as well as
many other use cases.

This change adds a Jenkins integration to Review Board. This will enable
users to add this integration and have new review requests trigger
builds on a Jenkins server.

This additionally features a Jenkins plugin. This plugin features two
build steps. The first build step, to occur prior to the main job's
build steps, applies the review request's diff using rbt patch. The
second, post-build step notifies review board of the build's status.

There is still additional work to be done, such as completion of unit
tests for rbintegrations and creating unit tests for rbjenkins.

Some testing has been done. All testing has been using a git repository,
so I still need to test other repositories to see how they will function.
This is mostly a POC for triggering Jenkins builds. Future revisions will
feature unit tests.

  • 0
  • 0
  • 20
  • 0
  • 20
Description From Last Updated
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

André Klitzing
  1. 
      
    1. Thanks for the feedback on this, André! This project's being developed by one of our UCOSP students this semester. He's quickly getting up to speed on not just this project but on Review Board's code and various use cases as well. I'm sure he'll benefit greatly from your feedback and from how you're making use of Review Board are your company.

      James, André's a user of Review Board who has provided us many patches to enhance Review Bot and other parts of the project. His feedback will hopefully be useful to you!

    2. Thanks! I'm happy to be a beta tester for this feature. 😄
      I will try it later. I found some things for improvements. But I will wait a little bit until the major parts are added... :-)
      By the way... I saw 'git only at the moment' - just ask me if you need help for Mercurial.

    3. Great! Thanks for helping test this! I'll let you know once it's in a more usable state, and then give you some instructions for configuring it. I'm not certain that the 'git only' restriction still applies, I'll have to do some testing with other SCMs first.

    4. Hi André, I feel like this latest revision is decent enough shape to be tested. Let me know if you still are wanting to try it out, I can write up some instructions for you on how to set it up.

    5. Hi James, yeah, thanks! Little introduction would be nice. :-)

  2. rbintegrations/jenkinsci/api.py (Diff revision 1)
     
     
     
     
     

    would be nice if the jobname could use some variables that will be replaced.

    Example:

    jobname = '{branch}_review_build'
    

    So it would call default_review_build job for default branch.

    1. If I understand you correctly, this would force users to have jobs with specific names. Is this something we want? Do we want to automatically create these jobs?

    2. If the job name is configurable in the integration's configuration page, then we could support having variables in it that would be replaced. This would be an optional feature, and if the job name is simply "review_build", it wouldn't need a job per branch.

    3. Oh I see! Yeah that'll be a nice feature to have, I'll add this in.

    4. Yes, thanks for explanation! I mean it totally optional!

    5. The latest revision includes two job variables, {repository} and {branch}. Let me know if there is anything else you feel could be added!

  3. rbintegrations/jenkinsci/integration.py (Diff revision 1)
     
     
     
     
     

    is it possible to provide the parent diff also?

    1. Yup that is a possibility. I guess the idea is to handle reviews that have parent reviews which have not yet been submitted?

    2. When a diff is uploaded to a review request, it may have a parent diff associated with it. This is useful if you have this branch structure:

       o [my-branch]
       |
       o [parent-branch]
       |
       | [origin/master]
      ...
      

      and you want to post just the commit at my-branch for review. In this example, parent-branch is local, so Review Board can't get access to it from the repository. So, RBTools posts the commit at my-branch as the diff to review, and uses the one from origin/master..parent-branch as the parent diff.

      In order to then test against this, Jenkins will need that parent diff informatino, if associated with the review request. It would then apply the parent diff first, then apply the normal diff on top of it.

    3. I see, thanks for the info! Do the existing CI integrations handle this use case as well? I don't see any mention of parent diffs in their code, they seem to just fetch and use the latest diffset.

    4. The CircleCI integration uses rbt patch to apply the patch. rbt patch --diff-revision <revision> <review_request> will apply both the parent diff and the diff itself.

    5. 👍 Again... thanks for explanation!

    6. Thanks Barret for pointing me in the right direction! The next revision will use rbt patch rather than the current mechanism of applying a patch.

    7. By the way.... hopefully this is an optional task. We need our own command to patch on different parts. :-)
      That's why jenkins-reviewbot of VMWare added that: https://github.com/vmware/jenkins-reviewbot/commit/1c5497ee8c59b98b010f4b2c789855c0ab4b197d

    8. Currently this patching is done via a build step that you would have to add to your job. Thus it is up to the end user to choose whether or not to use it. More advanced users could choose not to use it and instead develop their own method of patching their files, using the Review Board web API to fetch the diff.

  4. 
      
James Shephard
James Shephard
Review request changed

Change Summary:

This change features the notification part of the Jenkins plugin. Now
this has a complete end-to-end workflow. New review requests will
trigger builds on Jenkins, the builds will apply the patch for that
given review request, and then Jenkins will notify RB of the success
or failure of the build.

Description:

~  

Preliminary work on Jenkins integration. This is enough to have the

~   integration configurable on Review Board, as well as enough to trigger a
~   remote Jenkins build. This doesn't support notifications yet (so builds
~   are always in progress), nor does it have any tests. The code is based
~   off the travis-ci integration and shares a good chunk of it.

  ~

Jenkins is a continuous integration environment, widely used in industry

  ~ for running automated test suites, performing deployments, as well as
  ~ many other use cases.

  ~
  ~

This change adds a Jenkins integration to Review Board. This will enable

  + users to add this integration and have new review requests trigger
  + builds on a Jenkins server.

  +
  +

This additionally features a Jenkins plugin. This plugin features two

  + build steps. The first build step, to occur prior to the main job's
  + build steps, applies the review request's diff using rbt patch. The
  + second, post-build step notifies review board of the build's status.

  +
  +

There is still additional work to be done. Notably, there are no unit

  + tests. There was also the request of having variable in job names (i.e.
  + {branch}), which still needs to be added.

Testing Done:

~  

Minimal testing has been done. This is mostly a POC for triggering Jenkins builds.

  ~

Some testing has been done. All testing has been using a git repository,

  + so I still need to test other repositories to see how they will function.
  + This is mostly a POC for triggering Jenkins builds. Future revisions will
  + feature unit tests.

Commit:

-f5c992e3ec9130330cc6812ae6cbdb0891708681
+3d153125b6425c1e56b0251fabec84b1a41d6e03

Diff:

Revision 3 (+968 -1)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

Barret Rennie
  1. 
      
  2. You are missing a lot of doc comments on methods.

    1. Thanks for the comments. I'll address the doc comments in the next revision.

  3. I don't knwo that we have a style guide for Java, but 8 spaces as a continutation indent seems excessive.

    1. Ah, this is the default indentation for function parameters on a new line in intellij. I'll fix up the styling in the next revision so that it better follows the python style guide.

  4. If reviewBoardURL is a java.net.URL then we can do:

    URL url = new URL(
        reviewBoardURL,
        String.format("/api/review-requests/%s/status-updates/%s/", reviewId, statusId));
    
  5. Comments should be full sentences and end in periods.

  6. I think this is inferrable from the SC_OK bit.

  7. rbintegrations/util/urlrequest.py (Diff revision 3)
     
     

    Do we want to just use reviewboard.hostingsvcs.service.URLRequest here? We are duplicating logic across projects.

    This is something we should address with Christian and David.

    1. Yeah that'd be better, it's exactly the same code. I'll ask them what their thoughts are.

  8. 
      
James Shephard
James Shephard
Review request changed

Change Summary:

  • Added in job name variables. So far this includes {repository} and
    {branch}.
  • Begun work on unit tests in the rbintegrations codebase.

Description:

   

Jenkins is a continuous integration environment, widely used in industry

    for running automated test suites, performing deployments, as well as
    many other use cases.

   
   

This change adds a Jenkins integration to Review Board. This will enable

    users to add this integration and have new review requests trigger
    builds on a Jenkins server.

   
   

This additionally features a Jenkins plugin. This plugin features two

    build steps. The first build step, to occur prior to the main job's
    build steps, applies the review request's diff using rbt patch. The
    second, post-build step notifies review board of the build's status.

   
~  

There is still additional work to be done. Notably, there are no unit

~   tests. There was also the request of having variable in job names (i.e.
  ~

There is still additional work to be done, such as completion of unit

  ~ tests for rbintegrations and creating unit tests for rbjenkins.

-   {branch}), which still needs to be added.

Commit:

-e1506158ca349f7196591b54caea41af39103619
+ee3da305bffd1c2a2beaa59069a64983324c35c9

Diff:

Revision 5 (+1417 -38)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...