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

Convert PullRequests to pipeline of jobs #2836

Merged

Conversation

AdamBrousseau
Copy link
Contributor

@AdamBrousseau AdamBrousseau commented Sep 11, 2018

  • PRs currently run in separate, stand-alone jobs
    and this is not maintenance friendly.
  • There are many benefits to switching to the common
    workflow of "pipeline of jobs" used by all other builds.
  • This PR adds support for PR builds to use
    Pipeline-Build-Test-All as a top-level job.
  • This also adds support in pipeline-functions to accomodate
    various functionality needed by PRs.
  • New features:
    • Github commit status updates before and after a Compile or Test job
    • Auto-Cancel Test jobs when new ones are launched from the same PR
    • New Parameter BULD_IDENTIFIER which is useful for determining
      what a build belongs to (Nightly, OMR, PR#123 etc)

Issue #2728
[skip ci]

Signed-off-by: Adam Brousseau adam.brousseau88@gmail.com

@AdamBrousseau AdamBrousseau force-pushed the convert_pr_to_pipeline branch 9 times, most recently from 0810ca5 to ec43af0 Compare September 12, 2018 18:58
@AdamBrousseau
Copy link
Contributor Author

AdamBrousseau commented Sep 12, 2018

TODO
  • Pass down ghpr params in order to convince the build job to checkout the PR
    • ghprbPullId
    • ghprbGhRepository
    • ghprbCommentBody
    • ghprbTargetBranch
    • ghprbActualCommit
  • Parse comment to determine
    • PLATFORMS
    • VERSIONS
    • TESTS_TARGETS
  • Archive
  • Determine how to pass the test jobs the merge SHA instead of HEAD
    • I assume if we determine the merge SHA in the beginning then the get_source will fail to find the ref.
  • Update commit status
  • Limit "sanity" to sanity.functional
  • Test PRs from ext repos
  • Test "All"
    • JDKs (not allowed)
    • Platforms (allowed)
    • Targets (not allowed)
  • Test OMR Acceptance
  • Test Nightly
  • Test Personal
  • Update README
  • Email dev mailing list

@AdamBrousseau AdamBrousseau force-pushed the convert_pr_to_pipeline branch 11 times, most recently from 79cb4c6 to db96f7c Compare September 13, 2018 03:50
@AdamBrousseau AdamBrousseau force-pushed the convert_pr_to_pipeline branch 7 times, most recently from d3c46cc to 899c2aa Compare September 21, 2018 17:07
- `Jenkins test sanity.functional zlinux jdk8,jdk11`
- Request an extended functional and system build on pLinux for a single version
- `Jenkins test extended.functional,extended.system plinux jdk8`
- Request a sanity build on z,p Linux for multiple versions
- `Jenkins test sanity zlinux,plinux jdk8,jdk9`
Copy link
Member

Choose a reason for hiding this comment

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

How about removing jdk9 here since it is obsolete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -80,6 +105,8 @@ You can also request a Pull Request build from the Eclipse OpenJ9 repository - [
- Ex. If you have a dependent change and only want one platform, depends comes last
- `Jenkins test sanity zlinux jdk8 depends eclipse/omr#123`

###### Note: When specifying a dependent change in an OpenJDK extensions repo, you can only build the SDK version that matches the repo where the dependent change lives. Eg. You cannot build JDK8 with a PR in openj9-openjdk-jdk9.
Copy link
Member

Choose a reason for hiding this comment

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

Also here, remove the ref to the obsolete jdk9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@pshipton
Copy link
Member

pshipton commented May 6, 2019

Not sure "Fixes #2728" is appropriate if the conversion to Pipeline builds doesn't occur when this is merged.

@@ -84,8 +84,6 @@ build_discarder:
OMR: 20
OpenJDK: 20
Personal: 50
# tmp PRs
pullRequest: 50
Copy link
Member

Choose a reason for hiding this comment

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

Is it appropriate to remove this before converting PRs to pipeline builds?

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 think this was never actually used. I believe I added it as part of #2138 in preparation for when this PR was delivered but it is now obsolete and replaced with #4738.

Copy link
Member

Choose a reason for hiding this comment

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

#4738 must be a typo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

error("Unknown PLATFORM short:'${SHORT}'\nExpected one of:${SHORT_NAMES}")
}
}
echo "PLATFORMS:'${PLATFORMS}'"
Copy link
Member

Choose a reason for hiding this comment

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

How is 'depends' handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment to comment block at the beginning of setup_pull_request()

* 3+ - jdk versions
*/
def PARSED_COMMENT = params.ghprbCommentBody.toLowerCase().tokenize(' ')
if (PARSED_COMMENT[0] != 'jenkins') {
Copy link
Member

Choose a reason for hiding this comment

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

We should use a different trigger than 'jenkins' at least temporarily, so we can run pipeline PR builds concurrently with the old style PR builds for testing purposes. One suggestion is 'pipeline'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this check since it is technically redundant. The trigger regex in the jenkins job verifies PARSED_COMMENT[0]. We can change it to pipeline on the new jobs until we are ready to change over completely.

- PRs currently run in separate, stand-alone jobs
  and this is not maintenance friendly.
- There are many benefits to switching to the common
  workflow of "pipeline of jobs" used by all other builds.
- This PR adds support for PR builds to use
  Pipeline-Build-Test-All as a top-level job.
- This also adds support in pipeline-functions to accomodate
  various functionality needed by PRs.
- Update README to reflect changes.
- Auto-Cancel Test jobs when new ones are launched from the same PR
- Only allow restarts for UNSTABLE builds.

Issue eclipse-openj9#2728
[skip ci]

Signed-off-by: Adam Brousseau <adam.brousseau88@gmail.com>
@pshipton pshipton merged commit 8120512 into eclipse-openj9:master May 7, 2019
AdamBrousseau added a commit to AdamBrousseau/openj9 that referenced this pull request May 7, 2019
- GIT_BRANCH is passed from Github for PR builds. This
  param was overiding the default value of 'master' in
  PR builds which causes PRs to fail to find the ref.

Related eclipse-openj9#2836 eclipse-openj9#4443 eclipse-openj9#4783
[skip ci]

Signed-off-by: Adam Brousseau <adam.brousseau88@gmail.com>
AdamBrousseau added a commit to AdamBrousseau/openj9 that referenced this pull request May 7, 2019
- Current check uses ghprbGhRepository which is now used for non-pr builds
- Change to pull id as this param is only passed for old and new style pr builds

Related eclipse-openj9#2836
[skip ci]

Signed-off-by: Adam Brousseau <adam.brousseau88@gmail.com>
LinHu2016 pushed a commit to LinHu2016/openj9 that referenced this pull request May 7, 2019
- GIT_BRANCH is passed from Github for PR builds. This
  param was overiding the default value of 'master' in
  PR builds which causes PRs to fail to find the ref.

Related eclipse-openj9#2836 eclipse-openj9#4443 eclipse-openj9#4783
[skip ci]

Signed-off-by: Adam Brousseau <adam.brousseau88@gmail.com>
LinHu2016 pushed a commit to LinHu2016/openj9 that referenced this pull request May 7, 2019
- Current check uses ghprbGhRepository which is now used for non-pr builds
- Change to pull id as this param is only passed for old and new style pr builds

Related eclipse-openj9#2836
[skip ci]

Signed-off-by: Adam Brousseau <adam.brousseau88@gmail.com>
AdamBrousseau added a commit to AdamBrousseau/openj9 that referenced this pull request May 15, 2019
- xlinux
- xlinuxXL
- osx
- win
- win32
- aix
- plinux
- zlinux
- Also change the values to be lists in order to
  support multiple values

[skip ci]
Related eclipse-openj9#2836

Signed-off-by: Adam Brousseau <adam.brousseau88@gmail.com>
AdamBrousseau added a commit to AdamBrousseau/openj9 that referenced this pull request Jun 11, 2019
- We no longer run our own testing so this
  code is no longer needed.
- Remove PullRequest pipelines as they are no longer in use.
- Combine 'build' and 'pullRequest' variable setup cases
  as they are identical.
- Also remove temp reverse spec map used prior to eclipse-openj9#2836.

Related eclipse-openj9#1450
Depends eclipse-openj9#2728 eclipse-openj9#2638 eclipse-openj9#2467 eclipse-openj9#2272 eclipse-openj9#2836 eclipse-openj9#4443
[skip ci]
Signed-off-by: Adam Brousseau <adam.brousseau88@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants