Opened 3 years ago

Closed 2 years ago

#20564 closed enhancement (fixed)

Add Jenkins configuration for running metrics-lib's unit tests and producing a .jar file

Reported by: karsten Owned by: hiro
Priority: Medium Milestone:
Component: Metrics Version:
Severity: Normal Keywords:
Cc: hiro, iwakeh Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We should start using Jenkins for metrics builds, and ideally we should start with something simple. Let's have it run CollecTor's unit tests as a start. Step two would be to run Checkstyle or Cobertura, but those require .jar files that are not in Debian Stable. So, step one for now.

weasel suggests these two Git repositories:

git clone git@git-rw.torproject.org:project/jenkins/tools
git clone git@git-rw.torproject.org:project/jenkins/jobs

In particular, tools/slaves/linux/tb-manual and the corresponding settings in the jobs repository might serve as template for us.

The CollecTor repository is here:

https://gitweb.torproject.org/collector.git/

Build requirements can be derived from the .jar files listed here:

https://gitweb.torproject.org/collector.git/tree/build.xml

Just came to mind: we might have to package metrics-lib first, because Jenkins jobs cannot download anything from the network.

Child Tickets

Attachments (4)

metrics-lib-job.patch (1.0 KB) - added by hiro 3 years ago.
Add job for metrics-lib
jenkins-metrics-lib.patch (1.2 KB) - added by hiro 3 years ago.
Prepare metrics lib for building on jenkins
metrics-lib-build.patch (3.2 KB) - added by hiro 3 years ago.
Add build for metrics-lib
revert-commit.patch (1.2 KB) - added by hiro 3 years ago.
revert-patch

Download all attachments as: .zip

Change History (42)

comment:1 in reply to:  description ; Changed 3 years ago by karsten

Summary: Add Jenkins configuration for running CollecTor's unit testsAdd Jenkins configuration for running metrics-lib's unit tests and producing a .jar file

Replying to karsten:

Just came to mind: we might have to package metrics-lib first, because Jenkins jobs cannot download anything from the network.

weasel rightly suggests that we could have a separate job to build metrics-lib's .jar file and then share artifacts with the CollecTor job.

Maybe we should start with a Jenkins configuration for building metrics-lib then. The metrics-lib repository is here:

https://gitweb.torproject.org/metrics-lib.git/

comment:2 in reply to:  1 ; Changed 3 years ago by iwakeh

Replying to karsten:

Replying to karsten:

Just came to mind: we might have to package metrics-lib first, because Jenkins jobs cannot download anything from the network.

weasel rightly suggests that we could have a separate job to build metrics-lib's .jar file and then share artifacts with the CollecTor job.

The CollecTor build depends on an official release version of metrics-lib, not simply 'HEAD' of the master branch.

Four options come to mind:

  1. If there is no other (allowed) way to include an official metrics-lib jar, it could be part of CollecTor's git, after all descriptor-1.5.0.jar has only 189kB. This has the advantage that the jenkins job does exactly what is done during development and release.
  2. With building metrics-lib from the release tag (which could be done as suggested in a separate jenkins job or as part of the CollecTor job) we'd have to update the release tag version of the jenkins build when CollecTor switches to a new metrics-lib version.
  3. Create a metrics-lib jenkins job that builds all versions tagged as release (and automatically builds new release tags too) and provides these artifacts for all other Metrics' java builds (CollecTor, Onionoo, etc).
  4. Well, we could introduce the metrics-lib sub-module again into the CollecTor project, but always set to the release tag instead of HEAD. This seems not that optimal to introduce something like this into the development setup; I think.

Are there more options? Which should we use?

comment:3 in reply to:  2 ; Changed 3 years ago by karsten

Replying to iwakeh:

Replying to karsten:

Replying to karsten:

Just came to mind: we might have to package metrics-lib first, because Jenkins jobs cannot download anything from the network.

weasel rightly suggests that we could have a separate job to build metrics-lib's .jar file and then share artifacts with the CollecTor job.

The CollecTor build depends on an official release version of metrics-lib, not simply 'HEAD' of the master branch.

Four options come to mind:

Great list! I could imagine us doing either of the approaches above, with only mild preferences for some of them. Let's go through them:

  1. If there is no other (allowed) way to include an official metrics-lib jar, it could be part of CollecTor's git, after all descriptor-1.5.0.jar has only 189kB. This has the advantage that the jenkins job does exactly what is done during development and release.

My only concern is that this doesn't scale well in terms of the Git repository growing and growing over time. But it could be okay.

  1. With building metrics-lib from the release tag (which could be done as suggested in a separate jenkins job or as part of the CollecTor job) we'd have to update the release tag version of the jenkins build when CollecTor switches to a new metrics-lib version.

