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

Adding minimal set of privileges required for vSphere Cloud Provider #2989

Merged
merged 1 commit into from Apr 12, 2017
Merged

Adding minimal set of privileges required for vSphere Cloud Provider #2989

merged 1 commit into from Apr 12, 2017

Conversation

divyenpatel
Copy link
Member

@divyenpatel divyenpatel commented Mar 24, 2017

Changes:
Added minimal set of privileges required for vSphere Cloud Provider.
Removed resolved known issues.

Notes of Reviewers:
Verified privileges on vCenter 6.5 with Kubernetes 1.5.3 cluster.

Performed following operations
Create Storage Class, Create PVC, Create POD, Delete POD, Delete Volume.
Verified Disk attach/detach volume create/delete is working fine with these set of privileges.

CC: @abrarshivani @BaluDontu @luomiao @tusharnt @pdhamdhere @kerneltime


This change is Reviewable

@@ -55,6 +55,31 @@ export GOVC_INSECURE=1
govc vm.change -e="disk.enableUUID=1" -vm=<VMNAME>
```

* Create Role and User with Required Privileges for 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.

vSPhere -> vSphere

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@@ -55,6 +55,31 @@ export GOVC_INSECURE=1
govc vm.change -e="disk.enableUUID=1" -vm=<VMNAME>
```

* Create Role and User with Required Privileges for vSPhere Cloud Provider

vSphere Cloud Provider requires following minimal set of privileges to interact with vCenter.
Copy link
Contributor

Choose a reason for hiding this comment

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

"requires following" -> "requires the following"
Replace period at the end of the sentence with colon ":"

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@kerneltime
Copy link
Contributor

kerneltime commented Mar 27, 2017

Based on feedback kubernetes/kubernetes#43471 and my own experience, read permission on whole VC is needed. Is kubernetes/kubernetes#43471 and myself missing something in why this PR does not include it?

@divyenpatel
Copy link
Member Author

@kerneltime regarding read permission on whole VC, I am following up with @eicnix to know about vCenter version.

On 6.5 vCenter, I was able to monitor PV creation task without read only permission on whole VC.

@divyenpatel
Copy link
Member Author

@kerneltime we were missing permission at VC Level. if permission is given at Data Center or Host Level, we get null pointer exception.

Verified same set of Privileges works fine on vCenter 5.5

see latest comment at kubernetes/kubernetes#43471

@divyenpatel
Copy link
Member Author

@erictune can you approve this PR?

@kerneltime
Copy link
Contributor

/approve

@kerneltime
Copy link
Contributor

/lgtm

@chenopis chenopis self-assigned this Apr 12, 2017
@chenopis chenopis merged commit 854ac8a into kubernetes:master Apr 12, 2017
chenopis added a commit that referenced this pull request Apr 12, 2017
chenopis added a commit that referenced this pull request Apr 12, 2017
gyliu513 pushed a commit to gyliu513/kubernetes.github.io that referenced this pull request Apr 26, 2017
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants