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

feat(zfspv): pvc should be bound only if volume has been created. #121

Merged
merged 1 commit into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ Spec:
Pool Name: zfspv-pool
Recordsize: 4k
Volume Type: DATASET
Status:
State: Ready
Events: <none>
```

Expand Down Expand Up @@ -488,7 +490,7 @@ Name: pvc-e1230d2c-b32a-48f7-8b76-ca335b253dcd
Namespace: openebs
Labels: kubernetes.io/nodename=zfspv-node1
Annotations: <none>
API Version: openebs.io/v1alpha1
API Version: zfs.openebs.io/v1alpha1
Kind: ZFSVolume
Metadata:
Creation Timestamp: 2019-11-22T09:49:29Z
Expand All @@ -505,6 +507,8 @@ Spec:
Pool Name: zfspv-pool
Snap Name: pvc-34133838-0d0d-11ea-96e3-42010a800114@snapshot-3cbd5e59-4c6f-4bd6-95ba-7f72c9f12fcd
Volume Type: DATASET
Status:
State: Ready
Events: <none>

Here you can note that this resource has Snapname field which tells that this volume is created from that snapshot.
Expand Down
1 change: 1 addition & 0 deletions changelogs/unreleased/121-pawanpraka1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes an issue where PVC was bound to unusable PV created using incorrect values provided in PVC/Storageclass
4 changes: 3 additions & 1 deletion deploy/sample/zfspvcr.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: openebs.io/v1alpha1
apiVersion: zfs.openebs.io/v1alpha1
kind: ZFSVolume
metadata:
name: pvc-34133838-0d0d-11ea-96e3-42010a800114
Expand All @@ -16,3 +16,5 @@ spec:
recordsize: 8k
thinProvision: "off"
volumeType: DATASET
status:
state: Ready
16 changes: 16 additions & 0 deletions deploy/yamls/zfsvolume-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ spec:
description: Size of the volume
name: Size
type: string
- JSONPath: .status.state
description: Status of the volume
pawanpraka1 marked this conversation as resolved.
Show resolved Hide resolved
name: Status
type: string
- JSONPath: .spec.volblocksize
description: volblocksize of volume
name: volblocksize
Expand Down Expand Up @@ -216,6 +220,18 @@ spec:
- poolName
- volumeType
type: object
status:
properties:
state:
description: State specifies the current state of the volume provisioning
pawanpraka1 marked this conversation as resolved.
Show resolved Hide resolved
request. The state "Pending" means that the volume creation request
has not processed yet. The state "Ready" means that the volume has
been created and it is ready for the use.
enum:
- Pending
- Ready
type: string
type: object
required:
- spec
type: object
Expand Down
16 changes: 16 additions & 0 deletions deploy/zfs-operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ spec:
description: Size of the volume
name: Size
type: string
- JSONPath: .status.state
description: Status of the volume
name: Status
type: string
- JSONPath: .spec.volblocksize
description: volblocksize of volume
name: volblocksize
Expand Down Expand Up @@ -237,6 +241,18 @@ spec:
- poolName
- volumeType
type: object
status:
properties:
state:
description: State specifies the current state of the volume provisioning
request. The state "Pending" means that the volume creation request
has not processed yet. The state "Ready" means that the volume has
been created and it is ready for the use.
enum:
- Pending
- Ready
type: string
type: object
required:
- spec
type: object
Expand Down
2 changes: 2 additions & 0 deletions docs/import-existing-volume.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ spec:
ownerNodeID: pawan-3 # should be the nodename where ZPOOL is running
poolName: zfspv-pool # poolname where the volume is present
volumeType: DATASET # whether it is a DATASET or ZVOL
Status:
State: Ready
```

Modify the parameters :-
Expand Down
13 changes: 12 additions & 1 deletion pkg/apis/openebs.io/zfs/v1alpha1/zfsvolume.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
// +kubebuilder:printcolumn:name="ZPool",type=string,JSONPath=`.spec.poolName`,description="ZFS Pool where the volume is created"
// +kubebuilder:printcolumn:name="Node",type=string,JSONPath=`.spec.ownerNodeID`,description="Node where the volume is created"
// +kubebuilder:printcolumn:name="Size",type=string,JSONPath=`.spec.capacity`,description="Size of the volume"
// +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.state`,description="Status of the volume"
// +kubebuilder:printcolumn:name="volblocksize",type=string,JSONPath=`.spec.volblocksize`,description="volblocksize of volume"
// +kubebuilder:printcolumn:name="recordsize",type=string,JSONPath=`.spec.recordsize`,description="recordsize of created zfs dataset"
// +kubebuilder:printcolumn:name="Filesystem",type=string,JSONPath=`.spec.fsType`,description="filesystem created on the volume"
Expand All @@ -39,7 +40,8 @@ type ZFSVolume struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec VolumeInfo `json:"spec"`
Spec VolumeInfo `json:"spec"`
Status VolStatus `json:"status,omitempty"`
}

// MountInfo contains the volume related info
Expand Down Expand Up @@ -198,3 +200,12 @@ type VolumeInfo struct {
// Default Value: ext4.
FsType string `json:"fsType,omitempty"`
}

type VolStatus struct {
// State specifies the current state of the volume provisioning request.
// The state "Pending" means that the volume creation request has not
// processed yet. The state "Ready" means that the volume has been created
// and it is ready for the use.
// +kubebuilder:validation:Enum=Pending;Ready
State string `json:"state,omitempty"`
}
17 changes: 17 additions & 0 deletions pkg/apis/openebs.io/zfs/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions pkg/builder/volbuilder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ func (b *Builder) WithVolumeType(vtype string) *Builder {
return b
}

// WithVolumeStatus sets ZFSVolume status
func (b *Builder) WithVolumeStatus(status string) *Builder {
b.volume.Object.Status.State = status
return b
}

// WithFsType sets filesystem for the ZFSVolume
func (b *Builder) WithFsType(fstype string) *Builder {
b.volume.Object.Spec.FsType = fstype
Expand Down
29 changes: 27 additions & 2 deletions pkg/driver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func CreateZFSVolume(req *csi.CreateVolumeRequest) (string, error) {
WithThinProv(tp).
WithOwnerNode(selected).
WithVolumeType(vtype).
WithVolumeStatus(zfs.ZFSStatusPending).
WithFsType(fstype).
WithCompression(compression).Build()

Expand Down Expand Up @@ -161,7 +162,9 @@ func CreateZFSClone(req *csi.CreateVolumeRequest, snapshot string) (string, erro
selected := snap.Spec.OwnerNodeID

volObj, err := volbuilder.NewBuilder().
WithName(volName).Build()
WithName(volName).
WithVolumeStatus(zfs.ZFSStatusPending).
Build()

volObj.Spec = snap.Spec
volObj.Spec.SnapName = snapshot
Expand All @@ -187,12 +190,22 @@ func (cs *controller) CreateVolume(
volName := req.GetName()
pool := req.GetParameters()["poolname"]
size := req.GetCapacityRange().RequiredBytes
contentSource := req.GetVolumeContentSource()

if err = cs.validateVolumeCreateReq(req); err != nil {
return nil, err
}

contentSource := req.GetVolumeContentSource()
selected, state, err := zfs.GetZFSVolumeState(req.Name)

if err == nil {
// ZFSVolume CR has been created, check if it is in Ready state
if state == zfs.ZFSStatusReady {
goto CreateVolumeResponse
}
return nil, status.Errorf(codes.Internal, "volume %s creation is Pending", volName)
}

if contentSource != nil && contentSource.GetSnapshot() != nil {
snapshotID := contentSource.GetSnapshot().GetSnapshotId()

Expand All @@ -205,6 +218,18 @@ func (cs *controller) CreateVolume(
return nil, status.Error(codes.Internal, err.Error())
}

_, state, err = zfs.GetZFSVolumeState(req.Name)

if err != nil {
return nil, status.Errorf(codes.Internal, "createvolume: failed to fetch the volume %v", err.Error())
}

if state == zfs.ZFSStatusPending {
return nil, status.Errorf(codes.Internal, "volume %s is being created", volName)
kmova marked this conversation as resolved.
Show resolved Hide resolved
}

CreateVolumeResponse:

sendEventOrIgnore(volName, strconv.FormatInt(int64(size), 10), "zfs-localpv", analytics.VolumeProvision)

topology := map[string]string{zfs.ZFSTopologyKey: selected}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions pkg/zfs/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,21 @@ func GetZFSVolume(volumeID string) (*apis.ZFSVolume, error) {
return vol, err
}

// GetZFSVolumeState returns ZFSVolume OwnerNode and State for
// the given volume. CreateVolume request may call it again and
// again until volume is "Ready".
func GetZFSVolumeState(volID string) (string, string, error) {
getOptions := metav1.GetOptions{}
vol, err := volbuilder.NewKubeclient().
WithNamespace(OpenEBSNamespace).Get(volID, getOptions)

if err != nil {
return "", "", err
}

return vol.Spec.OwnerNodeID, vol.Status.State, nil
}

// UpdateZvolInfo updates ZFSVolume CR with node id and finalizer
func UpdateZvolInfo(vol *apis.ZFSVolume) error {
finalizers := []string{ZFSFinalizer}
Expand All @@ -168,6 +183,7 @@ func UpdateZvolInfo(vol *apis.ZFSVolume) error {

newVol, err := volbuilder.BuildFrom(vol).
WithFinalizer(finalizers).
WithVolumeStatus(ZFSStatusReady).
WithLabels(labels).Build()

if err != nil {
Expand Down