Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0224 fix docker entrypoint env overrides #10027

Merged

Conversation

zmstone
Copy link
Member

@zmstone zmstone commented Feb 24, 2023

No description provided.

Prior to this change EMQX_NODE__NAME is ignored by docker entrypoint
script which will in turn set EQMX_NODE_NAME by resolving
the node name and domain name respectively.
@zmstone zmstone force-pushed the 0224-fix-docker-entrypoint-env-overrides branch from 67f6941 to 9bb5abf Compare February 24, 2023 15:01
@zmstone zmstone marked this pull request as ready for review February 24, 2023 16:17
@zmstone zmstone requested review from a team and Rory-Z as code owners February 24, 2023 16:17
Copy link
Member

@Rory-Z Rory-Z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add EMQX_NODE_NAME to etc/emqx.conf, because when the user executes bin/emqx ctl in the container, bin/emqx may not get the environment variables from docker-entrypoint.sh

EMQX_ZONES__DEFAULT__MQTT__MAX_PACKET_SIZE <--> zones.default.mqtt.max_packet_size
EMQX_DASHBOARD__DEFAULT_PASSWORD <--> dashboard.default_password
EMQX_NODE__COOKIE <--> node.cookie
EMQX_LISTENERS__SSL__default__ENABLE <--> listeners.ssl.default.enable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EMQX_LISTENERS__SSL__default__ENABLE <--> listeners.ssl.default.enable
EMQX_LISTENERS__SSL__DEFAULT__ENABLE <--> listeners.ssl.default.enable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is intentionally left lower case to demo that lower case letters also work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, maybe add some note, it's easy to get confused

@Rory-Z
Copy link
Member

Rory-Z commented Feb 27, 2023

Looks EMQX 4.4 has same issue

@zmstone
Copy link
Member Author

zmstone commented Feb 28, 2023

I think we should add EMQX_NODE_NAME to etc/emqx.conf, because when the user executes bin/emqx ctl in the container, bin/emqx may not get the environment variables from docker-entrypoint.sh

The node name is retrieved from parsing the ps -ef command output before executing the ctl command.

@zmstone
Copy link
Member Author

zmstone commented Feb 28, 2023

Looks EMQX 4.4 has same issue

For 4.4 the node name is retrieved from the latest vm.<time>.args file.
If the node is never initialized or not running when executing non-boot commands, the command is doomed to fail anyway.

@Rory-Z
Copy link
Member

Rory-Z commented Mar 1, 2023

I think we should add EMQX_NODE_NAME to etc/emqx.conf, because when the user executes bin/emqx ctl in the container, bin/emqx may not get the environment variables from docker-entrypoint.sh

The node name is retrieved from parsing the ps -ef command output before executing the ctl command.

When I create a EMQX container like this: docker run -d --name emqx -e EMQX_NODE_NAME="emqx@fake" emqx/emqx:5.0.14
check log, the EMQX looks good:

$ docker logs -f emqx
WARNING: Default (insecure) Erlang cookie is in use.
WARNING: Configure node.cookie in /opt/emqx/etc/emqx.conf or override from environment variable EMQX_NODE__COOKIE
WARNING: Use the same config value for all nodes in the cluster.
EMQX_RPC__PORT_DISCOVERY [rpc.port_discovery]: manual
EMQX_LOG__FILE_HANDLERS__DEFAULT__ENABLE [log.file_handlers.default.enable]: false
EMQX_LOG__CONSOLE_HANDLER__ENABLE [log.console_handler.enable]: true
EMQX_NODE__NAME [node.name]: emqx@fake
Listener ssl:default on 0.0.0.0:8883 started.
Listener tcp:default on 0.0.0.0:1883 started.
Listener ws:default on 0.0.0.0:8083 started.
Listener wss:default on 0.0.0.0:8084 started.
Listener http:dashboard on :18083 started.
EMQX 5.0.14 is running now!

the ps -ef looks is normal

emqx@73c6ca2d0ba5:/opt/emqx$ ps -ef |grep emqx
emqx           1       0  7 06:37 ?        00:00:25 emqx -Bd -spp true -A 4 -IOt 4 -SDio 8 -e 262144 -zdbbl 8192 -Q 1048576 -P 2097152 -- -root /opt/emqx -progname /opt/emqx/bin/emqx -- -home /home/emqx -- -noshell -noshell -noinput -boot /opt/emqx/releases/5.0.14/start -boot_var RELEASE_LIB /opt/emqx/lib -boot_var ERTS_LIB_DIR /opt/emqx/lib -mode embedded -config /opt/emqx/data/configs/app.2023.03.01.06.37.31.config -stdlib restricted_shell emqx_restricted_shell -kernel net_ticktime 120 -shutdown_time 30000 -pa data/patches -mnesia dump_log_write_threshold 5000 -mnesia dump_log_time_threshold 60000 -pa /opt/emqx/releases/5.0.14/consolidated -setcookie emqxsecretcookie -name emqx@fake -mnesia dir "/opt/emqx/data/mnesia/emqx@fake" -- -start_epmd false -epmd_module ekka_epmd -proto_dist ekka -- foreground -emqx_data_dir /opt/emqx/data --

but emqx ctl is not working:

$ docker exec -it emqx bash
emqx@73c6ca2d0ba5:/opt/emqx$ emqx ctl status
Node emqx@fake not responding to pings.
ERROR: node_is_not_running!
emqx@73c6ca2d0ba5:/opt/emqx$ emqx_ctl status
Node emqx@fake not responding to pings.
ERROR: node_is_not_running!

The EMQX opensource 4.4.14 has same issue

@zmstone
Copy link
Member Author

zmstone commented Mar 1, 2023

emqx@fake

in this example, emqx ctl actually managed to find the node name.
but failed to ping the node because fake is not a valid hostname.

@zmstone zmstone merged commit ceec7e9 into emqx:master Mar 1, 2023
@zmstone zmstone deleted the 0224-fix-docker-entrypoint-env-overrides branch March 1, 2023 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants