-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[improve][build] Add a default username in the image #21695
Conversation
--- ### Motivation Add a default username in the pulsar image. When using HDFS offloader, it requires a username to transfer the file.
@zymap Please add the following content to your PR description and select a checkbox:
|
Could you use the ARG to define the default user? |
Can you provide more information about this requirement? Given that OpenShift doesn't use the username, it means the HDFS offloader won't work correctly there. |
How about using |
@michaeljmarshall
|
@lhotari Seems it doesn't work... I think It should be okay to add a username for the existing user ID. Do you have any concerns? |
/pulsarbot rerun-failure-checks |
The OpenShift does not mention the username.
And I also tried the postgres images they mentioned here https://docs.openshift.com/container-platform/3.11/using_images/db_images/postgresql.html. It also has the username
So it should be okay to add the username by default? |
I think openshift doesn't like uid/gid in docker image. Openshift uses the uid/gid to map SELinux context to different syscap on the host.
Should work in openshift. https://www.redhat.com/en/blog/a-guide-to-openshift-and-uids |
@yuweisung OpenShift ignores the UID in the image. In Openshift, the container user is a member of the root group and that's the only thing that matters. This is explained in https://www.redhat.com/en/blog/a-guide-to-openshift-and-uids .
|
@zymap Yes, it's fine to have a username for the existing user ID. Technically having a username only means that there's a line in the /etc/passwd file that matchers the UID. It's not more than that. The changes in this PR LGTM now. |
@lhotari Thank you for your fix! I pushed failed, then found you already fixed it. |
@zymap The concern was already raised by @michaeljmarshall when he said "Given that OpenShift doesn't use the username, it means the HDFS offloader won't work correctly there.". How did you set |
@zymap This is how the user name is set in Hadoop Java library: https://stackoverflow.com/a/16788971 |
Just export HADOOP_USER_NAME=xxx, then run the HDFS offloader follow this https://pulsar.apache.org/docs/next/tiered-storage-filesystem/ |
@zymap this issue isn't resolved. The solution in this PR doesn't work on OpenShift. |
@lhotari |
@zymap did you look at https://stackoverflow.com/a/16788971 ? Instead of relying on the OS user, the Java API of the Hadoop library should be used to configure the remote user. |
Oh. I misunderstood. Let me try to set it in the code to see if it works. |
### Motivation Add a default username in the pulsar image. When using HDFS offloader, it requires a username to transfer the file. (cherry picked from commit d5f0097)
### Motivation Add a default username in the pulsar image. When using HDFS offloader, it requires a username to transfer the file. (cherry picked from commit d5f0097)
Fixes #xyz
Main Issue: #xyz
PIP: #xyz
Motivation
Add a default username in the pulsar image. When using HDFS offloader, it requires a username to transfer the file.
Modifications
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: