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

[CI] Stop testing quay.io images and always build from source #589

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

zakkak
Copy link
Collaborator

@zakkak zakkak commented Oct 16, 2023

No description provided.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Oct 16, 2023
@zakkak zakkak requested review from jerboaa and Karm October 16, 2023 11:01
@zakkak zakkak added the testing CI and testing related issues and PRs label Oct 16, 2023
@jerboaa
Copy link
Collaborator

jerboaa commented Oct 16, 2023

  • Stop testing 23.1 builds from source, 23.1 is now being constantly
    tested against Quarkus main on the Quarkus repository.

Where would I be able to look at those test results? Do you have a link to the changeset that adds support for 23.1 on Quarkus?

Copy link
Collaborator

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

I'm not sure I agree with removing 23.1 build and test support. We surely want to continue testing that branch with updated JDK 21 EA builds, mandrel-integration-tests and so on (which Quarkus presumably doesn't do?!)? 23.1 is the latest LTS Mandrel release until GraalVM for JDK 25 comes out? A weekly test run doesn't hurt too much for this.

@zakkak
Copy link
Collaborator Author

zakkak commented Oct 16, 2023

Where would I be able to look at those test results? Do you have a link to the changeset that adds support for 23.1 on Quarkus?

That would be quarkusio/quarkus#36457 and since the time it got merged all Quarkus PRs are tested with 23.1 (from quay.io)

I'm not sure I agree with removing 23.1 build and test support. We surely want to continue testing that branch with updated JDK 21 EA builds, mandrel-integration-tests and so on (which Quarkus presumably doesn't do?!)?

The same still holds for 22.3 and 23.0, right?

23.1 is the latest LTS Mandrel release until GraalVM for JDK 25 comes out? A weekly test run doesn't hurt too much for this.

Well they add up...

We are interested in testing:

  1. Q 2.13 branch with:
    1. Mandrel 22.3 quay.io image (to catch regressions due to base image updates)
    2. Mandrel 22.3 built from source using the latest JDK 17-EA (to catch regressions due to changes in the JDK)
  2. Q 3.2 branch with:
    1. Mandrel 23.0 quay.io image (to catch regressions due to base image updates)
    2. Mandrel 23.0 built from source using the latest JDK 17-EA (to catch regressions due to changes in the JDK)
  3. Q main branch with:
    1. Mandrel 23.0 quay.io image (to catch regressions due to base image updates)
    2. Mandrel 23.0 built from source using the latest JDK 17-EA (to catch regressions due to changes in the JDK)
    3. Mandrel 23.1 built from source using the latest JDK 21-EA (to catch regressions due to changes in the JDK)

In total that's 7 configurations each taking about 5 hours...

At this point it might be worth setting up the weekly GH actions to also create a builder-image
and test all moving parts in a single run. That would leave us with 4 configurations. WDYT?

@zakkak zakkak changed the title Update weekly workflow [CI] Update weekly workflow Oct 17, 2023
@jerboaa
Copy link
Collaborator

jerboaa commented Nov 8, 2023

We are interested in testing:

1. Q 2.13 branch with:
   
   1. Mandrel 22.3 quay.io image (to catch regressions due to base image updates)
   2. Mandrel 22.3 built from source using the latest JDK 17-EA (to catch regressions due to changes in the JDK)

2. Q 3.2 branch with:
   
   1. Mandrel 23.0 quay.io image (to catch regressions due to base image updates)
   2. Mandrel 23.0 built from source using the latest JDK 17-EA (to catch regressions due to changes in the JDK)

3. Q main branch with:
   
   1. Mandrel 23.0 quay.io image (to catch regressions due to base image updates)
   2. Mandrel 23.0 built from source using the latest JDK 17-EA (to catch regressions due to changes in the JDK)
   3. Mandrel 23.1 built from source using the latest JDK 21-EA (to catch regressions due to changes in the JDK)

In total that's 7 configurations each taking about 5 hours...

