Add generic Jenkins files pipelines #2093

vsebe · 2018-06-06T16:07:39Z

add Jenkins file for build and test any platform
add pipeline wrapper for any platform
These Jenkins files allow the OpenJ9 extensions build and test for any
supported version and platform. New platforms can be added easily
without creating new Jenkins files only parameterized Jenkins jobs.
Pipeline-Build-Test-Any-Platform could also replace the all of the
Pipeline-Build-Test-JDK<version>-<platform> wrappers.
Build-Test-Any-Platform could replace all of the
Build-JDK<version>-<platform>, Test-Sanity-JDK<version>-<platform>, Test-Extended-JDK<version>-<platform> and PullRequest-*-JDK<version>-<platform> scripts.

Fixes #1897

[ci skip]

Signed-off-by: Violeta Sebe vsebe@ca.ibm.com

vsebe · 2018-06-06T17:01:35Z

@AdamBrousseau please review

AdamBrousseau · 2018-06-06T18:48:43Z

buildenv/jenkins/common/variables-functions

+    // check mandatory parameters if any
+    try {
+        validate_arguments(ARGS)
+    } catch(MissingPropertyException e){


Will this catch only when ARGS is not defined, or will it also catch the perror(s) we may throw inside the validate_arguments function?

only when ARGS is not defined

@AdamBrousseau - ready for review, thx

AdamBrousseau · 2018-06-06T18:51:04Z

buildenv/jenkins/common/variables-functions

+
+def check_parameter(ARG_NAME) {
+    if (!params.get(ARG_NAME)) {
+        perror(ARG_NAME)


What is the purpose of moving the error out to a new function if we only use it in one place?

AdamBrousseau · 2018-06-06T18:51:57Z

buildenv/jenkins/common/variables-functions

+
+def validate_arguments(ARGS) {
+    for (arg in ARGS) {
+        check_parameter(arg)


Same comment, what is the purpose or value moving this out if we don't use it anywhere else?

AdamBrousseau · 2018-06-06T19:09:13Z

buildenv/jenkins/jobs/builds/Build-Test-Any-Platform

+    } else {
+        if (params.UPSTREAM_JOB_NAME && params.UPSTREAM_JOB_NUMBER) {
+            // fetch resources from upstream job, run tests, archive results, clean up
+            testFile.test_all_with_fetch()


This will not work because JAVA_BIN was set on master when the test file was loaded
https://github.com/eclipse/openj9/pull/2093/files#diff-b19f95b3c3121dd4c7d986c2633a9deaR41

Given #1450 is very close, I think we should not worry about covering both compile and test jobs with this change
We should however, probably still do the PRs in this file, since we haven't ironed out where that story is headed yet.

Added a 2nd checkout for the test script

AdamBrousseau · 2018-06-06T19:14:42Z

buildenv/jenkins/jobs/pipelines/Pipeline-Build-Test-Any-Platform

+ * SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 OR LicenseRef-GPL-2.0 WITH Assembly-exception
+ *******************************************************************************/
+
+ARGS = ['SDK_VERSION', 'PLATFORM', 'TESTS_TARGETS']


The current pipeline jobs don't use an argument for TEST_TARGETS, they get the default from the variables-function file
https://github.com/eclipse/openj9/blob/master/buildenv/jenkins/common/variables-functions#L267

Is this an intended change in beaviour?

on second thought we can dispense of it

pshipton · 2018-06-06T22:11:18Z

@smlambert fyi

smlambert · 2018-06-07T16:11:54Z

It will be good to add more details in the commit message about this PR, what it intends to accomplish / what issue it fixes.

By the look of the title, it seems it may help eliminate duplication in Jenkinsfiles in the repo, is that true? If so, is the plan to remove duplicate files in a later PR?

How does this PR relate to the one linked to it #2097, does it replace the need for it?

AdamBrousseau · 2018-06-07T16:13:07Z

related #2117

vsebe · 2018-06-07T17:51:38Z

@smlambert - yes to both your question.
It is intended to eliminate the duplicated Jenkins files: Pipeline-Build-Test-JDK*, Build-JDK*, Test-Sanity-JDK*, PullRequest-*, etc.

Pipeline-Build-Test-Any-Platform replaces Pipeline-Build-JDK<version>-<spec> files
Build-Test-Any-Platform replaces Build-JDK<version>-<spec>, Test-Sanity-JDK<version>-<spec>, Test-Extended-JDK<version>-<spec>, PullRequest-*-JDK<version>-<spec> files.

We'll eliminate the duplicates in another PR.

PR #2097 adds windows 32 bits platform. There are no Jenkins file for win x86, e.g. Pipeline-Build-JDK9-win_x86, TestBuild-Test-JDK9-win_x86, etc.
To build and test OpenJ9 JDK8 on win x86 32bits we'll create parameterized Jenkins jobs that'll use Pipeline-Build-Test-Any-Platform and Build-Test-Any-Platform scripts.

AdamBrousseau · 2018-06-08T18:07:59Z

buildenv/jenkins/jobs/builds/Build-Test-Any-Platform

+ *******************************************************************************/
+
+ARGS = ['SDK_VERSION', 'PLATFORM']
+


I wonder if we should also wrap all this in a timeout for protection?

There is a timeout already set in the upstream pipeline - Pipeline-Build-Test-All

But this will also be used for PR builds which are not triggered by Pipeline-Build-Test-All

added 10h timeout

AdamBrousseau · 2018-06-08T18:11:41Z

buildenv/jenkins/jobs/builds/Build-Test-Any-Platform

+
+            variableFile = load 'buildenv/jenkins/common/variables-functions'
+
+            if (!params.UPSTREAM_JOB_NAME && !params.UPSTREAM_JOB_NUMBER) {


Should probably add a comment here about what this if means.

comment added

AdamBrousseau · 2018-06-08T18:14:02Z

buildenv/jenkins/common/variables-functions

+        }
+
+        if (arg == 'PLATFORM') {
+            SPEC = params.get(arg)


Should probably add a comment about why we use SPEC and PLATFORM and the difference between a groovy variable and a Jenkins Param and why SPEC conflicts with a compile.

comment added

AdamBrousseau · 2018-06-08T18:28:29Z

buildenv/jenkins/jobs/builds/Build-Test-Any-Platform

+            }
+
+            if (run_tests()) {
+                variableFile.set_job_variables('test')


For PR builds with testing, we will hit set_job_variables('build') then set_job_variables('test') but we should be hitting set_job_variables('pullRequest'). This will technically work as-is because our build/test labels (machines) are currently the same at OpenJ9. But we would essentially be setting up for a build node and then changing to a test node, which is incorrect because we need to compile on a build node. We may need another if here that looks for a param that is passed in a PR build.
https://ci.eclipse.org/openj9/view/Pull%20Requests/job/PullRequest-Sanity-JDK8-win_x86-64_cmprssptrs-OpenJ9/lastSuccessfulBuild/parameters/
ghprbPullId is usually a good one.

added check for PR

AdamBrousseau · 2018-06-08T18:32:13Z

buildenv/jenkins/jobs/builds/Build-Test-Any-Platform

+}
+
+node("${NODE}") {
+    if (!run_tests()) {


We don't have a try/finally with cleanWs() here, presumably because both build_all() and test_all() have that functionality. Is it better to move that functionality here and not duplicate them in the build and test files? It might also allow us to remove build_pr() since that function is designed to not cleanup after a PR compile. That way we could have something more like, if compile; build_all; if test; test_all; done

There are 130 Jenkins files still relying on build and test scripts. timeout, timestamps, cleanWs() should be done here but at this point it only duplicates an existing functionality. The refactoring should take place after we replace all the platform specific Jenkins files with Pipeline-Buld-Test-Any-Platform, Buld-Test-Any-Platform scripts.

AdamBrousseau · 2018-06-08T18:37:43Z

buildenv/jenkins/jobs/pipelines/Pipeline-Build-Test-Any-Platform

+ *******************************************************************************/
+
+ARGS = ['SDK_VERSION', 'PLATFORM']
+


timeout wrapper?

there is a timeout in Pipeline-Build-Test-All

AdamBrousseau · 2018-06-08T18:38:24Z

buildenv/jenkins/jobs/builds/Build-Test-Any-Platform

+ARGS = ['SDK_VERSION', 'PLATFORM']
+
+node('master') {
+    timestamps {


should move timestamps outside of node to show queue time

AdamBrousseau · 2018-06-08T18:38:44Z

buildenv/jenkins/jobs/pipelines/Pipeline-Build-Test-Any-Platform

+ARGS = ['SDK_VERSION', 'PLATFORM']
+
+node('master') {
+    timestamps {


move timestamps outside of node

@AdamBrousseau ready for review, thx

AdamBrousseau · 2018-06-11T19:58:35Z

Looks good now except for my one comment about a timeout in build-test-any

AdamBrousseau · 2018-06-12T17:00:35Z

adam test sanity

AdamBrousseau · 2018-06-12T17:57:07Z

adam test sanity depends eclipse/omr#master

AdamBrousseau · 2018-06-12T18:48:15Z

adam test sanity depends eclipse/omr#master

AdamBrousseau · 2018-06-12T18:48:38Z

PR Compile
https://ci.eclipse.org/openj9/job/Adam_PullRequest-Sanity-JDK9-linux_ppc-64_cmprssptrs_le-OpenJ9/39/
PR Sanity
https://ci.eclipse.org/openj9/job/Adam_PullRequest-Sanity-JDK9-linux_ppc-64_cmprssptrs_le-OpenJ9/40/

AdamBrousseau

I think this is good to go

vsebe · 2018-06-13T18:23:59Z

@pshipton please review

pshipton · 2018-06-13T18:50:54Z

@smlambert or delegate needs to sign off on this first.

vsebe · 2018-06-13T19:23:05Z

@smlambert - this does not change the current implementation of the test pipelines.
The pipelines workflow, the way the test pipeline are hooked up, the input for the test pipeline, how tests run remain unchanged.

AdamBrousseau · 2018-06-13T19:42:52Z

One more comment, can you add Fixes #1897 to your commit comment please?

vsebe · 2018-06-13T20:04:35Z

@AdamBrousseau - added Fixes #1897 to the commit message

llxia · 2018-06-13T20:55:37Z

buildenv/jenkins/jobs/builds/Build-Test-Any-Platform

+                // pull request pipeline: compile, run tests, archive results,
+                // clean up
+                buildFile.build_pr()
+                testFile.test_all()


Build-Test-Any-Platform could replace all of the Build-Test-JDK<version>-<platform> and PullRequest-*-JDK<version>-<platform> scripts

What is Build-Test-JDK<version>-<platform>? I cannot find it in openj9 repo.
Just a note, once we switch to AdoptOpenJDK test pipeline for nightly, testFile.* will not be used. PullRequest-*-JDK<version>-<platform> will continue running until it is switched.

typo, should be Build-JDK<version>-<platform> - fixed
The AdoptOpenJDK test pipeline would use other scripts. We'll clean up the if statement in a different PR when we're ready to switch to the new test pipeline.

llxia · 2018-06-13T21:10:21Z

buildenv/jenkins/common/variables-functions

+            SPEC = params.get(arg)
+        }
+    }
+}


Why we set ARGS array and get params based on the array? Could we just directly use parameters? i.e.,

if( params.PLATFORM ) { SPEC = params.PLATFORM } else { // error }

Do we get SDK_VERSION value somewhere?

params could contains mandatory and optional parameters. This checks only for mandatory parameters.
Yes, the SDK_VERSION is a mandatory parameter for the Jenkins job. An error is thrown if it isn't set.

- add Jenkins file for build and test any platform - add pipeline wrapper for any platform These Jenkins files allow the OpenJ9 extensions build and test for any supported version and platform. New platforms can be added easily without creating new Jenkins files only parameterized Jenkins jobs. Pipeline-Build-Test-Any-Platform could also replace the all of the Pipeline-Build-Test-JDK<version>-<platform> wrappers. Build-Test-Any-Platform could replace all of the Build-JDK<version>-<platform>, Test-Sanity-JDK<version>-<platform>, Test-Extended-JDK<version>-<platform> and PullRequest-*-JDK<version>-<platform> scripts. Fixes eclipse-openj9#1897 [ci skip] Signed-off-by: Violeta Sebe <vsebe@ca.ibm.com>

vsebe · 2018-06-14T15:44:06Z

@pshipton please review

vsebe changed the title ~~Add generic Jenkins file for Windows 32bits pipelines~~ WIP: Add generic Jenkins file for Windows 32bits pipelines Jun 6, 2018

vsebe force-pushed the openj9.jenkins branch from 3a5c643 to bb30c9c Compare June 6, 2018 16:27

vsebe changed the title ~~WIP: Add generic Jenkins file for Windows 32bits pipelines~~ WIP: Add generic Jenkins files pipelines Jun 6, 2018

vsebe changed the title ~~WIP: Add generic Jenkins files pipelines~~ Add generic Jenkins files pipelines Jun 6, 2018

AdamBrousseau reviewed Jun 6, 2018

View reviewed changes

AdamBrousseau requested changes Jun 6, 2018

View reviewed changes

vsebe force-pushed the openj9.jenkins branch from bb30c9c to 277db6a Compare June 6, 2018 20:25

vsebe force-pushed the openj9.jenkins branch from 277db6a to ba8b46a Compare June 7, 2018 13:47

vsebe mentioned this pull request Jun 7, 2018

Add Windows x86 32 bits support to Jenkins pipeline #2097

Merged

vsebe force-pushed the openj9.jenkins branch from ba8b46a to b561bce Compare June 7, 2018 14:56

AdamBrousseau requested changes Jun 8, 2018

View reviewed changes

AdamBrousseau mentioned this pull request Jun 8, 2018

Move all job PROPERTIES to Jenkins Files for Build and Pipeline jobs #2138

Merged

vsebe force-pushed the openj9.jenkins branch 3 times, most recently from df0ee0f to 5f0861c Compare June 11, 2018 15:35

vsebe force-pushed the openj9.jenkins branch from 5f0861c to 1bb695a Compare June 12, 2018 14:04

pshipton added the comp:build label Jun 12, 2018

AdamBrousseau approved these changes Jun 13, 2018

View reviewed changes

AdamBrousseau mentioned this pull request Jun 13, 2018

Refactor Jenkins Build files #1897

Closed

4 tasks

vsebe force-pushed the openj9.jenkins branch from 1bb695a to ca33de7 Compare June 13, 2018 20:02

llxia reviewed Jun 13, 2018

View reviewed changes

vsebe force-pushed the openj9.jenkins branch from ca33de7 to 3919003 Compare June 14, 2018 13:49

llxia approved these changes Jun 14, 2018

View reviewed changes

smlambert merged commit 7e5a97b into eclipse-openj9:master Jun 14, 2018

vsebe deleted the openj9.jenkins branch May 8, 2019 13:55

		*******************************************************************************/

		ARGS = ['SDK_VERSION', 'PLATFORM']


		variableFile = load 'buildenv/jenkins/common/variables-functions'

		if (!params.UPSTREAM_JOB_NAME && !params.UPSTREAM_JOB_NUMBER) {

Add generic Jenkins files pipelines #2093

Add generic Jenkins files pipelines #2093

Conversation

vsebe commented Jun 6, 2018 • edited Loading

vsebe commented Jun 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pshipton commented Jun 6, 2018

smlambert commented Jun 7, 2018

AdamBrousseau commented Jun 7, 2018

vsebe commented Jun 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamBrousseau commented Jun 11, 2018

AdamBrousseau commented Jun 12, 2018

AdamBrousseau commented Jun 12, 2018

AdamBrousseau commented Jun 12, 2018

AdamBrousseau commented Jun 12, 2018

AdamBrousseau left a comment

Choose a reason for hiding this comment

vsebe commented Jun 13, 2018

pshipton commented Jun 13, 2018

vsebe commented Jun 13, 2018

AdamBrousseau commented Jun 13, 2018

vsebe commented Jun 13, 2018

Choose a reason for hiding this comment

vsebe Jun 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsebe Jun 14, 2018 • edited Loading

Choose a reason for hiding this comment

vsebe commented Jun 14, 2018

vsebe commented Jun 6, 2018 •

edited

Loading

vsebe Jun 14, 2018 •

edited

Loading

vsebe Jun 14, 2018 •

edited

Loading