Skip to content

Conversation

@bobfuru
Copy link
Contributor

@bobfuru bobfuru commented May 4, 2020

@bobfuru
Copy link
Contributor Author

bobfuru commented May 4, 2020

@liangxia or @qinpingli - PTAL

@openshift-docs-preview-bot

The preview will be available shortly at:

@qinpingli
Copy link

@duanwei33 Please help take a look at this.

@duanwei33
Copy link

@bobfuru

  1. The volume capacity size is not the same in PV(2Gi) and PVC(5Gi) definition.
  2. For VMDKs creating, I see vSphere doc refer to govc and vmkfstools, I'm not familiar with the vmware-vdiskmanager command, how can we get the datastore name after this command, which will be used in PV creating?
  3. For the command "$ vmkfstools -c /vmfs/volumes/DatastoreName/volumes/.vmdk", better to replace "DatastoreName" by "", or we can use the same name according the one PV used.

@bobfuru
Copy link
Contributor Author

bobfuru commented May 6, 2020

Thank you, @duanwei33. See my comments below:

  1. The volume capacity size is not the same in PV(2Gi) and PVC(5Gi) definition.

Good catch. I changed capacity to 1Gi in both places to match another example on the page.

  1. For VMDKs creating, I see vSphere doc refer to govc and vmkfstools, I'm not familiar with the vmware-vdiskmanager command, how can we get the datastore name after this command, which will be used in PV creating?

I'm also not familiar and did not take part in the original creation of this doc. IIUC, the user will only need to include [datastore-name] if they used the vmkfstools command. I made this more explicit in callout #4 by stating "If you used vmkfstools".

  1. For the command "$ vmkfstools -c /vmfs/volumes/DatastoreName/volumes/.vmdk", better to replace "DatastoreName" by "", or we can use the same name according the one PV used.

I changed it to <datastore-name> to match the <disk-name> variable at the end of the command.

With these changes, does this look good to you?

@duanwei33
Copy link

@bobfuru Thanks for the update, it looks good to me.

@bobfuru bobfuru added the peer-review-needed Signifies that the peer review team needs to review this PR label May 7, 2020
@lamek lamek 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 May 7, 2020
@lamek
Copy link

lamek commented May 7, 2020

LGTM

@bobfuru bobfuru merged commit c3d1ec6 into openshift:master May 7, 2020
@bobfuru
Copy link
Contributor Author

bobfuru commented May 7, 2020

/cherrypick enterprise-4.5

@openshift-cherrypick-robot

@bobfuru: new pull request created: #21917

Details

In response to this:

/cherrypick enterprise-4.5

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.

@bobfuru
Copy link
Contributor Author

bobfuru commented May 7, 2020

/cherrypick enterprise-4.4

@bobfuru
Copy link
Contributor Author

bobfuru commented May 7, 2020

/cherrypick enterprise-4.3

@bobfuru
Copy link
Contributor Author

bobfuru commented May 7, 2020

/cherrypick enterprise-4.2

@openshift-cherrypick-robot

@bobfuru: new pull request created: #21918

Details

In response to this:

/cherrypick enterprise-4.4

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

@bobfuru: new pull request created: #21919

Details

In response to this:

/cherrypick enterprise-4.3

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

@bobfuru: new pull request created: #21920

Details

In response to this:

/cherrypick enterprise-4.2

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.

@bobfuru bobfuru deleted the BZ1783437 branch May 7, 2020 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants