azagrebin commented on a change in pull request #11245:
URL: https://github.com/apache/flink/pull/11245#discussion_r430979974
##########
File path:
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/configuration/KubernetesConfigOptions.java
##########
@@ -121,8 +122,11 @@
public static final ConfigOption<String> CONTAINER_IMAGE =
key("kubernetes.container.image")
.stringType()
- .defaultValue("flink:latest")
- .withDescription("Image to use for Flink containers.");
+ .defaultValue("flink:" + EnvironmentInformation.getVersion() +
"-scala_" + EnvironmentInformation.getScalaVersion())
+ .withDescription("Image to use for Flink containers. " +
+ "The specified image MUST be based upon Apache Flink " +
Review comment:
What is the conclusion here?
##########
File path: docs/ops/deployment/docker.md
##########
@@ -46,6 +46,11 @@ For example, you can use the following aliases:
* `flink:latest` → `flink:<latest-flink>-scala_<latest-scala>`
* `flink:1.11` → `flink:1.11.<latest-flink-1.11>-scala_2.11`
+It is recommended to always use an explicit version tag of the docker image
that specifies both the needed Flink and Scala
Review comment:
```suggestion
<span class="label label-info">Note</span> It is recommended to always use
an explicit version tag of the docker image that specifies both the needed
Flink and Scala
```
This is indeed good addition. I would highlight it with a note label.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/util/EnvironmentInformation.java
##########
@@ -103,6 +103,15 @@ public static String getGitCommitTimeString() {
return getVersionsInstance().gitCommitTimeStr;
}
+ /**
+ * Returns the exact name of the Dockerimage for Flink that is needed
to run in.
+ *
+ * @return The "name:tag" of the Flink docker image.
+ */
+ public static String getFlinkDockerImageNameAndTag() {
Review comment:
My concern is also that `EnvironmentInformation` is responsible for the
real runtime information about the execution environment. The real docker image
may be different.
What are the expected users of this method? If there are indeed more than
just a default value for native kubernetes, maybe it makes sense to introduce
`DockerUtils` class or so.
##########
File path: docs/ops/deployment/kubernetes.md
##########
@@ -262,7 +261,7 @@ spec:
spec:
containers:
- name: jobmanager
- image: flink:{% if site.is_stable
%}{{site.version}}-scala{{site.scala_version_suffix}}{% else %}latest{% endif %}
+ image: flink:{% if site.is_stable
%}{{site.version}}-scala{{site.scala_version_suffix}}{% else %}latest # This
contains the latest released Flink built against Scala 2.12{% endif %}
Review comment:
Is it not easier to keep information about `latest->2.12` in one place:
docker hub tag page?
Maybe we could point to tags page here if you think that Scala version can
be confusing for users?
If we switch to Scala 2.11 in `latest` tag, this becomes stale.
##########
File path: docs/ops/deployment/kubernetes.md
##########
@@ -159,7 +159,6 @@ with the `kubectl` command:
## Appendix
### Common cluster resource definitions
-
Review comment:
This change seems to be unrelated.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