Right, this seems like it could produce extra work for the Jenkins admins every time we put out a new metrics-lib release. We might want to avoid this.

  1. Create a metrics-lib jenkins job that builds all versions tagged as release (and automatically builds new release tags too) and provides these artifacts for all other Metrics' java builds (CollecTor, Onionoo, etc).

We might even create a job that builds the last three versions of metrics-lib and all versions released in the past 12 months. Or we just start with all versions and remember these idea in case the approach doesn't scale anymore.

  1. Well, we could introduce the metrics-lib sub-module again into the CollecTor project, but always set to the release tag instead of HEAD. This seems not that optimal to introduce something like this into the development setup; I think.

Not too bad. We should first check whether fetching a Git submodule counts as network access or not.

Are there more options? Which should we use?

My preferred approaches are: 3, 1, 4, 2.

comment:4 in reply to:  3 Changed 3 years ago by iwakeh

Replying to karsten:

...

My preferred approaches are: 3, 1, 4, 2.

My list of preferences is 1, 3. Numbers two and four I would rule out. Number two for obvious administration overhead and number four would cause trouble when developing: dependencies for the sub-module have to be provided and it is awkward to refer to the sub-module and depend on a release at the same time.

The addition of descriptor-<version>.jar as in suggestion 1. shouldn't cause any problems, I think. After all linux kernel development adds way bigger and more often changing binaries to their git repos without problems. Solution 1. would require the least effort and actually be very convenient for development.

Solution 3. (building all metrics-lib releases in a separate jenkins job) is also fine, but requires some more effort than adding a file to the git repo. There shouldn't be a problem with scaling as the job can be configured to only keep the few metrics-lib jars and erase all other files.

  1. A fifth option is (given that tor-git access is fine) to have a metrics-build-resources git repo that contains all non-debian dependencies. Then we also run metrics ant tasks in jenkins, that depend on more modern libraries (e.g. checkstyle). And, the metrics-lib release only needs to be added to this repo and not to all other java projects, too.

We could start with solution 1. (without checkstyle etc.) or 5. (incl. all code metrics) as these require the least effort. If necessary, 1. could be replaced 3. when all jenkins jobs are in place for Metrics' products, but that wouldn't address the reason for 5.

comment:5 Changed 3 years ago by hiro

Regarding solution 1. We might include metrics-lib as a gitmodule in CollecTor. This way the git repository will not grow over time, while at the same time we ensure sync with releases of the metrics-lib.

comment:6 Changed 3 years ago by karsten

weasel, hiro, and I talked a bit about this in #tor-dev. The short summary is that there are various issues with the solutions above.

What we could do is always build the latest release of metrics-lib, or even HEAD, and use that in code bases like CollecTor. And if that breaks the build, because CollecTor doesn't work with the latest release or commit of metrics-lib, we'll learn that we need to fix that in the near future. After all, it's not that Jenkins builds packages for production, it's there to spot problems. I'm inclined to just try this and revisit this discussion in case it doesn't work for us at all.

See also the tor-debian-release-source job for comparison.

comment:7 Changed 3 years ago by karsten

weasel suggests to add a second branch to metrics-lib, say, release, create two jobs metrics-lib-master and metrics-lib-release, and test applications against both. Sounds good to me!

comment:8 in reply to:  7 Changed 3 years ago by iwakeh

Replying to karsten:

weasel suggests to add a second branch to metrics-lib, say, release, create two jobs metrics-lib-master and metrics-lib-release, and test applications against both. Sounds good to me!

That's really the best solution for supplying metrics-lib with the following assumptions:

  • check-ins on the respective branches trigger the jenkins jobs, i.e. the metrics-lib-release-job is only triggered when a new check-in occurs on this branch and after that the descriptor-<version>.jar is available for all other Metrics' jenkins jobs
  • The collector jenkins job on HEAD/master is triggered on check-in (in collector.git) and each time one of the two metrics-lib jenkins jobs ran (successfully).
  • Analogously with onionoo etc.

It doesn't solve the non-debian stable library topic. But, we can just start running the ant tasks in jenkins that don't require additional libraries.

comment:9 Changed 3 years ago by karsten

Great, we have a solution!

I just pushed the commit that is tagged as descriptor-1.5.0 to a new release branch in the official metrics-lib repository: https://gitweb.torproject.org/metrics-lib.git/. I'm also going to push again to that branch after tagging the next release.

hiro, would you want to look into creating Jenkins jobs metrics-lib-master and metrics-lib-release for building those two branches?

comment:10 Changed 3 years ago by hiro

Sure, missed this update. Was working on building the jobs already. One question I have is regarding necessary jars that are needed to build and run the test task. Some of these are included in debian stable, but not w/ the same version needed to build metrics-lib or collector. Shall these be included manually?

Last edited 3 years ago by hiro (previous) (diff)

comment:11 in reply to:  10 Changed 3 years ago by iwakeh

Replying to hiro:

Sure, missed this update. Was working on building the jobs already.

Thanks for taking this task! I'll set it on assigned.

One question I have is regarding necessary jars that are needed to build and run the test task. Some of these are included in debian stable, but not w/ the same version needed to build metrics-lib or collector.

I thought the versions of all jars used for compile and test were available in debian jessie.
Could you post the list of different versions on the test system? So, I can check the build and alter the dependencies if necessary.

Shall these be included manually?

Is there a way of adding non-debian-stable jars to builds?
I thought that option was ruled out for administrative reasons.

comment:12 Changed 3 years ago by iwakeh

Owner: set to hiro
Status: newassigned

comment:13 Changed 3 years ago by hiro

Thanks iwakeh.
One example that I found is: https://packages.debian.org/jessie/checkstyle
We require v6.17 that isn't available on jessie. I will make a complete list and update this ticket.

comment:14 Changed 3 years ago by iwakeh

The build.xml file has different paths for different tasks.

Currently, we're excluding checkstyle (cf. description) as this is indeed not in debian jessie.

The path for compile and test tasks are classpath and test.classpath.
The cobertura.classpath ought to be fine, but I'm not 100% sure.

It would be nice to have the results of the following tasks available after a run:

  • ant test
  • ant coverage
  • ant docs

I assume the Jenkins build slave environment is empty before a build. If not, the task ant clean needs to run first.

comment:15 Changed 3 years ago by hiro

I have verified we can build metrics-lib without any problem. The only issue is the version of checkstyle. I have downgraded to the version present on jessie and, unless there are other reasons to stick with version 6.17, we could use 5.9. I am going to move forward and create jenkins jobs.

comment:16 Changed 3 years ago by iwakeh

Unfortunately, checkstyle 6.17 is necessary for the checks we need. If possible, please use the new checkstyle, otherwise just leave the ant checks target out of the Jenkins build for now.

Thanks for verifying those versions!

comment:17 Changed 3 years ago by hiro

Submitting a patch of the build, the job and the metrics-lib ready for jenkins.

Changed 3 years ago by hiro

Attachment: metrics-lib-job.patch added

Add job for metrics-lib

Changed 3 years ago by hiro

Attachment: jenkins-metrics-lib.patch added

Prepare metrics lib for building on jenkins

comment:18 Changed 3 years ago by iwakeh

@hiro: Thanks for these patches!

From what I can tell just reading the patches I'd suggest some tweaks:

  • slaves/linux/metrics-lib-master/build:
    • Please run ant -Djenkins=true -f build.xml test docs coverage jar
    • checkstyle-5.9 can be omitted (cf. comment:16)

comment:19 in reply to:  18 Changed 3 years ago by hiro

Replying to iwakeh:

@hiro: Thanks for these patches!

From what I can tell just reading the patches I'd suggest some tweaks:

  • slaves/linux/metrics-lib-master/build:
    • Please run ant -Djenkins=true -f build.xml test docs coverage jar

Will do.

Here I thought to leave the latest version in case we want to add some specific checks that can be run on 5.9. But I am happy to remove it if we decide to leave this for now.

comment:20 Changed 3 years ago by hiro

Attaching updated patch for metrics-lib build.

Changed 3 years ago by hiro

Attachment: metrics-lib-build.patch added

Add build for metrics-lib

comment:21 Changed 3 years ago by karsten

One thing I wonder is whether we should use wildcards like commons-compress*.jar when copying files from /usr/share/java to lib/ to avoid updating yet another place when upgrading to new library versions.

Other than that I cannot comment much on the patch. Does it work? :)

comment:22 Changed 3 years ago by hiro

We were wondering with weasel if it is necessary to copy jars over to the project lib folder and we can just link? I went for copy since we provide the file in the archive that we distribute through dist.

Meanwhile I have cleaned up the build script and made it available here:
https://gitweb.torproject.org/user/hiro/jenkins-tools.git/diff/?h=metrics-lib-jobs

The job is also available here:
https://gitweb.torproject.org/user/hiro/jenkins-jobs.git/diff/?h=metrics-lib-jobs

How can we test if the build actually works on Jenkins? I have tested on my local instance but things might be totally different ;)

comment:23 in reply to:  22 Changed 3 years ago by iwakeh

Replying to hiro:

We were wondering with weasel if it is necessary to copy jars over to the project lib folder and we can just link? I went for copy since we provide the file in the archive that we distribute through dist.

Links would be sufficient. Dists are not yet intended to be build on Jenkins and I suppose the ant tar-task could follow links and include these files properly.

Meanwhile I have cleaned up the build script and made it available here:
https://gitweb.torproject.org/user/hiro/jenkins-tools.git/diff/?h=metrics-lib-jobs

The job is also available here:
https://gitweb.torproject.org/user/hiro/jenkins-jobs.git/diff/?h=metrics-lib-jobs

Nice :-)

How can we test if the build actually works on Jenkins? I have tested on my local instance but things might be totally different ;)

weasel will have the answer what further tests are necessary before the changes can be deployed and sufficient access privileges to deploy all.

comment:24 Changed 3 years ago by hiro

Iwakeh: sounds cool. I have updated the pull request to use links instead of copies (https://gitweb.torproject.org/user/hiro/jenkins-tools.git/diff/?h=metrics-lib-jobs). Let me know how we move from here.

Last edited 3 years ago by hiro (previous) (diff)

comment:25 Changed 3 years ago by hiro

Also one more thing that we can consider is to include the system java path in the build.xml file.
Something like:

  <property name="libs" value="/usr/share/java"/>

In which case we might not need the lib folder at all.
The problem I see with this is that we might lose in compatibility with different sys setups. I do not think it might be a big issue, but if we move in this direction we should definitely mention this is the installation instructions.

comment:26 in reply to:  25 Changed 3 years ago by iwakeh

Replying to hiro:

...
The problem I see with this is that we might lose in compatibility with different sys setups. ...

We try to avoid any system dependencies in the build process. After all it's a java project that ought to run on many operation systems. I think the linking solution for the jenkins job is fine.

Could you find out from the admin-team how to get your patches into the main Jenkins setup?
I'm curious to see the very first Metrics' Jenkins job in action.
Thanks!

comment:28 Changed 3 years ago by hiro

The build was added to jenkins but we need to also merge https://trac.torproject.org/projects/tor/attachment/ticket/20564/jenkins-metrics-lib.patch to metrics-lib. This small patch exclude the checks task for jenkins and add the clean task to test dependencies.

comment:29 in reply to:  28 Changed 3 years ago by karsten

Replying to hiro:

The build was added to jenkins but we need to also merge https://trac.torproject.org/projects/tor/attachment/ticket/20564/jenkins-metrics-lib.patch to metrics-lib. This small patch exclude the checks task for jenkins and add the clean task to test dependencies.

Ah, sorry for not noticing that patch earlier. Merged with a trivial whitespace tweak.

comment:30 Changed 3 years ago by hiro

Hi, we have a first build of metrics-lib: https://jenkins.torproject.org/job/metrics-lib-master/
I had to exclude the coverage task for a version mismatch of cobertura on debian stable. I think we can continue and build both onionoo and collector.

comment:31 Changed 3 years ago by iwakeh

@karsten:
I just noticed this commit in metrics-lib, which I think should be reverted.

  1. the addition of unless="jenkins" is unnecessary as the checks task is not depended on and only called directly.
  2. the addition of clean to depends in the test task is not needed and wasn't discussed. I suggested to call ant clean in addition to the various other tasks. So, that change is unnecessary too.


comment:32 Changed 3 years ago by karsten

Oh, okay. I tried out the update build script locally and it didn't break in obvious ways. But I see your points there.

hiro, can you take a look at the comment above and see if you can change the Jenkins scripts accordingly to still work after reverting that commit? I'll hold back reverting until we're sure how to move forward. Thanks!

comment:33 Changed 3 years ago by iwakeh

Status: assignedneeds_information

This tasks is finished according to its description: There is a metrics-lib Jenkins job that tests and runs the jar task. So I'd suggest closing this ticket after karsten's question in comment:32
is answered. Until then it's set to needs_information.

@hiro: Thanks for setting up this very first metrics-lib-master-Jenkins job!

All other Metrics' Jenkins integration plans are on hold now until the next Metrics-in-person meeting. We need to answer quite a few mostly non-technical questions regarding Metrics' Jenkins jobs.
In addition, Jenkins-integration is not part of our current main sponsor and we need to focus on the sponsored tasks.

Changed 3 years ago by hiro

Attachment: revert-commit.patch added

revert-patch

comment:34 Changed 3 years ago by hiro

Attached revert patch.
I have also modified the build job and it is available here: https://gitweb.torproject.org/user/hiro/jenkins-tools.git/commit/?h=metrics-lib-jobs

comment:35 Changed 3 years ago by iwakeh

Thanks!

One little change:
Please also add clean (as this ought to be removed from the dependencies of the test task).
Afaics, the build machine is setup anew for each run, but just in case that changes.

-ant test docs coverage jar
+ant clean test docs coverage jar

So, commit d154982e5a974bf451086241e79f9b3d9f56b2c6 in Metrics-lib can be reverted. Fine!

comment:37 Changed 3 years ago by karsten

Applied revert patch. Thanks!

comment:38 Changed 2 years ago by hiro

Resolution: fixed
Status: needs_informationclosed
Note: See TracTickets for help on using tickets.