zentol commented on a change in pull request #32:
URL: https://github.com/apache/flink-docker/pull/32#discussion_r461508280
##########
File path: generator.sh
##########
@@ -0,0 +1,80 @@
+#!/bin/bash -e
+
+# Defaults, can vary between versions
+export SOURCE_VARIANTS=(java11-debian debian )
+
+export LATEST_SCALA="2.12"
+
+function generate() {
Review comment:
I'd split this into 2 functions; one for generating the image, one for
generating the meta data.
Right now its a function doing 2 things with (pretty much) 2 non-overlapping
argument sets.
Then we don't have to deal with it for `add-custom.sh`.
##########
File path: generator.sh
##########
@@ -0,0 +1,80 @@
+#!/bin/bash -e
+
+# Defaults, can vary between versions
+export SOURCE_VARIANTS=(java11-debian debian )
+
+export LATEST_SCALA="2.12"
+
+function generate() {
+ # define variables
+ dir=$1
+ binary_download_url=$2
+ asc_download_url=$3
+ gpg_key=$4
+ check_gpg=$5
+ flink_release=$6
+ flink_version=$7
+ scala_version=$8
+ source_variant=$9
+
+ from_docker_image="openjdk:8-jre"
+ if [[ $source_variant =~ "java11" ]] ; then
+ from_docker_image="openjdk:11-jre"
+ fi
+
+ ########################################
+ ### generate "Dockerfile" file
+ ########################################
+
+ # overwrite variable based on $source_variant to support non-debian
releases
+ source_file="Dockerfile-debian"
+
+ mkdir "$dir"
+ cp docker-entrypoint.sh "$dir/docker-entrypoint.sh"
+
+ # '&' has special semantics in sed replacement patterns
+ escaped_binary_download_url=$(echo "$binary_download_url" | sed
's/&/\\\&/')
+
+ # generate Dockerfile
+ sed \
+ -e "s,%%BINARY_DOWNLOAD_URL%%,${escaped_binary_download_url}," \
+ -e "s,%%ASC_DOWNLOAD_URL%%,$asc_download_url," \
+ -e "s/%%GPG_KEY%%/$gpg_key/" \
+ -e "s/%%CHECK_GPG%%/${check_gpg}/" \
+ -e "s/%%FROM_IMAGE%%/${from_docker_image}/" \
+ "$source_file.template" > "$dir/Dockerfile"
+
+ ########################################
+ ### generate "release.metadata" file
+ ########################################
+
+ # docker image tags:
+ java11_suffix=""
+ if [[ $source_variant =~ "java11" ]] ; then
Review comment:
its a bit weird to encode the java version into the source_variant when
the scala version has its own argument.
I'd rather have the java version as an explicit argument, with a dedicated
set of `-java8` tags, + it being considered the default java version for the
tag set without java versions.
##########
File path: generator.sh
##########
@@ -0,0 +1,80 @@
+#!/bin/bash -e
+
+# Defaults, can vary between versions
Review comment:
can be removed now; everything on the dev branches can vary between
versions
##########
File path: generator.sh
##########
@@ -0,0 +1,80 @@
+#!/bin/bash -e
+
+# Defaults, can vary between versions
+export SOURCE_VARIANTS=(java11-debian debian )
+
+export LATEST_SCALA="2.12"
+
+function generate() {
+ # define variables
+ dir=$1
+ binary_download_url=$2
+ asc_download_url=$3
+ gpg_key=$4
+ check_gpg=$5
+ flink_release=$6
+ flink_version=$7
+ scala_version=$8
+ source_variant=$9
+
+ from_docker_image="openjdk:8-jre"
+ if [[ $source_variant =~ "java11" ]] ; then
+ from_docker_image="openjdk:11-jre"
+ fi
+
+ ########################################
+ ### generate "Dockerfile" file
+ ########################################
+
+ # overwrite variable based on $source_variant to support non-debian
releases
Review comment:
this seems to do the opposite; you're hard-coding debian.
It seems overly complicated to add `java11-debian` into `SOURCE_VARIANTS`
only to overwrite it later with `debian` anyway.
A separate java version argument would be a simpler solution.
##########
File path: generator.sh
##########
@@ -0,0 +1,80 @@
+#!/bin/bash -e
+
+# Defaults, can vary between versions
+export SOURCE_VARIANTS=(java11-debian debian )
+
+export LATEST_SCALA="2.12"
+
+function generate() {
+ # define variables
+ dir=$1
+ binary_download_url=$2
+ asc_download_url=$3
+ gpg_key=$4
+ check_gpg=$5
+ flink_release=$6
+ flink_version=$7
+ scala_version=$8
+ source_variant=$9
+
+ from_docker_image="openjdk:8-jre"
+ if [[ $source_variant =~ "java11" ]] ; then
+ from_docker_image="openjdk:11-jre"
+ fi
+
+ ########################################
+ ### generate "Dockerfile" file
+ ########################################
+
+ # overwrite variable based on $source_variant to support non-debian
releases
+ source_file="Dockerfile-debian"
+
+ mkdir "$dir"
+ cp docker-entrypoint.sh "$dir/docker-entrypoint.sh"
+
+ # '&' has special semantics in sed replacement patterns
+ escaped_binary_download_url=$(echo "$binary_download_url" | sed
's/&/\\\&/')
+
+ # generate Dockerfile
+ sed \
+ -e "s,%%BINARY_DOWNLOAD_URL%%,${escaped_binary_download_url}," \
+ -e "s,%%ASC_DOWNLOAD_URL%%,$asc_download_url," \
+ -e "s/%%GPG_KEY%%/$gpg_key/" \
+ -e "s/%%CHECK_GPG%%/${check_gpg}/" \
+ -e "s/%%FROM_IMAGE%%/${from_docker_image}/" \
+ "$source_file.template" > "$dir/Dockerfile"
+
+ ########################################
+ ### generate "release.metadata" file
+ ########################################
+
+ # docker image tags:
+ java11_suffix=""
+ if [[ $source_variant =~ "java11" ]] ; then
+ java11_suffix="-java11"
+ fi
+ # example "1.2.0-scala_2.11-java11"
+ full_tag=${flink_version}-scala_${scala_version}${java11_suffix}
+
+ # example "1.2-scala_2.11-java11"
+ short_tag=${flink_release}-scala_${scala_version}${java11_suffix}
+
+ # example "scala_2.12-java11"
+ scala_tag="scala_${scala_version}${java11_suffix}"
+
+ tags="$full_tag, $short_tag, $scala_tag"
+
+ if [[ "$scala_version" == "$LATEST_SCALA" ]]; then
+ # we are generating the image for the latest scala version, add:
+ # "1.2.0-java11"
+ # "1.2-java11"
+ # "latest-java11"
+ tags="$tags, ${flink_version}${java11_suffix},
${flink_release}${java11_suffix}, latest${java11_suffix}"
+ fi
+
+ echo "Tags: $tags" >> $dir/release.metadata
+
+ # We currently only support amd64 with Flink.
+ echo "Architectures: amd64" >> $dir/release.metadata
Review comment:
I'm not sure if we can get by with this; if we want ARM support
(FLINK-14241) then we'll something more sophisticated. Not saying it has to be
the docker magic we had before (which I'd kinda prefer not to use), but we
should have a plan on how to support other architectures.
----------------------------------------------------------------
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]
