[FLINK-28574] Bump the JOSDK version to 3.2.1 #362

SteNicholas · 2022-09-05T13:53:18Z

What is the purpose of the change

Java Operator SDK has released 3.2.1 version. Bump the JOSDK version to 3.2.1.

Brief change log

Bump the Java Operator SDK to 3.2.1

Verifying this change

(Please pick either of the following options)

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

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

Added integration tests for end-to-end deployment with large payloads (100MB)
Extended integration test for recovery after master (JobManager) failure
Manually verified the change by running a 4 node cluster with 2 JobManagers and 4 TaskManagers, a stateful streaming program, and killing one JobManager and two TaskManagers during the execution, verifying that recovery happens correctly.

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

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

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

mbalassi · 2022-09-06T10:25:00Z

Thanks @SteNicholas, it seems that bumping the fabric8 client has further implications - please double check the test failures:

[INFO] Running org.apache.flink.kubernetes.operator.kubeclient.factory.StandaloneKubernetesJobManagerFactoryTest
Error:  Tests run: 7, Failures: 0, Errors: 7, Skipped: 0, Time elapsed: 0.182 s <<< FAILURE! - in org.apache.flink.kubernetes.operator.kubeclient.factory.StandaloneKubernetesJobManagerFactoryTest
Error:  org.apache.flink.kubernetes.operator.kubeclient.factory.StandaloneKubernetesJobManagerFactoryTest.testDeploymentMetadata  Time elapsed: 0.101 s  <<< ERROR!
java.lang.NoSuchMethodError: 'io.fabric8.kubernetes.api.model.ServicePortFluent io.fabric8.kubernetes.api.model.ServiceSpecFluent$PortsNested.withNewTargetPort(java.lang.Integer)'
	at org.apache.flink.kubernetes.operator.kubeclient.factory.StandaloneKubernetesJobManagerFactoryTest.setup(StandaloneKubernetesJobManagerFactoryTest.java:75)

Error:  org.apache.flink.kubernetes.operator.kubeclient.factory.StandaloneKubernetesJobManagerFactoryTest.testTemplateMetadata  Time elapsed: 0.015 s  <<< ERROR!
java.lang.NoSuchMethodError: 'io.fabric8.kubernetes.api.model.ServicePortFluent io.fabric8.kubernetes.api.model.ServiceSpecFluent$PortsNested.withNewTargetPort(java.lang.Integer)'
	at org.apache.flink.kubernetes.operator.kubeclient.factory.StandaloneKubernetesJobManagerFactoryTest.setup(StandaloneKubernetesJobManagerFactoryTest.java:75)

Error:  org.apache.flink.kubernetes.operator.kubeclient.factory.StandaloneKubernetesJobManagerFactoryTest.testFlinkConfConfigMap  Time elapsed: 0.003 s  <<< ERROR!
java.lang.NoSuchMethodError: 'io.fabric8.kubernetes.api.model.ServicePortFluent io.fabric8.kubernetes.api.model.ServiceSpecFluent$PortsNested.withNewTargetPort(java.lang.Integer)'
	at org.apache.flink.kubernetes.operator.kubeclient.factory.StandaloneKubernetesJobManagerFactoryTest.setup(StandaloneKubernetesJobManagerFactoryTest.java:75)

morhidi · 2022-09-06T10:40:08Z

Thanks @SteNicholas for the contrib! Few thoughts on this change:

Why do we need to bump the fabric8 client? I guess nor Flink neither JOSDK itself has made this step yet.
All .0 upgrades on JOSDK have brought some unexpected surprises so far :/ So, if there's no specific feature we are after from JOSDK I'd put this PR on hold at least till we release 1.2 from the Operator.

SteNicholas · 2022-09-07T01:49:10Z

@morhidi

Why do we need to bump the fabric8 client? I guess nor Flink neither JOSDK itself has made this step yet.

I follow the bump step of JOSDK and bump the fabric8 client to 6.1.1.

All .0 upgrades on JOSDK have brought some unexpected surprises so far :/ So, if there's no specific feature we are after from JOSDK I'd put this PR on hold at least till we release 1.2 from the Operator.

Holding this PR at least till we release 1.2 makes sense to me.

morhidi · 2022-09-13T06:30:58Z

Hi @SteNicholas

It'd be great if you could try to fix the CI errors, i.e. NoSuchMethod. There is also the KubernetesClientMetrics class which introduces an interceptor to be able to measure the Kubernetes API Server access that might break due to the version bump.

Thanks,
Matyas

SteNicholas · 2022-09-15T08:21:46Z

@morhidi, @mbalassi, the java-operator-sdk released the 3.2.1 version which reverts the upgrade made in 3.2.0 to Fabric8 Client 6.1. Hence the version of the Fabric8 Client in v3.2.1 is 5.12.3. I will bump the version of the java-operator-sdk to 3.2.1 later. WDYT?

morhidi · 2022-09-15T08:29:03Z

@morhidi, @mbalassi, the java-operator-sdk released the 3.2.1 version which reverts the upgrade made in 3.2.0 to Fabric8 Client 6.1. Hence the version of the Fabric8 Client in v3.2.1 is 5.12.3. I will bump the version of the java-operator-sdk to 3.2.1 later. WDYT?

Somewhat expected :) Makes sense.

SteNicholas · 2022-09-17T08:26:48Z

@morhidi @mbalassi , the Java Operator SDK releases the 3.2.1 and 4.0.0 version, in which the Fabric Client version is 5.12.3 in v3.2.1 and the version is 6.1.1 in v4.0.0.

I have bumped the version of Java Operator SDK to v3.2.1 that separates executor service for workflow. PTAL.

BTW, Fabric8 Kubernetes Client 6.1.1 is most likely a breaking change, hence we could bump the version to 4.0.0 in Flink K8S operator 1.3 version. WDYT?

morhidi · 2022-09-19T10:46:02Z

flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/FlinkOperator.java

@@ -113,6 +113,9 @@ private void overrideOperatorConfigs(ConfigurationServiceOverrider overrider) {
        } else {
            LOG.info("Configuring operator with {} reconciliation threads.", parallelism);
            overrider.withConcurrentReconciliationThreads(parallelism);
+            // feat: separate executor service for workflow
+            // https://github.com/java-operator-sdk/java-operator-sdk/pull/1371
+            overrider.withExecutorService(Executors.newFixedThreadPool(parallelism));


Hi @SteNicholas Could you please explain why we need this extra config here? Is it ok if we control it with the same config?

normally only overrider.withConcurrentReconciliationThreads(parallelism); if you don't want to replace the executor service with a different flavor (thus other than a fixed thread pool).

see: https://github.com/java-operator-sdk/java-operator-sdk/blob/d9e869c3bf5319163d8d884529f6b4a926bd2d85/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java#L127-L129

@csviri @morhidi, this change comes from FlinkOperatorTest.java#L54, which get the ExecutorService with default thread number 10 and the concurrentReconciliationThreads() returns the default thread number.

Talked to @csviri offline, he'll clarify this part in a bit.

yes, sorry, this seems to be an issue with the overrider, created a PR to fix it
operator-framework/java-operator-sdk#1484

pls for now override the service creation like above: overrider.withExecutorService

@morhidi, like @csviri mentioned, the current way that overrides the service creation is correct.

@csviri, could the fix be released in 3.2.2? Otherwise I will remove this line when bumping the version to 4.0.0. cc @morhidi

@SteNicholas yes, will backport it to 3.x

morhidi · 2022-09-19T10:48:07Z

Hi @SteNicholas, if we stick with the current fabric8 version I don't see any problems merging this PR with the upcoming FOv1.2 release.

morhidi · 2022-09-19T13:00:37Z

+1 LGTM

SteNicholas changed the title ~~[FLINK-28574] Bump the JOSDK version to 3.2.0~~ [FLINK-28574] Bump the JOSDK version to 3.2.1 Sep 15, 2022

SteNicholas force-pushed the FLINK-28574 branch 2 times, most recently from b09f25c to ba1a374 Compare September 16, 2022 10:31

[FLINK-28574] Bump the JOSDK version to 3.2.1

3ce3009

SteNicholas force-pushed the FLINK-28574 branch from ba1a374 to 3ce3009 Compare September 17, 2022 08:24

morhidi reviewed Sep 19, 2022

View reviewed changes

SteNicholas requested a review from morhidi September 19, 2022 12:41

gyfora merged commit 1a5b066 into apache:main Sep 19, 2022

[FLINK-28574] Bump the JOSDK version to 3.2.1 #362

[FLINK-28574] Bump the JOSDK version to 3.2.1 #362

Uh oh!

Conversation

SteNicholas commented Sep 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief change log

Verifying this change

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

Documentation

Uh oh!

mbalassi commented Sep 6, 2022

Uh oh!

morhidi commented Sep 6, 2022

Uh oh!

SteNicholas commented Sep 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

morhidi commented Sep 13, 2022

Uh oh!

SteNicholas commented Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

morhidi commented Sep 15, 2022

Uh oh!

SteNicholas commented Sep 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morhidi commented Sep 19, 2022

Uh oh!

morhidi commented Sep 19, 2022

Uh oh!

Uh oh!

SteNicholas commented Sep 5, 2022 •

edited

Loading

SteNicholas commented Sep 7, 2022 •

edited

Loading

SteNicholas commented Sep 15, 2022 •

edited

Loading

SteNicholas commented Sep 17, 2022 •

edited

Loading