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

[FLINK-36460] Refactoring the CI matrix #893

Merged
merged 5 commits into from
Nov 1, 2024

Conversation

SamBarker
Copy link
Contributor

@SamBarker SamBarker commented Oct 10, 2024

What is the purpose of the change

As observed in FLINK-36332 the CI workflow has become difficult to mange. This PR is attempting to split tests along sensible lines to make the matrix more manageable.

To do so this PR splits the edge to edge workflow in two. A common workflow definition: e2e.yaml which is then reused from multiple different test matrices.

I think this split allows better control over the variables without having to vary each dimension every time. By making the simplifying assumption that variations in the Java version are adequately tested by the maven builds and the smoke tests it effectively reduces the number of test jobs from ~225 to about 100 (TBC)

Brief change log

Simplify CI workflows.

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changes to the CustomResourceDescriptors: no
  • Core observer or reconciler logic that is regularly executed: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable

@SamBarker SamBarker force-pushed the ci_jobs_refactor branch 4 times, most recently from 6442c91 to 56c87b0 Compare October 14, 2024 04:22
@SamBarker SamBarker force-pushed the ci_jobs_refactor branch 3 times, most recently from 8cf03c3 to b7f0ee0 Compare October 15, 2024 02:57
@SamBarker SamBarker force-pushed the ci_jobs_refactor branch 3 times, most recently from cfc7967 to b854817 Compare October 25, 2024 04:13
@SamBarker SamBarker marked this pull request as ready for review October 25, 2024 07:55
@SamBarker
Copy link
Contributor Author

@gyfora alternative CI matrix as requested. Even if the matrix proposed in this PR doesn't feel "right" it sets things up so that the matrix is considerably easier to change.

@SamBarker
Copy link
Contributor Author

This shows it in action on my fork: Screenshot 2024-10-25 at 8 58 16 pm

Copy link
Contributor

@gyfora gyfora left a comment

Choose a reason for hiding this comment

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

Looks great @SamBarker !

@SamBarker
Copy link
Contributor Author

SamBarker commented Oct 31, 2024

I think I've finally got the excludes to align with main. sigh That was a lot more fuss than I wanted and doesn't end up as clean as I was hoping. However I think this represents a much cleaner way to setup test matrices.

The tests are still very slow to run as each test has to build its own copies of the containers. I think thats resolvable without publishing temporary artefacts to a container registry. Some combination of docker advice and GitHub sharing data docs along with this discussion.

That feels like a separate PR.

@gyfora could you trigger another run of the PR please (its gone green on my fork)

Copy link
Contributor

@mxm mxm left a comment

Choose a reason for hiding this comment

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

Cool! This looks a lot cleaner.

@gyfora gyfora merged commit 1a5ee08 into apache:main Nov 1, 2024
98 checks passed
@SamBarker SamBarker deleted the ci_jobs_refactor branch November 3, 2024 20:09
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.

3 participants