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.

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

Update documentation for vSphere Persistent Volume support #5465

Merged
merged 6 commits into from
Oct 2, 2017

Conversation

BaluDontu
Copy link
Contributor

@BaluDontu BaluDontu commented Sep 15, 2017

Update documentation for vSphere persistent volume support inside kubernetes.


This change is Reviewable

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 15, 2017
@BaluDontu
Copy link
Contributor Author

@steveperry-53 can you please take a look at it and merge if everything looks good.

@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Sep 15, 2017

Deploy preview ready!

Built with commit 4a24de1

https://deploy-preview-5465--kubernetes-io-master-staging.netlify.com

@BaluDontu
Copy link
Contributor Author

@Bradamant3 can you please take a look at it and merge if everything looks good.

@thockin
Copy link
Member

thockin commented Sep 25, 2017

@Bradamant3 Can you take a look at this?


2. Create a persistent volume with a disk format on a user specified datastore.
2. Create a persistent volume with a disk format on a user specified datastore.
Copy link
Member

Choose a reason for hiding this comment

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

Should be "Create a StorageClass"?

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
Member

Choose a reason for hiding this comment

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

Does this second example also encompass the 1st? Can you combine the two?

Copy link
Member

Choose a reason for hiding this comment

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

Also this second example should be "Create a StorageClass" too

datastore: VSANDatastore
```
- `diskformat`: `thin`, `zeroedthick` and `eagerzeroedthick`. Default: `"thin"`.
- `datastore`: The user can also specify the datastore in the Storageclass. The volume will be created on the datastore specified in the storage class which in this case is `VSANDatastore`. This field is optional. If not specified as in previous YAML description, the volume will be created on the datastore specified in the vsphere config file used to initialize the vSphere Cloud Provider.
Copy link
Member

Choose a reason for hiding this comment

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

StorageClass

If the datastore is not specified, then the volume will be....

vSphere

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!

```
One of the most important features of vSphere for Storage Management is Policy based Management. Storage Policy Based Management (SPBM) is a storage policy framework that provides a single unified control plane across a broad range of data services and storage solutions. SPBM enables vSphere administrators to overcome upfront storage provisioning challenges, such as capacity planning, differentiated service levels and managing capacity headroom

As we discussed in previously StorageClass specifies provisioner and parameters. And using these parameters you can define the policy for that particular PV which will be dynamically provisioned.
Copy link
Member

Choose a reason for hiding this comment

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

I think these two lines can be replaced with: "The SPBM policies can be specified in the StorageClass using the storagePolicyName parameter". And then show an example.

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. For the example, I don't want to give right here rather would want to point to our documentation. This way we don't need to modify at multiple places.

@BaluDontu
Copy link
Contributor Author

@msau42 : I made the changes as per your review comments. Please take a look.

Copy link
Contributor

@chenopis chenopis left a comment

Choose a reason for hiding this comment

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

Please fix the formatting issues and minor grammar suggestions.

cacheReservation: "20"
datastore: VSANDatastore
```
One of the most important features of vSphere for Storage Management is Policy based Management. Storage Policy Based Management (SPBM) is a storage policy framework that provides a single unified control plane across a broad range of data services and storage solutions. SPBM enables vSphere administrators to overcome upfront storage provisioning challenges, such as capacity planning, differentiated service levels and managing capacity headroom
Copy link
Contributor

Choose a reason for hiding this comment

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

In the first sentence, "policy based management" should all be in lowercase since it's not a particular name. The capitalization in the second sentence is okay because SPBM is the name of the framework.

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!

datastore: VSANDatastore
```
- `diskformat`: `thin`, `zeroedthick` and `eagerzeroedthick`. Default: `"thin"`.
- `datastore`: The user can also specify the datastore in the StorageClass. The volume will be created on the datastore specified in the storage class which in this case is `VSANDatastore`. This field is optional. If the datastore is not specified, then the volume will be created on the datastore specified in the vSphere config file used to initialize the vSphere Cloud Provider.
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comma: "...class, which in this case is `VSANDatastore`."

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!

@@ -573,68 +573,51 @@ parameters:

#### vSphere

1. Create a persistent volume with a user specified disk format.
1. Create a StorageClass with a user specified disk format.
Copy link
Contributor

Choose a reason for hiding this comment

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

