Skip to content

[Docker] Enable filesystem permission updates in derivative docker im… - #12956

Merged
merlimat merged 1 commit into
apache:masterfrom
michaeljmarshall:remove-unnecessary-volume
Nov 29, 2021
Merged

[Docker] Enable filesystem permission updates in derivative docker im…#12956
merlimat merged 1 commit into
apache:masterfrom
michaeljmarshall:remove-unnecessary-volume

Conversation

@michaeljmarshall

Copy link
Copy Markdown
Member

…ages

Master Issue: #11269 and #8242

Motivation

@roelrymenants points out in #8242:

On OpenShift the user id in the container is random. In this case, the /pulsar folder has problematic permissions. The VOLUME statement even locks these into place for derivative images.

I realized today that because we removed the standalone docker image in #11657, we can now removed the VOLUME command in the dockerfile and make it easier for end users to extend the pulsar docker image. I describe moving the VOLUME command here #8796 (comment).

I think I should be able to make the pulsar docker images OpenShift compliant and non-root by default in early December. Until then, it is still valuable to update docker image to make it easier for end users to extend the image.

Modifications

  • Remove the VOLUME directive from the pulsar dockerfile.

Verifying this change

Now that we removed the standalone docker image, there is no need for the VOLUME command.

Does this pull request potentially affect one of the following parts:

This is not a breaking change.

Documentation

  • no-need-doc

This is a minor feature that does not need documentation. I'll provide docs when I update the pulsar docker image to be non-root/OpenShift compliant.

@michaeljmarshall

Copy link
Copy Markdown
Member Author

@lhotari - PTAL

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Nov 24, 2021
@freeznet

Copy link
Copy Markdown
Contributor

@timmyyuan please help to review this PR when you have time, thanks.

@timmyyuan

Copy link
Copy Markdown
Contributor

LGTM

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lgtm

@merlimat merlimat added this to the 2.10.0 milestone Nov 29, 2021
@merlimat
merlimat merged commit 96d9067 into apache:master Nov 29, 2021
fxbing pushed a commit to fxbing/pulsar that referenced this pull request Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants