Skip to content

Docker: better clickhouse-server entrypoint#18954

Merged
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
filimonov:docker-entrypoint
Jan 14, 2021
Merged

Docker: better clickhouse-server entrypoint#18954
alexey-milovidov merged 2 commits intoClickHouse:masterfrom
filimonov:docker-entrypoint

Conversation

@filimonov
Copy link
Copy Markdown
Contributor

@filimonov filimonov commented Jan 11, 2021 •
edited
Loading

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Docker image: several improvements for clickhouse-server entrypoint.

Detailed description / Documentation draft:

  1. detect if folders are accessible instead of checking owners (which can vary and do vary)
  2. die fast in the case when the config is not readable
  3. die on errors (fixes clickhouse-server docker initdb script sourcing ignores errors #10183 )
  4. use the same entrypoint for ubuntu-based and alpine-based containers:
    • rewrite wget a readiness check to be compatible with alpine
    • add bash to alpine
    • autodetect gosu vs su-exec
  5. nullglob to skip '*' in the empty folder
  6. shellcheck fixes.

Images with those changes are available here

filimonovq/clickhouse-server:21.1.1.5643
filimonovq/clickhouse-server:21.1.1.5643-alpine

P.S. maybe we should start publishing alpine-based images?

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Jan 11, 2021
stigsb
stigsb reviewed Jan 12, 2021
View reviewed changes
docker/server/entrypoint.sh
fi
# will try to send ping clickhouse via http_port (max 12 retries by default, with 1 sec timeout and 1 sec delay between retries)
tries=${CLICKHOUSE_INIT_TIMEOUT:-12}
while ! wget --spider -T 1 -q "http://127.0.0.1:$HTTP_PORT/ping" 2>/dev/null; do
Copy link
Copy Markdown
Contributor

@stigsb stigsb Jan 12, 2021

Choose a reason for hiding this comment

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

Probably don't need --spider option anymore now?

Copy link
Copy Markdown
Contributor Author

@filimonov filimonov Jan 12, 2021

Choose a reason for hiding this comment

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

Well --spider just means 'it will not download the pages, just check that they are there'. We still don't need to store 'ok' responce in some file, so i think we can leave it. Alternative is to redirect output to stderr/stdout/ or /dev/null.

@alexey-milovidov alexey-milovidov self-assigned this Jan 14, 2021
@alexey-milovidov alexey-milovidov merged commit fc5e09d into ClickHouse:master Jan 14, 2021
@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Jan 14, 2021 •
edited
Loading

P.S. maybe we should start publishing alpine-based images?

@filimonov Are there any reasons to not use it as default and the only option?

@filimonov
Copy link
Copy Markdown
Contributor Author

filimonov commented Jan 14, 2021

Compatibility issues.

  1. Some people (may be even some CI tools) can build their own containers based on the official own relying on the ubuntu commands and features which work differently on alpine.

For example imagine they install some odbc drivers using apt-get commands.

  1. Also (ust came to my mind right now) - uid of clickhouse user differs in alpine and in clickhouse.
docker run -it filimonovq/clickhouse-server:21.1.1.5643 bash
root@d6b4c588d7cf:/# id clickhouse
uid=999(clickhouse) gid=999(clickhouse) groups=999(clickhouse)

docker run -it filimonovq/clickhouse-server:21.1.1.5643-alpine bash
bash-5.0# id clickhouse 
uid=100(clickhouse) gid=1000(clickhouse) groups=1000(clickhouse),1000(clickhouse)

That is not critical but maybe a bad surprise #18927 (comment)

BTW - maybe we should create clickhouse user in the Dockerfile with fixed uid/gid ahead / before deb installallation will to it and will pick 'some' free uid for the user automatically?

UPD1. Others do that - see:
https://github.com/docker-library/postgres/blob/03e769531fff4c97cb755e4a608b24935ceeee27/13/Dockerfile#L14-L18
https://github.com/docker-library/mysql/blob/b0a0e546712534b30ac13e8b2829f23654facbc5/8.0/Dockerfile.debian#L9-L10

UPD2. I will send one more PR fixing that.

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Jan 14, 2021

Some people (may be even some CI tools)

We can keep scripts for Ubuntu container but publish Alpine container as the official.

BTW - maybe we should create clickhouse user in the Dockerfile with fixed uid/gid ahead / before deb installallation

Yes, we can tune Install.cpp for it.

@filimonov
Copy link
Copy Markdown
Contributor Author

filimonov commented Jan 14, 2021 •
edited
Loading

One more thing to change - prepare clickhouse-client image for alpine (BTW - as for me that separate client image is a bit useless, as you can use clickhouse-server for everything).

@alexey-milovidov
Copy link
Copy Markdown
Member

alexey-milovidov commented Jan 14, 2021

Yes, and we can rename it to just clickhouse.

@filimonov filimonov deleted the docker-entrypoint branch January 14, 2021 12:19
@filimonov
Copy link
Copy Markdown
Contributor Author

filimonov commented Jan 14, 2021

p.2 : #19096

This was referenced Jan 20, 2021
Closed
Merged
@naylinn69 naylinn69 mentioned this pull request Mar 24, 2021
Closed
@filimonov filimonov mentioned this pull request Mar 24, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexey-milovidov alexey-milovidov alexey-milovidov approved these changes

+1 more reviewer

@stigsb stigsb stigsb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@alexey-milovidov alexey-milovidov

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

clickhouse-server docker initdb script sourcing ignores errors

4 participants

@filimonov @alexey-milovidov @stigsb @robot-clickhouse