Skip to content
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.

Sign up for GitHub

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

Jump to bottom

BZ#1601757 - added considerations for using MySQL and PostgreSQL with azure file #10930

Merged
merged 1 commit into from Aug 5, 2018
Merged

BZ#1601757 - added considerations for using MySQL and PostgreSQL with azure file #10930

merged 1 commit into from Aug 5, 2018

Conversation

gaurav-nelson
Copy link
Contributor

For https://bugzilla.redhat.com/show_bug.cgi?id=1601757

  • Added as a topic inside Before you begin section.

@gaurav-nelson gaurav-nelson added this to the Next Release milestone Jul 20, 2018
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 20, 2018
@gaurav-nelson
Copy link
Contributor Author

@tanaka-takayoshi PTAL

@tanaka-takayoshi
Copy link

tanaka-takayoshi commented Jul 20, 2018 •
edited by gaurav-nelson

@gaurav-nelson It looks good for me. However, I'd like some MySQL/PostgreSQL expert to review it. If you know suitable persons, please let them know.
I'm also looking for them.
The point is:

  • Regarding MySQL container, is it suitable to run with directory permission 0777 and file permission 0744?
  • Regarding PostgreSQL, is creating a symbolic link necessary?

@tanaka-takayoshi
Copy link

@hhorak
Hi, I hear you're developing MySQL container. Could you review this document change?
I'd like to know the directory permission 0777 and file permission 0744 is suitable for MySQL container. Should we reduce the permission?

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 24, 2018
@openshift-docs-preview-bot
Copy link

openshift-docs-preview-bot commented Jul 24, 2018 •
edited by gaurav-nelson

The preview will be availble shortly at:

@hhorak
Copy link

hhorak commented Jul 24, 2018

@tanaka-takayoshi The data of the database shouldn't be readable by others, and also the executable flag is not necessary for files. The group read-write is used for cases when user specifies a random UID, docker sets GUID to 0. So, if this trick with GUID 0 is not necessary, I'd say what should be enough is directory permission 0700 and file permission 0600 -- at least that should be enough if the container runs with the same UID as the volume directory. I'm not able to verify though, whether this is working for the NFS/Azure use case, that's something way beyond my ability to check.

@tanaka-takayoshi
Copy link

@hhorak Thaks for that.
I just run a quick test with the following mount options and it works fine. So dir_mode=0700 and file_mode=0600 are enough when the directory/file owner is the same as the container process user.

  mountOptions:
  - dir_mode=0700
  - file_mode=0600
  - uid=(conatiner_process_uid)
  - gid=0

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 26, 2018
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 26, 2018
@gaurav-nelson
Copy link
Contributor Author

Thanks @tanaka-takayoshi @hhorak
I have made updates to the PR to include these permissions.

@gaurav-nelson gaurav-nelson added the peer-review-needed Signifies that the peer review team needs to review this PR label Jul 26, 2018
bfallonf
bfallonf reviewed Jul 27, 2018
View reviewed changes
install_config/persistent_storage/persistent_storage_azure_file.adoc Outdated
.Considerations when using MySQL and PostgresSQL with Azure file
* The owner UID of the Azure File mounted directory is different from the UID of a container.
* Both MySQL and PostgreSQL containers change the file owner permission in the mounted directory. Because of the mismatch between the owner UID and container process UID, this operation fails. Therefore to run MySQL with Azure File:
** Specify the UID in `runAsUser` variable in the pod specification file.
Copy link

Choose a reason for hiding this comment

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

s/in runAsUser variable/in the runAsUser variable

bfallonf
bfallonf reviewed Jul 27, 2018
View reviewed changes
install_config/persistent_storage/persistent_storage_azure_file.adoc Outdated
securityContext:
runAsUser: 1000125000
----
** Specify the `uid` in mount options:
Copy link

Choose a reason for hiding this comment

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

Is this also in the pod spec? And because consistency, maybe:

Specify the UID in the mountOptions stanza [in the pod specification file?]:

bfallonf
bfallonf reviewed Jul 27, 2018
View reviewed changes
install_config/persistent_storage/persistent_storage_azure_file.adoc Outdated
- gid=0
----

* Azure File does not support link:https://docs.microsoft.com/en-us/rest/api/storageservices/features-not-supported-by-the-azure-file-service[symbolic link]. Therefore when PostgreSQL creates a symbolic link in the Azure File directory, the pod fails to start.
Copy link

Choose a reason for hiding this comment

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

This makes it sound like it will happen and there's nothing you can do about it. Should it be "If PostgreSQL creates a symbolic link, the pod will fail"? Is there a way around this?

@bfallonf
Copy link

@gaurav-nelson I made some comments and asked some tough questions...

@kalexand-rh kalexand-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jul 27, 2018
@gaurav-nelson gaurav-nelson mentioned this pull request Jul 31, 2018
Closed
@vikram-redhat
Copy link
Contributor

@gaurav-nelson - added enterprise-3.11 as a reminder.

@gaurav-nelson
Copy link
Contributor Author

Updated for PostgreSql based on sclorg/postgresql-container#286 (comment)

@gaurav-nelson gaurav-nelson merged commit 9e7a7d3 into openshift:master Aug 5, 2018
@gaurav-nelson
Copy link
Contributor Author

/cherrypick enterprise-3.9

@gaurav-nelson
Copy link
Contributor Author

/cherrypick enterprise-3.10

@gaurav-nelson
Copy link
Contributor Author

/cherrypick enterprise-3.11

@openshift-cherrypick-robot
Copy link

@gaurav-nelson: new pull request created: #11375

In response to this:

/cherrypick enterprise-3.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Aug 5, 2018
Merged
@openshift-cherrypick-robot
Copy link

@gaurav-nelson: new pull request created: #11376

In response to this:

/cherrypick enterprise-3.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Aug 5, 2018
Merged
@openshift-cherrypick-robot
Copy link

@gaurav-nelson: new pull request created: #11377

In response to this:

/cherrypick enterprise-3.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Aug 5, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bfallonf bfallonf bfallonf left review comments

Assignees
No one assigned
Labels
branch/enterprise-3.9 branch/enterprise-3.10 branch/enterprise-3.11 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
Next Release
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@gaurav-nelson @tanaka-takayoshi @openshift-docs-preview-bot @hhorak @bfallonf @vikram-redhat @openshift-cherrypick-robot @openshift-bot @openshift-ci-robot @kalexand-rh