Add e2e test for external pv provisioning #39545
Conversation
|
Hi @wongma7. 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 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. I understand the commands that are listed here. |
k8s-ci-robot
added
the
cncf-cla: yes
|
This change is |
k8s-github-robot
added
size/L
fejta
added
the
sig/storage
|
@k8s-bot ok to test |
|
Jenkins Bazel Build failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins GCE etcd3 e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins GKE smoke e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins GCE e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins GCI GKE smoke e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins kops AWS e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins verification failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins GCI GCE e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins CRI GCE Node e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins Kubemark GCE e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
|
Jenkins GCE Node e2e failed for commit c73e3c74bdeb54bf036dbfb92aff0c2234d609f7. Full PR test history. The magic incantation to run this job again is 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. I understand the commands that are listed here. |
| framework.ExpectNoError(err, "Cannot locate the provisioner pod %v: %v", provisionerPod.Name, err) | ||
|
|
||
| By("sleeping a bit to give the provisioner time to start") | ||
| time.Sleep(5 * time.Second) |
There was a problem hiding this comment.
Is this sleep necessary? The test creates a PVC and then waits until it's provisioned by the pod and it could be faster than 5 seconds. Question is, if the NFS server in the pod is ready at that time.
There was a problem hiding this comment.
No, seems I was being too cautious, removed sleep. The NFS server & caches & all should be ready by the time PVC is created. In the tiny chance they aren't, we wait just 15 seconds for the provisioner to retry provisioning.
| // speeds up the test! | ||
| // Three minutes should be enough to clean up the pods properly. | ||
| // We've seen GCE PD detach to take more than 1 minute. | ||
| By("Sleeping to let kubelet destroy all pods") |
There was a problem hiding this comment.
sync period is now 15 seconds, perhaps we no longer need this sleep. I'll post a PR that removes it.
| ) | ||
|
|
||
| func testDynamicProvisioning(client clientset.Interface, claim *v1.PersistentVolumeClaim) { | ||
| func testDynamicProvisioning(client clientset.Interface, claim *v1.PersistentVolumeClaim, external bool) { |
There was a problem hiding this comment.
IMO, using expectedSize parameter would be more useful than external bool. Strictly speaking, fact that all current cloud providers round to 1GB chunks is just a coincidence, it's not a guaranteed property of all internal provisioners.
|
@jsafrane PTAL, I addressed your comments :) |
jsafrane
added
the
release-note-none
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
Automatic merge from submit-queue (batch tested with PRs 37557, 39545) |
|
After merging this PR suites ci-kubernetes-e2e-gci-gce-slow and ci-kubernetes-e2e-gci-gke-slow started failing. I'm going to revert it. https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-gci-gce-slow/1672 |
Automatic merge from submit-queue (batch tested with PRs 40574, 40806, 40308, 40771, 39440) Add bootstrap cluster role for external pv provisioners The set of permissions an external provisioner #30285 running as a pod will need. Technically in order to dynamically provision PVs one doesn't need to "update" PVCs or "watch" events but the controller https://github.com/kubernetes-incubator/nfs-provisioner/tree/master/controller we are recommending people use does those things to: set lock annotations on PVCs and watch `ProvisioningSucceeded`/`ProvisioningFailed` events. Some external provisioners may need additional permissions, for example nfs-provisioner requires "get" access to Services and Endpoints when run "statefully." I think in that case we would recommend creating a new ClusterRole specific to that provisioner, using this as a base? (This was to be a part of my redo/fix of the external e2e test #39545 but I'm submitting it as a separate PR for now due to some issues I had with running nfs-provisioner on gce.) @kubernetes/sig-auth-misc ?
Automatic merge from submit-queue Add e2e test for external provisioners this is a retry of #39545 This time around: * take advantage of the system:persistent-volume-provisioner bootstrap cluster role to grant the external provisioner pod serviceaccount permissions * add storageclass suffix so that the first and third test don't conflict when one creates the storageclass with the same name before the other also tested more thoroughly myself on gce :) @jsafrane if you would like to re-review
Automatic merge from submit-queue (batch tested with PRs 40574, 40806, 40308, 40771, 39440) Add bootstrap cluster role for external pv provisioners The set of permissions an external provisioner kubernetes/kubernetes#30285 running as a pod will need. Technically in order to dynamically provision PVs one doesn't need to "update" PVCs or "watch" events but the controller https://github.com/kubernetes-incubator/nfs-provisioner/tree/master/controller we are recommending people use does those things to: set lock annotations on PVCs and watch `ProvisioningSucceeded`/`ProvisioningFailed` events. Some external provisioners may need additional permissions, for example nfs-provisioner requires "get" access to Services and Endpoints when run "statefully." I think in that case we would recommend creating a new ClusterRole specific to that provisioner, using this as a base? (This was to be a part of my redo/fix of the external e2e test kubernetes/kubernetes#39545 but I'm submitting it as a separate PR for now due to some issues I had with running nfs-provisioner on gce.) @kubernetes/sig-auth-misc ?
fixes #36170