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

[BP-1.20][FLINK-34194] Update CI to Ubuntu 22.04 (Jammy) #25827

Merged
merged 2 commits into from
Jan 3, 2025

Conversation

mehdid93
Copy link
Contributor

What is the purpose of the change

This PR backport the changes done of the PR made by @zentol (#25708) in master for version 1.20.X to be used in dependencies upgrade.

Brief change log

  • Ubuntu CI image version is no more v18 but v22

Verifying this change

Please make sure both new and modified tests in this PR follow the conventions for tests defined in our code quality guide.

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 changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
  • The S3 file system connector: no

Documentation

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

@flinkbot
Copy link
Collaborator

flinkbot commented Dec 20, 2024

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@afedulov
Copy link
Contributor

afedulov commented Jan 2, 2025

@zentol you worked on the original upgrade. Do you know of any obstacles speaking against backporting it to 1.20 and 1.19 branches?
@XComp pinging you since IIRC you also worked a lot on the CI

@XComp
Copy link
Contributor

XComp commented Jan 2, 2025

The reason for not proceeding here was summarized in this FLINK-34194 comment.

We ran into some problems with the docs generation with the updated CI Docker image version (due to Maven being used instead of the Maven wrapper). I just reverted the change rather than fixing it due to time constraints on my side.

@afedulov
Copy link
Contributor

afedulov commented Jan 2, 2025

@XComp
Thanks, Matthias, I see you revert by it seems there was actually a later followup by Chesnay that is currently on master
03ab6a7
I assume the docs generation got addressed in that one?
I am curious to know if you are aware of any constraints for porting it back to the previous releases.

@XComp
Copy link
Contributor

XComp commented Jan 3, 2025

I assume the docs generation got addressed in that one?
I am curious to know if you are aware of any constraints for porting it back to the previous releases.

Yes, @zentol picked it up to help fixing FLINK-36716. He pushed another Docker image that still provides the Maven binaries. I'm going to close this PR to avoid confusion.

We might want to create backports for 03ab6a7 if we need to fix the issue in the other release branches as well.

@XComp XComp closed this Jan 3, 2025
@afedulov
Copy link
Contributor

afedulov commented Jan 3, 2025

@XComp got it thanks. I think there was some misunderstanding since this PR contained the aforementioned commit from Chesnay with some modifications and a follow-up commit that reverted them, so effectively it is exactly the backport of what we have on master. I will open a clean backport PR to avoid confusion.

@XComp XComp reopened this Jan 3, 2025
@XComp
Copy link
Contributor

XComp commented Jan 3, 2025

Oops, that was my mistake. I didn't check the author of the commit 🤦 I assumed it was an old PR that was created by me. Sorry for the confusion. I reopened the PR

Copy link
Contributor

@XComp XComp left a comment

Choose a reason for hiding this comment

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

The backport PR looks good 👍 I guess, we can use this one (after squashing the commits) and don't need to create a new PR here.

Any objections from your end, @afedulov ?

@afedulov
Copy link
Contributor

afedulov commented Jan 3, 2025

I am not quite sure what to read into this comment: #25708 (comment)
Do we need to continue building 1.x line with JDK 8?

@afedulov
Copy link
Contributor

afedulov commented Jan 3, 2025

If that is the case than LGTM, the second commit reverts back to 8 after resolving conflicts initially
565408f

@XComp
Copy link
Contributor

XComp commented Jan 3, 2025

Do we need to continue building 1.x line with JDK 8?

Yes, only with 2.0 (current master) it was decided to deprecate JDK 8

@XComp XComp merged commit b882ab7 into apache:release-1.20 Jan 3, 2025
@XComp
Copy link
Contributor

XComp commented Jan 6, 2025

Do you have an idea what could be wrong?

The CI build you are referring to does not contain the fixes of this FLINK-34194 1.20 backport PR, AFAIS.

The CI build refers to this branch (the link can be gathered from the CI run's main page). That branch has this git history which does not contain any commit of mentioning FLINK-34194

@afedulov
Copy link
Contributor

afedulov commented Jan 6, 2025

Oh, of course, makes sense.

jonathan-rene pushed a commit to jonathan-rene/flink that referenced this pull request Jan 8, 2025
* Update CI to Ubuntu 22.04 (Jammy)

---------

Co-authored-by: Chesnay Schepler <chesnay@apache.org>
afedulov pushed a commit to afedulov/flink that referenced this pull request Jan 9, 2025
* Update CI to Ubuntu 22.04 (Jammy)

---------

Co-authored-by: Chesnay Schepler <chesnay@apache.org>
afedulov pushed a commit that referenced this pull request Jan 13, 2025
* Update CI to Ubuntu 22.04 (Jammy)

---------

Co-authored-by: Chesnay Schepler <chesnay@apache.org>
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.

5 participants