Refact docker-entrypoint.sh. #616
Conversation
The sed command substitution of bbmustache style template is stale emqx no longer releases an unrendered config template
| @@ -19,6 +18,9 @@ RUN apk add git \ | |||
| libc-dev \ | |||
| libstdc++ | |||
|
|
|||
| COPY tmp/qemu-$QEMU_ARCH-stati* /usr/bin/ | |||
| COPY . /emqx-rel | |||
|
|
|||
There was a problem hiding this comment.
Sort the build stages: run apk add first for more efficient cache.
There was a problem hiding this comment.
Hi, @PMExtra sorry for the delay
We need qemu to help us compile other cpu architecture images on x86, so the COPY tmp/qemu-$QEMU_ARCH-stati* /usr/bin/ statement needs to be above, You can check https://github.com/multiarch/qemu-user-static
There was a problem hiding this comment.
@zhanghongtong Thanks for your guidance. But I have some questions.
- Why don't we use qemu-user-static like:
FROM multiarch/qemu-user-static:${QEMU_ARCH} AS qemu
FROM ${BUILD_FROM} AS builder
COPY --from=qemu /usr/bin/qemu-$QEMU_ARCH-static /usr/bin
- Why should we copy
qemu-$QEMU_ARCH-stati*rather thanqemu-$QEMU_ARCH-static
Are these all designed to adapt x86_64? (copy nothing if QEMU_ARCH=x86_64)
There was a problem hiding this comment.
Hi, @PMExtra
Why don't we use qemu-user-static like:
FROM multiarch/qemu-user-static:${QEMU_ARCH} AS qemu FROM ${BUILD_FROM} AS builder COPY --from=qemu /usr/bin/qemu-$QEMU_ARCH-static /usr/bin
The multiarch/qemu-user-static image requires the —privileged option, so we did not choose it
Why should we copy qemu-$QEMU_ARCH-stati* rather than qemu-$QEMU_ARCH-static
This is a historical code, it seems there is no difference between the two
Are these all designed to adapt x86_64?
Yes, we will build arm, arm64, 386 and s390 docker images on x86_64 machines, you can see them in GitHub actions
There was a problem hiding this comment.
@zhanghongtong
抱歉我的英语可能没法准确表达我的意思，暂时中文沟通。
第一个问题我想问的是：目前我们应该是在make脚本中下载qemu的依赖到tmp目录，然后build时从tmp目录复制进去。但我们为什么不直接从multiarch/qemu-user-static复制依赖呢？README中也介绍过这种使用方法，这个应该和--privileged没有关系的，只是从其中复制文件，并不需要运行容器。
第二个问题我觉得差异在于，使用globs通配符，当文件不存在时会被忽略，不会报错。而直接写完整路径，如果文件不存在，会build失败。
想到这里我怀疑之所以有这两个设计，是不是为了把复制qemu依赖变成一个可选步骤。因为在x86_64上构建x86_64的镜像，其实是不需要qemu的，事实上，也没有multiarch/qemu-user-static:x86_64这样的镜像。
所以如果按照我的说法来做，就需要对 x86_64 和其它需要qemu的架构，分开成两个 Dockerfile 文件。而这两个看似有缺点的设计，却能够让所有架构包括x86_64自身都使用同样的Dockerfile来构建。
以上是我的想法，只是想核实一下我的理解是否正确。请不吝赐教。
另外，我已经提交commit把qemu的顺序提到 apk add 前面了，如果没有别的问题，这个PR应该可以合并了。
There was a problem hiding this comment.
@zhanghongtong Docker CI checks are all success. Please let me know if anything I can do.
There was a problem hiding this comment.
Hi, @PMExtra
第一个问题我想问的是：目前我们应该是在make脚本中下载qemu的依赖到tmp目录，然后build时从tmp目录复制进去。但我们为什么不直接从multiarch/qemu-user-static复制依赖呢？README中也介绍过这种使用方法，这个应该和--privileged没有关系的，只是从其中复制文件，并不需要运行容器。
I think this is a good idea, can you help us verify that this is work? If successful, we don’t need to download qemu anymore
第二个问题我觉得差异在于，使用globs通配符，当文件不存在时会被忽略，不会报错。而直接写完整路径，如果文件不存在，会build失败。
I have tried this point, even if wildcards are used, an error will still be reported when the file does not exist
|
|
||
| RUN ln -s /opt/emqx/bin/* /usr/local/bin/ | ||
| RUN apk add --no-cache curl ncurses-libs openssl sudo libstdc++ | ||
| RUN ln -s /opt/emqx/bin/* /usr/local/bin/ |
There was a problem hiding this comment.
Sort the build stages: run apk add first for more efficient cache.
There was a problem hiding this comment.
HI, @PMExtra
Just like above, the COPY must be before the RUN
| ## EMQ docker image start script | ||
| # Huang Rui <[email protected]> | ||
| # EMQ X Team <[email protected]> | ||
|
|
||
| ## Shell setting | ||
| if [[ ! -z "$DEBUG" ]]; then | ||
| if [[ -n "$DEBUG" ]]; then |
There was a problem hiding this comment.
| set -ex | ||
| else | ||
| set -e | ||
| fi | ||
|
|
||
| shopt -s nullglob |
There was a problem hiding this comment.
For globs loop.
It's used to enumerate plugins config, related https://github.com/koalaman/shellcheck/wiki/SC2045
| ## Local IP address setting | ||
|
|
||
| LOCAL_IP=$(hostname -i |grep -E -oh '((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])'|head -n 1) | ||
| LOCAL_IP=$(hostname -i | grep -oE '((25[0-5]|(2[0-4]|1[0-9]|[1-9]|)[0-9])\.){3}(25[0-5]|(2[0-4]|1[0-9]|[1-9]|)[0-9])' | head -n 1) |
There was a problem hiding this comment.
Improve ip regex. (https://stackoverflow.com/a/36760050/6401252)
The grep -h flag is redundant because it read input from stdin instead of file.
|
|
||
| ## EMQ Base settings and plugins setting | ||
| # Base settings in /opt/emqx/etc/emqx.conf | ||
| # Plugin settings in /opt/emqx/etc/plugins | ||
|
|
||
| _EMQX_HOME="/opt/emqx" | ||
| _EMQX_HOME='/opt/emqx' |
There was a problem hiding this comment.
Format: quote raw string in single quotes rather than double quotes.
|
|
||
| if [[ -z "$EMQX_NAME" ]]; then | ||
| export EMQX_NAME="$(hostname)" | ||
| EMQX_NAME="$(hostname)" | ||
| export EMQX_NAME |
There was a problem hiding this comment.
| fi | ||
| export EMQX_HOST |
There was a problem hiding this comment.
Remove unnecessary variables, merge export at last.
| # echo value of $VAR hiding secrets if any | ||
| # SYNOPSIS | ||
| # echo_value KEY VALUE | ||
| echo_value() { |
There was a problem hiding this comment.
Prevent use global variable across functions.
Explicit pass arguments to echo_value.
| # get MASK_CONFIG | ||
| MASK_CONFIG_FILTER="$MASK_CONFIG_FILTER|password|passwd|key|token|secret" | ||
| FORMAT_MASK_CONFIG_FILTER=$(echo $MASK_CONFIG_FILTER |sed -e "s/^[^A-Za-z0-9_]\{1,\}//g"|sed -e "s/[^A-Za-z0-9_]\{1,\}/\|/g") | ||
|
|
||
| FORMAT_MASK_CONFIG_FILTER=$(echo "$MASK_CONFIG_FILTER" | sed -r -e 's/^[^A-Za-z0-9_]+//' -e 's/[^A-Za-z0-9_]+$//' -e 's/[^A-Za-z0-9_]+/|/g') |
There was a problem hiding this comment.
Improve regex and trim unexpected characters from both head and tail.
| # check if contains sensitive value | ||
| if [ ! -z $(echo $(echo $VAR_NAME | tr '.' ' ') |grep -w -o -E "$FORMAT_MASK_CONFIG_FILTER") ]; then | ||
| echo "$VAR_NAME=***secret***" | ||
| if echo "$key" | grep -iqwE "$FORMAT_MASK_CONFIG_FILTER"; then |
There was a problem hiding this comment.
https://github.com/koalaman/shellcheck/wiki/SC2143
add -i for ignore case.
|
I closed and reopened this pr for re-trigger GitHub actions. |
|
I've tried 3 times of CI check. But it would be cancelled after several minutes seems like randomly. |
|
@zmstone Please review it. |
|
Thanks. I'll have a look later. |
Fix qemu.
Fix bug.
* chore(docker): Delete stale code The sed command substitution of bbmustache style template is stale emqx no longer releases an unrendered config template * Refactor docker-entrypoint.sh.
* chore(docker): Delete stale code The sed command substitution of bbmustache style template is stale emqx no longer releases an unrendered config template * Refactor docker-entrypoint.sh.
Related #612