At this point it might be worth setting up the weekly GH actions to also create a builder-image and test all moving parts in a single run. That would leave us with 4 configurations. WDYT?

I'm not sure I agree on this test plan. I've yet to see a change in the base image updates to cause a major headache for us. So I'd skip those tests. I'm also not sure why we care about Q main + 23.0 combo. I thought it's 3.2 only at this point. That leaves us with Q main + 23.1. Since 23.1 isn't used in any Quarkus release yet, that needs the most testing, IMO. The Quarkus CI isn't good enough, since it uses released bits (Mandrel and JDK), so we don't catch EA JDK build issues (which we've definitely seen). So for weekly CI testing the focus, in my opinion, should be:

  1. Q 2.13 branch with 22.3 built from source and EA JDK 17.
  2. Q 3.2 branch with 23.0 built from source and EA JDK 17.
  3. Q main with 23.1 built from source and EA JDK 21.

@zakkak
Copy link
Collaborator Author

zakkak commented Nov 8, 2023

I'm not sure I agree on this test plan. I've yet to see a change in the base image updates to cause a major headache for us. So I'd skip those tests.

That would probably be fine indeed given that these images are supposed to be used out in the field and we will learn sooner or later if something broke.

I'm also not sure why we care about Q main + 23.0 combo. I thought it's 3.2 only at this point. That leaves us with Q main + 23.1. Since 23.1 isn't used in any Quarkus release yet, that needs the most testing, IMO.

AFAIK it's still to be decided wheter the next Quarkus LTS release will move to 23.1 (i.e. JDK 21) or stick to 23.0 (i.e. JDK 17), thus why we need to keep testing it at least for the time being.

The Quarkus CI isn't good enough, since it uses released bits (Mandrel and JDK), so we don't catch EA JDK build issues (which we've definitely seen). So for weekly CI testing the focus, in my opinion, should be:

Q 2.13 branch with 22.3 built from source and EA JDK 17.
Q 3.2 branch with 23.0 built from source and EA JDK 17.
Q main with 23.1 built from source and EA JDK 21.

OK as long as we keep 23.0 for Q main. @Karm WDYT?

@jerboaa
Copy link
Collaborator

jerboaa commented Nov 8, 2023

I'm also not sure why we care about Q main + 23.0 combo. I thought it's 3.2 only at this point. That leaves us with Q main + 23.1. Since 23.1 isn't used in any Quarkus release yet, that needs the most testing, IMO.

AFAIK it's still to be decided wheter the next Quarkus LTS release will move to 23.1 (i.e. JDK 21) or stick to 23.0 (i.e. JDK 17), thus why we need to keep testing it at least for the time being.

Fair enough.

The Quarkus CI isn't good enough, since it uses released bits (Mandrel and JDK), so we don't catch EA JDK build issues (which we've definitely seen). So for weekly CI testing the focus, in my opinion, should be:

Q 2.13 branch with 22.3 built from source and EA JDK 17.
Q 3.2 branch with 23.0 built from source and EA JDK 17.
Q main with 23.1 built from source and EA JDK 21.

OK as long as we keep 23.0 for Q main.

Sure.

@zakkak zakkak force-pushed the 2023-10-16-update-weekly-ci branch from 50fbb7f to 2d86cd9 Compare November 8, 2023 13:52
@zakkak zakkak changed the title [CI] Update weekly workflow [CI] Stop testing quay.io images and always build from source Nov 8, 2023
Copy link
Collaborator

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

Looks fine.

@zakkak zakkak force-pushed the 2023-10-16-update-weekly-ci branch from 2d86cd9 to 83a620d Compare November 8, 2023 20:47
@zakkak
Copy link
Collaborator Author

zakkak commented Nov 9, 2023

22.3 and 23.0 failures are due to #608 and #607 respectively

@zakkak zakkak merged commit 8ddee70 into graalvm:default Nov 9, 2023
121 of 126 checks passed
@zakkak zakkak deleted the 2023-10-16-update-weekly-ci branch November 9, 2023 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement. testing CI and testing related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants