Adds support for GitLabAPI V4 while keeping support for V3

[Argelbargel]

This is an enhancement of PR #596 which adds support for gitlab-api v4 while keeping support for v3
(as i forked jenkins/gitlab-plugin i could not find a simple way to add to @ZJvandeWeg 's PR).
GitLabConnectionConfig has a new configuration option "API-Level" in the advanced configuration section where you can switch between v3 and v4. Default is "autodetect" which tries to determine which api-endpoint is available (v3 is preferred for now). The result of the autodetection is cached after the first request to the server.

Additionally, i've tried to change the name of the classes so that the class-names correspond with the names of the getters and variables, thus GitLabApi is now called GitLabClient.

This is just some fancy code to switch the interface-definitions used to create a gitlab-rest-client. i'm not sure if switching between apis will work for existing jobs which do not have both the id and iid of merge-requests stored somewhere - could be that we have to change other parts to build a seamless upgrade path to v4.

[bikebilly]

@Argelbargel that is a very good approach! Can't wait to see it released.

[Argelbargel]

@bikebilly: if you're willing to install the plugin manually, you can try out a development-release from here: https://github.com/Argelbargel/gitlab-plugin/releases/gitlab-v4-SNAPSHOT/

[omehegan]

@Argelbargel this is great, thanks for taking the initiative to extend the original feature PR!

For the record, I understand that GitLab has decided they are not quite ready to deprecate the v3 API, so it will still work at least in the 10.0 release.

[Argelbargel]

That's my understanding too - so we have enough time for a thorough review and perhaps might find some users to try out dev-snapshots in real-world environments.

[bikebilly]

@omehegan API v3 have been deprecated back in GitLab 9.0, but until actual removal they will continue to work as usual.

Removal was discussed to happen in 10.0, but we decided to postpone so users have more time to update to the new version of the plugin (and other reasons unrelated to Jenkins).

Anyone interested can follow the plan at https://gitlab.com/gitlab-org/gitlab-ce/issues/36819.

[Argelbargel]

Has this any chance of being accepted anytime soon? At the current pace of changes in master, the effort of merging and fixing conflicts kinda nears the point where it'll be easier to start over when support for v4 is really required. No offense meant, it's partly my own fault, but nonetheless...

[bikebilly]

@Argelbargel @omehegan if you don't have any strong reasons for not merging it, please do as soon as possible. We decided to postpone removal of API v3 mainly because of this plugin, and if you wait to merge since API are still available in the next version, we are in a deadlock. We have strong pressure on removing API v3, so I kindly ask you if we can move forward here. Please let us know if we can be of any help in the process. Thanks.

[mreichel]

The pr looks pretty good to me. @Argelbargel could you look into the merge conflicts?

[Argelbargel]

@mreichel: i did. that was the cause for my comment - as both mine and the current merges in master heavily change/rename the same classes i got a litte frustrated ;-)

I'll look further into it, it seems many of the conflicts occur because of changes in intendation/import ordering and such funny little things... is there a ruleset for formating for this project which contributors can use?

[Alexxtheonly]

@Argelbargel Here are some screenshots:

Since the gitlab update to 10.x the builds don't show up anymore, thought the gitlab-plugin/1.4.9-SNAPSHOT/gitlab-plugin-1.4.9-20170921.213040-1.hpi might fix that. That's how I found this pull request.

(The yellow minion is our jenkins user)

Jenkins builds did never show up in 'jobs', so i guess it's ok.

Unfortunately I can not post a screenshot with working gitlab-plugin, because downgrading gitlab broke something i can't remember anymore(sorry).

That's all i can provide for now, at least till tomorrow.

[Argelbargel]

Okay, just to get this right: the builds do not show up with the currently released version too? So while a problem it's not a regression caused by the changes in this pull-request?

[Alexxtheonly]

Yes, since we updated gitlab to 10.x we do not see any builds. I thought the reason for this was the removed support for APIv3.

[bikebilly]

@Alexxtheonly in GitLab 10.x we still support APIv3 (even if deprecated in favor of APIv4), so that is unlikely to be the problem: https://about.gitlab.com/2017/09/22/gitlab-10-0-released/#api-v3.

We removed CI API v1 (used by GitLab Runner < 9.0), but I don't think the plugin is relying on that.

[Argelbargel]

@bikebilly: no, as far as i know, the plugin does not use the CI API.
@omehegan: if the problem reported by @Alexxtheonly is not a regression caused by the changes made in this PR, i propose that we merge this quite soon and build at least another snaphot the get more feedback and unblock the other PRs waiting on this.
@Alexxtheonly: i think you should open a separate issue for your problem as it is not related or at least not caused by the changes made in this PR. If you could perhaps add a simple project setup with which to reproduce your problem, that would be a great help. Perhaps add the versions of the plugins you're using, too. If your problem is not caused by your gitlab-instance (did you check the gitlab-logs for errors?), i think this could be caused by some underlying library like jackson-xml or so.

[omehegan]

@Argelbargel can you squash your changes? I will release another snapshot shortly.

[omehegan]

http://repo.jenkins-ci.org/snapshots/org/jenkins-ci/plugins/gitlab-plugin/1.4.9-SNAPSHOT/gitlab-plugin-1.4.9-20171018.215942-2.hpi has been released with the latest changes in this branch. Please test!

[Alexxtheonly]

Did some further digging, our issues have nothing to do with this pull request. I'm sorry for any inconvenience caused.

The issue for us is, when configuring 'Gitlab host URL' it fails when https:// is used. But works with http://.
Log:
I/O exception (java.net.SocketException) caught when connecting to {s}->https://gitlab.accenon.de:443: Connection reset

Currently I don't know what exactly is causing this, but quick searches suggest it is related to our certificate (not self signed).

With http:// the connection check is successful, however, gitlab reports 301 as status.

Got response from GitLab:
URL: http://domain.com/api/v3/projects/someProject/statuses/fe779b3782d0d7d3891c4e6b4d2e110756e0c7a7
Status: 301 Moved Permanently

Again sorry for any inconvenience caused. I will come back to report my test results regarding this pull request when I've resolved the other issue.

[Argelbargel]

@omehegan: will do - or couldn't you just use the "squash and merge option" when merging this into master (click on the down-arrow right to "Merge pull request")?

Should i leave the travis-scripts in here or will you update the jenkins build to execute the integration-tests?

[Alexxtheonly]

With help from @Argelbargel and a lot of trial and error, jenkins and gitlab are obeying my commands again :)

My tests in regards to this PR were all successful. I can confirm it works without any issues.
v3, v4 and autodetect.

[omehegan]

@Argelbargel oh I've never tried to squash-merge in GitHub, but OK! I can get Jenkins to run the additional tests (I think), let me give that a try.

@christ66 @mreichel would either of you like to give this PR a last check before we merge it?

[Argelbargel]

@omehegan: that would be great. Don't think running the tests against Gitlab 8.x is that important but integrating against 9.x and upwards would be good to catch the differences to come...

[mreichel]

The last changes mainly affect the build process. I can't run the integration test profile on OSX or Linux because the Docker plugin is giving me some trouble. But I don't think that is a show stopper.

[Argelbargel]

Hi guys, any feedback on this? There is a really long weekend coming up here in germany, so I'll have time to put some work into this if required.

[omehegan]

@Argelbargel it sounds like @mreichel is happy, I didn't realize that the only real changes since her review and @christ66's were with the Travis build stuff. So I think we should go ahead and merge, and release this as a one-change release. I will do that this week, and we should ask GitLab to encourage their users to upgrade and test after we do that, just because they have the most frequent direct contact with users of this plugin.

[omehegan]

OK, plugin version 1.5.0 has been released with only this change. It should be available in the update center within a few hours. Thanks everyone for your help getting this feature completed! If you discover any potential problems, please open new issues for them.

[bikebilly]

Thanks everyone for your great job! We'll give the good news to our customers so they can upgrade to the new version.

Is APIv4 the default, or do you need to force them in the configuration? I'm wondering if upgrading the plugin is enough to test APIv4 calls, as we still support both versions.

[Argelbargel]

@bikebilly : the default setting is "autodetect" which uses v3 when available and switches to v4 otherwise. The result of the autodetection is cached so it might be neccessary to restart jenkins after an upgrade to gitlab 11. Or we could easily change the order in which the apis are detected.
If users want to enforce the usage of v4 they can do so by setting the api-level via the advanced gitlab-configuration-options.

I'll provide an implementaiton of the autodetection which will not require a jenkins restart as soon as i can get my hands on a docker-image for gitlab 11 from https://github.com/sameersbn/docker-gitlab/releases - for now i chose this, admittedly a bit cumbersome variant, as it has the least possibility to break older setups.

[bikebilly]

Oh I didn't see it coming early, sorry. That could be a problem as all the users that upgrade to the new plugin version will not test the APIv4 implementation (as we still support APIv3). This will not allow "involuntary" testing, but maybe it is a safer option.

What about this plan:

• we'll ask some customers to manually set APIv4 usage, and have some tests for a while
• if no problems are detected, you can change the plugin to make APIv4 the default option when autodetect
• APIv4 will be used by all users as they upgrade to this new version
• if no problems are spotted, we can release GitLab 11.0 without APIv3 support

It seems to me the best approach to avoid problems, and we actually don't already know when GitLab 11.0 will be out. Also, it is not strictly related to APIv3 removal, that can happen even later if it is really a necessity (but hopefully it will not be the case).

[stefanandres]

I'm waiting to test this. I've upgraded the plugin to 1.5.0 already, how can I force it to use v4 in a multibranch pipeline job using the Jenkinsfile?

[Argelbargel]

@bikebilly: good plan. and i'll try to re-implement the autodetection without requiring a restart right away ;-)

@stefanandres: you have to select it in the advanced-section for the gitlab-connection configuration in the system-wide-configuration

As long as you're not creating the gitlab-connection in your Jenkinsfile, the global settings should have effect for all jobs in the jenkins-instance

[stefanandres]

Ah thanks, in the general settings.

I can confirm that the v4 works as good as v3.

Though personally I still have the bug the runtime-status changes are not sent to gitlab. (that is something that worked with 1.3.0 or so and I've changed nothing in the configuration so far, as far as I know)

(Jenkinsfile: https://gist.github.com/stefanandres/7468a46b02267736a0cbb39c0622d356)
(log https://gist.github.com/stefanandres/c5f300463986a87232ec225c50cdb5f6)

[mattiasb]

@stefanandres You need to wrap your code inside a gitlabBuilds step. Something like:

gitlabBuilds(builds: ['build']) {
    gitlabCommitStatus('build') {
      stage name: 'Pre-Cleanup'
      deleteDir()
      […]
      }
    }
}

This is likely considered off-topic for this PR though so you should probably open a separate issue or ask somewhere else (no idea where).

[bikebilly]

@bikebilly: good plan. and i'll try to re-implement the autodetection without requiring a restart right away ;-)

@Argelbargel sounds very good to me. I already asked if we can select some customers for the first phase of the test for APIv4 support.

[Argelbargel]

@bikebilly: i've changed the order in which the apis are detected and improved the autodetection. See #650 ;-)