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

Make setup-and-test configurable #38

Merged
merged 2 commits into from Feb 2, 2021
Merged

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Jan 28, 2021

The idea is to make it simple to allow an ecosystem CI run to test different Quarkus versions.

For example Kogito's GH Actions definition (at https://github.com/kiegroup/kogito-runtimes/blob/master/.github/workflows/quarkus-snapshot.yaml) could be changed to:

name: "Quarkus ecosystem CI"
on:
  watch:
    types: [started]

  # For this CI to work, ECOSYSTEM_CI_TOKEN needs to contain a GitHub with rights to close the Quarkus issue that the user/bot has opened,
  # while 'ECOSYSTEM_CI_REPO_PATH' needs to be set to the corresponding path in the 'quarkusio/quarkus-ecosystem-ci' repository

env:
  ECOSYSTEM_CI_REPO: quarkusio/quarkus-ecosystem-ci
  ECOSYSTEM_CI_REPO_FILE: context.yaml
  JAVA_VERSION: 11

  #########################
  # Repo specific setting #
  #########################

  ECOSYSTEM_CI_REPO_PATH:  kogito-quarkus

jobs:
  quarkus-master:
    name: "Build against latest Quarkus snapshot"
    runs-on: ubuntu-latest
    if: github.actor == 'quarkusbot'

    steps:
      - name: Install yq
        run: sudo add-apt-repository ppa:rmescandon/yq && sudo apt update && sudo apt install yq -y

      - name: Set up Java
        uses: actions/setup-java@v1
        with:
          java-version: ${{ env.JAVA_VERSION }}

      - name: Checkout repo
        uses: actions/checkout@v2
        with:
          path: current-repo
          ref: master

      - name: Checkout Ecosystem
        uses: actions/checkout@v2
        with:
          repository: ${{ env.ECOSYSTEM_CI_REPO }}
          ref: master
          path: ecosystem-ci

      - name: Test against Quarkus master
        run: ./ecosystem-ci/setup-and-test
        env:
          ECOSYSTEM_CI_TOKEN: ${{ secrets.ECOSYSTEM_CI_TOKEN }}

  quarkus-lts:
    name: "Build against latest Quarkus LTS"
    runs-on: ubuntu-latest
    if: github.actor == 'quarkusbot'

    steps:
      - name: Install yq
        run: sudo add-apt-repository ppa:rmescandon/yq && sudo apt update && sudo apt install yq -y

      - name: Set up Java
        uses: actions/setup-java@v1
        with:
          java-version: ${{ env.JAVA_VERSION }}

      - name: Checkout repo
        uses: actions/checkout@v2
        with:
          path: current-repo
          ref: master

      - name: Checkout Ecosystem
        uses: actions/checkout@v2
        with:
          repository: ${{ env.ECOSYSTEM_CI_REPO }}
          ref: master
          path: ecosystem-ci

      - name: Test against Quarkus master
        run: ./ecosystem-ci/setup-and-test
        env:
          ECOSYSTEM_CI_TOKEN: ${{ secrets.ECOSYSTEM_CI_TOKEN }}
          ISSUE_NUM=9999 #needs to be created
          QUARKUS_BRANCH=1.11

Basically this adds a second job to be run on the same trigger, but configured the setup-and-test script by overriding the issue number and quarkus branch name.
Setup could potentially be different, but I show the 2 jobs approach here since we want the test to run for both Quarkus branches whether or not either fails

@geoand geoand force-pushed the configurable-setup-and-test branch from 6e2a470 to 804839b Compare January 28, 2021 15:50
@geoand
Copy link
Contributor Author

geoand commented Jan 28, 2021

cc @evacchi @gsmet

@evacchi
Copy link

evacchi commented Jan 28, 2021

cc @radtriste @danielezonca

cd quarkus
git checkout ${QUARKUS_SHA}
# Checkout Quarkus
if [[ -z "${QUARKUS_BRANCH}" ]]; then
Copy link

Choose a reason for hiding this comment

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

@radtriste that looks like what you were suggesting

@maxandersen
Copy link
Contributor

shouldn't the "overrides" come from the yaml driving the build ?

@geoand
Copy link
Contributor Author

geoand commented Jan 28, 2021

The problem is that we have one trigger for different builds. I suppose we could have second YAML file and simply set the env var to that file.
I just think it would be confusing what comes from where...

@maxandersen
Copy link
Contributor

what i was thinking was to change context from:

timestamp: 2021-01-28_07-24-37
issues:
  latestCommit: 9142
  repo: quarkusio/quarkus
quarkus:
  version: 999-SNAPSHOT
  sha: 6cdd2078f1e99eddc4e739f28c7d7808ce8af12b

to something like:

timestamp: 2021-01-28_07-24-37
issues:
  latestCommit: 9142
  repo: quarkusio/quarkus
quarkus:
  version: 999-SNAPSHOT
  sha: 42323434234
alternatives:
	"latest": 
		timestamp: 2021-01-28_07-24-37
	  	issues:
	    	latestCommit: 9142
	    	repo: quarkusio/quarkus
	  	quarkus:
	    version: 999-SNAPSHOT
	    sha: 6cdd2078f1e99eddc4e739f28c7d7808ce8af12b
	"next.x":
	  timestamp: 2021-01-28_07-24-37
	  issues:
	    latestCommit: 9142
	    repo: quarkusio/quarkus
	  quarkus:
	    version: 1.11.1-SNAPSHOT
	    sha: 6cdd203435234094

then have that be what drives the build.

note, here i'm assuming we would also set version numbers on branches but if we didn't we would just need to ensure the builds does not share ~/.m2 repo to avoid overlapping 999-snapshot

@geoand
Copy link
Contributor Author

geoand commented Jan 28, 2021

That seems reasonable, but I really don't want to spend any more time than necessary on this (as of now) isolated case.
I would prefer getting something in for Kogito now and kicking the stone down the road as we haven't heard of this need from anyone else.

@maxandersen
Copy link
Contributor

On last two synccups we had ask to be able to build ecosystem against other ébranches for ie. Vertx 4 - isn't this the same ?

@maxandersen
Copy link
Contributor

And to be clear. Doing something now for kogito is fine. Just thought would be good to discuss how to fix the multi branch setup.

@geoand
Copy link
Contributor Author

geoand commented Jan 28, 2021

And to be clear. Doing something now for kogito is fine. Just thought would be good to discuss how to fix the multi branch setup.

Right 👍.

I'm definitely in favor of a proper solution when the need actually arises.
For now we have Kogito that really wants this feature, so I would prefer to get this in now and the work on the proper solution later or (or Guillaume could when he finds some time).

@geoand
Copy link
Contributor Author

geoand commented Jan 28, 2021

On last two synccups we had ask to be able to build ecosystem against other ébranches for ie. Vertx 4 - isn't this the same ?

I don't think the Vert.x branch thing is the same

@geoand
Copy link
Contributor Author

geoand commented Jan 29, 2021

I just pushed a second commit which is simplified version of what @maxandersen proposed.
With this version, Kogito could would now change its info.yaml in this repository to:

url: https://github.com/kiegroup/kogito-runtimes # Github repo of your extension
issues:
  repo: quarkusio/quarkus # this should be left as is when CI updates are going to be reported on the Quarkus repository
  latestCommit: 12486 # this is the number of the issue you created above
alternatives:
  lts:
    issue: 99999999 # some new issue number
    quakusBranch: 1.11

and update their pipeline definition (in its own repo) to something like:

name: "Quarkus ecosystem CI"
on:
  watch:
    types: [started]

  # For this CI to work, ECOSYSTEM_CI_TOKEN needs to contain a GitHub with rights to close the Quarkus issue that the user/bot has opened,
  # while 'ECOSYSTEM_CI_REPO_PATH' needs to be set to the corresponding path in the 'quarkusio/quarkus-ecosystem-ci' repository

