You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/09/16 18:35:38 UTC
[GitHub] [flink-kubernetes-operator] sap1ens opened a new pull request, #370: [FLINK-29288] Make it possible to use job jars in the system classpath
sap1ens opened a new pull request, #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370
## What is the purpose of the change
Allowing to use job jars located in the system classpath (e.g. `/opt/flink/lib`). Currently it's not possible.
## Brief change log
- Making `spec.job.jarURI` optional, removing validation.
- Updating `FlinkConfigBuilder` to not define `pipeline.jars` and `pipeline.classpaths` if the `jarURI` is not specified.
- Updating `UserLibMountDecorator` to not create a `usrlib` folder if `pipeline.classpaths` is not defined. Otherwise Flink will try to load the provided class from the `usrlib` folder which is empty.
## Verifying this change
This change added tests and can be verified as follows:
- Running updated tests: `DefaultValidatorTest`, `FlinkConfigBuilderTest`, `UserLibMountDecoratorTest`.
- I've also deployed an operator with a custom Docker image built based on this branch to a staging environment. I confirmed that my custom Flink Docker image with a job jar in `/opt/flink/lib` can be executed successfully.
## 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, but it does make a required parameter optional
- Core observer or reconciler logic that is regularly executed: no
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? docs
---
**NOTE**: I find `UserLibMountDecorator` implementation a bit weird. It looks like right now `/opt/flink/usrlib` is always created in the Application mode. The class comments mention `pipeline.classpaths`, but it seems like the classpaths specified this way can be located anywhere. [Flink's DefaultPackagedProgramRetriever explicitly adds](https://github.com/apache/flink/blob/40d50f177c8ac63057dea2c755173a0dc138ede5/flink-clients/src/main/java/org/apache/flink/client/program/DefaultPackagedProgramRetriever.java#L104-L110) `usrlib` and `pipeline.classpaths` separately. In my opinion, the operator should let the user create `usrlib` if needed. It's not in the scope of this PR, but I'm happy to create a follow-up issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] sap1ens commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
sap1ens commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1250355459
Re: [CI failure](https://github.com/apache/flink-kubernetes-operator/actions/runs/3069970323/jobs/4972514311), it looks like the Maven step completed successfully (`[INFO] BUILD SUCCESS`), but for some reason, the run stopped with `Error: Process completed with exit code 1.`. Is it a known issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r977587900
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java:
##########
@@ -180,10 +180,6 @@ private Optional<String> validateJobSpec(
return Optional.empty();
}
- if (StringUtils.isNullOrWhitespaceOnly(job.getJarURI())) {
- return Optional.of("Jar URI must be defined");
- }
Review Comment:
For me your code change seemed to work with the following base image:
```
FROM flink:1.15.1
RUN cp /opt/flink/examples/streaming/StateMachineExample.jar /opt/flink/lib/.
```
SessionJob:
```
apiVersion: flink.apache.org/v1beta1
kind: FlinkSessionJob
metadata:
name: basic-session-job-only-example
spec:
deploymentName: basic-session-deployment-only-example
job:
parallelism: 4
upgradeMode: stateless
entryClass: org.apache.flink.streaming.examples.statemachine.StateMachineExample
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1255241160
Thank you for the great work @sap1ens , I will merge this once the CI passes!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] sap1ens commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
sap1ens commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r977209625
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java:
##########
@@ -180,10 +180,6 @@ private Optional<String> validateJobSpec(
return Optional.empty();
}
- if (StringUtils.isNullOrWhitespaceOnly(job.getJarURI())) {
- return Optional.of("Jar URI must be defined");
- }
Review Comment:
Hey @jeesmon, I've tweaked `AbstractFlinkService` to pass an empty noop jar, however now I'm seeing an exception on the JobManager side:
```
Caused by: java.lang.ClassNotFoundException: org.apache.flink.streaming.examples.statemachine.StateMachineExample
at java.net.URLClassLoader.findClass(Unknown Source) ~[?:?]
at java.lang.ClassLoader.loadClass(Unknown Source) ~[?:?]
at org.apache.flink.util.FlinkUserCodeClassLoader.loadClassWithoutExceptionHandling(FlinkUserCodeClassLoader.java:68) ~[flink-dist-1.15.2.jar:1.15.2]
at org.apache.flink.util.ChildFirstClassLoader.loadClassWithoutExceptionHandling(ChildFirstClassLoader.java:65) ~[flink-dist-1.15.2.jar:1.15.2]
at org.apache.flink.util.FlinkUserCodeClassLoader.loadClass(FlinkUserCodeClassLoader.java:52) ~[flink-dist-1.15.2.jar:1.15.2]
at java.lang.ClassLoader.loadClass(Unknown Source) ~[?:?]
at org.apache.flink.runtime.execution.librarycache.FlinkUserCodeClassLoaders$SafetyNetWrapperClassLoader.loadClass(FlinkUserCodeClassLoaders.java:172) ~[flink-dist-1.15.2.jar:1.15.2]
at java.lang.Class.forName0(Native Method) ~[?:?]
at java.lang.Class.forName(Unknown Source) ~[?:?]
at org.apache.flink.client.program.PackagedProgram.loadMainClass(PackagedProgram.java:479) ~[flink-dist-1.15.2.jar:1.15.2]
at org.apache.flink.client.program.PackagedProgram.<init>(PackagedProgram.java:153) ~[flink-dist-1.15.2.jar:1.15.2]
at org.apache.flink.client.program.PackagedProgram.<init>(PackagedProgram.java:65) ~[flink-dist-1.15.2.jar:1.15.2]
at org.apache.flink.client.program.PackagedProgram$Builder.build(PackagedProgram.java:691) ~[flink-dist-1.15.2.jar:1.15.2]
at org.apache.flink.runtime.webmonitor.handlers.utils.JarHandlerUtils$JarHandlerContext.toPackagedProgram(JarHandlerUtils.java:182) ~[flink-dist-1.15.2.jar:1.15.2]
```
My job config is the following:
```
apiVersion: flink.apache.org/v1beta1
kind: FlinkSessionJob
metadata:
name: basic-session-job-only-example-noop2
spec:
deploymentName: basic-session-deployment-only-example
job:
entryClass: org.apache.flink.streaming.examples.statemachine.StateMachineExample
parallelism: 4
upgradeMode: stateless
```
I've manually copied the jar file with the StateMachineExample class to the /opt/flink/lib folder.
After analyzing the code it looks like Flink creates and utilizes a special classloader that only uses the uploaded jar and nothing else. Of course, it can't find anything since the jar is empty.
What do I miss?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r973734058
##########
flink-kubernetes-standalone/src/main/java/org/apache/flink/kubernetes/operator/kubeclient/parameters/StandaloneKubernetesJobManagerParameters.java:
##########
@@ -87,4 +88,8 @@ public Boolean getAllowNonRestoredState() {
}
return null;
}
+
+ public Boolean isPipelineClasspathDefined() {
+ return flinkConfig.contains(PipelineOptions.CLASSPATHS);
+ }
Review Comment:
I don't really understand this check. Why can't we always mount the usrlib dir? What if PipelineOptions.CLASSPATHS points to a directory outside usrlib?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r976840850
##########
flink-kubernetes-standalone/src/main/java/org/apache/flink/kubernetes/operator/kubeclient/parameters/StandaloneKubernetesJobManagerParameters.java:
##########
@@ -87,4 +88,8 @@ public Boolean getAllowNonRestoredState() {
}
return null;
}
+
+ public Boolean isPipelineClasspathDefined() {
+ return flinkConfig.contains(PipelineOptions.CLASSPATHS);
+ }
Review Comment:
I agree that we should keep it, but did you look at the changes @usamj ? The questions is whether the change makes sense or not to support this feature
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] usamj commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
usamj commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r976786813
##########
flink-kubernetes-standalone/src/main/java/org/apache/flink/kubernetes/operator/kubeclient/parameters/StandaloneKubernetesJobManagerParameters.java:
##########
@@ -87,4 +88,8 @@ public Boolean getAllowNonRestoredState() {
}
return null;
}
+
+ public Boolean isPipelineClasspathDefined() {
+ return flinkConfig.contains(PipelineOptions.CLASSPATHS);
+ }
Review Comment:
As you mentioned without the decorator the user would have to perform more work to use the Operator.
I feel ease of use of the operator is important which is why I added this responsibility to the operator and believe that it should stay.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r974159354
##########
flink-kubernetes-standalone/src/main/java/org/apache/flink/kubernetes/operator/kubeclient/parameters/StandaloneKubernetesJobManagerParameters.java:
##########
@@ -87,4 +88,8 @@ public Boolean getAllowNonRestoredState() {
}
return null;
}
+
+ public Boolean isPipelineClasspathDefined() {
Review Comment:
this method should return `boolean` instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1250325337
If I understand it correctly this would in theory also solve https://issues.apache.org/jira/browse/FLINK-29144 as a side effect :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] sap1ens commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
sap1ens commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1255216631
@gyfora I squashed the commits, this is ready to be merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora merged pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora merged PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
jeesmon commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r977593522
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java:
##########
@@ -180,10 +180,6 @@ private Optional<String> validateJobSpec(
return Optional.empty();
}
- if (StringUtils.isNullOrWhitespaceOnly(job.getJarURI())) {
- return Optional.of("Jar URI must be defined");
- }
Review Comment:
Thanks @gyfora for confirming. That's exactly what I did when I tested. I was going to test it again this morning but you already did it :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1250366042
> Re: [CI failure](https://github.com/apache/flink-kubernetes-operator/actions/runs/3069970323/jobs/4972514311), it looks like the Maven step completed successfully (`[INFO] BUILD SUCCESS`), but for some reason, the run stopped with `Error: Process completed with exit code 1.`. Is it a known issue?
```
Please generate the java doc via 'mvn clean install -DskipTests -Pgenerate-docs' again
Error: Process completed with exit code 1.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r973754287
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java:
##########
@@ -180,10 +180,6 @@ private Optional<String> validateJobSpec(
return Optional.empty();
}
- if (StringUtils.isNullOrWhitespaceOnly(job.getJarURI())) {
- return Optional.of("Jar URI must be defined");
- }
Review Comment:
You removed validation of JarURI for both FlinkDeployment and FlinkSessionJob but at this moment the FlinkSessionJob still requires it (otherwise I think we might get a NPE somewhere else). This is why I thought it would be simplest to include the `no-op` jar changes also in this PR
cc @jeesmon
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] sap1ens commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
sap1ens commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r977817257
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java:
##########
@@ -180,10 +180,6 @@ private Optional<String> validateJobSpec(
return Optional.empty();
}
- if (StringUtils.isNullOrWhitespaceOnly(job.getJarURI())) {
- return Optional.of("Jar URI must be defined");
- }
Review Comment:
Alright, I added the jar after starting the container, thanks @jeesmon for telling me that :)
Thanks for checking!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] sap1ens commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
sap1ens commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1250355101
> If I understand it correctly this would in theory also solve https://issues.apache.org/jira/browse/FLINK-29144 as a side effect :)
In a way yes, you can place multiple jars in the system classpath instead and not specify anything as `jarURI`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r973760343
##########
flink-kubernetes-standalone/src/main/java/org/apache/flink/kubernetes/operator/kubeclient/parameters/StandaloneKubernetesJobManagerParameters.java:
##########
@@ -87,4 +88,8 @@ public Boolean getAllowNonRestoredState() {
}
return null;
}
+
+ public Boolean isPipelineClasspathDefined() {
+ return flinkConfig.contains(PipelineOptions.CLASSPATHS);
+ }
Review Comment:
I think without the `UserLibMountDecorator` it would be more cumbersome for the user to use the standalone mode. @usamj could probably provide more context here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] sap1ens commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
sap1ens commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r973753540
##########
flink-kubernetes-standalone/src/main/java/org/apache/flink/kubernetes/operator/kubeclient/parameters/StandaloneKubernetesJobManagerParameters.java:
##########
@@ -87,4 +88,8 @@ public Boolean getAllowNonRestoredState() {
}
return null;
}
+
+ public Boolean isPipelineClasspathDefined() {
+ return flinkConfig.contains(PipelineOptions.CLASSPATHS);
+ }
Review Comment:
> Why can't we always mount the usrlib dir?
Great question! Check the end of my PR description for more context. It turns out, when [StandaloneApplicationClusterEntryPoint](https://github.com/apache/flink/blob/40d50f177c8ac63057dea2c755173a0dc138ede5/flink-container/src/main/java/org/apache/flink/container/entrypoint/StandaloneApplicationClusterEntryPoint.java#L104) starts, it tries to find the `usrlib` folder. And if it's not null [it'll be used for loading the classes, not the system classpath](https://github.com/apache/flink/blob/40d50f177c8ac63057dea2c755173a0dc138ede5/flink-clients/src/main/java/org/apache/flink/client/program/DefaultPackagedProgramRetriever.java#L143-L147). So, we simply can't have a `usrlib` folder when we want to load classes from `/opt/flink/lib`.
> What if PipelineOptions.CLASSPATHS points to a directory outside usrlib?
Exactly. I don't know the context and all the use-cases, but in my mind `UserLibMountDecorator` shouldn't be part of the operator - let the user create/mount the right folder and place jars there. But deleting it in the context of this PR is a big change IMO, so added a check based on the comment on the `UserLibMountDecorator` class: `Mount the Flink User Lib directory to enable Flink to pick up a Jars defined in pipeline.classpaths.`. Which is think is misleading. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] sap1ens commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
sap1ens commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1250354948
> Also this does not address the SessionJobs as we discussed in the Slack thread with @jeesmon.
Yes, but aside from providing the "no-op" jar, which I thought is something that @jeesmon is working on separately, is there anything else that's missing here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
jeesmon commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r974199626
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java:
##########
@@ -180,10 +180,6 @@ private Optional<String> validateJobSpec(
return Optional.empty();
}
- if (StringUtils.isNullOrWhitespaceOnly(job.getJarURI())) {
- return Optional.of("Jar URI must be defined");
- }
Review Comment:
@gyfora @sap1ens What I did in our side was:
* Created a new maven module `flink-kubernetes-noop` to create a noop jar. I believe Gyula had mentioned we may be able to use some maven magic to generate this noop jar without adding another module, but I didn't invest time to look at it yet. I was trying to keep this change separate with minimal diff from upstream.
* Updated Dockerfile to copy noop jar to $FLINK_HOME/lib folder. We use a different Dockerfile in our side as we have to use photon based base image.
As we are using helm to deploy FlinkSessionJob CR, I didn't make any operator code change to use noop jar when jarURI is not present. Instead in helm template of FlinkSessionJob I set jarURI to `file:///opt/flink/lib/noop-0.0.1.jar` if jarURI is absent and entryClass is present in values.yaml.
If the above path is still beneficial, I can create a PR. But please include changes as part of this PR instead of what I outlined above if it fits better with this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r974285814
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/validation/DefaultValidator.java:
##########
@@ -180,10 +180,6 @@ private Optional<String> validateJobSpec(
return Optional.empty();
}
- if (StringUtils.isNullOrWhitespaceOnly(job.getJarURI())) {
- return Optional.of("Jar URI must be defined");
- }
Review Comment:
I don't think we need a new maven module, we could simply commit the empty jar into the project itself and avoid the complications :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
jeesmon commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1255026752
@gyfora Are we fine with including a pre-built jar with this PR? Just double checking :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#issuecomment-1255073155
> @gyfora Are we fine with including a pre-built jar with this PR? Just double checking :)
>
> flink-kubernetes-operator/src/main/resources/noop.jar
yes of course, why not? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] sap1ens commented on a diff in pull request #370: [FLINK-29288] Make it possible to use job jars in the system classpath
Posted by GitBox <gi...@apache.org>.
sap1ens commented on code in PR #370:
URL: https://github.com/apache/flink-kubernetes-operator/pull/370#discussion_r976723478
##########
flink-kubernetes-standalone/src/main/java/org/apache/flink/kubernetes/operator/kubeclient/parameters/StandaloneKubernetesJobManagerParameters.java:
##########
@@ -87,4 +88,8 @@ public Boolean getAllowNonRestoredState() {
}
return null;
}
+
+ public Boolean isPipelineClasspathDefined() {
Review Comment:
Resolved
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org