places vsphere volumes as per zones #74654

subramanian-neelakantan · 2019-02-27T10:49:44Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
This is a cherry-pick of PRs #72731 and PR #72687 to be merged into release-1.13. When this PR is merged, vSphere volumes will get placed as per zone requested in allowedTopologies property of a storage class. Also the newly created volume is labelled with the zone information. See the base PRs for more details.

Which issue(s) this PR fixes:

Fixes #73301
refers to issue #67703

Special notes for your reviewer:
All the existing vsphere e2e tests were run against this PR and found to be passing.

This change ensures that volumes get provisioned based on the zone information provided in allowedTopologies.

Storage class spec:
kind: StorageClass
apiVersion: storage.k8s.io/v1
metadata:
  name: fastpolicy1
provisioner: kubernetes.io/vsphere-volume
parameters:
    diskformat: zeroedthick
    storagePolicyName: vSAN Default Storage Policy
allowedTopologies:
- matchLabelExpressions:
  - key: failure-domain.beta.kubernetes.io/zone
    values:
    - zone1

PV creation Logs:

I0109 11:17:52.321372       1 vsphere.go:1147] Starting to create a vSphere volume with volumeOptions: &{CapacityKB:1048576 Tags:map[kubernetes.io/created-for/pvc/namespace:default kubernetes.io/created-for/pvc/name:pvcsc-1-policy kubernetes.io/created-for/pv/name:pvc-34650c12-1400-11e9-aef4-005056804cc9] Name:kubernetes-dynamic-pvc-34650c12-1400-11e9-aef4-005056804cc9 DiskFormat:zeroedthick Datastore: VSANStorageProfileData: StoragePolicyName:vSAN Default Storage Policy StoragePolicyID: SCSIControllerType: Zone:[zone1]}
...
I0109 11:17:59.430113       1 vsphere.go:1334] The canonical volume path for the newly created vSphere volume is "[vsanDatastore] 98db185c-6683-d8c7-bc55-0200435ec5da/kubernetes-dynamic-pvc-34650c12-1400-11e9-aef4-005056804cc9.vmdk"

This change also applies zone labels to vSphere Volumes automatically. The zone labels are visible on the PV:

$ kubectl get pv --show-labels
NAME           CAPACITY   ACCESSMODES   STATUS    CLAIM            REASON    AGE       LABELS
pv-abc                5Gi        RWO                   Bound     default/claim1                    46s       failure-domain.beta.kubernetes.io/region=VC1,failure-domain.beta.kubernetes.io/zone=cluster-1

Does this PR introduce a user-facing change?:

Applies zone labels to vSphere Volumes and honors allowedTopologies when provisioning.

/sig vmware
/sig storage
/sig api-machinery
/cc @vladimirvivien @frapposelli

k8s-ci-robot · 2019-02-27T10:49:51Z

Hi @subramanian-neelakantan. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

frapposelli · 2019-02-27T10:52:31Z

/ok-to-test

subramanian-neelakantan · 2019-02-27T12:48:40Z

/retest

frapposelli · 2019-02-28T14:22:39Z

/lgtm
/approve

/assign @tpepper

This is for the next 1.13 patch release

frapposelli · 2019-02-28T16:59:59Z

/assign @SandeepPissay @divyenpatel @deads2k @derekwaynecarr

PTAL

roycaihw · 2019-02-28T21:12:33Z

cc @cheftako

SandeepPissay · 2019-03-01T20:23:37Z

/approve

tpepper · 2019-03-01T22:38:05Z

In the future please use the cherry-pick automation.

As I read the description and saw the size/XL this came across as a feature. Then I looked at the resolved issue and it is marked as a feature. I need some convincing this is actually a cherry pick candidate.

subramanian-neelakantan · 2019-03-02T04:03:03Z

In the future please use the cherry-pick automation.

Will do. Thanks for letting me know the right process.

As I read the description and saw the size/XL this came across as a feature. Then I looked at the resolved issue and it is marked as a feature. I need some convincing this is actually a cherry pick candidate.

This cherry-pick contains two related PRs - #72731 and #72687 - both of which fix slightly different issues. vSphere cloud provider already implements the zones interface. With Zones support the nodes get zone labels as per the tags in vcenter inventory and users could provision pods to zones. However it did not work well when volumes were not placed in specific configurations. PR #72687 fixes issue #73301 (zone labels need to be auto-applied to a volume, so that pod scheduler can choose the right pod as per volume) and PR #72731 fixes the issue of placing the volume in a datastore of the right zone. These issues were causing wrong and strange behaviour of volumes in a vsphere environment that uses zones when the datastore were not shared across all k8s nodes.

I apologize for not giving the reference of the right issues in the description. I have done that now. Kindly take another look and let me know if you have questions.

subramanian-neelakantan · 2019-03-07T09:10:47Z

@tpepper PTAL

subramanian-neelakantan · 2019-03-20T12:23:48Z

@deads2k @derekwaynecarr can you take a look at this PR for a cherry pick into 1.13? This needs your approve before it can be considered for a merge.

derekwaynecarr · 2019-03-22T03:33:40Z

echo the sentiment on using the automation. the admission changes look fine.

/approve

k8s-ci-robot · 2019-03-22T03:34:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, frapposelli, SandeepPissay, subramanian-neelakantan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/cloudprovider/providers/vsphere/OWNERS~~ [SandeepPissay,frapposelli]
~~pkg/volume/vsphere_volume/OWNERS~~ [SandeepPissay]
~~plugin/pkg/admission/OWNERS~~ [derekwaynecarr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

subramanian-neelakantan · 2019-03-22T07:04:55Z

/test pull-kubernetes-integration

subramanian-neelakantan · 2019-03-25T04:35:07Z

Cleaned up the release notes in description.

subramanian-neelakantan and others added 2 commits February 27, 2019 14:32

Applies zone labels to newly created vsphere volumes

f48a328

Provision vsphere volume honoring zones

8081a56

k8s-ci-robot requested review from frapposelli and vladimirvivien February 27, 2019 10:49

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. labels Feb 27, 2019

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 27, 2019

k8s-ci-robot assigned tpepper and frapposelli Feb 28, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 28, 2019

k8s-ci-robot assigned deads2k, derekwaynecarr, divyenpatel and SandeepPissay Feb 28, 2019

embano1 mentioned this pull request Mar 18, 2019

vSphere Cloud Provider does not work when deployed across Zones with zone-local Storage #67703

Closed

4 tasks

tpepper added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Mar 18, 2019

k8s-ci-robot removed the do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. label Mar 18, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 22, 2019

k8s-ci-robot merged commit 55c118a into kubernetes:release-1.13 Mar 22, 2019

subramanian-neelakantan deleted the vsphere_zone_1_13 branch March 25, 2019 04:35

alloran mentioned this pull request May 25, 2019

VSphere cloud provider zone-local volume provisioning does not support multiple vcenters inventory #78336

Closed

places vsphere volumes as per zones #74654

places vsphere volumes as per zones #74654

Uh oh!

Conversation

subramanian-neelakantan commented Feb 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Feb 27, 2019

Uh oh!

frapposelli commented Feb 27, 2019

Uh oh!

subramanian-neelakantan commented Feb 27, 2019

Uh oh!

frapposelli commented Feb 28, 2019

Uh oh!

frapposelli commented Feb 28, 2019

Uh oh!

roycaihw commented Feb 28, 2019

Uh oh!

SandeepPissay commented Mar 1, 2019

Uh oh!

tpepper commented Mar 1, 2019

Uh oh!

subramanian-neelakantan commented Mar 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

subramanian-neelakantan commented Mar 7, 2019

Uh oh!

subramanian-neelakantan commented Mar 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

derekwaynecarr commented Mar 22, 2019

Uh oh!

k8s-ci-robot commented Mar 22, 2019

Uh oh!

subramanian-neelakantan commented Mar 22, 2019

Uh oh!

subramanian-neelakantan commented Mar 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

subramanian-neelakantan commented Feb 27, 2019 •

edited

Loading

subramanian-neelakantan commented Mar 2, 2019 •

edited

Loading

subramanian-neelakantan commented Mar 20, 2019 •

edited

Loading