Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Review status: 0 of 23 files reviewed at latest revision, 16 unresolved discussions.

examples/experimental/persistent-volume-provisioning/README.md, line 146 [r1] (raw file):

<!-- END MUNGE: EXAMPLE quobyte/quobyte-storage-class.yaml -->

- **apiServer** API Server of Quobyte in the format http(s)://api-server:7860

apiServer is rather ambiguous, make it quobyteApiServer or something that is distinguishable from kube-apiserver.

examples/experimental/persistent-volume-provisioning/README.md, line 147 [r1] (raw file):

- **apiServer** API Server of Quobyte in the format http(s)://api-server:7860
* **registry** Quobyte registry to use to mount the volume. You can specifiy the registry as <host>:<port> pair or if you want to specify multiple registries you just have to put a semicolon between them e.q. <host1>:<port>,<host2>:<port>,<host3>:<port>. The host can be an IP address or if you have a working DNS you can also provide the DNS names.

semicolon->comma

examples/experimental/persistent-volume-provisioning/README.md, line 150 [r1] (raw file):

* **adminSecretName** secret that holds information about the Quobyte user and the password to authenticate agains the API server.
* **adminSecretNamespace** The namespace for **adminSecretName**. Default is `default`.
* **user** maps all access to this user. Default is `root`.

what does "all access" mean?

examples/experimental/persistent-volume-provisioning/quobyte/quobyte-storage-class.yaml, line 7 [r1] (raw file):

provisioner: kubernetes.io/quobyte
parameters:
    apiServer: http://139.59.210.140:7860

https if possible

pkg/volume/quobyte/quobyte.go, line 50 [r1] (raw file):

  user      string
  password  string
  apiServer string

quobyteApiServer for less confusion, same for the rest.

Comments from Reviewable

@brendandburns we update the Quobyte API client and remove the external github.com/gorilla dependencies from it.

@rootfs got some time for reviewing the code?

I still see the gorilla library being added. Missing a push? or perhaps just need to remove the commit?

Thanks

@rootfs can we tag this PR to be tested?

So in the first step this PR is fine with a reference to the secrets in the PV Annotations but in a second step (when external provisioners are available) we should refactor this into an external provisioners? Are there already some design docs for external provisioners?

@k8s-bot ok to test

looks OK to me, @brendandburns, @rootfs opinions?

@johscheuer, can you please provide an e2e tests for Quobyte volume plugin (mount/unmount) in a subsequent PR? Look at test/e2e/volumes.go for an example how to test with containerized storage clusters like Gluster or Ceph - typically some minimal (and often also degraded) server is started as a pod, all we need is that it provides a minimal volume that mounts.

Test for provisioning would be helpful too (test/e2e/volume_provisioning.go).

@jsafrane I will have a look at but only in two weeks I'm stuffed with other things at the moment that I have to finish first. When I correctly remember we decided to not implement a 2e2 for Quobyte these reasons: #24977 (comment) (+following)

LGTM, pass @brendandburns for tags.

tagged.

@brendandburns looks like my last push got stuck :( I fixed a small typo before the lgtm and I thought my computer had pushed it looks like I was wrong.. Sorry!

@k8s-bot test this issue #32770

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

GCE e2e build/test passed for commit 02db13b.

• Test Results
• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue

Thanks!