Skip to content

Commit

Permalink
Merge pull request rook#1995 from rtnpro/support-storageclass-reclaim…
Browse files Browse the repository at this point in the history 
…policy

Support storageclass reclaimpolicy
  • Loading branch information
@travisn
travisn authored Sep 14, 2018
2 parents ae4e336 + ed08977 commit bcb7137
Show file tree
Hide file tree
Showing 14 changed files with 225 additions and 80 deletions.
2 changes: 2 additions & 0 deletions Documentation/block.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ parameters:
clusterNamespace: rook-ceph
# Specify the filesystem type of the volume. If not specified, it will use `ext4`.
fstype: xfs
# Optional, default reclaimPolicy is "Delete". Other options are: "Retain", "Recycle" as documented in https://kubernetes.io/docs/concepts/storage/storage-classes/
reclaimPolicy: Retain
```
Create the storage class.
Expand Down
1 change: 1 addition & 0 deletions PendingReleaseNotes.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Rook Ceph block storage provisioner can now correctly create erasure coded block images. See [Advanced Example: Erasure Coded Block Storage](Documentation/block.md#advanced-example-erasure-coded-block-storage) for an example usage.
- [Network File System (NFS)](https://github.com/nfs-ganesha/nfs-ganesha/wiki) is now supported by Rook with a new operator to deploy and manage this widely used server. NFS servers can be automatically deployed by creating an instance of the new `nfsservers.nfs.rook.io` custom resource. See the [NFS server user guide](Documentation/nfs.md) to get started with NFS.
- The minimum version of Kubernetes supported by Rook changed from `1.7` to `1.8`.
- `reclaimPolicy` parameter of `StorageClass` definition is now supported.

## Breaking Changes
- Mons are [named consistently](https://github.com/rook/rook/issues/1751) with other daemons with the letters a, b, c, etc.
Expand Down
26 changes: 16 additions & 10 deletions pkg/operator/ceph/provisioner/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ func (ctrl *ProvisionController) shouldProvision(claim *v1.PersistentVolumeClaim

// Kubernetes 1.4 provisioning, evaluating class.Provisioner
claimClass := helper.GetPersistentVolumeClaimClass(claim)
provisioner, _, err := ctrl.getStorageClassFields(claimClass)
provisioner, _, _, err := ctrl.getStorageClassFields(claimClass)
if err != nil {
glog.Errorf("Error getting claim %q's StorageClass's fields: %v", claimToClaimKey(claim), err)
return false
Expand Down Expand Up @@ -767,7 +767,7 @@ func (ctrl *ProvisionController) provisionClaimOperation(claim *v1.PersistentVol
return nil
}

provisioner, parameters, err := ctrl.getStorageClassFields(claimClass)
provisioner, reclaimPolicyField, parameters, err := ctrl.getStorageClassFields(claimClass)
if err != nil {
glog.Errorf("Error getting claim %q's StorageClass's fields: %v", claimToClaimKey(claim), err)
return nil
Expand All @@ -780,9 +780,15 @@ func (ctrl *ProvisionController) provisionClaimOperation(claim *v1.PersistentVol
return nil
}

var reclaimPolicy v1.PersistentVolumeReclaimPolicy
if reclaimPolicyField == nil {
reclaimPolicy = v1.PersistentVolumeReclaimDelete
} else {
reclaimPolicy = *reclaimPolicyField
}

options := VolumeOptions{
// TODO SHOULD be set to `Delete` unless user manually configures other reclaim policy.
PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete,
PersistentVolumeReclaimPolicy: reclaimPolicy,
PVName: pvName,
PVC: claim,
Parameters: parameters,
Expand Down Expand Up @@ -1082,25 +1088,25 @@ func (ctrl *ProvisionController) scheduleOperation(operationName string, operati
}
}

func (ctrl *ProvisionController) getStorageClassFields(name string) (string, map[string]string, error) {
func (ctrl *ProvisionController) getStorageClassFields(name string) (string, *v1.PersistentVolumeReclaimPolicy, map[string]string, error) {
classObj, found, err := ctrl.classes.GetByKey(name)
if err != nil {
return "", nil, err
return "", nil, nil, err
}
if !found {
return "", nil, fmt.Errorf("StorageClass %q not found", name)
return "", nil, nil, fmt.Errorf("StorageClass %q not found", name)
// 3. It tries to find a StorageClass instance referenced by annotation
// `claim.Annotations["volume.beta.kubernetes.io/storage-class"]`. If not
// found, it SHOULD report an error (by sending an event to the claim) and it
// SHOULD retry periodically with step i.
}
switch class := classObj.(type) {
case *storage.StorageClass:
return class.Provisioner, class.Parameters, nil
return class.Provisioner, class.ReclaimPolicy, class.Parameters, nil
case *storagebeta.StorageClass:
return class.Provisioner, class.Parameters, nil
return class.Provisioner, class.ReclaimPolicy, class.Parameters, nil
}
return "", nil, fmt.Errorf("Cannot convert object to StorageClass: %+v", classObj)
return "", nil, nil, fmt.Errorf("Cannot convert object to StorageClass: %+v", classObj)
}

func claimToClaimKey(claim *v1.PersistentVolumeClaim) string {
Expand Down
35 changes: 18 additions & 17 deletions pkg/operator/ceph/provisioner/controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ func TestController(t *testing.T) {
{
name: "provision for claim-1 but not claim-2",
objs: []runtime.Object{
newStorageClass("class-1", "foo.bar/baz"),
newStorageClass("class-2", "abc.def/ghi"),
newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete),
newStorageClass("class-2", "abc.def/ghi", v1.PersistentVolumeReclaimDelete),
newClaim("claim-1", "uid-1-1", "class-1", "", nil),
newClaim("claim-2", "uid-1-2", "class-2", "", nil),
},
provisionerName: "foo.bar/baz",
provisioner: newTestProvisioner(),
expectedVolumes: []v1.PersistentVolume{
*newProvisionedVolume(newStorageClass("class-1", "foo.bar/baz"), newClaim("claim-1", "uid-1-1", "class-1", "", nil)),
*newProvisionedVolume(newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete), newClaim("claim-1", "uid-1-1", "class-1", "", nil)),
},
},
{
Expand Down Expand Up @@ -127,7 +127,7 @@ func TestController(t *testing.T) {
{
name: "provisioner fails to provision for claim-1: no pv is created",
objs: []runtime.Object{
newStorageClass("class-1", "foo.bar/baz"),
newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete),
newClaim("claim-1", "uid-1-1", "class-1", "", nil),
},
provisionerName: "foo.bar/baz",
Expand All @@ -148,7 +148,7 @@ func TestController(t *testing.T) {
{
name: "try to provision for claim-1 but fail to save the pv object",
objs: []runtime.Object{
newStorageClass("class-1", "foo.bar/baz"),
newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete),
newClaim("claim-1", "uid-1-1", "class-1", "", nil),
},
provisionerName: "foo.bar/baz",
Expand Down Expand Up @@ -177,14 +177,14 @@ func TestController(t *testing.T) {
{
name: "provision for claim-1 but not claim-2, because it is ignored",
objs: []runtime.Object{
newStorageClass("class-1", "foo.bar/baz"),
newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete),
newClaim("claim-1", "uid-1-1", "class-1", "", nil),
newClaim("claim-2", "uid-1-2", "class-1", "", nil),
},
provisionerName: "foo.bar/baz",
provisioner: newIgnoredProvisioner(),
expectedVolumes: []v1.PersistentVolume{
*newProvisionedVolume(newStorageClass("class-1", "foo.bar/baz"), newClaim("claim-1", "uid-1-1", "class-1", "", nil)),
*newProvisionedVolume(newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete), newClaim("claim-1", "uid-1-1", "class-1", "", nil)),
},
},
}
Expand Down Expand Up @@ -261,7 +261,7 @@ func TestMultipleControllers(t *testing.T) {
ctrls[i] = NewProvisionController(client, test.provisionerName, provisioner, "v1.5.0", CreateProvisionedPVInterval(10*time.Millisecond))
ctrls[i].claimSource = claimSource
ctrls[i].claims.Add(newClaim("claim-1", "uid-1-1", "class-1", "", nil))
ctrls[i].classes.Add(newStorageClass("class-1", "foo.bar/baz"))
ctrls[i].classes.Add(newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete))
stopChs[i] = make(chan struct{})
}

Expand Down Expand Up @@ -294,28 +294,28 @@ func TestShouldProvision(t *testing.T) {
{
name: "should provision",
provisionerName: "foo.bar/baz",
class: newStorageClass("class-1", "foo.bar/baz"),
class: newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete),
claim: newClaim("claim-1", "1-1", "class-1", "", nil),
expectedShould: true,
},
{
name: "claim already bound",
provisionerName: "foo.bar/baz",
class: newStorageClass("class-1", "foo.bar/baz"),
class: newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete),
claim: newClaim("claim-1", "1-1", "class-1", "foo", nil),
expectedShould: false,
},
{
name: "no such class",
provisionerName: "foo.bar/baz",
class: newStorageClass("class-1", "foo.bar/baz"),
class: newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete),
claim: newClaim("claim-1", "1-1", "class-2", "", nil),
expectedShould: false,
},
{
name: "not this provisioner's job",
provisionerName: "foo.bar/baz",
class: newStorageClass("class-1", "abc.def/ghi"),
class: newStorageClass("class-1", "abc.def/ghi", v1.PersistentVolumeReclaimDelete),
claim: newClaim("claim-1", "1-1", "class-1", "", nil),
expectedShould: false,
},
Expand All @@ -324,15 +324,15 @@ func TestShouldProvision(t *testing.T) {
{
name: "should provision 1.5",
provisionerName: "foo.bar/baz",
class: newStorageClass("class-2", "abc.def/ghi"),
class: newStorageClass("class-2", "abc.def/ghi", v1.PersistentVolumeReclaimDelete),
claim: newClaim("claim-1", "1-1", "class-1", "",
map[string]string{annStorageProvisioner: "foo.bar/baz"}),
expectedShould: true,
},
{
name: "unknown provisioner 1.5",
provisionerName: "foo.bar/baz",
class: newStorageClass("class-1", "foo.bar/baz"),
class: newStorageClass("class-1", "foo.bar/baz", v1.PersistentVolumeReclaimDelete),
claim: newClaim("claim-1", "1-1", "class-1", "",
map[string]string{annStorageProvisioner: "abc.def/ghi"}),
expectedShould: false,
Expand Down Expand Up @@ -487,12 +487,13 @@ func newTestProvisionController(
return ctrl
}

func newStorageClass(name, provisioner string) *storagebeta.StorageClass {
func newStorageClass(name, provisioner string, reclaimPolicy v1.PersistentVolumeReclaimPolicy) *storagebeta.StorageClass {
return &storagebeta.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Provisioner: provisioner,
Provisioner: provisioner,
ReclaimPolicy: &reclaimPolicy,
}
}

Expand Down Expand Up @@ -658,7 +659,7 @@ func (i *ignoredProvisioner) Provision(options VolumeOptions) (*v1.PersistentVol
return nil, &IgnoredError{"Ignored"}
}

return newProvisionedVolume(newStorageClass("class-1", "foo.bar/baz"), newClaim("claim-1", "uid-1-1", "class-1", "", nil)), nil
return newProvisionedVolume(newStorageClass("class-1", "foo.bar/baz", options.PersistentVolumeReclaimPolicy), newClaim("claim-1", "uid-1-1", "class-1", "", nil)), nil
}

func (i *ignoredProvisioner) Delete(volume *v1.PersistentVolume) error {
Expand Down
57 changes: 50 additions & 7 deletions pkg/operator/ceph/provisioner/provisioner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,14 @@ func TestProvisionImage(t *testing.T) {
}

provisioner := New(context, "foo.io")
volume := newVolumeOptions(newStorageClass("class-1", "foo.io/block", map[string]string{"pool": "testpool", "clusterNamespace": "testCluster", "fsType": "ext3", "dataPool": ""}), newClaim("claim-1", "uid-1-1", "class-1", "", "class-1", nil))
volume := newVolumeOptions(newStorageClass("class-1", "foo.io/block", map[string]string{"pool": "testpool", "clusterNamespace": "testCluster", "fsType": "ext3", "dataPool": ""}, v1.PersistentVolumeReclaimRetain), newClaim("claim-1", "uid-1-1", "class-1", "", "class-1", nil), v1.PersistentVolumeReclaimRetain)

pv, err := provisioner.Provision(volume)
assert.Nil(t, err)

assert.Equal(t, "pvc-uid-1-1", pv.Name)
assert.NotNil(t, pv.Spec.PersistentVolumeSource.FlexVolume)
assert.Equal(t, v1.PersistentVolumeReclaimRetain, pv.Spec.PersistentVolumeReclaimPolicy)
assert.Equal(t, "foo.io/rook-system", pv.Spec.PersistentVolumeSource.FlexVolume.Driver)
assert.Equal(t, "ext3", pv.Spec.PersistentVolumeSource.FlexVolume.FSType)
assert.Equal(t, "testCluster", pv.Spec.PersistentVolumeSource.FlexVolume.Options["clusterNamespace"])
Expand All @@ -83,13 +84,14 @@ func TestProvisionImage(t *testing.T) {
assert.Equal(t, "pvc-uid-1-1", pv.Spec.PersistentVolumeSource.FlexVolume.Options["image"])
assert.Equal(t, "", pv.Spec.PersistentVolumeSource.FlexVolume.Options["dataPool"])

volume = newVolumeOptions(newStorageClass("class-1", "foo.io/block", map[string]string{"pool": "testpool", "clusterNamespace": "testCluster", "fsType": "ext3", "dataPool": "iamdatapool"}), newClaim("claim-1", "uid-1-1", "class-1", "", "class-1", nil))
volume = newVolumeOptions(newStorageClass("class-1", "foo.io/block", map[string]string{"pool": "testpool", "clusterNamespace": "testCluster", "fsType": "ext3", "dataPool": "iamdatapool"}, v1.PersistentVolumeReclaimRecycle), newClaim("claim-1", "uid-1-1", "class-1", "", "class-1", nil), v1.PersistentVolumeReclaimRecycle)

pv, err = provisioner.Provision(volume)
assert.Nil(t, err)

assert.Equal(t, "pvc-uid-1-1", pv.Name)
assert.NotNil(t, pv.Spec.PersistentVolumeSource.FlexVolume)
assert.Equal(t, v1.PersistentVolumeReclaimRecycle, pv.Spec.PersistentVolumeReclaimPolicy)
assert.Equal(t, "foo.io/rook-system", pv.Spec.PersistentVolumeSource.FlexVolume.Driver)
assert.Equal(t, "ext3", pv.Spec.PersistentVolumeSource.FlexVolume.FSType)
assert.Equal(t, "testCluster", pv.Spec.PersistentVolumeSource.FlexVolume.Options["clusterNamespace"])
Expand All @@ -99,6 +101,46 @@ func TestProvisionImage(t *testing.T) {
assert.Equal(t, "iamdatapool", pv.Spec.PersistentVolumeSource.FlexVolume.Options["dataPool"])
}

func TestReclaimPolicyForProvisionedImages(t *testing.T) {
clientset := test.New(3)
namespace := "ns"
configDir, _ := ioutil.TempDir("", "")
os.Setenv("POD_NAMESPACE", "rook-system")
defer os.Setenv("POD_NAMESPACE", "")
defer os.RemoveAll(configDir)
executor := &exectest.MockExecutor{
MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) {
if strings.Contains(command, "ceph-authtool") {
cephtest.CreateConfigDir(path.Join(configDir, namespace))
}

if command == "rbd" && args[0] == "create" {
return `[{"image":"pvc-uid-1-1","size":1048576,"format":2}]`, nil
}

if command == "rbd" && args[0] == "ls" && args[1] == "-l" {
return `[{"image":"pvc-uid-1-1","size":1048576,"format":2}]`, nil
}
return "", nil
},
}

context := &clusterd.Context{
Clientset: clientset,
Executor: executor,
ConfigDir: configDir,
}

provisioner := New(context, "foo.io")
for _, reclaimPolicy := range []v1.PersistentVolumeReclaimPolicy{v1.PersistentVolumeReclaimDelete, v1.PersistentVolumeReclaimRetain, v1.PersistentVolumeReclaimRecycle} {
volume := newVolumeOptions(newStorageClass("class-1", "foo.io/block", map[string]string{"pool": "testpool", "clusterNamespace": "testCluster", "fsType": "ext3", "dataPool": "iamdatapool"}, reclaimPolicy), newClaim("claim-1", "uid-1-1", "class-1", "", "class-1", nil), reclaimPolicy)
pv, err := provisioner.Provision(volume)
assert.Nil(t, err)

assert.Equal(t, reclaimPolicy, pv.Spec.PersistentVolumeReclaimPolicy)
}
}

func TestParseClassParameters(t *testing.T) {
cfg := make(map[string]string)
cfg["pool"] = "testPool"
Expand Down Expand Up @@ -143,22 +185,23 @@ func TestParseClassParametersInvalidOption(t *testing.T) {
assert.EqualError(t, err, "invalid option \"foo\" for volume plugin rookVolumeProvisioner")
}

func newVolumeOptions(storageClass *storagebeta.StorageClass, claim *v1.PersistentVolumeClaim) controller.VolumeOptions {
func newVolumeOptions(storageClass *storagebeta.StorageClass, claim *v1.PersistentVolumeClaim, reclaimPolicy v1.PersistentVolumeReclaimPolicy) controller.VolumeOptions {
return controller.VolumeOptions{
PersistentVolumeReclaimPolicy: v1.PersistentVolumeReclaimDelete,
PersistentVolumeReclaimPolicy: reclaimPolicy,
PVName: "pvc-" + string(claim.ObjectMeta.UID),
PVC: claim,
Parameters: storageClass.Parameters,
}
}

func newStorageClass(name, provisioner string, parameters map[string]string) *storagebeta.StorageClass {
func newStorageClass(name, provisioner string, parameters map[string]string, reclaimPolicy v1.PersistentVolumeReclaimPolicy) *storagebeta.StorageClass {
return &storagebeta.StorageClass{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Provisioner: provisioner,
Parameters: parameters,
Provisioner: provisioner,
Parameters: parameters,
ReclaimPolicy: &reclaimPolicy,
}
}

Expand Down
8 changes: 4 additions & 4 deletions tests/framework/clients/block.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,17 @@ func (b *BlockOperation) CreatePvc(claimName, storageClassName, mode string) (st
return b.k8sClient.ResourceOperation("create", b.manifests.GetBlockPvcDef(claimName, storageClassName, mode))
}

func (b *BlockOperation) CreateStorageClass(poolName, storageClassName, namespace string, varClusterName bool) (string, error) {
return b.k8sClient.ResourceOperation("create", b.manifests.GetBlockStorageClassDef(poolName, storageClassName, namespace, varClusterName))
func (b *BlockOperation) CreateStorageClass(poolName, storageClassName, reclaimPolicy, namespace string, varClusterName bool) (string, error) {
return b.k8sClient.ResourceOperation("create", b.manifests.GetBlockStorageClassDef(poolName, storageClassName, reclaimPolicy, namespace, varClusterName))
}

func (b *BlockOperation) DeletePvc(claimName, storageClassName, mode string) error {
_, err := b.k8sClient.ResourceOperation("delete", b.manifests.GetBlockPvcDef(claimName, storageClassName, mode))
return err
}

func (b *BlockOperation) DeleteStorageClass(poolName, storageClassName, namespace string) error {
_, err := b.k8sClient.ResourceOperation("delete", b.manifests.GetBlockStorageClassDef(poolName, storageClassName, namespace, false))
func (b *BlockOperation) DeleteStorageClass(poolName, storageClassName, reclaimPolicy, namespace string) error {
_, err := b.k8sClient.ResourceOperation("delete", b.manifests.GetBlockStorageClassDef(poolName, storageClassName, reclaimPolicy, namespace, false))
return err
}

Expand Down
Loading

0 comments on commit bcb7137

Please sign in to comment.