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.17][FLINK-18356] Update CI image #23757

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

XComp
Copy link
Contributor

@XComp XComp commented Nov 20, 2023

What is the purpose of the change

1.17 backport PR for parent PR #23717

Brief change log

I also backport the change for the docs build for the sake of consistency even though it's not necessary to fix FLINK-18356.

This backport required the upgrade of Maven to 3.8.6 (FLINK-28203).

Verifying this change

CI should pass without problems and should include the optional check

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

@XComp XComp changed the title [FLINK-18356] Update CI image [BP-1.17][FLINK-18356] Update CI image Nov 20, 2023
@XComp XComp requested a review from zentol November 20, 2023 07:54
@flinkbot
Copy link
Collaborator

flinkbot commented Nov 20, 2023

CI report:

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

@XComp
Copy link
Contributor Author

XComp commented Nov 20, 2023

@flinkbot run azure

@XComp
Copy link
Contributor Author

XComp commented Nov 20, 2023

The license check failure cannot be reproduced locally. Let's see whether it's reproducible in CI.

@XComp XComp force-pushed the FLINK-18356-1.17 branch 3 times, most recently from b134356 to 4ae9836 Compare November 23, 2023 07:37
@XComp
Copy link
Contributor Author

XComp commented Nov 23, 2023

I cannot get my head around why just upgrading the Docker image (4ae9836) leads to a LicenseCheck failure. We have to backport FLINK-32834 and FLINK-28203 to fix CI.

Update: Ok, looking into the issues: FLINK-28203 has to be backported because of the Maven version update. But the LicenseChecker issue seems to be unrelated. I still have to investigate whether FLINK-32834 is actually not needed.

Update 2: Looks like backporting FLINK-32834 is actually not necessary and I was misguided by error initially.

flink-dist/pom.xml Outdated Show resolved Hide resolved
flink-dist/pom.xml Outdated Show resolved Hide resolved
@snuyanzin
Copy link
Contributor

Thanks for driving this 👍

After looking at FLINK-28203 i noticed that there are also files
tools/ci/compile.sh
tools/ci/maven-utils.sh
tools/ci/verify_bundled_optional.sh

shouldn't they also be part of this PR?

@XComp
Copy link
Contributor Author

XComp commented Nov 24, 2023

After looking at FLINK-28203 i noticed that there are also files
tools/ci/compile.sh
tools/ci/maven-utils.sh
tools/ci/verify_bundled_optional.sh

shouldn't they also be part of this PR?

You're right. Thanks for this thorough review. I screwed up the backports. But anyway, @zentol pushed a dedicated image for the release-1.17 branch that includes Maven 3.2.5 which I'm gonna use now. That makes life easier. 8)

Comment on lines 42 to 43
# (see chesnay/flink-ci:java_8_11_17_maven_386_v3 which is used in branches for
# Flink 1.18+) but still use Maven 3.2.5 (to avoid backporting FLINK-28016)
Copy link
Contributor

@snuyanzin snuyanzin Nov 24, 2023

Choose a reason for hiding this comment

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

nit:
should we refer to concrete image here?
I'm asking since now there is a new one available chesnay/flink-ci:java_8_11_17_21_maven_386 which potentially could be used for 1.19

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - I had the naive view that we won't change the Docker image anymore until the EOL of 1.17 :-D

@snuyanzin
Copy link
Contributor

snuyanzin commented Nov 24, 2023

Thanks for the update
it looks good from my side,
I left a minor comment however it's up to you to ignore it or not

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.

4 participants