From 3fab614e4092cd58225cb2e160b82a672e77d45d Mon Sep 17 00:00:00 2001 From: Yashpal Choudhary Date: Wed, 17 Feb 2021 18:09:43 +0530 Subject: [PATCH] feat(provisioning) enable pod resheduling cause of node insufficient capacity It also involves some refactoring of csi param parsing and volume provisioing workflow improving timeout, error handling & idempotency. Signed-off-by: Yashpal Choudhary --- deploy/lvm-operator.yaml | 16 +- deploy/yamls/lvmvolume-crd.yaml | 16 +- pkg/apis/openebs.io/lvm/v1alpha1/lvmvolume.go | 27 ++- .../lvm/v1alpha1/zz_generated.deepcopy.go | 23 ++- pkg/driver/controller.go | 168 ++++++++++-------- pkg/driver/params.go | 72 ++++++++ pkg/driver/schd_helper.go | 3 +- pkg/lvm/lvm_util.go | 24 +++ pkg/lvm/volume.go | 72 ++++++-- pkg/mgmt/volume/volume.go | 55 ++++-- 10 files changed, 369 insertions(+), 107 deletions(-) create mode 100644 pkg/driver/params.go diff --git a/deploy/lvm-operator.yaml b/deploy/lvm-operator.yaml index df0a031a..2cac6b91 100644 --- a/deploy/lvm-operator.yaml +++ b/deploy/lvm-operator.yaml @@ -123,14 +123,28 @@ spec: description: VolStatus string that specifies the current state of the volume provisioning request. properties: + error: + description: Error denotes the error occurred during provisioning/expanding + a volume. Error field should only be set when State becomes Failed. + properties: + code: + description: VolumeErrorCode represents the error code to represent + specific class of errors. + type: string + message: + type: string + type: object 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. + been created and it is ready for the use. "Failed" means that volume + provisioning has been failed and will not be retried by node agent + controller. enum: - Pending - Ready + - Failed type: string type: object required: diff --git a/deploy/yamls/lvmvolume-crd.yaml b/deploy/yamls/lvmvolume-crd.yaml index a78e5a91..19833740 100644 --- a/deploy/yamls/lvmvolume-crd.yaml +++ b/deploy/yamls/lvmvolume-crd.yaml @@ -102,14 +102,28 @@ spec: description: VolStatus string that specifies the current state of the volume provisioning request. properties: + error: + description: Error denotes the error occurred during provisioning/expanding + a volume. Error field should only be set when State becomes Failed. + properties: + code: + description: VolumeErrorCode represents the error code to represent + specific class of errors. + type: string + message: + type: string + type: object 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. + been created and it is ready for the use. "Failed" means that volume + provisioning has been failed and will not be retried by node agent + controller. enum: - Pending - Ready + - Failed type: string type: object required: diff --git a/pkg/apis/openebs.io/lvm/v1alpha1/lvmvolume.go b/pkg/apis/openebs.io/lvm/v1alpha1/lvmvolume.go index 0f621524..f5a6ef7b 100644 --- a/pkg/apis/openebs.io/lvm/v1alpha1/lvmvolume.go +++ b/pkg/apis/openebs.io/lvm/v1alpha1/lvmvolume.go @@ -83,7 +83,30 @@ 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 + // and it is ready for the use. "Failed" means that volume provisioning + // has been failed and will not be retried by node agent controller. + // +kubebuilder:validation:Enum=Pending;Ready;Failed State string `json:"state,omitempty"` + + // Error denotes the error occurred during provisioning/expanding a volume. + // Error field should only be set when State becomes Failed. + Error *VolumeError `json:"error,omitempty"` +} + +// VolumeError specifies the error occurred during volume provisioning. +type VolumeError struct { + Code VolumeErrorCode `json:"code,omitempty"` + Message string `json:"message,omitempty"` } + +// VolumeErrorCode represents the error code to represent +// specific class of errors. +type VolumeErrorCode string + +const ( + // Internal represents system internal error. + Internal VolumeErrorCode = "Internal" + // InsufficientCapacity represent lvm vg doesn't + // have enough capacity to fit the lv request. + InsufficientCapacity VolumeErrorCode = "InsufficientCapacity" +) diff --git a/pkg/apis/openebs.io/lvm/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/openebs.io/lvm/v1alpha1/zz_generated.deepcopy.go index 16c02c22..a95595d0 100644 --- a/pkg/apis/openebs.io/lvm/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/openebs.io/lvm/v1alpha1/zz_generated.deepcopy.go @@ -91,7 +91,7 @@ func (in *LVMVolume) DeepCopyInto(out *LVMVolume) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) out.Spec = in.Spec - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) return } @@ -165,6 +165,11 @@ func (in *SnapStatus) DeepCopy() *SnapStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolStatus) DeepCopyInto(out *VolStatus) { *out = *in + if in.Error != nil { + in, out := &in.Error, &out.Error + *out = new(VolumeError) + **out = **in + } return } @@ -178,6 +183,22 @@ func (in *VolStatus) DeepCopy() *VolStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VolumeError) DeepCopyInto(out *VolumeError) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VolumeError. +func (in *VolumeError) DeepCopy() *VolumeError { + if in == nil { + return nil + } + out := new(VolumeError) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolumeInfo) DeepCopyInto(out *VolumeInfo) { *out = *in diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index 8bfdbe4c..7312f2d1 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -18,7 +18,6 @@ package driver import ( "fmt" - "github.com/openebs/lvm-localpv/pkg/builder/snapbuilder" "strconv" "strings" "time" @@ -31,8 +30,9 @@ import ( "k8s.io/klog" errors "github.com/openebs/lib-csi/pkg/common/errors" - "github.com/openebs/lib-csi/pkg/common/helpers" schd "github.com/openebs/lib-csi/pkg/scheduler" + lvmapi "github.com/openebs/lvm-localpv/pkg/apis/openebs.io/lvm/v1alpha1" + "github.com/openebs/lvm-localpv/pkg/builder/snapbuilder" "github.com/openebs/lvm-localpv/pkg/builder/volbuilder" "github.com/openebs/lvm-localpv/pkg/lvm" csipayload "github.com/openebs/lvm-localpv/pkg/response" @@ -101,107 +101,124 @@ func getRoundedCapacity(size int64) int64 { return ((size + Mi - 1) / Mi) * Mi } -func waitForReadyVolume(volname string) error { - for true { - vol, err := lvm.GetLVMVolume(volname) - if err != nil { - return status.Errorf(codes.Internal, - "lvm: wait failed, not able to get the volume %s %s", volname, err.Error()) +// waitForLVMVolume waits for completion of any processing of lvm volume. +// It returns the final status of lvm volume along with a boolean denoting +// whether it should be rescheduled on some other volume group or node. +// In case volume ends up in failed state and rescheduling is required, +// func is also deleting the lvm volume resource, so that it can be +// re provisioned on some other node. +func waitForLVMVolume(ctx context.Context, + vol *lvmapi.LVMVolume) (*lvmapi.LVMVolume, bool, error) { + var reschedule bool // tracks if rescheduling is required or not. + var err error + if vol.Status.State == lvm.LVMStatusPending { + if vol, err = lvm.WaitForLVMVolumeProcessed(ctx, vol.GetName()); err != nil { + return nil, false, err } + } + // if lvm volume is ready, return the provisioned node. + if vol.Status.State == lvm.LVMStatusReady { + return vol, false, nil + } - switch vol.Status.State { - case lvm.LVMStatusReady: - return nil + // Now it must be in failed state if not above. See if we need + // to reschedule the lvm volume. + var errMsg string + if volErr := vol.Status.Error; volErr != nil { + errMsg = volErr.Message + if volErr.Code == lvmapi.InsufficientCapacity { + reschedule = true } - time.Sleep(time.Second) + } else { + errMsg = fmt.Sprintf("failed lvmvol must have error set") } - return nil -} -func waitForVolDestroy(volname string) error { - for true { - _, err := lvm.GetLVMVolume(volname) - if err != nil { - if k8serror.IsNotFound(err) { - return nil - } - return status.Errorf(codes.Internal, - "lvm: destroy wait failed, not able to get the volume %s %s", volname, err.Error()) + if reschedule { + // if rescheduling is required, we can deleted the existing lvm volume object, + // so that it can be recreated. + if err = lvm.DeleteVolume(vol.GetName()); err != nil { + return nil, false, status.Errorf(codes.Aborted, + "failed to delete volume %v: %v", vol.GetName(), err) } - time.Sleep(time.Second) + if err = lvm.WaitForLVMVolumeDestroy(ctx, vol.GetName()); err != nil { + return nil, false, status.Errorf(codes.Aborted, + "failed to delete volume %v: %v", vol.GetName(), err) + } + return vol, true, status.Error(codes.ResourceExhausted, errMsg) } - return nil + + return vol, false, status.Error(codes.Aborted, errMsg) } // CreateLVMVolume create new lvm volume for csi volume request -func CreateLVMVolume(req *csi.CreateVolumeRequest) (string, error) { +func CreateLVMVolume(ctx context.Context, req *csi.CreateVolumeRequest, + params *VolumeParams) (*lvmapi.LVMVolume, error) { volName := strings.ToLower(req.GetName()) - size := getRoundedCapacity(req.GetCapacityRange().RequiredBytes) - - // parameter keys may be mistyped from the CRD specification when declaring - // the storageclass, which kubectl validation will not catch. Because - // parameter keys (not values!) are all lowercase, keys may safely be forced - // to the lower case. - originalParams := req.GetParameters() - parameters := helpers.GetCaseInsensitiveMap(&originalParams) - - vg := parameters["volgroup"] - schld := parameters["scheduler"] - shared := parameters["shared"] + capacity := strconv.FormatInt(getRoundedCapacity( + req.GetCapacityRange().RequiredBytes), 10) - capacity := strconv.FormatInt(int64(size), 10) + vol, err := lvm.GetLVMVolume(volName) + if err != nil { + if !k8serror.IsNotFound(err) { + return nil, status.Errorf(codes.Aborted, + "failed get lvm volume %v: %v", volName, err.Error()) + } + vol, err = nil, nil + } - if vol, err := lvm.GetLVMVolume(volName); err == nil { + if vol != nil { if vol.DeletionTimestamp != nil { - if _, ok := parameters["wait"]; ok { - if err := waitForVolDestroy(volName); err != nil { - return "", err - } + if err = lvm.WaitForLVMVolumeDestroy(ctx, volName); err != nil { + return nil, err } } else { if vol.Spec.Capacity != capacity { - return "", status.Errorf(codes.AlreadyExists, + return nil, status.Errorf(codes.AlreadyExists, "volume %s already present", volName) } - return vol.Spec.OwnerNodeID, nil + var reschedule bool + vol, reschedule, err = waitForLVMVolume(ctx, vol) + // If the lvm volume becomes ready or we can't reschedule failed volume, + // return the err. + if err == nil || !reschedule { + return vol, err + } } } - nmap, err := getNodeMap(schld, vg) + nmap, err := getNodeMap(params.Scheduler, params.VolumeGroup) if err != nil { - return "", status.Errorf(codes.Internal, "get node map failed : %s", err.Error()) + return nil, status.Errorf(codes.Internal, "get node map failed : %s", err.Error()) } // run the scheduler selected := schd.Scheduler(req, nmap) if len(selected) == 0 { - return "", status.Error(codes.Internal, "scheduler failed, not able to select a node to create the PV") + return nil, status.Error(codes.Internal, "scheduler failed, not able to select a node to create the PV") } owner := selected[0] - - klog.Infof("scheduled the volume %s/%s on node %s", vg, volName, selected) + klog.Infof("scheduling the volume %s/%s on node %s", params.VolumeGroup, volName, owner) volObj, err := volbuilder.NewBuilder(). WithName(volName). WithCapacity(capacity). - WithVolGroup(vg). + WithVolGroup(params.VolumeGroup). WithOwnerNode(owner). WithVolumeStatus(lvm.LVMStatusPending). - WithShared(shared).Build() + WithShared(params.Shared).Build() if err != nil { - return "", status.Error(codes.Internal, err.Error()) + return nil, status.Error(codes.Internal, err.Error()) } - err = lvm.ProvisionVolume(volObj) + vol, err = lvm.ProvisionVolume(volObj) if err != nil { - return "", status.Errorf(codes.Internal, - "not able to provision the volume %s", err.Error()) + return nil, status.Errorf(codes.Internal, "not able to provision the volume %s", err.Error()) } - - return owner, nil + vol, _, err = waitForLVMVolume(ctx, vol) + return vol, err } // CreateVolume provisions a volume @@ -210,43 +227,38 @@ func (cs *controller) CreateVolume( req *csi.CreateVolumeRequest, ) (*csi.CreateVolumeResponse, error) { - var err error - var selected string + if err := cs.validateVolumeCreateReq(req); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } - if err = cs.validateVolumeCreateReq(req); err != nil { - return nil, err + params, err := NewVolumeParams(req.GetParameters()) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, + "failed to parse csi volume params: %v", err) } volName := strings.ToLower(req.GetName()) - parameters := req.GetParameters() - // lower case keys, cf CreateLVMVolume() - vg := helpers.GetInsensitiveParameter(¶meters, "volgroup") size := getRoundedCapacity(req.GetCapacityRange().GetRequiredBytes()) contentSource := req.GetVolumeContentSource() - pvcName := helpers.GetInsensitiveParameter(¶meters, "csi.storage.k8s.io/pvc/name") + var vol *lvmapi.LVMVolume if contentSource != nil && contentSource.GetSnapshot() != nil { return nil, status.Error(codes.Unimplemented, "") } else if contentSource != nil && contentSource.GetVolume() != nil { return nil, status.Error(codes.Unimplemented, "") } else { - selected, err = CreateLVMVolume(req) + vol, err = CreateLVMVolume(ctx, req, params) } if err != nil { return nil, err } + sendEventOrIgnore(params.PVCName, volName, + strconv.FormatInt(int64(size), 10), + "lvm-localpv", analytics.VolumeProvision) - if _, ok := parameters["wait"]; ok { - if err := waitForReadyVolume(volName); err != nil { - return nil, err - } - } - - sendEventOrIgnore(pvcName, volName, strconv.FormatInt(int64(size), 10), "lvm-localpv", analytics.VolumeProvision) - - topology := map[string]string{lvm.LVMTopologyKey: selected} - cntx := map[string]string{lvm.VolGroupKey: vg} + topology := map[string]string{lvm.LVMTopologyKey: vol.Spec.OwnerNodeID} + cntx := map[string]string{lvm.VolGroupKey: params.VolumeGroup} return csipayload.NewCreateVolumeResponseBuilder(). WithName(volName). diff --git a/pkg/driver/params.go b/pkg/driver/params.go new file mode 100644 index 00000000..e6e169d4 --- /dev/null +++ b/pkg/driver/params.go @@ -0,0 +1,72 @@ +/* + Copyright © 2021 The OpenEBS Authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package driver + +import ( + "github.com/openebs/lib-csi/pkg/common/helpers" +) + +// VolumeParams holds collection of supported settings that can +// be configured in storage class. +type VolumeParams struct { + // VolumeGroup specifies vg name to use for + // provisioning logical volumes. + VolumeGroup string + + Scheduler string + Shared string + + // extra optional metadata passed by external provisioner + // if enabled. See --extra-create-metadata flag for more details. + // https://github.com/kubernetes-csi/external-provisioner#recommended-optional-arguments + PVCName string + PVCNamespace string + PVName string +} + +// NewVolumeParams parses the input params and instantiates new VolumeParams. +func NewVolumeParams(m map[string]string) (*VolumeParams, error) { + params := &VolumeParams{ // set up defaults, if any. + Scheduler: CapacityWeighted, + Shared: "no", + } + // parameter keys may be mistyped from the CRD specification when declaring + // the storageclass, which kubectl validation will not catch. Because + // parameter keys (not values!) are all lowercase, keys may safely be forced + // to the lower case. + m = helpers.GetCaseInsensitiveMap(&m) + params.VolumeGroup = m["volgroup"] + + // parse string params + stringParams := map[string]*string{ + "scheduler": ¶ms.Scheduler, + "shared": ¶ms.Shared, + } + for key, param := range stringParams { + value, ok := m[key] + if !ok { + continue + } + *param = value + } + + params.PVCName = m["csi.storage.k8s.io/pvc/name"] + params.PVCNamespace = m["csi.storage.k8s.io/pvc/namespace"] + params.PVName = m["csi.storage.k8s.io/pv/name"] + + return params, nil +} diff --git a/pkg/driver/schd_helper.go b/pkg/driver/schd_helper.go index 06a92613..ab406bd2 100644 --- a/pkg/driver/schd_helper.go +++ b/pkg/driver/schd_helper.go @@ -17,9 +17,10 @@ limitations under the License. package driver import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "strconv" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/openebs/lvm-localpv/pkg/builder/volbuilder" "github.com/openebs/lvm-localpv/pkg/lvm" ) diff --git a/pkg/lvm/lvm_util.go b/pkg/lvm/lvm_util.go index 3337a03e..ba09493c 100644 --- a/pkg/lvm/lvm_util.go +++ b/pkg/lvm/lvm_util.go @@ -17,6 +17,7 @@ limitations under the License. package lvm import ( + "fmt" "os" "os/exec" @@ -40,6 +41,28 @@ const ( LVExtend = "lvextend" ) +// ExecError holds the process output along with underlying +// error returned by exec.CombinedOutput function. +type ExecError struct { + Output []byte + Err error +} + +// Error implements the error interface. +func (e *ExecError) Error() string { + return fmt.Sprintf("%v - %v", string(e.Output), e.Err) +} + +func newExecError(output []byte, err error) error { + if err == nil { + return nil + } + return &ExecError{ + Output: output, + Err: err, + } +} + // builldLVMCreateArgs returns lvcreate command for the volume func buildLVMCreateArgs(vol *apis.LVMVolume) []string { var LVMVolArg []string @@ -89,6 +112,7 @@ func CreateVolume(vol *apis.LVMVolume) error { out, err := cmd.CombinedOutput() if err != nil { + err = newExecError(out, err) klog.Errorf( "lvm: could not create volume %v cmd %v error: %s", volume, args, string(out), ) diff --git a/pkg/lvm/volume.go b/pkg/lvm/volume.go index 5780f628..81364eef 100644 --- a/pkg/lvm/volume.go +++ b/pkg/lvm/volume.go @@ -15,12 +15,17 @@ package lvm import ( + "context" "os" "strconv" + "time" apis "github.com/openebs/lvm-localpv/pkg/apis/openebs.io/lvm/v1alpha1" "github.com/openebs/lvm-localpv/pkg/builder/snapbuilder" "github.com/openebs/lvm-localpv/pkg/builder/volbuilder" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + k8serror "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog" ) @@ -77,16 +82,14 @@ func init() { // ProvisionVolume creates a LVMVolume CR, // watcher for volume is present in CSI agent -func ProvisionVolume( - vol *apis.LVMVolume, -) error { +func ProvisionVolume(vol *apis.LVMVolume) (*apis.LVMVolume, error) { - _, err := volbuilder.NewKubeclient().WithNamespace(LvmNamespace).Create(vol) + createdVol, err := volbuilder.NewKubeclient().WithNamespace(LvmNamespace).Create(vol) if err == nil { klog.Infof("provisioned volume %s", vol.Name) } - return err + return createdVol, err } // DeleteVolume deletes the corresponding LVM Volume CR @@ -107,6 +110,52 @@ func GetLVMVolume(volumeID string) (*apis.LVMVolume, error) { return vol, err } +// WaitForLVMVolumeProcessed waits till the lvm volume becomes +// ready or failed (i.e reaches to terminal state). +func WaitForLVMVolumeProcessed(ctx context.Context, volumeID string) (*apis.LVMVolume, error) { + timer := time.NewTimer(0) + defer timer.Stop() + for { + select { + case <-ctx.Done(): + return nil, status.FromContextError(ctx.Err()) + case <-timer.C: + } + vol, err := GetLVMVolume(volumeID) + if err != nil { + return nil, status.Errorf(codes.Aborted, + "lvm: wait failed, not able to get the volume %s %s", volumeID, err.Error()) + } + if vol.Status.State == LVMStatusReady || + vol.Status.State == LVMStatusFailed { + return vol, nil + } + timer.Reset(1 * time.Second) + } +} + +// WaitForLVMVolumeDestroy waits till the lvm volume gets deleted. +func WaitForLVMVolumeDestroy(ctx context.Context, volumeID string) error { + timer := time.NewTimer(0) + defer timer.Stop() + for { + select { + case <-ctx.Done(): + return status.FromContextError(ctx.Err()) + case <-timer.C: + } + _, err := GetLVMVolume(volumeID) + if err != nil { + if k8serror.IsNotFound(err) { + return nil + } + return status.Errorf(codes.Aborted, + "lvm: destroy wait failed, not able to get the volume %s %s", volumeID, err.Error()) + } + timer.Reset(1 * time.Second) + } +} + // GetLVMVolumeState returns LVMVolume OwnerNode and State for // the given volume. CreateVolume request may call it again and // again until volume is "Ready". @@ -123,17 +172,20 @@ func GetLVMVolumeState(volID string) (string, string, error) { } // UpdateVolInfo updates LVMVolume CR with node id and finalizer -func UpdateVolInfo(vol *apis.LVMVolume) error { - finalizers := []string{LVMFinalizer} - labels := map[string]string{LVMNodeKey: NodeID} - +func UpdateVolInfo(vol *apis.LVMVolume, state string) error { if vol.Finalizers != nil { return nil } + var finalizers []string + labels := map[string]string{LVMNodeKey: NodeID} + switch state { + case LVMStatusReady: + finalizers = append(finalizers, LVMFinalizer) + } newVol, err := volbuilder.BuildFrom(vol). WithFinalizer(finalizers). - WithVolumeStatus(LVMStatusReady). + WithVolumeStatus(state). WithLabels(labels).Build() if err != nil { diff --git a/pkg/mgmt/volume/volume.go b/pkg/mgmt/volume/volume.go index b5525ee2..dfec066a 100644 --- a/pkg/mgmt/volume/volume.go +++ b/pkg/mgmt/volume/volume.go @@ -18,6 +18,7 @@ package volume import ( "fmt" + "strings" "time" apis "github.com/openebs/lvm-localpv/pkg/apis/openebs.io/lvm/v1alpha1" @@ -74,28 +75,56 @@ func (c *VolController) enqueueVol(obj interface{}) { // synVol is the function which tries to converge to a desired state for the // LVMVolume -func (c *VolController) syncVol(Vol *apis.LVMVolume) error { +func (c *VolController) syncVol(vol *apis.LVMVolume) error { var err error // LVM Volume should be deleted. Check if deletion timestamp is set - if c.isDeletionCandidate(Vol) { - err = lvm.DestroyVolume(Vol) + if c.isDeletionCandidate(vol) { + err = lvm.DestroyVolume(vol) if err == nil { - err = lvm.RemoveVolFinalizer(Vol) + err = lvm.RemoveVolFinalizer(vol) } - } else { - // if finalizer is not set then it means we are creating - // the volume. And if it is set then volume has already been - // created and this event is for property change only. - if Vol.Status.State != lvm.LVMStatusReady { - err = lvm.CreateVolume(Vol) - if err == nil { - err = lvm.UpdateVolInfo(Vol) - } + return err + } + // if status is Pending then it means we are creating the volume. + // Otherwise, we are just ignoring the event. + switch vol.Status.State { + case lvm.LVMStatusFailed: + klog.Warningf("Skipping retrying lvm volume provisioning as its already in failed state: %+v", vol.Status.Error) + return nil + case lvm.LVMStatusReady: + klog.Info("lvm volume already provisioned") + return nil + } + + if err = lvm.CreateVolume(vol); err != nil { + // validate if it's insufficient space error & accordingly set up the error code. + if volErr := c.transformLVMError(err); volErr.Code == apis.InsufficientCapacity { + vol.Status.Error = volErr + err = lvm.UpdateVolInfo(vol, lvm.LVMStatusFailed) } + return err } + err = lvm.UpdateVolInfo(vol, lvm.LVMStatusReady) return err } +func (c *VolController) transformLVMError(err error) *apis.VolumeError { + volErr := &apis.VolumeError{ + Code: apis.Internal, + Message: err.Error(), + } + execErr, ok := err.(*lvm.ExecError) + if !ok { + return volErr + } + + if strings.Contains(strings.ToLower(string(execErr.Output)), + "insufficient free space") { + volErr.Code = apis.InsufficientCapacity + } + return volErr +} + // addVol is the add event handler for LVMVolume func (c *VolController) addVol(obj interface{}) { Vol, ok := obj.(*apis.LVMVolume)