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.

Sign up for GitHub

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

WIP: Persistent Storage API #5105

Closed
wants to merge 17 commits into from
Closed

WIP: Persistent Storage API #5105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5604fa2
PersistentVolume & PersistentVolumeClaim API
markturansky Mar 3, 2015
f279e05
moved funcs from VolMgr pkg to volume
markturansky Mar 5, 2015
fdb9dbd
Introduced PersistentVolumeSource, added comments to all types, and r…
markturansky Mar 5, 2015
5557c16
Fixed registry for PV and PVC after rebase
markturansky Mar 6, 2015
3c9b668
Addressed feedback on ClaimRef, volume phases, volume/claim capacity
markturansky Mar 10, 2015
294d3c0
Rebased and updated to changes
markturansky Mar 13, 2015
69e3e0d
Added PV & PVC validation
markturansky Mar 14, 2015
9d4b203
inlined PersistentVolumeSource
markturansky Mar 14, 2015
41f0ca8
rebased
markturansky Mar 14, 2015
e1423a2
rebased and updated to changes
markturansky Mar 17, 2015
e15ff33
added descriptions to each versioned types.go
markturansky Mar 17, 2015
f16f5dd
rebased, added fields.Selector
markturansky Mar 18, 2015
d4e6bab
added missing descriptions, removed future PV structs, and rebased
markturansky Mar 19, 2015
bebacb3
rebased and updated to changes
markturansky Mar 20, 2015
aa694df
Added test-cmd.sh tests to create/delete PV & PVC
markturansky Mar 20, 2015
b7cff5b
removed name check, per feedback
markturansky Mar 20, 2015
7ec38af
improved comment for PersistentVolumePhase, tweaked wording of desire…
markturansky Mar 23, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions examples/examples_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ func validateObject(obj runtime.Object) (errors []error) {
for i := range t.Items {
errors = append(errors, validateObject(&t.Items[i])...)
}
case *api.PersistentVolume:
errors = validation.ValidatePersistentVolume(t)
case *api.PersistentVolumeClaim:
api.ValidNamespace(ctx, &t.ObjectMeta)
errors = validation.ValidatePersistentVolumeClaim(t)
default:
return []error{fmt.Errorf("no validation defined for %#v", obj)}
}
Expand Down Expand Up @@ -150,6 +155,16 @@ func TestExampleObjectSchemas(t *testing.T) {
"kitten-rc": &api.ReplicationController{},
"nautilus-rc": &api.ReplicationController{},
},
"../examples/persistent-volumes/volumes": {
"local-01": &api.PersistentVolume{},
"local-02": &api.PersistentVolume{},
"gce": &api.PersistentVolume{},
},
"../examples/persistent-volumes/claims": {
"claim-01": &api.PersistentVolumeClaim{},
"claim-02": &api.PersistentVolumeClaim{},
"claim-03": &api.PersistentVolumeClaim{},
},
}

for path, expected := range cases {
Expand Down
10 changes: 10 additions & 0 deletions examples/persistent-volumes/claims/claim-01.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
kind: PersistentVolumeClaim
apiVersion: v1beta3
metadata:
name: myclaim-1
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 3
10 changes: 10 additions & 0 deletions examples/persistent-volumes/claims/claim-02.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
kind: PersistentVolumeClaim
apiVersion: v1beta3
metadata:
name: myclaim-2
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 8
17 changes: 17 additions & 0 deletions examples/persistent-volumes/claims/claim-03.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"kind": "PersistentVolumeClaim",
"apiVersion": "v1beta3",
"metadata": {
"name": "myclaim-3"
}, "spec": {
"accessModes": [
"ReadWriteOnce",
"ReadOnlyMany"
],
"resources": {
"requests": {
"storage": "10G"
}
}
}
}
69 changes: 69 additions & 0 deletions examples/persistent-volumes/simpletest/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# How To Use Persistent Volumes

This guide assumes knowledge of Kubernetes fundamentals and that a user has a cluster up and running.

## Create volumes

Persistent Volumes are intended for "network volumes", such as GCE Persistent Disks, NFS shares, and AWS EBS volumes.

The `HostPath` VolumeSource was included in the Persistent Volumes implementation for ease of testing.

Create persistent volumes by posting them to the API server:

```

cluster/kubectl.sh create -f examples/persistent-volumes/volumes/local-01.yaml
cluster/kubectl.sh create -f examples/persistent-volumes/volumes/local-02.yaml

cluster/kubectl.sh get pv

NAME LABELS CAPACITY ACCESSMODES STATUS CLAIM
pv0001 map[] 10737418240 RWO
pv0002 map[] 5368709120 RWO


In the log:

I0302 10:20:45.663225 1920 persistent_volume_manager.go:115] Managing PersistentVolume[UID=b16e91d6-c0ef-11e4-8be4-80e6500a981e]
I0302 10:20:55.667945 1920 persistent_volume_manager.go:115] Managing PersistentVolume[UID=b41f4f0e-c0ef-11e4-8be4-80e6500a981e]

```

## Create claims

You must be in a namespace to create claims.

```

cluster/kubectl.sh create -f examples/persistent-volumes/claims/claim-01.yaml
cluster/kubectl.sh create -f examples/persistent-volumes/claims/claim-02.yaml

NAME LABELS STATUS VOLUME
myclaim-1 map[]
myclaim-2 map[]

```


## Matching and binding

```

PersistentVolumeClaim[UID=f4b3d283-c0ef-11e4-8be4-80e6500a981e] bound to PersistentVolume[UID=b16e91d6-c0ef-11e4-8be4-80e6500a981e]



cluster/kubectl.sh get pv

NAME LABELS CAPACITY ACCESSMODES STATUS CLAIM
pv0001 map[] 10737418240 RWO myclaim-1 / f4b3d283-c0ef-11e4-8be4-80e6500a981e
pv0002 map[] 5368709120 RWO myclaim-2 / f70da891-c0ef-11e4-8be4-80e6500a981e


cluster/kubectl.sh get pvc

NAME LABELS STATUS VOLUME
myclaim-1 map[] b16e91d6-c0ef-11e4-8be4-80e6500a981e
myclaim-2 map[] b41f4f0e-c0ef-11e4-8be4-80e6500a981e

```
10 changes: 10 additions & 0 deletions examples/persistent-volumes/simpletest/namespace.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"kind": "Namespace",
"apiVersion":"v1beta3",
"metadata": {
"name": "myns",
"labels": {
"name": "development"
}
}
}
16 changes: 16 additions & 0 deletions examples/persistent-volumes/simpletest/nginx.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
kind: Pod
apiVersion: v1beta3
metadata:
name: mypod
spec:
containers:
- image: nginx:latest
name: myfrontend
volumeMounts:
- mountPath: "/usr/share/nginx/html"
name: mypd
volumes:
- name: mypd
source:
hostPath:
path: "/Users/markturansky/site-content"
18 changes: 18 additions & 0 deletions examples/persistent-volumes/simpletest/pod.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
kind: Pod
apiVersion: v1beta3
metadata:
name: mypod
spec:
containers:
- image: dockerfile/nginx
name: myfrontend
volumeMounts:
- mountPath: "/var/www/html"
name: mypd
volumes:
- name: mypd
source:
persistentVolumeClaim:
accessMode: ReadWriteOnce
claimRef:
name: myclaim-1
10 changes: 10 additions & 0 deletions examples/persistent-volumes/volumes/gce.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
kind: PersistentVolume
apiVersion: v1beta3
metadata:
name: pv0003
spec:
capacity:
storage: 10
gcePersistentDisk:
pdName: "abc123"
fsType: "ext4"
11 changes: 11 additions & 0 deletions examples/persistent-volumes/volumes/local-01.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
kind: PersistentVolume
apiVersion: v1beta3
metadata:
name: pv0001
labels:
type: local
Copy link

Choose a reason for hiding this comment

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

Having a local, temporary directory referred to as a "Persistent" volume will be a source of confusion to users. I have no suggested improvement yet - let me finish reading first :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, this has caused confusion already in reviewers. I added comments that specifically state that a hostPath volume should be used only and exclusively for development and testing.

Copy link

Choose a reason for hiding this comment

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

Oh, and Mark, please excuse the very late feedback, and somewhat
unstructured nature of it. I'm just going through all of the storage stuff
as best I can to get up to speed, making comments/notes as I go. Feel free
to ignore all of my input for now, and I can provide consolidated feedback
at the end, if that works better for you.

On Thu, Mar 19, 2015 at 11:40 AM, Mark Turansky notifications@github.com
wrote:

In examples/persistent-volumes/volumes/local-01.yaml
#5105 (comment)
:

@@ -0,0 +1,11 @@
+kind: PersistentVolume
+apiVersion: v1beta3
+metadata:

  • name: pv0001
  • labels:
  • type: local

Agreed, this has caused confusion already in reviewers. I added comments
that specifically state that a hostPath volume should be used only and
exclusively for development and testing.

—
Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/5105/files#r26784869
.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I glad to receive the feedback and will correct the obvious and actionable stuff. Words and terms are easily changed, too, if there is consensus on a term. Many of them have already been put through the wringer.

spec:
capacity:
storage: 10Gi
Copy link

Choose a reason for hiding this comment

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

nit: can we refer to this as 'space' rather than 'storage' - the latter is too general and not what we're after here, I think.

Copy link

Choose a reason for hiding this comment

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

Or size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was originally sized. "storage" was thought more specific. I'm happy to change it to whatever the consensus is.

Copy link

Choose a reason for hiding this comment

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

The thing that you're going to need to be careful of here is that there are highly likely to be multiple storage resource dimensions here over time, primarily:

  1. space,
  2. IOPS
  3. bandwidth

Typically you will want to carefully control oversubscription of all of those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I originally had those 3 (gleaned from the cloud storage APIs) but was encouraged to use just size for the first version. I introduced ResourceRequirements.Requests in this PR, which can grow to accommodate more than just storage. Similarly, the PersistentVolumeManager can attempt to match on more than just size.

hostPath:
path: "/tmp/data01"
11 changes: 11 additions & 0 deletions examples/persistent-volumes/volumes/local-02.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
kind: PersistentVolume
apiVersion: v1beta3
metadata:
name: pv0002
labels:
type: local
spec:
capacity:
storage: 5Gi
hostPath:
path: "/tmp/data02"
43 changes: 43 additions & 0 deletions hack/test-cmd.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,49 @@ __EOF__
# Post-condition: no replication controller is running
kube::test::get_object_assert rc "{{range.items}}{{.$id_field}}:{{end}}" ''

######################
# Persistent Volumes #
######################

### Create and delete persistent volume examples
# Pre-condition: no persistent volumes currently exist
kube::test::get_object_assert pv "{{range.items}}{{.$id_field}}:{{end}}" ''
# Command
kubectl create -f examples/persistent-volumes/volumes/local-01.yaml "${kube_flags[@]}"
kube::test::get_object_assert pv "{{range.items}}{{.$id_field}}:{{end}}" 'pv0001:'
kubectl delete pv pv0001 "${kube_flags[@]}"
kubectl create -f examples/persistent-volumes/volumes/local-02.yaml "${kube_flags[@]}"
kube::test::get_object_assert pv "{{range.items}}{{.$id_field}}:{{end}}" 'pv0002:'
kubectl delete pv pv0002 "${kube_flags[@]}"
kubectl create -f examples/persistent-volumes/volumes/gce.yaml "${kube_flags[@]}"
kube::test::get_object_assert pv "{{range.items}}{{.$id_field}}:{{end}}" 'pv0003:'
kubectl delete pv pv0003 "${kube_flags[@]}"
# Post-condition: no PVs
kube::test::get_object_assert pv "{{range.items}}{{.$id_field}}:{{end}}" ''

############################
# Persistent Volume Claims #
############################

### Create and delete persistent volume claim examples
# Pre-condition: no persistent volume claims currently exist
kube::test::get_object_assert pvc "{{range.items}}{{.$id_field}}:{{end}}" ''
# Command
kubectl create -f examples/persistent-volumes/claims/claim-01.yaml "${kube_flags[@]}"
kube::test::get_object_assert pvc "{{range.items}}{{.$id_field}}:{{end}}" 'myclaim-1:'
kubectl delete pvc myclaim-1 "${kube_flags[@]}"

kubectl create -f examples/persistent-volumes/claims/claim-02.yaml "${kube_flags[@]}"
kube::test::get_object_assert pvc "{{range.items}}{{.$id_field}}:{{end}}" 'myclaim-2:'
kubectl delete pvc myclaim-2 "${kube_flags[@]}"

kubectl create -f examples/persistent-volumes/claims/claim-03.json "${kube_flags[@]}"
kube::test::get_object_assert pvc "{{range.items}}{{.$id_field}}:{{end}}" 'myclaim-3:'
kubectl delete pvc myclaim-3 "${kube_flags[@]}"
# Post-condition: no PVCs
kube::test::get_object_assert pvc "{{range.items}}{{.$id_field}}:{{end}}" ''



#########
# Nodes #
Expand Down
3 changes: 2 additions & 1 deletion pkg/api/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ var standardResources = util.NewStringSet(
string(ResourcePods),
string(ResourceQuotas),
string(ResourceServices),
string(ResourceReplicationControllers))
string(ResourceReplicationControllers),
string(ResourceStorage))

func IsStandardResourceName(str string) bool {
return standardResources.Has(str)
Expand Down
7 changes: 4 additions & 3 deletions pkg/api/latest/latest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,10 @@ func init() {
// the list of kinds that are scoped at the root of the api hierarchy
// if a kind is not enumerated here, it is assumed to have a namespace scope
kindToRootScope := map[string]bool{
"Node": true,
"Minion": true,
"Namespace": true,
"Node": true,
"Minion": true,
"Namespace": true,
"PersistentVolume": true,
Copy link

Choose a reason for hiding this comment

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

I think we're going to want PersistentVolumes to be inside namespaces. In the future general case, individual users will be able to create their own PV's, and want to put them in namespaces to avoid clashes. In the first iteration, where the "storage administrator" creates all PV's on behalf of users, would there be any downside to having those inside namespaces too (even if it's just a global "admin-created-volumes" namespace or similar)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@quinton-hoole I think the current implementations of VolumeSource supports what you are asking.

Example, Tim was thinking the new NFSMount plugin can stay in VolumeSource as well as be represented in the new PersistentVolumeSource. He saw re-use for that in a pod beyond an admin-provisioned volume. In this case, the NFS share is a pet and continues to live outside the lifecycle of a pod. It can reused by subsequent pods.

Does the correctly address what you are saying?

Also, I don't see a problem with pinning some PVs to a specific namespace (the admin provisions it purposefully that way), except for how the code handles namespaced PVs versus non-namespaced ones.

}

// enumerate all supported versions, get the kinds, and register with the mapper how to address our resources
Expand Down
8 changes: 8 additions & 0 deletions pkg/api/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ func init() {
&Secret{},
&SecretList{},
&DeleteOptions{},
&PersistentVolume{},
&PersistentVolumeList{},
&PersistentVolumeClaim{},
&PersistentVolumeClaimList{},
)
// Legacy names are supported
Scheme.AddKnownTypeWithName("", "Minion", &Node{})
Expand Down Expand Up @@ -87,3 +91,7 @@ func (*NamespaceList) IsAnAPIObject() {}
func (*Secret) IsAnAPIObject() {}
func (*SecretList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
func (*PersistentVolume) IsAnAPIObject() {}
func (*PersistentVolumeList) IsAnAPIObject() {}
func (*PersistentVolumeClaim) IsAnAPIObject() {}
func (*PersistentVolumeClaimList) IsAnAPIObject() {}
2 changes: 1 addition & 1 deletion pkg/api/rest/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx api.Context, obj runtime.Obje
return nil
}

// CheckGeneratedNameError checks whether an error that occured creating a resource is due
// CheckGeneratedNameError checks whether an error that occurred creating a resource is due
// to generation being unable to pick a valid name.
func CheckGeneratedNameError(strategy RESTCreateStrategy, err error, obj runtime.Object) error {
if !errors.IsAlreadyExists(err) {
Expand Down
Loading