env:
  ECOSYSTEM_CI_REPO: quarkusio/quarkus-ecosystem-ci
  ECOSYSTEM_CI_REPO_FILE: context.yaml
  JAVA_VERSION: 11

  #########################
  # Repo specific setting #
  #########################

  ECOSYSTEM_CI_REPO_PATH:  kogito-quarkus

jobs:
  quarkus-master:
    name: "Build against latest Quarkus snapshot"
    runs-on: ubuntu-latest
    if: github.actor == 'quarkusbot'

    steps:
      - name: Install yq
        run: sudo add-apt-repository ppa:rmescandon/yq && sudo apt update && sudo apt install yq -y

      - name: Set up Java
        uses: actions/setup-java@v1
        with:
          java-version: ${{ env.JAVA_VERSION }}

      - name: Checkout repo
        uses: actions/checkout@v2
        with:
          path: current-repo
          ref: master

      - name: Checkout Ecosystem
        uses: actions/checkout@v2
        with:
          repository: ${{ env.ECOSYSTEM_CI_REPO }}
          ref: master
          path: ecosystem-ci

      - name: Test against Quarkus master
        run: ./ecosystem-ci/setup-and-test
        env:
          ECOSYSTEM_CI_TOKEN: ${{ secrets.ECOSYSTEM_CI_TOKEN }}

  quarkus-lts:
    name: "Build against latest Quarkus LTS"
    runs-on: ubuntu-latest
    if: github.actor == 'quarkusbot'

    steps:
      - name: Install yq
        run: sudo add-apt-repository ppa:rmescandon/yq && sudo apt update && sudo apt install yq -y

      - name: Set up Java
        uses: actions/setup-java@v1
        with:
          java-version: ${{ env.JAVA_VERSION }}

      - name: Checkout repo
        uses: actions/checkout@v2
        with:
          path: current-repo
          ref: master

      - name: Checkout Ecosystem
        uses: actions/checkout@v2
        with:
          repository: ${{ env.ECOSYSTEM_CI_REPO }}
          ref: master
          path: ecosystem-ci

      - name: Test against Quarkus master
        run: ./ecosystem-ci/setup-and-test
        env:
          ECOSYSTEM_CI_TOKEN: ${{ secrets.ECOSYSTEM_CI_TOKEN }}
          ALTERNATIVE: lts

I think that this iteration is more than enough for the time being

@radtriste
Copy link
Contributor

Hi @geoand,
Taking your latest example, should we update context.yaml instead of info.yaml
in the CI definition, we set ECOSYSTEM_CI_REPO_FILE: context.yaml

@gsmet
Copy link
Member

gsmet commented Jan 29, 2021

Thanks @geoand . I think it's perfectly reasonable and I agree we shouldn't spend too much time on this.

@geoand
Copy link
Contributor Author

geoand commented Jan 29, 2021

@radtriste no, updating info.yaml is the way to go, as the context.yaml is automatically updated by ecosystem CI

@radtriste
Copy link
Contributor

radtriste commented Jan 29, 2021

oh ok. good for me then :)

@geoand
Copy link
Contributor Author

geoand commented Feb 1, 2021

If there are not any objections, I'll go ahead and merge this

setup-and-test Outdated Show resolved Hide resolved
@geoand geoand force-pushed the configurable-setup-and-test branch from 6c5a84b to bd9e8e6 Compare February 1, 2021 10:30
@geoand
Copy link
Contributor Author

geoand commented Feb 2, 2021

Merging.

If all goes to hell tomorrow, we'll know what's to blame :)

@geoand geoand merged commit 44620ca into master Feb 2, 2021
@geoand geoand deleted the configurable-setup-and-test branch February 2, 2021 10:34
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.

None yet

5 participants