Fix permission problems in docker by FrankChen021 · Pull Request #11299 · apache/druid

FrankChen021 · 2021-05-25T10:55:19Z

Description

To fix 11278, mv instruction is used to replace original ln instruction to eliminate symlink so that the docker image works with AWS Fargate. This change has been verified with the help of @mattmassicotte , reporter of that issue.

To fix 11298, /opt/shared is created in the Dockerfile so that a named volume could be used in the docker-compose. And a named volume druid_shared is used to share segments and task log files among Druid services

Because the problems we have encountered are platform independent, I test these changes on 3 different platform following the steps below

Test Steps

Execute command docker volume prune to clean up previous volumes
Launch the example docker-compose
Start an ingestion task | Task Successes
Task successes

Test Result

Host OS	Host OS Version	Docker Versoin	Test Result
CentOS	7.2.1511	20.10.6	OK
Ubuntu	20.04.2	20.10.2	OK
macOS	11.3	3.3.1	OK

This PR has:

been self-reviewed.
- using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
added documentation for new or modified features or behaviors.
added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
added or updated version, license, or notice information in licenses.yaml
added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
added integration tests.
been tested in a test Druid cluster.

jihoonson · 2021-05-26T08:24:49Z

@FrankChen021 thank you for making this PR. I tested your change on my machine, but, unfortunately, ingestion still fails with the same error. I'm using linux mint 20.1, which is based on Ubuntu Focal. Should we update the docker-compose.yml file per your suggestion in this PR instead of documenting it?

FrankChen021 · 2021-05-26T08:31:42Z

@jihoonson I don't know why host directory is used in the original docker-compose.yml, and I'm waiting for an answer from @clintropolis . If there's no special reason, I think it's acceptable to use a named volume in the example compose file. Before that, we have to execute a command to set the permission for linux systems.

FrankChen021 · 2021-05-26T08:41:46Z

@jihoonson will the ingestion success after executing the following command ?

sudo docker exec --user root middlemanager chown druid:druid /opt/data

clintropolis · 2021-05-27T06:49:56Z

@jihoonson I don't know why host directory is used in the original docker-compose.yml, and I'm waiting for an answer from @clintropolis . If there's no special reason, I think it's acceptable to use a named volume in the example compose file. Before that, we have to execute a command to set the permission for linux systems.

Sorry for the delay, as long as the same volume for storage is shared and available to the historical, middle manager, and overlord containers then everything should be ok. The important part is that it is shared between containers, because the storage directory is used as deep storage for the example docker cluster, so the segments and task logs that the peons running in the middle manager publish can be then read by the overlord and historical. I don't think it matters too much however this happens.

FrankChen021 · 2021-05-27T09:02:13Z

@clintropolis A named volume in docker-compose also shares contents among overlords, middle managers and historicals. At first I thought the reason why the host directory 'storge' is used has something to do with the disk size at first. Because by default, docker creates directory for a named volume under /var which usually has smaller disk size for druid data storage.

Since there's no special reasons to use a directory on the host OS to mount as a volume in docker, I think it's acceptable for us to use a named volume in the example docker-compose.yml for data storage, so that there won't be permission problems on Linux platforms. Another reason is that the docker-compose.yml is only an example, it's not recommended in production, the disk size problem of a named volume should not be a concern for us if we state it in the doc.

What do you think ? @clintropolis @jihoonson @xvrl
If you agree, I will push a new commit.

clintropolis · 2021-05-27T09:42:00Z

I think anything that makes the quick start work across platforms is agreeable; it is not very large data so disk size shouldn't matter afaik.

xvrl · 2021-05-28T05:52:03Z

@@ -58,7 +58,10 @@ COPY --chown=druid:druid --from=builder /opt /opt
 COPY distribution/docker/druid.sh /druid.sh
 RUN mkdir /opt/druid/var \


Suggested change

RUN mkdir /opt/druid/var \

RUN mkdir /opt/druid/var /opt/data \

xvrl · 2021-05-28T05:52:26Z

 RUN mkdir /opt/druid/var \
 && chown druid:druid /opt/druid/var \
- && chmod 775 /opt/druid/var
+ && chmod 775 /opt/druid/var \


Suggested change

&& chmod 775 /opt/druid/var \

&& chmod 775 /opt/druid/var /opt/data \

xvrl · 2021-05-28T05:52:42Z

@@ -58,7 +58,10 @@ COPY --chown=druid:druid --from=builder /opt /opt
 COPY distribution/docker/druid.sh /druid.sh
 RUN mkdir /opt/druid/var \
 && chown druid:druid /opt/druid/var \


Suggested change

&& chown druid:druid /opt/druid/var \

&& chown druid:druid /opt/druid/var /opt/data \

xvrl · 2021-05-28T06:04:11Z

 && chown druid:druid /opt/druid/var \
- && chmod 775 /opt/druid/var
+ && chmod 775 /opt/druid/var \
+ && mkdir /opt/data \


can we add a comment to explain the difference between /opt/data and /opt/druid/var ?

xvrl · 2021-05-28T06:08:32Z

-The example `docker-compose.yml` will create a container for each Druid service, as well as Zookeeper and a PostgreSQL container as the metadata store. Deep storage will be a local directory, by default configured as `./storage` relative to your `docker-compose.yml` file, and will be mounted as `/opt/data` and shared between Druid containers which require access to deep storage. The Druid containers are configured via an [environment file](https://github.com/apache/druid/blob/{{DRUIDVERSION}}/distribution/docker/environment).
+The example `docker-compose.yml` will create a container for each Druid service, as well as Zookeeper and a PostgreSQL container as the metadata store. 
+
+It will also create a named volumes `druid-data`, which is mounted as `opt/data` in container, as deep storage to keep and share segments and task logs among Druid services.


I feel druid-data and /opt/data is a little generic and doesn't really convey the idea that this is meant for shared storage. /opt/druid/var is also used for local "data" so the distinction is not very clear.

Maybe we should give it a more descriptive name such as /opt/shared, or something else if you have better suggestions, wdyt?

FrankChen021 · 2021-05-28T11:18:01Z

Thanks @xvrl , I have submitted a new commit to resolve all your comments. Test result will be updated in this weekend.

clintropolis

latest changes lgtm, will try to test soon

jihoonson

@FrankChen021 thanks. +1 after CI. The quick start ingestion works as it is after applying this change.

As I mentioned in the dev thread, it seems to me that the real problem is this docker image is not being tested properly. I'm not sure why the tests we have today don't catch this error. Do you have any plan for follow-ups to investigate this issue and add more tests if necessary?

FrankChen021 · 2021-05-29T16:08:52Z

As I mentioned in the dev thread, it seems to me that the real problem is this docker image is not being tested properly. I'm not sure why the tests we have today don't catch this error.

I have the same question as you. I will check it when I'm free.

FrankChen021 · 2021-05-29T16:12:14Z

I tested these changes on 3 different host OS, all worked. See the description of this PR.

* Create /opt/data to fix permission problem * eliminate symlink to avoid compatibility problem on AWS Fargate * Add a workaround section * Update instruction for named volume * Use named volume in docker-compose * Revert some doc change * Resolve review comments

* Create /opt/data to fix permission problem * eliminate symlink to avoid compatibility problem on AWS Fargate * Add a workaround section * Update instruction for named volume * Use named volume in docker-compose * Revert some doc change * Resolve review comments Co-authored-by: frank chen <frank.chen021@outlook.com>

FrankChen021 added 3 commits May 25, 2021 17:24

Create /opt/data to fix permission problem

eafe977

eliminate symlink to avoid compatibility problem on AWS Fargate

4c1556a

Add a workaround section

60038ad

FrankChen021 added the Docker https://hub.docker.com/r/apache/druid label May 25, 2021

FrankChen021 requested review from clintropolis, jihoonson and xvrl May 25, 2021 10:55

FrankChen021 linked an issue May 25, 2021 that may be closed by this pull request

storage directory permission problem when using the example docker-compose.yml #11298

Closed

jihoonson added the Bug label May 25, 2021

Update instruction for named volume

be1336a

FrankChen021 added 2 commits May 28, 2021 09:20

Use named volume in docker-compose

28e4b80

Revert some doc change

51ba67d

xvrl reviewed May 28, 2021

View reviewed changes

Resolve review comments

c8a1229

clintropolis reviewed May 28, 2021

View reviewed changes

clintropolis added this to the 0.21.1 milestone May 28, 2021

jihoonson approved these changes May 29, 2021

View reviewed changes

clintropolis approved these changes Jun 2, 2021

View reviewed changes

clintropolis merged commit d5139c9 into apache:master Jun 2, 2021

clintropolis mentioned this pull request Jun 2, 2021

[Backport] Fix permission problems in docker #11321

Merged

FrankChen021 deleted the data_dir_permission branch June 2, 2021 01:49

clintropolis mentioned this pull request Jun 3, 2021

[Draft] 0.21.1 Release Notes #11249

Closed

xvrl mentioned this pull request Jun 2, 2023

OBSDATA-1354: Druid 25 with confluent patches confluentinc/druid#136

Merged

		@@ -58,7 +58,10 @@ COPY --chown=druid:druid --from=builder /opt /opt
		COPY distribution/docker/druid.sh /druid.sh
		RUN mkdir /opt/druid/var \

	RUN mkdir /opt/druid/var \
	RUN mkdir /opt/druid/var /opt/data \

	&& chmod 775 /opt/druid/var \
	&& chmod 775 /opt/druid/var /opt/data \

	&& chown druid:druid /opt/druid/var \
	&& chown druid:druid /opt/druid/var /opt/data \

Conversation

FrankChen021 commented May 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test Steps

Test Result

Uh oh!

jihoonson commented May 26, 2021

Uh oh!

FrankChen021 commented May 26, 2021

Uh oh!

FrankChen021 commented May 26, 2021

Uh oh!

clintropolis commented May 27, 2021

Uh oh!

FrankChen021 commented May 27, 2021

Uh oh!

clintropolis commented May 27, 2021

Uh oh!

xvrl May 28, 2021

Choose a reason for hiding this comment

Uh oh!

xvrl May 28, 2021

Choose a reason for hiding this comment

Uh oh!

xvrl May 28, 2021

Choose a reason for hiding this comment

Uh oh!

xvrl May 28, 2021

Choose a reason for hiding this comment

Uh oh!

xvrl May 28, 2021

Choose a reason for hiding this comment

Uh oh!

FrankChen021 commented May 28, 2021

Uh oh!

clintropolis left a comment

Choose a reason for hiding this comment

Uh oh!

jihoonson left a comment

Choose a reason for hiding this comment

Uh oh!

FrankChen021 commented May 29, 2021

Uh oh!

FrankChen021 commented May 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

FrankChen021 commented May 25, 2021 •

edited

Loading