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

Update local provisioner docs for 3.10 #8899

Merged
merged 1 commit into from
May 21, 2018
Merged

Update local provisioner docs for 3.10 #8899

merged 1 commit into from
May 21, 2018

Conversation

jsafrane
Copy link
Contributor

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 25, 2018
qinpingli
qinpingli reviewed Apr 25, 2018
View reviewed changes
install_config/configuring_local.adoc Outdated
----
<1> Name of the StorageClass.
<2> Path to the directory on the host. It must be a subdirectory of *_/mnt/local-storage_*.
<3> Path to the directory in the provisioner pod. We recommend using the same directory structure as used on the host.
<3> Path to the directory in the provisioner pod. We recommend using the same directory structure as used on the host and `mountDir` can be omitted in this case.

With this configuration, the provisioner creates:

* One PV with StorageClass `local-ssd` for every subdirectory in *_/mnt/local-storage/ssd_*.
* One PV with StorageClass `local-hdd` for every subdirectory in *_/mnt/local-storage/hdd_*.
Copy link

Choose a reason for hiding this comment

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

In openshift 3.10, local provisioner will create PV just for mountpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@qinpingli
Copy link

qinpingli commented Apr 25, 2018 •
edited
Loading

In openshift 3.10, we can remove the "Adding New Devices" part or change this part.

A new mountpoint can be created after local provisioner ds is running. But the PV still maybe not be created for permission denied.

So, the administrators need run the command again:
$ chcon -R unconfined_u:object_r:svirt_sandbox_file_t:s0 /mnt/local-storage/

@qinpingli
Copy link

Do we need add a part for how to using local volume from WEBUI in this PR?

@jsafrane jsafrane force-pushed the local-provisioner-3.10 branch from 0f7ec4b to 0c8f854 Compare April 25, 2018 14:13
@jsafrane
Copy link
Contributor Author

In openshift 3.10, we can remove the "Adding New Devices" part or change this part.

I kept it there to mention SELinux relabeling.

@jsafrane
Copy link
Contributor Author

TODO: add documentation about block devices

@jsafrane
Copy link
Contributor Author

TODO: add documentation about block devices

block devices are alpha upstream, so no documentation nor support

ghost
ghost reviewed Apr 30, 2018
View reviewed changes
install_config/configuring_local.adoc Outdated
@@ -27,37 +27,9 @@ The local volume provisioner allows using local storage within {product-title} a

[NOTE]
====
Local volumes is an alpha feature and may change in a future release of {product-title}.
Local volumes is a technical preview feature and may change in a future release of {product-title}.
Copy link

Choose a reason for hiding this comment

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

s/technical preview/Technology Preview

You should also add the IMPORTANT note: https://mojo.redhat.com/docs/DOC-1124368

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with IMPORTANT from the mojo page.

ghost
ghost reviewed Apr 30, 2018
View reviewed changes
install_config/configuring_local.adoc Outdated
The `LocalPersistentVolumes` alpha feature now also requires the `VolumeScheduling` alpha feature. This is a breaking change, and the following changes are required:
[WARNING]
====
Syntax of the ConfigMap has changed between {product-title} 3.9 and 3.10. Since this feature is in technical preview, the ConfigMap is not converted automatically during update.
Copy link

Choose a reason for hiding this comment

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

s/technical preview/Technology Preview

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

ghost
ghost reviewed Apr 30, 2018
View reviewed changes
install_config/configuring_local.adoc Outdated
@@ -189,13 +157,14 @@ The provisioner runs as root to be able to clean up the directories when a PV is

[[local-volume-adding-new-devices]]
=== Adding New Devices
Adding a new device requires several manual steps:
Adding new device is semi-automatic. The provisioner periodically checks for new mounts in configured directories. Admin just needs to create a new subdirectory there, mount a device there and allow pods to use the device by applying SELinux label on it:
Copy link

Choose a reason for hiding this comment

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

s/Adding new device/Adding a new device
s/Admin just needs to/The administrator needs to
s/mount a device there/mount a device there,
s/by applying SELinux label on it:/by applying the SELinux label:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link

Choose a reason for hiding this comment

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

Can you consider making the other suggested changes? Thanks.

@ghost
Copy link

ghost commented Apr 30, 2018

@jsafrane I suggested a few edits. Thanks.

@jsafrane jsafrane force-pushed the local-provisioner-3.10 branch from 0c8f854 to f64da18 Compare May 2, 2018 08:10
@jsafrane
Copy link
Contributor Author

jsafrane commented May 2, 2018

@tmorriso-rh, I applied you suggestions, PTAL

@ghost ghost mentioned this pull request May 2, 2018
Closed
@ghost ghost self-assigned this May 11, 2018
@ghost
Copy link

ghost commented May 18, 2018

@jsafrane I think this is ready to merge. Can you remove/squash the extra commit? We require all PRs to have only 1 commit before we can merge. By squashing your merge commits into one commit, you are making it easier for us to understand the scope of your work. Thanks.

@jsafrane
Local storage updates for 3.10
d2a449a 
- Fixed local provisioner config map syntax for 3.10
- Removed alpha features for local storage.
@jsafrane jsafrane force-pushed the local-provisioner-3.10 branch from f64da18 to d2a449a Compare May 21, 2018 07:11
@jsafrane
Copy link
Contributor Author

Squashed, PTAL

@ahardin-rh ahardin-rh merged commit 29d3f01 into openshift:master May 21, 2018
@ahardin-rh
Copy link
Contributor

/cherrypick enterprise-3.10

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request May 21, 2018
Merged
@openshift-cherrypick-robot
Copy link

@ahardin-rh: new pull request created: #9465

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.

@ahardin-rh ahardin-rh added this to the Future Release milestone May 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qinpingli qinpingli qinpingli left review comments

Assignees
No one assigned
Labels
branch/enterprise-3.10 size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
OCP 3.10 GA
Development

Successfully merging this pull request may close these issues.
6 participants
@jsafrane @qinpingli @ahardin-rh @openshift-cherrypick-robot @vikram-redhat @openshift-ci-robot