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

Added support for the GitLab 8.1 commit API #127

Merged
merged 5 commits into from
Nov 11, 2015
Merged

Added support for the GitLab 8.1 commit API #127

merged 5 commits into from
Nov 11, 2015

Conversation

tomelfring
Copy link
Contributor

Fixes #111 & fixes #122
This adds support for the GitLab 8.1 commit API.
Added an extra option so it can be enabled (disabled by default)

Will add result to all pushrequests & mergerequests using the gitlab api.

Please note: it uses the java-gitlab-api v1.1.10-SNAPSHOT which isn't available (yet) through the official maven repository. See issue: timols/java-gitlab-api#80

@omehegan
Copy link
Member

@thommy101 thanks for doing the work for this! I have a good Jenkins test setup in place, so I would like to do some verification which this change to make sure it is backwards-compatible with older Gitlab versions, etc. We're still running Gitlab 7.14.3 here.

@tomelfring
Copy link
Contributor Author

@omehegan You're welcome. I have only tested it at the latest version of Gitlab, so I don't know if it's fully backwards-compatible. If i'm right everything is still the same, and the commit api is under the newly added checkbox.

@@ -18,6 +18,9 @@
<f:entry title="Add note with build status on merge requests" field="addNoteOnMergeRequest">
<f:checkbox default="true" />
</f:entry>
<f:entry title="Use GitLab CI features (GitLab 8.1 required!)" field="addCiMessage">
Copy link
Member

Choose a reason for hiding this comment

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

Do we also need a getter for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:/ I had one, but needed to create a new branch.. Guess I missed some lines in the progress

Copy link
Member

Choose a reason for hiding this comment

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

Lucky, I barely know Java and only thought to check for this because of your comment in your other PR :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll check to the code tomorrow, and add any missing things which I overlooked.. It's still backward compatability testable, because the code still defaults to false :)

@omehegan
Copy link
Member

@thommy101 can you give an overview of what exactly this PR enables? I understand that Gitlab broke this integration with their 8.0 release, then came up with a new API for us to use in 8.1. I'm just pretty unclear on how the 8.1 solution differs from the 7.x one.

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@tomelfring
Copy link
Contributor Author

I don't know exactly how it worked in Gitlab previous to version 8. Started using Gitlab about three weeks ago.
But I can try to explain what is added with these commitits.

As described in the official blog of Gitlab ( https://about.gitlab.com/2015/10/22/gitlab-8-1-released/ ) there is a better implementation of CI in Gitlab. You can now see the build status of commits / mergerequests in the commit overview. For each checked commit there is a green checkmark or a red cross, for good and bad build respectivly.
This commits add the checkmarks though the API in said overview and in the merge request overview.

@omehegan
Copy link
Member

@thommy101 cool. Are you using this GitLab interface then? http://doc.gitlab.com/ce/api/commits.html#post-the-status-to-commit

@tomelfring
Copy link
Contributor Author

@omehegan This project is making use of the library timols/java-gitlab-api which handles all requests. The version needed is currently not released to the official repo's (hence why the builds are failing).
The library itself makes use of your mentiond gitlab interface.

@omehegan
Copy link
Member

@thommy101 got it, it's making sense now. Thanks! I'll try to do some testing today.

@tomelfring
Copy link
Contributor Author

There are still some little issues i've just found. I'm giving the wrong information back to Gitlab. Working on fixing it ;)

@tomelfring tomelfring changed the title Added support for the GitLab 8.1 commit API [WIP] Added support for the GitLab 8.1 commit API Oct 30, 2015
Also a fix for MergeRequests containing source branch instead of commit
hash
@tomelfring
Copy link
Contributor Author

Fixed it, the correct information is now being send.

@tomelfring tomelfring changed the title [WIP] Added support for the GitLab 8.1 commit API Added support for the GitLab 8.1 commit API Nov 2, 2015
@omehegan
Copy link
Member

omehegan commented Nov 4, 2015

@slide if you would like to help review this, from a general Java and Jenkins-plugin perspective, feel free! I'm new to this :)

@pioto
Copy link

pioto commented Nov 5, 2015

Off-hand... it feels like the "status" should be an enum (since there are only a few valid options), rather than a String. But I guess that's probably more of a suggestion for the GitLab Java library being used.

Could you also add something to the README.md file with details about how this works, should be enabled, etc?

@EmteZogaf
Copy link
Contributor

Version 1.1.11 of the Gitlab Java API Wrapper has been released on Maven Central.

@tomelfring
Copy link
Contributor Author

I've updated the dependency and tested it with my own setup (GitLab 8.1.3) and still works flawless.
I do agree with @pioto, that status should be an enum, but it doesn't feel right to implement it here. It should go with the gitlab library itself.

I would love to add some to the readme, but i'm still getting used to all those merge requests, and I'm afraid something will get messed up though the fact that I haven't got the latest version of the README.md file.

@omehegan
Copy link
Member

omehegan commented Nov 9, 2015

I'm testing this against GitLab 7.x today.

@thommy101 can you add some help text to go with the 'Use GitLab CI features (GitLab 8.1 required!)' UI option?

@omehegan
Copy link
Member

OK, this works in Gitlab 7.14.3 so it seems backwards-compatible. If you can add some help text then I think I'd be happy to merge it.

@tomelfring
Copy link
Contributor Author

I've added some documentation about the new CI feature to the readme.md and to the configuration page itself (see question mark next to the line "Use GitLab CI features (GitLab 8.1 required!)".

omehegan added a commit that referenced this pull request Nov 11, 2015
Added support for the GitLab 8.1 commit API
@omehegan omehegan merged commit 113a759 into jenkinsci:master Nov 11, 2015
public GitlabCommitStatus createCommitStatus(GitlabAPI api, String status, String targetUrl) {
try {
if(objectAttributes.lastCommit!=null) {
return api.createCommitStatus(sourceProject, objectAttributes.getLastCommit().getId(), status, objectAttributes.getLastCommit().getId(), "Jenkins", targetUrl, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the commit status api documentation for the optional ref it says

ref (optional) - The ref (branch or tag) to which the status refers

But here the commit id is assigned to it again.
Shouldn't it be objectAttributes.getSourceBranch()?

Copy link
Member

Choose a reason for hiding this comment

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

@thommy101 can you comment on the above?

Copy link
Member

Choose a reason for hiding this comment

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

@EmteZogaf can you submit a PR for this so it doesn't get lost as just a comment on the source? Thanks!
EDIT: nevermind, you DID open one. Thanks :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@EmteZogaf @omehegan , I'm using the commit id as it's the closest I could find to match the default behavior of GitLab CI itself.
I haven't used the official CI myself (only using Java at the moment). I've looked at the blog post (https://about.gitlab.com/2015/09/22/gitlab-8-0-released/) and they are using the commit id.

@tomelfring
Copy link
Contributor Author

@shochdoerfer Thanks for comfirming! ;)

@cfarence
Copy link

I just installed the plugin and I can't find the option to enable gitlab ci features. I was able to set the host url and api key in global settings.

@tomelfring
Copy link
Contributor Author

@cfarence, The option for gitlab ci features are defined for each job, not globaly. You can find them under configuration when you've the job open.

@vstriz
Copy link

vstriz commented Nov 23, 2015

Is it possible to have manually triggered build job to update Gitlab status? Everything is working properly now with Gitlab 8.1 and this plugin when Merge Request is triggered by push from gitlab. However I sometimes have situations where I need to do a rebuild without changing any code. I've setup the parameters to the job and do "Build with Parameters". The job succeeds but doesn't update the status back on gitlab. Is there anyway to configure parameters such that this happens?

@cfarence
Copy link

@tommy101 that's what I meant to say, I set the auth in global, but can't find the options in the projects configuration.

@omehegan
Copy link
Member

@cfarence they become visible when you click the 'Build when a change is pushed to GitLab' check box under Build Triggers in the job configuration. If you do not see that, I'm guessing you are using an unsupported job type (only Freestyle jobs work fully at the moment).

@tomelfring
Copy link
Contributor Author

I'm currently using maven project, and they also work flawless.

@cfarence
Copy link

I have found the setting, can't believe I didn't look there before :). I committing to the repo and jenkins didn't do anything. I have added the web hook in gitlab and using the test button gitlab says everything went ok. The option "Build when a change is pushed to GitLab" is checked and Build on Push and Merge requests are checked as well. I also enabled use CI features and tried doing a manual build and the commit status was not updated. Not really sure why it isn't working.

Using the latest gitlab, latest jenkins, and latest Gitlab plugin. Its a freestyle project that I'm trying to get working.

@vstriz
Copy link

vstriz commented Nov 23, 2015

@cfarence I also have not been able to get status to update when manually triggering the job in jenkins with "Build Now". I would love to know how to get that to work.

It does however work fine for gtilab initiated builds. I.e. update a branch in gitlab by pushing into it. That should trigger the build job and then update status when build completes. That part is working now very well.

@thatarchguy
Copy link

I configured Gitlab Url and API token in jenkins. I've enabled the new options for the job in Jenkins. I could not get jenkins to post back to Gitlab about builds without enabling CI on my project in Gitlab.

So gitlab is saying "failed" for my merge requests, but if I go to each commit I can see jenkins passed it. Also I can see jenkins posting (as my user) +1 to the merge. Did I configure something wrong? If I disable CI on my gitlab project I do not see any status, just the +1 in the comments. Is this the expected behaviour?

2015-11-23-175403_883x494_scrot

2015-11-23-175422_894x666_scrot

2015-11-23-175552_667x79_scrot

@EmteZogaf
Copy link
Contributor

@cfarence @vstriz If you trigger the build manually from within Jenkins it won't work. The gitlab plugin gets all the needed information from the gitlab web hook which is then missing. But you can manually fire the web hook in gitlab by accessing the project settings go to 'web hooks' an click 'Test Hook' next to the configured web hook. This will pretend a push to the master, i think. So you probably have to confgure the job in Jenkins to rebuild merge requests in updates to the source and master branch, i guess. But I haven't deeply tested it yet as. I always use this option for my Jenkins jobs.

@cfarence
Copy link

@EmteZogaf I've tried testing the web hook in gitlab and it says it successfully sent it, though jenkins doesn't say anything. I have build when a change is pushed to Gitlab and use the url it gives me for the web hook.

@EmteZogaf
Copy link
Contributor

This is my jenkins job configuration:
jenkins-gitlab-conf
And the gitlab webhook:
gitlab-webhook
When I press Test Hook jenkins starts a build for the master branch as well as one build for each open merge request.

@vstriz
Copy link

vstriz commented Nov 24, 2015

@EmteZogaf Well it would be nice to have. Wouldn't it be possible to get enough information with build parameters. I guess this is really a feature request at this point. If you have parametrized inputs of merge-request id and the source-branch-name, I think you should be able to get all information you need about merge request from gitlab-api (including the commit #) to do the build.

@fontealpina
Copy link

@thatarchguy exactly same issue using gitlab 8.1, any news?
Maybe is necessary to configure a .gitlab-ci.yml file?

@sebastienbonami
Copy link
Contributor

On my side the plugin seems to work great, but I get the annoying ".gitlab-ci.yml not found in this commit" in the build section of every commit. Is it normal?

@fontealpina
Copy link

@sebastienbonami I noticed the same behaviour, and in my opinion it is not normal. It should be avoided, but I don't know if it is a bug or some configurations are missing.

@cfarence
Copy link

I think it's Gitlab not recognizing a build came from an external source. It makes sense if there is no builds at all for the commit, but if an external service Jenkins in this case puts a build there, the message should go away.

If I wasn't on my phone right now I would look at Gitlab to see if that is already mentioned, if not create a request for it.

@omehegan
Copy link
Member

@ayufan can you comment on the issues people are having on the GitLab side when their repo does not have a .gitlab-ci.yml file?

@omehegan
Copy link
Member

omehegan commented Dec 1, 2015

@vstriz I think the problem with manual builds not being linked to GitLab is, when you build manually none of this plugin's code is triggered. When GitLab triggers a build, the plugin creates a pointer URL in Jenkins to reference the build, and GitLab requests status using that URL. A manual run never creates this. I don't think it's just a matter of adding appropriate parameters to the job, unfortunately.

@ayufan
Copy link
Contributor

ayufan commented Dec 1, 2015

@sebastienbonami @omehegan Sure. We already go rid of: Undefined yaml error. I'll introduce change that will hide notice about missing YAML file if GitLab Builds are disabled.

Thanks for ping.

@ayufan
Copy link
Contributor

ayufan commented Dec 1, 2015

@razzeee
Copy link

razzeee commented Dec 8, 2015

@vstriz
Copy link

vstriz commented Dec 8, 2015

@omehegan I could be wrong, but I thought the plugin issues REST calls to gitlab (not the other way arround) with commit hash# in the path to set the build status. It should be able to do so when manually triggered by figuring out the hash# of the HEAD for the given branch and then update the gitlab. I'm pretty sure that should be possible.

dosire added a commit to dosire/gitlab-plugin that referenced this pull request Dec 8, 2015
@cfarence
Copy link

Just a heads up, they fixed the YAML file bug in Gitlab 8.3 set to release today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Time estimation for gitlab 8.0 commit api integration GitLab 8.0 Commit Status Broken