-
Notifications
You must be signed in to change notification settings - Fork 10.4k
[bitnami/seaweedfs] fix chown on volume-permissions (master & volume) #36386
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
Conversation
|
Thank you for initiating this pull request. We appreciate your effort. This is just a friendly reminder that signing your commits is important. Your signature certifies that you either authored the patch or have the necessary rights to contribute to the changes. You can find detailed information on how to do this in the “Sign your work” section of our contributing guidelines. Feel free to reach out if you have any questions or need assistance with the signing process. |
fad2e62 to
4e5a6bc
Compare
|
hi @carrodher |
fmulero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @ggtrd for bringing this issue to our attention and apply this fix.
I've just left a comment proposing a change, could you give it a glance?
| - | | ||
| find {{ .Values.master.persistence.mountPath }} -mindepth 1 -maxdepth 1 -not -name ".snapshot" -not -name "lost+found" | xargs chown -R {{ .Values.master.containerSecurityContext.runAsUser }}:{{ .Values.master.podSecurityContext.fsGroup }} | ||
| mkdir -p {{ .Values.master.persistence.mountPath }} | ||
| chown -R {{ .Values.master.containerSecurityContext.runAsUser }}:{{ .Values.master.podSecurityContext.fsGroup }} {{ .Values.master.persistence.mountPath }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is not needed and we could face issues with special volumes like lost+found and .snapshot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally agree, i've removed the find command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The line not needed was the chown, not the find. I've just created a follow up for this PR #36397
Signed-off-by: Geoffrey Gontard <geoffrey.gontard@cosmotech.com>
Signed-off-by: Bitnami Bot <bitnami.bot@broadcom.com>
|
Hey @fmulero i've removed the unwanted command as mentioned in your suggestion. I don't know if the failed job is a problem for you to go further? If yes, i'd like to understand what is the error |
Signed-off-by: Bitnami Bot <bitnami.bot@broadcom.com>
fmulero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The error was not related to your change, was an internal issue |
Description of the change
Fix "chown: missing operand after '1001:1001'" on master and volume
Benefits
After activated persistence, and when using volumePermissions, both master and volume pods are failing due to "chown: missing operand after '1001:1001'"
Also, it seems the volume "data-0" doesn't work (maybe SeaweedFS waits for "data" and not "data-0" ?), but i've solved this on my own by setting "data" in values since i need only one volume. This last point is not part of the PR, just wanted to inform you.
Possible drawbacks
None
Applicable issues
None
Additional information
None
Checklist
Chart.yamlaccording to semver. This is not necessary when the changes only affect README.md files.