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 2021/01/13 10:33:48 UTC

[GitHub] [flink] zentol opened a new pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

zentol opened a new pull request #14630:
URL: https://github.com/apache/flink/pull/14630


   This PR moves most of the docker-entrypoint.sh logic into a new script within the distribution to reduce the maintenance burden and release issues.
   Everything but the jmalloc switch and dropping of privileges has been moved.
   
   Some minor adjustments have been made to the file;
   - _FLINK_HOME_DETERMINED is now always set to true, not just in the native-kubernetes/generic modes
   - bin/config.sh is now always sourced, not just in the native-kubernetes/generic modes
   - use more specific directories (e.g., FLINK_OPT_DIR) where possible.
   
   This branch works against a dev flink-docker branch for CI purposes.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "760192452",
       "triggerType" : "MANUAL"
     }, {
       "hash" : "fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * a19d8569e3badcb3c1877a9df03ab388edacc0c2 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053) 
   * fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759563380


   Yeah I wasn't sure either about the naming business. I'm not too bothered by the current name, and think that entrypoint-runner is a bit misleading since I would would assume the runner would call the entrypoint.
   Maybe docker-utils.sh would do the trick.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] rmetzger commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

rmetzger commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r556603593



##########
File path: flink-end-to-end-tests/test-scripts/common_docker.sh
##########
@@ -48,7 +48,7 @@ function build_image() {
     local server_pid=$!
 
     echo "Preparing Dockeriles"

Review comment:
       ```suggestion
       echo "Preparing Dockerfiles"
   ```

##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13

Review comment:
       ```suggestion
   # Deprecated, should be removed in Flink release 1.13
   ```

##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13

Review comment:
       This PR is against master, which will eventually become release 1.13?

##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+

Review comment:
       Maybe add a comment why the docker-entrypoint is located here.

##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13
+COMMAND_NATIVE_KUBERNETES="native-k8s"
+COMMAND_HISTORY_SERVER="history-server"
+COMMAND_DISABLE_JEMALLOC="disable-jemalloc"
+
+args=("$@")
+echo "${args[@]}"
+
+bin=`dirname "$0"`
+bin=`cd "$bin"; pwd`
+
+# FLINK_HOME is set by the docker image

Review comment:
       Does it make sense to check if the variable is set?
   
   If not, return an error that the variable is not set, and that the script should not be called directly?

##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash

Review comment:
       The script shows up in `build-target/bin`. If we keep it there, we need to add a message that it shall not be used manually.
   
   Maybe, it makes sense to move the script to `opt/docker/docker-entrypoint.sh` to not further pollute `bin/`? (This is really just a thought for discussion)




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762209580

@tianon Note that the stance against blackboxes and against kubernetes-specific branches creates quite a few issues, and at this point I honestly do not know what is acceptable and what isn't.

a)
If deferring the entire logic to some script outside the image is a problem, then so should any approach that allows an arbitrary command to be run (i.e., what was introduced in https://github.com/apache/flink-docker/pull/49). Both do _something unspecified_, having _some unspecified requirements_. I cannot comprehend why the latter was seemingly accepted though; we could now remove all the other branches and route everything through the generic path, leaving us in eventually the same state that we are in with this PR.
b)
The reality is we need some hook to activate some kubernetes-specific code; that's the whole point of our _native_ kubernetes integration (you _can_ still run things on kubernetes without it, this is just an _option_). We can't have kubernetes-specific branches in the image, and with generic branches being ruled out by a) the only alternative is adding a new `isKubernetes` flag argument to existing scripts in the distribution. This flag would not be mentioned in the image at all (it would just one of many arguments passed as parameter to the bin/ scripts), effectively being a hidden kubernetes branch.
I feel like that _shouldn't_ be acceptable because we'd just be tip-toeing around the issue and the scripts using that flag, from the perspective of the image, could be considered a black box.
To be specific, the jobmanager.sh would then either start a generic jobmanager (one that would be used outside docker environments) or one specific to kubernetes based on a given argument. But if that were allowed, then I'd think a script that starts either a jobmanager or taskmanager based on some argument should also be allowed. You can apply this logic a few more times and end up with a single scripts starting some process, which was already rejected.
But then I don't see a way how applications could implement anything optional that is specific to kubernetes and conform to your rules.

----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762796099

> How much of that is actually specific to using Flink in Docker[...]?

As it stands, I'd say everything. Let's go through it shall we:

1. Setting of certain options:
* jobmanager.rpc.address: Outside of docker this must be set manually depending on the way you deploy Flink, but we can make use of knowledge about dockers networking to simplify the setup.
* *.port: Outside of docker these are determined randomly by default, but we set them to static values here since we know that with docker they cannot conflict with others, and it simplifies the network rules setup for users.
* taskmanager.numberOfTaskSlots: This is essentially legacy behavior that we inherited. Outside of docker there are set manually by users based on their resources/requirements.
2. copying plugins
* It was recommended to us to allow any modifications that are usually done manually be made possible via environment variables. This is one such example. There are no plans to move this upstream.
3. FLINK_PROPERTIES
* Similarly to copying plugins, this was added as a convenience for modifying the configuration. Outside of docker this file is modified manually, and there is little benefit making this generally available.
4. envsubst stuff: Again some inherited stuff we can't throw out as of yet. There is a current discussion on the Flink dev mailing list to support this in Flink itself.
5. jemalloc stuff
* This switch was introduced due to a problem that is specific to the distribution used by docker image. While it is something that is potentially useful in other cases we have no interest in accommodating all possible platforms in such detail.
6. drop_privs_cmd
* Relies on the existence of a user account that is setup in the Dockerfile.
7. wrapping of commands
* primarily exists to set the `start-foreground` flag, as by default Flink processes run in the background. This is easier to do if there is some abstraction layer in between the user and Flink; in your model we'd have to inject a new parameter into the script arguments.

> This isn't as much a matter of "acceptable" vs "unacceptable".

I'd beg to differ, given that https://github.com/docker-library/official-images/pull/9249 has now been sitting around for over a month, forcing us to set a secondary docker image distribution channel. Instead such cleanups could've simply been relegated to the next version.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] flinkbot commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759360662

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

## Automated Checks
Last check on commit 73767fc77812142a7cde258bbcfe8d58a4540a59 (Wed Jan 13 10:35:35 UTC 2021)

**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!

<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress

* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.

Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:

- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 73767fc77812142a7cde258bbcfe8d58a4540a59 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r556652487



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13

Review comment:
       I'm only moving the script, any logic/API changes should be relegated to follow-ups.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "760192452",
       "triggerType" : "MANUAL"
     }, {
       "hash" : "fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12062",
       "triggerID" : "fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * a19d8569e3badcb3c1877a9df03ab388edacc0c2 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053) 
   * fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12062) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-761885131


   > I'm confused
   
   Well that makes 2 of us.
   
   https://github.com/docker-library/official-images/pull/9249#issuecomment-756731803
   > If I understand you correctly, then it would be fine for us to move **_everything in the docker-entrypoint.sh_** into a **_new scripts_** within distribution, and **_just call that script_** from docker-entrypoint.sh with all passed arguments.
   
   https://github.com/docker-library/official-images/pull/9249#issuecomment-756902869
   > Yes, I'd love to see this.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "760192452",
       "triggerType" : "MANUAL"
     } ]
   }-->
   ## CI report:
   
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * a19d8569e3badcb3c1877a9df03ab388edacc0c2 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760156571


   I've only run CI; which should at least cover all branches. I think I broke the jmalloc switch, because it checks the wrong argument. Of course we don't have tests for that; why should you...


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r556656406



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13
+COMMAND_NATIVE_KUBERNETES="native-k8s"
+COMMAND_HISTORY_SERVER="history-server"
+COMMAND_DISABLE_JEMALLOC="disable-jemalloc"
+
+args=("$@")
+echo "${args[@]}"
+
+bin=`dirname "$0"`
+bin=`cd "$bin"; pwd`
+
+# FLINK_HOME is set by the docker image

Review comment:
       a sanity check is always good




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762209580

@tianon Note that the stance against blackboxes and against kubernetes-specific branches creates quite a few issues, and at this point I honestly do not know what is acceptable and what isn't.

I understand the goal of wanting the image to be run anywhere and not containing kubernetes-specific code.
But then, how exactly are applications meant to implement kubernetes-specific features?

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-761888056


   > if someone downloads the binary distribution of Flink and tries to run it on a VM, are they expected to set an appropriate [FLINK_]CLASSPATH variable?
   
   To answer your example: No, it is generally not required. They may though, if they want it to contain something specific that would not be put onto classpath by the distribution on it's own. An example might be jars that are put into a mounted directory.
   
   As for why the kubernetes mode does it: It assembles a plain `java ...` command, and uses `${FLINK_CLASSPATH}` as a placeholder for the jvm classpath parameter (because the actual value is determined within the container, not on the client-side), which is populated by the script.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762796099

> How much of that is actually specific to using Flink in Docker[...]?

As it stands, I'd say everything. Let's go through it shall we:

There are some things we can certainly simplify (like deduplicating the copy_plugins calls, or jemalloc being controlled by an environment variable (that is in fact already implemented and just needs to be merged)).
The idea to have users call scripts directly is generally a good one, but it does bear the risk of users using functionality that needs some docker-specific logic that we have yet to set up. Ultimately the idea was to provide a clear API on what commands are available from docker and to guard it against changes in the distribution.
Nevertheless I'd be interested in trying this out and checking in with others on what they think about it.

> This isn't as much a matter of "acceptable" vs "unacceptable".

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r557283929



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,176 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+#######################################
+# This script is called by the Flink docker images when starting a process.
+# It contains docker-specific features, and hard-codes a few options.
+#
+# Globals:
+#   FLINK_HOME                          - (required) path to the Flink home directory
+#   ENABLE_BUILT_IN_PLUGINS             - semi-colon (;) separated list of plugins to enable, e.g., "flink-plugin1.jar;flink-plugin2.jar"
+#   FLINK_PROPERTIES                    - additional flink-conf.yaml entries as a multi-line string
+#   JOB_MANAGER_RPC_ADDRESS             - RPC address of the job manager
+#   TASK_MANAGER_NUMBER_OF_TASK_SLOTS   - number of slots for task executors
+#######################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13
+COMMAND_NATIVE_KUBERNETES="native-k8s"
+COMMAND_HISTORY_SERVER="history-server"
+
+args=("$@")
+echo "${args[@]}"

Review comment:
       my bad, this was for debugging.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r556656634



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13
+COMMAND_NATIVE_KUBERNETES="native-k8s"
+COMMAND_HISTORY_SERVER="history-server"
+COMMAND_DISABLE_JEMALLOC="disable-jemalloc"
+
+args=("$@")
+echo "${args[@]}"
+
+bin=`dirname "$0"`
+bin=`cd "$bin"; pwd`
+
+# FLINK_HOME is set by the docker image

Review comment:
       particularly since devs might call the scripts directly to test things.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762796099

> How much of that is actually specific to using Flink in Docker[...]?

As it stands, I'd say everything. Let's go through it shall we:

1. Setting of certain options:
* jobmanager.rpc.address: Outside of docker this must be set manually depending on the way you deploy Flink, but we can make use of knowledge about dockers networking to simplify the setup.
* *.port: Outside of docker these are determined randomly by default, but we set them to static values here since we know that with docker they cannot conflict with others, and it simplifies the network rules setup for users.
* taskmanager.numberOfTaskSlots: This is essentially legacy behavior that we inherited. Outside of docker there are set manually by users based on their resources/requirements.
2. copying plugins
* It was recommended to us to allow any modifications that are usually done manually be made possible via environment variables. This is one such example. There are no plans to move this upstream.
3. FLINK_PROPERTIES
* Similarly to copying plugins, this was added as a convenience for modifying the configuration. Outside of docker this file is modified manually, and there is little benefit making this generally available.
4. envsubst stuff: Again some inherited stuff we can't throw out as of yet. There is a current discussion on the Flink dev mailing list to support this in Flink itself.
5. jemalloc stuff
* This switch was introduced due to a problem that is specific to the distribution used by docker image. While it is something that is potentially useful in other cases we have no interest in accommodating all possible platforms to such a degree.
6. drop_privs_cmd
* Relies on the existence of a user account that is setup in the Dockerfile.
7. wrapping of commands
* primarily exists to set the `start-foreground` flag, as by default Flink processes run in the background. This is easier to do if there is some abstraction layer in between the user and Flink; in your model we'd have to inject a new parameter into the script arguments.

There are some things we can certainly simplify (like deduplicating the copy_plugins calls, or jemalloc being controlled by an environment variable (that is in fact already implemented and just needs to be merged)).
The idea to have users call scripts directly is generally a good one, but it does bear the risk of users using functionality that needs some docker-specific logic that we have yet to set up. Ultimately, our intend neither is nor ever was was to provide an image that allows everything in the distribution to be used, but to only ease the setup of singular Flink processes. Maybe this is where some of the dissonance comes from.
Nevertheless I'd be interested in trying this out and checking in with others on what they think about it.

> This isn't as much a matter of "acceptable" vs "unacceptable".

I'd beg to differ, given that https://github.com/docker-library/official-images/pull/9249 has now been sitting around for over a month, forcing us to set up a secondary docker image distribution channel. Instead such cleanups could've simply been relegated to the next version.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8589d475ea766eb3a44018272beb7ba23cb9bbec Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001) 
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * b3370da98fb5903795c396172842e33c2bbf7575 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] wangyang0918 commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

wangyang0918 commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r557070055



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13

Review comment:
       In the master branch, the native K8s will not need the `native-k8s` command anymore. It will generate the command and arguments as followings.
   
   ```
     - args:
       - bash
       - -c
       - $JAVA_HOME/bin/java -classpath $FLINK_CLASSPATH -Xmx1073741824 -Xms1073741824
         -XX:MaxMetaspaceSize=268435456 -Dlog.file=/opt/flink/log/jobmanager.log -Dlogback.configurationFile=file:/opt/flink/conf/logback-console.xml
         -Dlog4j.configuration=file:/opt/flink/conf/log4j2.xml -Dlog4j.configurationFile=file:/opt/flink/conf/log4j2.xml
         org.apache.flink.kubernetes.entrypoint.KubernetesApplicationClusterEntrypoint
         ...
       command:
       - /docker-entrypoint.sh
   ``` 
   We already have a ticket FLINK-20676 to track this.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * b3370da98fb5903795c396172842e33c2bbf7575 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762796099

> How much of that is actually specific to using Flink in Docker[...]?

As it stands, I'd say everything. Let's go through it shall we:

There are some things we can certainly simplify (like deduplicating the copy_plugins calls, or jemalloc being controlled by an environment variable (that is in fact already implemented and just needs to be merged)).
The idea to have users call scripts directly is generally a good one, but it does bear the risk of users using functionality that needs some docker-specific logic that we have yet to set up. Ultimately, our intend neither is nor ever was was to provide an image that allows everything in the distribution to be used (it handles wayyyy to many things for this be make sense imo), but to only ease the setup of singular Flink processes. Maybe this is where some of the dissonance comes from.
Nevertheless I'd be interested in trying this out and checking in with others on what they think about it.

> This isn't as much a matter of "acceptable" vs "unacceptable".

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760098909


   @wangyang0918 I have outlined this approach and already gotten their approval. https://github.com/docker-library/official-images/pull/9249#issuecomment-756902869
   (note that I had the same hunch as you though!)


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] wangyang0918 commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

wangyang0918 commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760791491


   It seems that we are still not on the same page.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8589d475ea766eb3a44018272beb7ba23cb9bbec Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001) 
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * b3370da98fb5903795c396172842e33c2bbf7575 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009) 
   * a19d8569e3badcb3c1877a9df03ab388edacc0c2 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] tianon commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

tianon commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-761165429


   I agree that the Docker image being a complete black box is a bad thing, which is exactly why I'm confused by this PR.  Let me try to rephrase what I'm suggesting:
   
   Take a look at the `docker-entrypoint.sh` as it stands today, and identify what problems it solves that are _not_ currently solved by the existing `flink` wrapper script(s) that are shipped with the upstream Flink distribution.
   
   Determine what makes sense to add to the Flink distribution to help with those use cases that are currently not being serviced, and use the integration of those changes into `docker-entrypoint.sh` as a "barometer" of sorts for whether you're succeeding at decreasing the amount of above-and-beyond scripting necessary to accomplish these core Flink use cases (because at the end of the day, the length of `docker-entrypoint.sh` is demonstrating that there are gaps in how Flink is expected to be run).
   
   I am not suggesting that the code in the current entrypoint script should be simply moved elsewhere, because that doesn't actually solve the underlying problem that the way the Flink distribution officially expects to be run and the way it's being run have diverged.
   
   As a concrete example, if someone downloads the binary distribution of Flink and tries to run it on a VM, are they expected to set an appropriate `CLASSPATH` variable?  Why or why not?  How can we improve the experience for them in such a way that it also improves the experience for Docker?  (Alternatively, if they are not, why does the Docker image need to, and how can we bring those two usages into a more symbiotic alignment?)


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762209580

@tianon Note that the stance against blackboxes and against kubernetes-specific branches creates quite a few issues, and at this point I honestly do not know what is acceptable and what isn't.

a)
If deferring the entire logic to some script outside the image is a problem, then so should any approach that allows an arbitrary command to be run (i.e., what was introduced in https://github.com/apache/flink-docker/pull/49). Both do _something unspecified_, having _some unspecified requirements_. I cannot comprehend why the latter was seemingly accepted though; we could now remove all the other branches and route everything through the generic path, leaving us in eventually the same state that we are in with this PR.
b)
The reality is we need some hook to activate some kubernetes-specific code; that's the whole point of our _native_ kubernetes integration (you _can_ still run things on kubernetes without it, this is just an _option_). We can't have kubernetes-specific branches in the image, and with generic branches being ruled out by a) the only alternative is adding a new `isKubernetes` flag argument to existing scripts in the distribution. This flag would not be mentioned in the image at all (it would just one of many arguments passed as parameter to the bin/ scripts), effectively being a hidden kubernetes branch.
I feel like that _shouldn't_ be acceptable because we'd just be tip-toeing around the issue and the scripts using that flag would again, from the perspective of the image, could be considered a black box.
To be specific, the jobmanager.sh would then either start a generic jobmanager (one that would be used outside docker environments) or one specific to kubernetes based on a given argument. But if that were allowed, then I'd think a script that starts either a jobmanager or taskmanager based on some argument should also be allowed. You can apply this logic a few more times and end up with a single scripts starting some process, which was already rejected.
But then I don't see a way how applications could implement anything optional that is specific to kubernetes and conform to your rules.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760192452


   @flinkbot run azure


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-761888056


   > if someone downloads the binary distribution of Flink and tries to run it on a VM, are they expected to set an appropriate [FLINK_]CLASSPATH variable?
   
   To answer your example: No, it is generally not required. They may though, if they want it to contain something specific that would not be put onto classpath by the distribution on it's own. An example might be jars that are put into a mounted directory.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-761888056


   > if someone downloads the binary distribution of Flink and tries to run it on a VM, are they expected to set an appropriate [FLINK_]CLASSPATH variable?
   
   To answer your example: No, it is not required.
   
   As for why the kubernetes mode does it: It assembles a plain `java ...` command, and uses `${FLINK_CLASSPATH}` as a placeholder for the jvm classpath parameter (because the actual value is determined within the container based on it's contents, not on the client-side), which is populated by the script.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] wangyang0918 commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

wangyang0918 commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r557073151



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,176 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+#######################################
+# This script is called by the Flink docker images when starting a process.
+# It contains docker-specific features, and hard-codes a few options.
+#
+# Globals:
+#   FLINK_HOME                          - (required) path to the Flink home directory
+#   ENABLE_BUILT_IN_PLUGINS             - semi-colon (;) separated list of plugins to enable, e.g., "flink-plugin1.jar;flink-plugin2.jar"
+#   FLINK_PROPERTIES                    - additional flink-conf.yaml entries as a multi-line string
+#   JOB_MANAGER_RPC_ADDRESS             - RPC address of the job manager
+#   TASK_MANAGER_NUMBER_OF_TASK_SLOTS   - number of slots for task executors
+#######################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13
+COMMAND_NATIVE_KUBERNETES="native-k8s"
+COMMAND_HISTORY_SERVER="history-server"
+
+args=("$@")
+echo "${args[@]}"

Review comment:
       We should not print any text in the pass through mode. Otherwise, the PR for docker-library/official-images will fail since CI will run the this test[1].
   
   [1]. https://github.com/docker-library/official-images/blob/master/test/tests/override-cmd/run.sh




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762796099

> How much of that is actually specific to using Flink in Docker[...]?

As it stands, I'd say everything. Let's go through it shall we:

There are some things we can certainly simplify (like deduplicating the copy_plugins calls, or jemalloc being controlled by an environment variable (that is in fact already implemented and just needs to be merged)).
The idea to have users call scripts directly is generally a good one, but it does bear the risk of users using functionality that needs some docker-specific logic that we have yet to set up. Ultimately, our intend neither is nor ever was was to provide an image that allows everything in the distribution to be used, but to only ease the setup of singular Flink processes. Maybe this is where some of the dissonance comes from.
Nevertheless I'd be interested in trying this out and checking in with others on what they think about it.

> This isn't as much a matter of "acceptable" vs "unacceptable".

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] tianon commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

tianon commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762526116


   Ok, let me try to phrase this in a simpler way.
   
   https://github.com/apache/flink-docker/blob/d2ff00e0176e09cdbbde76412026054016a8551c/1.12/scala_2.12-java11-debian/docker-entrypoint.sh is almost 200 lines long with a lot of little bits of duplicated code.
   
   How much of that is actually specific to using Flink in Docker, and how much of that is something that users *outside* Docker might be interested in / use?
   
   It seems like a lot of it is just (explicit, separate) wrappers around other existing Flink scripts, and even many of those are sharing common behavior (`copy_plugins_if_required`, for example).
   
   Could they be combined in some way such that the "Docker specific" behavior happens once and the upstream scripts are then just standard fallthrough behavior?
   
   For example, right now users do commands like `docker run ... flink history-server ...` in order to run `historyserver.sh`.  What I would expect instead is that `$FLINK_HOME/bin` is in `PATH`, that `docker-entrypoint.sh` looks at `$1` to do any extra Docker-specific initialization for `historyserver.sh`, and users then do `docker run ... flink historyserver.sh ...` (where now, the "command" users are running in the container matches the exact name of the same command they would run for a standard install of Flink).
   
   Rough mockup:
   
   ```bash
   copy_plugins_if_required
   
   case "$1" in
   	standalone-job.sh) ... ;;
   	historyserver.sh) ... ;;
   	...
   esac
   
   exec "$@"
   ```
   
   If having something that allows users to run something "prettier" like `flink history-server ...` is a desired goal, shouldn't that be a standard wrapper script in the upstream distribution, rather than something created specifically just for the Docker image?  That could even be implemented as a set of symlinks to the official scripts with their "prettier" preferred names, if you want to keep the maintenance burden low.
   
   ---
   
   As another explicit example of complexity, instead of disabling jemalloc via an explicit "command" which then has to be stripped and is order-dependent, if that were set via environment variable the logic would be much simpler:
   
   ```bash
   if [ -z "${FLINK_DISABLE_JEMALLOC:-}" ]; then
   	export LD_PRELOAD=$LD_PRELOAD:/usr/lib/x86_64-linux-gnu/libjemalloc.so
   fi
   ```
   
   (To make this part of an "official" script in the actual distribution you'll likely want to make it more forgiving / detecting of the path to `libjemalloc.so`, but you get the idea.)
   
   ---
   
   This isn't as much a matter of "acceptable" vs "unacceptable" but rather that the script is consistently growing in complexity and I'm trying to understand why it is not simpler and/or which parts of it are actually Docker-specific and necessary, because it currently reads as a whole new launcher script for Flink and that seems like something that ought to be part of the Flink distribution instead of something Docker-specific.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * b3370da98fb5903795c396172842e33c2bbf7575 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009) 
   * a19d8569e3badcb3c1877a9df03ab388edacc0c2 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759563380


   Yeah I wasn't sure either about the naming business. I'm not too bothered by the current name, and think that entrypoint-runner is a bit misleading since it would would assume the runner would call the entrypoint.
   Maybe docker-utils.sh would do the trick.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 73767fc77812142a7cde258bbcfe8d58a4540a59 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984) 
   * 8589d475ea766eb3a44018272beb7ba23cb9bbec Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762796099

> How much of that is actually specific to using Flink in Docker[...]?

As it stands, I'd say everything. Let's go through it shall we:

1. Setting of certain options:
* jobmanager.rpc.address: Outside of docker this must be set manually depending on the way you deploy Flink, but we can make use of knowledge about dockers networking to simplify the setup.
* *.port: Outside of docker these are determined randomly by default, but we set them to static values here since we know that with docker they cannot conflict with others, and it simplifies the network rules setup for users.
* taskmanager.numberOfTaskSlots: This is essentially legacy behavior that we inherited. Outside of docker there are set manually by users based on their resources/requirements.
2. copying plugins
* It was recommended to us to allow any modifications that are usually done manually be made possible via environment variables. This is one such example. There are no plans to move this upstream.
3. FLINK_PROPERTIES
* Similarly to copying plugins, this was added as a convenience for modifying the configuration. Outside of docker this file is modified manually, and there is little benefit making this generally available.
4. envsubst stuff: Again some inherited stuff we can't throw out as of yet. There is a current discussion on the Flink dev mailing list to support this in Flink itself.
5. jemalloc stuff
* This switch was introduced due to a problem that is specific to the distribution used by docker image. While it is something that is potentially useful in other cases we have no interest in accommodating all possible platforms to such a degree.
6. drop_privs_cmd
* Relies on the existence of a user account that is setup in the Dockerfile.
7. wrapping of commands
* primarily exists to set the `start-foreground` flag, as by default Flink processes run in the background. This is easier to do if there is some abstraction layer in between the user and Flink; in your model we'd have to inject a new parameter into the script arguments.

There are some things we can certainly simplify (like deduplicating the copy_plugins calls, or jemalloc being controlled by an environment variable (that is in fact already implemented and just needs to be merged)).
The idea to have users call scripts directly is generally a good one, but it does bear the risk of users using functionality that needs some docker-specific logic that we have yet to set up. Ultimately, our intend neither is nor ever was was to provide an image that allows everything in the distribution to be used, but to only ease the setup of singular Flink processes. Maybe this is where some of the dissonance comes from.
Nevertheless I'd be interested in trying this out and checking in with others on what they think about it.

> This isn't as much a matter of "acceptable" vs "unacceptable".

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 73767fc77812142a7cde258bbcfe8d58a4540a59 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "FAILURE",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * a19d8569e3badcb3c1877a9df03ab388edacc0c2 Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] wangyang0918 commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

wangyang0918 commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r557073151



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,176 @@
+#!/usr/bin/env bash
+
+###############################################################################
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+
+#######################################
+# This script is called by the Flink docker images when starting a process.
+# It contains docker-specific features, and hard-codes a few options.
+#
+# Globals:
+#   FLINK_HOME                          - (required) path to the Flink home directory
+#   ENABLE_BUILT_IN_PLUGINS             - semi-colon (;) separated list of plugins to enable, e.g., "flink-plugin1.jar;flink-plugin2.jar"
+#   FLINK_PROPERTIES                    - additional flink-conf.yaml entries as a multi-line string
+#   JOB_MANAGER_RPC_ADDRESS             - RPC address of the job manager
+#   TASK_MANAGER_NUMBER_OF_TASK_SLOTS   - number of slots for task executors
+#######################################
+
+COMMAND_STANDALONE="standalone-job"
+# Deprecated, should be remove in Flink release 1.13
+COMMAND_NATIVE_KUBERNETES="native-k8s"
+COMMAND_HISTORY_SERVER="history-server"
+
+args=("$@")
+echo "${args[@]}"

Review comment:
       We should not print any text in the pass-through mode. Otherwise, the PR for docker-library/official-images will fail since CI will run the this test[1].
   
   [1]. https://github.com/docker-library/official-images/blob/master/test/tests/override-cmd/run.sh




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760922830


   @tianon This is just a first step to facilitate easier refactorings. Migrating more (ideally all) of the docker-specific logic into the scripts is the long-term goal, which would be hampered if we'd have to move things bit by bit from one repo (flink-docker) to another (flink).
   The concern that @wangyang0918 had was that the docker image is now a blackbox (because it does not define in any way what it accepts), and this is what I believe we agreed on to be fine (how else could we move the logic to the distribution).
   
   In any case moving this script into the distribution should already be an overall plus because this logic is now at least accessible even outside docker use-cases.
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 73767fc77812142a7cde258bbcfe8d58a4540a59 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 73767fc77812142a7cde258bbcfe8d58a4540a59 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984) 
   * 8589d475ea766eb3a44018272beb7ba23cb9bbec UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-766786159


   > Our experience with "we'll fix this in the next version" is that it doesn't happen.
   
   I can understand that mindset.
   
   We're happy to take you up on that compromise; we are cleaning up the script right now with the last PR to be merged today, and will update the official-images PR shortly.
   The follow-up regarding Kubernetes will be tracked in FLINK-21128.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r556656134



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash

Review comment:
       there's a lot of stuff in bin/ at this point that users should never use directly (bash-java-utils, config.sh, flink-console.sh, flink-daemon.sh, find-flink-home.sh), that I'm not too concerned about adding one more.
   I see the point of trying to hide it, but this whole "put-into-opt-but-load-it-automatically-in-some-scenarios" business that the table-api introduced is rubbing me the wrong way.
   The overall semantics for bin/opt seem rather ill-defined.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760098909


   @wangyang0918 I have outlined this approach and already gotten their approval. https://github.com/docker-library/official-images/pull/9249#issuecomment-756902869
   (note that I had the same hunch as you though, and was actually surprised that they were fine with 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760098909


   @wangyang0918 I have outlined this approach and already gotten their approval. https://github.com/docker-library/official-images/pull/9249#issuecomment-756902869


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8589d475ea766eb3a44018272beb7ba23cb9bbec Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762796099

> How much of that is actually specific to using Flink in Docker[...]?

As it stands, I'd say everything. Let's go through it shall we:

There are some things we can certainly simplify (like deduplicating the copy_plugins calls, or jemalloc being controlled by an environment variable (that is in fact already implemented and just needs to be merged)).
The idea to have users call scripts directly is generally a good one, but it does bear the risk of users using functionality that needs some docker-specific logic that we have yet to set up. Ultimately, our intend neither is nor ever was was to provide an image that allows everything in the distribution to be used, but to only ease the setup of singular Flink processes. Maybe this is where some of the dissonance comes from.
Nevertheless I'd be interested in trying this out and checking in with others on what they think about it.

> This isn't as much a matter of "acceptable" vs "unacceptable".

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] zentol closed pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol closed pull request #14630:
URL: https://github.com/apache/flink/pull/14630


   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-766786159


   > Our experience with "we'll fix this in the next version" is that it doesn't happen.
   
   I can understand that mindset.
   
   We're happy to take you up on that compromise; we are cleaning up the script right now with the last PR to be merged today, and will update the official-images PR shortly.
   The follow-up regarding Kubernetes will be tracked in FLINK-21128.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 8589d475ea766eb3a44018272beb7ba23cb9bbec Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001) 
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * b3370da98fb5903795c396172842e33c2bbf7575 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] wangyang0918 commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

wangyang0918 commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762308338


   Actually, I am also confused about what is acceptable to keep in `docker-entrypoint.sh` and what is not. Maybe at the very begging we should not introduce the native K8s related logics to `docker-entrypoint.sh`.
   
   For how to implement kubernetes-specific features, I think we could have a dedicated entrypoint for native Kubernetes integration `kubernetes-entrypoint.sh`. After then, `docker-entrypoint.sh` is used only for standalone containerized deployment. Both `kubernetes-entrypoint.sh` and `docker-entrypoint.sh` could share some common functions(e.g. `copy_plugins_if_required `, `disable_jemalloc` , etc.).


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760922830


   @tianon This is just a first step to facilitate easier refactorings. Migrating more (ideally all) of the docker-specific logic into the scripts is the long-term goal, which would be hampered if we'd have to move things bit by bit from one repo (flink-docker) to another (flink).
   The concern that @wangyang0918 had was that the docker image is now a blackbox (because it does not define in any way what it accepts), and this is what I believe we agreed on to be fine (how else could we move the logic to the distribution).
   


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-762796099

> How much of that is actually specific to using Flink in Docker[...]?

As it stands, I'd say everything. Let's go through it shall we:

1. Setting of certain options:
* jobmanager.rpc.address: Outside of docker this must be set manually depending on the way you deploy Flink, but we can make use of knowledge about dockers networking to simplify the setup.
* *.port: Outside of docker these are determined randomly by default, but we set them to static values here since we know that with docker they cannot conflict with others, and it simplifies the network rules setup for users.
* taskmanager.numberOfTaskSlots: This is essentially legacy behavior that we inherited. Outside of docker there are set manually by users based on their resources/requirements.
2. copying plugins
* It was recommended to us to allow any modifications that are usually done manually be made possible via environment variables. This is one such example. There are no plans to move this upstream.
3. FLINK_PROPERTIES
* Similarly to copying plugins, this was added as a convenience for modifying the configuration. Outside of docker this file is modified manually, and there is little benefit making this generally available.
4. envsubst stuff: Again some inherited stuff we can't throw out as of yet. There is a current discussion on the Flink dev mailing list to support this in Flink itself.
5. jemalloc stuff
* This switch was introduced due to a problem that is specific to the distribution used by docker image. While it is something that is potentially useful in other cases we have no interest in accommodating all possible platforms to such a degree.
6. drop_privs_cmd
* Relies on the existence of a user account that is setup in the Dockerfile.
7. wrapping of commands
* primarily exists to set the `start-foreground` flag, as by default Flink processes run in the background. This is easier to do if there is some abstraction layer in between the user and Flink; in your model we'd have to inject a new parameter into the script arguments.

There are some things we can certainly simplify (like deduplicating the copy_plugins calls, or jemalloc being controlled by an environment variable (that is in fact already implemented and just needs to be merged)).
The idea to have users call scripts directly is generally a good one, but it does bear the risk of users using functionality that needs some docker-specific logic that we have yet to set up. Ultimately, our intend neither is nor ever was was to provide an image that allows everything in the distribution to be used, but to only ease the setup of singular Flink processes. Maybe this is where some of the dissonance comes from.
Nevertheless I'd be interested in trying this out and checking in with others on what they think about it.

> This isn't as much a matter of "acceptable" vs "unacceptable".

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] rmetzger commented on a change in pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

rmetzger commented on a change in pull request #14630:
URL: https://github.com/apache/flink/pull/14630#discussion_r557343768



##########
File path: flink-dist/src/main/flink-bin/bin/docker-entrypoint.sh
##########
@@ -0,0 +1,161 @@
+#!/usr/bin/env bash

Review comment:
       You are right. We should clean up the bin directory in a separate effort.




----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] tianon commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

tianon commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-760564996


   > @wangyang0918 I have outlined this approach and already gotten their approval. [docker-library/official-images#9249 (comment)](https://github.com/docker-library/official-images/pull/9249#issuecomment-756902869)
   > (note that I had the same hunch as you though, and was actually surprised that they were fine with it)
   
   To clarify - I did _not_ mean simply moving the entrypoint script into Flink.  I meant moving the _functionality_ into Flink.  Many of the things that happen in that script are things that probably make sense for other (non-container) users of Flink as well, and it feels wrong for those implementations to happen in the Dockerization scripts.


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] tianon commented on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

tianon commented on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-763180710


   > I'd beg to differ, given that [docker-library/official-images#9249](https://github.com/docker-library/official-images/pull/9249) has now been sitting around for over a month, forcing us to set up a secondary docker image distribution channel.
   
   Our experience with "we'll fix this in the next version" is that it doesn't happen.
   
   The reason we're pushing back on 1.12 is that this functionality is introduced there, and as far as the image is concerned, isn't released yet, so I'm having a hard time understanding why things like the jemalloc usage can't be fixed in 1.12 before the images are released?  I get that the kubernetes functionality is unfortunately already baked in to the 1.12 upstream release, so we can't really "fix" that until 1.13.
   
   As a compromise (and to show good faith), if we can clean up the jemalloc usage (or instead remove/revert it pending 1.13), I'm happy to merge 1.12 and get that moving under the shared understanding that we find a better usage pattern for kubernetes in 1.13 (cc @yosifkit).


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] flinkbot edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

flinkbot edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-759372895


   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=11984",
       "triggerID" : "73767fc77812142a7cde258bbcfe8d58a4540a59",
       "triggerType" : "PUSH"
     }, {
       "hash" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12001",
       "triggerID" : "8589d475ea766eb3a44018272beb7ba23cb9bbec",
       "triggerType" : "PUSH"
     }, {
       "hash" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "656628f8e564ef8ee29032d392f1485a8b0d9eea",
       "triggerType" : "PUSH"
     }, {
       "hash" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12009",
       "triggerID" : "b3370da98fb5903795c396172842e33c2bbf7575",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "triggerType" : "PUSH"
     }, {
       "hash" : "a19d8569e3badcb3c1877a9df03ab388edacc0c2",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12053",
       "triggerID" : "760192452",
       "triggerType" : "MANUAL"
     }, {
       "hash" : "fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12062",
       "triggerID" : "fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 656628f8e564ef8ee29032d392f1485a8b0d9eea UNKNOWN
   * fb9fe1a4723b02d78c4cc37d2c0b3d2549aafa2e Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=12062) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>


----------------------------------------------------------------
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:
users@infra.apache.org

[GitHub] [flink] zentol edited a comment on pull request #14630: [FLINK-20915][docker] Move docker entrypoint to distribution

Posted by GitBox <gi...@apache.org>.

zentol edited a comment on pull request #14630:
URL: https://github.com/apache/flink/pull/14630#issuecomment-761888056






----------------------------------------------------------------
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:
users@infra.apache.org