The bullet formatting is now messed up. See https://deploy-preview-5465--kubernetes-io-master-staging.netlify.com/docs/concepts/storage/persistent-volumes/#vsphere

Do not indent the bullet numbers.

diskformat: zeroedthick
datastore: VSANDatastore
```
- `diskformat`: `thin`, `zeroedthick` and `eagerzeroedthick`. Default: `"thin"`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these unordered bullets under the second bullet point? You probably need to fix the formatting for this.


vSphere Infrastructure(VI) administrator can specify storage requirements for applications in terms of storage capabilities while creating a storage class inside Kubernetes. Please note that while creating a StorageClass, administrator should specify storage capability names used in the table above as these names might differ from the ones used by VSAN. For example - Number of disk stripes per object is referred to as stripeWidth in VSAN documentation however vSphere Cloud Provider uses a friendly name diskStripes.
Vsphere Infrastructure(VI) Admins will have the ability to specify custom Virtual SAN Storage Capabilities during dynamic volume provisioning. You can now define storage requirements, such as performance and availability, in the form of storage capabilities during dynamic volume provisioning. The storage capability requirements are converted into a Virtual SAN policy which are then pushed down to the Virtual SAN layer when a persistent volume (virtual disk) is being created. The virtual disk is distributed across the Virtual SAN datastore to meet the requirements.
Copy link
Contributor

Choose a reason for hiding this comment

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

add a space: "Vsphere Infrastructure (VI) Admins will..."

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!

@chenopis
Copy link
Contributor

@BaluDontu @thockin Is this supposed to be in the 1.8 release?

@BaluDontu
Copy link
Contributor Author

BaluDontu commented Sep 29, 2017

@chenopis : I hope it looks good now. I have verified from the preview under files changed - https://github.com/BaluDontu/kubernetes.github.io/blob/d981be88bedd6e9014f8731e633aa370672191fc/docs/concepts/storage/persistent-volumes.md and it looks good.

I want this change to be in master and 1.8 release.

@msau42
Copy link
Member

msau42 commented Sep 29, 2017

lgtm from tech

@chenopis
Copy link
Contributor

chenopis commented Oct 2, 2017

@chenopis
Copy link
Contributor

chenopis commented Oct 2, 2017

@BaluDontu If you check the "Allow maintainers to edit" checkbox, I can fix it for you.

@BaluDontu
Copy link
Contributor Author

@chenopis : I have enabled the checkbox. You can fix it.

However its quite confusing that https://github.com/BaluDontu/kubernetes.github.io/blob/d981be88bedd6e9014f8731e633aa370672191fc/docs/concepts/storage/persistent-volumes.md and preview doesn't show the same. The file https://github.com/BaluDontu/kubernetes.github.io/blob/d981be88bedd6e9014f8731e633aa370672191fc/docs/concepts/storage/persistent-volumes.md shows correct formatting for me.

Should we have to wait for preview link to check for formatting issues?

@chenopis
Copy link
Contributor

chenopis commented Oct 2, 2017

@BaluDontu Yeah, unfortunately, the GitHub rendering is not the same as the actual website rendering, so it's best to check the Netlify preview. You can always use the link provided in the comment from the k8sio-netlify-preview-bot (see image below). And that gets updated if you add commits.

screen shot 2017-10-02 at 3 59 50 pm

@chenopis chenopis merged commit cb4a6dc into kubernetes:master Oct 2, 2017
cnef pushed a commit to ghostcloud-cn/kubernetes.github.io that referenced this pull request Oct 3, 2017
…hub.io into format-example-code

* 'master' of https://github.com/kubernetes/kubernetes.github.io:
  Remove broken link (kubernetes#5726)
  unified style (kubernetes#5725)
  Fixes in style and more on CDK (kubernetes#5292)
  Use code style for inline code and commands (kubernetes#5724)
  Fix link. (kubernetes#5723)
  Update distribute-credentials-secure.md (kubernetes#5169)
  Update documentation for vSphere Persistent Volume support (kubernetes#5465)
  Update _redirects
  Update scale-intro.html
  Update cluster-management.md (kubernetes#4980)
  ingress grammar/spelling updates (kubernetes#5721)
  Fix an incorrect flag in configure-upgrade-etcd.md (kubernetes#5304)
  Updated PR template for GH workaround (kubernetes#5719)
  separate commands from output (kubernetes#5718)
  explain how to enable heapster on minikube (kubernetes#5710)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants