-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
Conversation
@steveperry-53 can you please take a look at it and merge if everything looks good. |
Deploy preview ready! Built with commit 4a24de1 https://deploy-preview-5465--kubernetes-io-master-staging.netlify.com |
@Bradamant3 can you please take a look at it and merge if everything looks good. |
@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. |
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.
Should be "Create a StorageClass"?
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.
Fixed!
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.
Does this second example also encompass the 1st? Can you combine the two?
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.
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. |
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.
StorageClass
If the datastore is not specified, then the volume will be....
vSphere
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.
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. |
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.
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.
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.
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.
4863899
to
67d2028
Compare
@msau42 : I made the changes as per your review comments. Please take a look. |
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.
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 |
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.
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.
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.
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. |
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.
add a comma: "...class,
which in this case is `VSANDatastore`."
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.
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. |
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 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"`. |
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.
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. |
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.
add a space: "Vsphere Infrastructure
(VI) Admins will..."
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.
Fixed!
@BaluDontu @thockin Is this supposed to be in the 1.8 release? |
@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. |
lgtm from tech |
@BaluDontu The formatting still doesn't look right when you check the preview: https://deploy-preview-5465--kubernetes-io-master-staging.netlify.com/docs/concepts/storage/persistent-volumes/#vsphere |
@BaluDontu If you check the "Allow maintainers to edit" checkbox, I can fix it for you. |
@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? |
@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. |
…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)
Update documentation for vSphere persistent volume support inside kubernetes.
This change is