From 7ff68b3589b2a6cab52bb287f1f88bdb9df07970 Mon Sep 17 00:00:00 2001 From: Aleksandr Zimin Date: Thu, 7 Mar 2024 00:45:55 +0300 Subject: [PATCH 1/4] fix Signed-off-by: Aleksandr Zimin --- .../api/v1alpha1/lvm_logical_volume.go | 9 ++-- .../sds-local-volume-csi/driver/controller.go | 23 ++++++--- images/sds-local-volume-csi/pkg/utils/func.go | 50 +++++++++++++------ 3 files changed, 56 insertions(+), 26 deletions(-) diff --git a/images/sds-local-volume-csi/api/v1alpha1/lvm_logical_volume.go b/images/sds-local-volume-csi/api/v1alpha1/lvm_logical_volume.go index 6f731809..84bad5af 100644 --- a/images/sds-local-volume-csi/api/v1alpha1/lvm_logical_volume.go +++ b/images/sds-local-volume-csi/api/v1alpha1/lvm_logical_volume.go @@ -37,10 +37,11 @@ type LVMLogicalVolume struct { } type LVMLogicalVolumeSpec struct { - Type string `json:"type"` - Size resource.Quantity `json:"size"` - LvmVolumeGroup string `json:"lvmVolumeGroup"` - Thin *ThinLogicalVolumeSpec `json:"thin"` + ActualLVNameOnTheNode string `json:"actualLVNameOnTheNode"` + Type string `json:"type"` + Size resource.Quantity `json:"size"` + LvmVolumeGroupName string `json:"lvmVolumeGroupName"` + Thin *ThinLogicalVolumeSpec `json:"thin"` } type ThinLogicalVolumeSpec struct { diff --git a/images/sds-local-volume-csi/driver/controller.go b/images/sds-local-volume-csi/driver/controller.go index dbd0b337..cc23d699 100644 --- a/images/sds-local-volume-csi/driver/controller.go +++ b/images/sds-local-volume-csi/driver/controller.go @@ -68,6 +68,7 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ } llvName := request.Name + lvName := request.Name d.log.Info(fmt.Sprintf("llv name: %s ", llvName)) llvSize := resource.NewQuantity(request.CapacityRange.GetRequiredBytes(), resource.BinarySI) @@ -103,8 +104,13 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ return nil, status.Errorf(codes.Internal, err.Error()) } - llvSpec := utils.GetLLVSpec(*d.log, selectedLVG, storageClassLVGParametersMap, preferredNode, LvmType, *llvSize) + llvSpec := utils.GetLLVSpec(*d.log, lvName, selectedLVG, storageClassLVGParametersMap, preferredNode, LvmType, *llvSize) d.log.Info(fmt.Sprintf("LVMLogicalVolumeSpec : %+v", llvSpec)) + resizeDelta, err := resource.ParseQuantity(internal.ResizeDelta) + if err != nil { + d.log.Error(err, "error ParseQuantity for ResizeDelta") + return nil, err + } d.log.Trace("------------ CreateLVMLogicalVolume start ------------") _, err = utils.CreateLVMLogicalVolume(ctx, d.cl, llvName, llvSpec) @@ -119,14 +125,15 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ d.log.Trace("------------ CreateLVMLogicalVolume end ------------") d.log.Trace("start wait CreateLVMLogicalVolume ") - resizeDelta, err := resource.ParseQuantity(internal.ResizeDelta) - if err != nil { - d.log.Error(err, "error ParseQuantity for ResizeDelta") - return nil, err - } + attemptCounter, err := utils.WaitForStatusUpdate(ctx, d.cl, *d.log, request.Name, "", *llvSize, resizeDelta) if err != nil { - d.log.Error(err, "error WaitForStatusUpdate") + deleteErr := utils.DeleteLVMLogicalVolume(ctx, d.cl, request.Name) + + d.log.Error(err, fmt.Sprintf("error WaitForStatusUpdate. Delete LVMLogicalVolume %s", request.Name)) + if deleteErr != nil { + d.log.Error(deleteErr, "error DeleteLVMLogicalVolume") + } return nil, err } d.log.Trace(fmt.Sprintf("stop waiting CreateLVMLogicalVolume, attempt counter = %d ", attemptCounter)) @@ -284,7 +291,7 @@ func (d *Driver) ControllerExpandVolume(ctx context.Context, request *csi.Contro }, nil } - lvg, err := utils.GetLVMVolumeGroup(ctx, d.cl, llv.Spec.LvmVolumeGroup, llv.Namespace) + lvg, err := utils.GetLVMVolumeGroup(ctx, d.cl, llv.Spec.LvmVolumeGroupName, llv.Namespace) if err != nil { return nil, status.Errorf(codes.Internal, "error getting LVMVolumeGroup: %v", err) } diff --git a/images/sds-local-volume-csi/pkg/utils/func.go b/images/sds-local-volume-csi/pkg/utils/func.go index c23185e1..f5152618 100644 --- a/images/sds-local-volume-csi/pkg/utils/func.go +++ b/images/sds-local-volume-csi/pkg/utils/func.go @@ -24,6 +24,7 @@ import ( "sds-local-volume-csi/api/v1alpha1" "sds-local-volume-csi/internal" "sds-local-volume-csi/pkg/logger" + "slices" "time" "gopkg.in/yaml.v2" @@ -40,6 +41,7 @@ const ( LLVTypeThin = "Thin" KubernetesApiRequestLimit = 3 KubernetesApiRequestTimeout = 1 + SDSLocalVolumeCSIFinalizer = "storage.deckhouse.io/sds-local-volume-csi" ) func CreateLVMLogicalVolume(ctx context.Context, kc client.Client, name string, LVMLogicalVolumeSpec v1alpha1.LVMLogicalVolumeSpec) (*v1alpha1.LVMLogicalVolume, error) { @@ -52,6 +54,7 @@ func CreateLVMLogicalVolume(ctx context.Context, kc client.Client, name string, ObjectMeta: metav1.ObjectMeta{ Name: name, OwnerReferences: []metav1.OwnerReference{}, + Finalizers: []string{SDSLocalVolumeCSIFinalizer}, }, Spec: LVMLogicalVolumeSpec, } @@ -77,18 +80,19 @@ func CreateLVMLogicalVolume(ctx context.Context, kc client.Client, name string, func DeleteLVMLogicalVolume(ctx context.Context, kc client.Client, LVMLogicalVolumeName string) error { var err error - llv := &v1alpha1.LVMLogicalVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: LVMLogicalVolumeName, - }, - TypeMeta: metav1.TypeMeta{ - Kind: v1alpha1.LVMLogicalVolumeKind, - APIVersion: v1alpha1.TypeMediaAPIVersion, - }, + + llv, err := GetLVMLogicalVolume(ctx, kc, LVMLogicalVolumeName, "") + if err != nil { + return fmt.Errorf("get LVMLogicalVolume %s: %w", LVMLogicalVolumeName, err) + } + + err = removeLLVFinalizerIfExist(ctx, kc, llv, SDSLocalVolumeCSIFinalizer) + if err != nil { + return fmt.Errorf("remove finalizers from LVMLogicalVolume %s: %w", LVMLogicalVolumeName, err) } for attempt := 0; attempt < KubernetesApiRequestLimit; attempt++ { - err := kc.Delete(ctx, llv) + err = kc.Delete(ctx, llv) if err == nil { return nil } @@ -373,7 +377,7 @@ func GetLVGList(ctx context.Context, kc client.Client) (*v1alpha1.LvmVolumeGroup return nil, fmt.Errorf("after %d attempts of getting LvmVolumeGroupList, last error: %w", KubernetesApiRequestLimit, err) } -func GetLLVSpec(log logger.Logger, selectedLVG v1alpha1.LvmVolumeGroup, storageClassLVGParametersMap map[string]string, nodeName, lvmType string, llvSize resource.Quantity) v1alpha1.LVMLogicalVolumeSpec { +func GetLLVSpec(log logger.Logger, lvName string, selectedLVG v1alpha1.LvmVolumeGroup, storageClassLVGParametersMap map[string]string, nodeName, lvmType string, llvSize resource.Quantity) v1alpha1.LVMLogicalVolumeSpec { var llvThin *v1alpha1.ThinLogicalVolumeSpec if lvmType == internal.LLMTypeThin { llvThin = &v1alpha1.ThinLogicalVolumeSpec{} @@ -381,10 +385,11 @@ func GetLLVSpec(log logger.Logger, selectedLVG v1alpha1.LvmVolumeGroup, storageC log.Info(fmt.Sprintf("[GetLLVSpec] Thin pool name: %s", llvThin.PoolName)) } return v1alpha1.LVMLogicalVolumeSpec{ - Type: lvmType, - Size: llvSize, - LvmVolumeGroup: selectedLVG.Name, - Thin: llvThin, + ActualLVNameOnTheNode: lvName, + Type: lvmType, + Size: llvSize, + LvmVolumeGroupName: selectedLVG.Name, + Thin: llvThin, } } @@ -396,3 +401,20 @@ func SelectLVG(storageClassLVGs []v1alpha1.LvmVolumeGroup, storageClassLVGParame } return v1alpha1.LvmVolumeGroup{}, fmt.Errorf("[SelectLVG] no LVMVolumeGroup found for node %s", nodeName) } + +func removeLLVFinalizerIfExist(ctx context.Context, kc client.Client, llv *v1alpha1.LVMLogicalVolume, finalizer string) error { + removed := false + + for i, val := range llv.Finalizers { + if val == finalizer { + llv.Finalizers = slices.Delete(llv.Finalizers, i, i+1) + removed = true + break + } + } + + if removed { + return UpdateLVMLogicalVolume(ctx, kc, llv) + } + return nil +} From cd4108cbba13a479421ac59e25b5532782afa117 Mon Sep 17 00:00:00 2001 From: Aleksandr Zimin Date: Thu, 7 Mar 2024 00:55:50 +0300 Subject: [PATCH 2/4] fix Signed-off-by: Aleksandr Zimin --- images/sds-local-volume-csi/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/images/sds-local-volume-csi/Dockerfile b/images/sds-local-volume-csi/Dockerfile index 5170d8e9..5b781991 100644 --- a/images/sds-local-volume-csi/Dockerfile +++ b/images/sds-local-volume-csi/Dockerfile @@ -1,7 +1,7 @@ ARG BASE_ALPINE=registry.deckhouse.io/base_images/alpine:3.16.3@sha256:5548e9172c24a1b0ca9afdd2bf534e265c94b12b36b3e0c0302f5853eaf00abb -ARG BASE_GOLANG_20_ALPINE_BUILDER=registry.deckhouse.io/base_images/golang:1.20.5-alpine3.18@sha256:51a47fb0851397db2f506c15c426735bc23de31177cbdd962880c0879d1906a4 +ARG BASE_GOLANG_21_ALPINE_BUILDER=registry.deckhouse.io/base_images/golang:1.21.4-alpine3.18@sha256:cf84f3d6882c49ea04b6478ac514a2582c8922d7e5848b43d2918fff8329f6e6 -FROM $BASE_GOLANG_20_ALPINE_BUILDER as builder +FROM $BASE_GOLANG_21_ALPINE_BUILDER as builder WORKDIR /go/src From 118d48cff27f310feeca05895dc5c396e8e82554 Mon Sep 17 00:00:00 2001 From: Aleksandr Zimin Date: Thu, 7 Mar 2024 15:16:54 +0300 Subject: [PATCH 3/4] Update images/sds-local-volume-csi/driver/controller.go Signed-off-by: Aleksandr Zimin --- images/sds-local-volume-csi/driver/controller.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/images/sds-local-volume-csi/driver/controller.go b/images/sds-local-volume-csi/driver/controller.go index cc23d699..fb6bb4ac 100644 --- a/images/sds-local-volume-csi/driver/controller.go +++ b/images/sds-local-volume-csi/driver/controller.go @@ -66,7 +66,12 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ d.log.Error(err, "error GetStorageClassLVGs") return nil, status.Errorf(codes.Internal, err.Error()) } - +// TODO: Consider refactoring the naming strategy for llvName and lvName. +// Currently, we use the same name for llvName (the name of the LVMLogicalVolume resource in Kubernetes) +// and lvName (the name of the LV in LVM on the node) because the PV name is unique within the cluster, +// preventing name collisions. This approach simplifies matching between nodes and Kubernetes by maintaining +// the same name in both contexts. Future consideration should be given to optimizing this logic to enhance +// code readability and maintainability. llvName := request.Name lvName := request.Name d.log.Info(fmt.Sprintf("llv name: %s ", llvName)) From b44d2140e91b622e643a7ff7d6269d7d3757529f Mon Sep 17 00:00:00 2001 From: Aleksandr Zimin Date: Thu, 7 Mar 2024 16:48:43 +0300 Subject: [PATCH 4/4] fixes Signed-off-by: Aleksandr Zimin --- .../sds-local-volume-csi/driver/controller.go | 14 +++++----- images/sds-local-volume-csi/pkg/utils/func.go | 27 ++++++++++++------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/images/sds-local-volume-csi/driver/controller.go b/images/sds-local-volume-csi/driver/controller.go index fb6bb4ac..5d94f974 100644 --- a/images/sds-local-volume-csi/driver/controller.go +++ b/images/sds-local-volume-csi/driver/controller.go @@ -61,7 +61,7 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ return nil, status.Errorf(codes.InvalidArgument, err.Error()) } - storageClassLVGs, storageClassLVGParametersMap, err := utils.GetStorageClassLVGsAndParameters(ctx, d.cl, *d.log, request.GetParameters()[internal.LvmVolumeGroupKey]) + storageClassLVGs, storageClassLVGParametersMap, err := utils.GetStorageClassLVGsAndParameters(ctx, d.cl, d.log, request.GetParameters()[internal.LvmVolumeGroupKey]) if err != nil { d.log.Error(err, "error GetStorageClassLVGs") return nil, status.Errorf(codes.Internal, err.Error()) @@ -83,7 +83,7 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ switch BindingMode { case internal.BindingModeI: d.log.Info(fmt.Sprintf("BindingMode is %s. Start selecting node", internal.BindingModeI)) - selectedNodeName, freeSpace, err := utils.GetNodeWithMaxFreeSpace(*d.log, storageClassLVGs, storageClassLVGParametersMap, LvmType) + selectedNodeName, freeSpace, err := utils.GetNodeWithMaxFreeSpace(d.log, storageClassLVGs, storageClassLVGParametersMap, LvmType) if err != nil { d.log.Error(err, "error GetNodeMaxVGSize") } @@ -109,7 +109,7 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ return nil, status.Errorf(codes.Internal, err.Error()) } - llvSpec := utils.GetLLVSpec(*d.log, lvName, selectedLVG, storageClassLVGParametersMap, preferredNode, LvmType, *llvSize) + llvSpec := utils.GetLLVSpec(d.log, lvName, selectedLVG, storageClassLVGParametersMap, preferredNode, LvmType, *llvSize) d.log.Info(fmt.Sprintf("LVMLogicalVolumeSpec : %+v", llvSpec)) resizeDelta, err := resource.ParseQuantity(internal.ResizeDelta) if err != nil { @@ -131,9 +131,9 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ d.log.Trace("start wait CreateLVMLogicalVolume ") - attemptCounter, err := utils.WaitForStatusUpdate(ctx, d.cl, *d.log, request.Name, "", *llvSize, resizeDelta) + attemptCounter, err := utils.WaitForStatusUpdate(ctx, d.cl, d.log, request.Name, "", *llvSize, resizeDelta) if err != nil { - deleteErr := utils.DeleteLVMLogicalVolume(ctx, d.cl, request.Name) + deleteErr := utils.DeleteLVMLogicalVolume(ctx, d.cl, d.log, request.Name) d.log.Error(err, fmt.Sprintf("error WaitForStatusUpdate. Delete LVMLogicalVolume %s", request.Name)) if deleteErr != nil { @@ -168,7 +168,7 @@ func (d *Driver) CreateVolume(ctx context.Context, request *csi.CreateVolumeRequ func (d *Driver) DeleteVolume(ctx context.Context, request *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { d.log.Info("method DeleteVolume") - err := utils.DeleteLVMLogicalVolume(ctx, d.cl, request.VolumeId) + err := utils.DeleteLVMLogicalVolume(ctx, d.cl, d.log, request.VolumeId) if err != nil { d.log.Error(err, "error DeleteLVMLogicalVolume") } @@ -320,7 +320,7 @@ func (d *Driver) ControllerExpandVolume(ctx context.Context, request *csi.Contro return nil, status.Errorf(codes.Internal, "error updating LVMLogicalVolume: %v", err) } - attemptCounter, err := utils.WaitForStatusUpdate(ctx, d.cl, *d.log, llv.Name, llv.Namespace, *requestCapacity, resizeDelta) + attemptCounter, err := utils.WaitForStatusUpdate(ctx, d.cl, d.log, llv.Name, llv.Namespace, *requestCapacity, resizeDelta) if err != nil { d.log.Error(err, "error WaitForStatusUpdate") return nil, err diff --git a/images/sds-local-volume-csi/pkg/utils/func.go b/images/sds-local-volume-csi/pkg/utils/func.go index f5152618..b6dce0b2 100644 --- a/images/sds-local-volume-csi/pkg/utils/func.go +++ b/images/sds-local-volume-csi/pkg/utils/func.go @@ -78,7 +78,7 @@ func CreateLVMLogicalVolume(ctx context.Context, kc client.Client, name string, return llv, nil } -func DeleteLVMLogicalVolume(ctx context.Context, kc client.Client, LVMLogicalVolumeName string) error { +func DeleteLVMLogicalVolume(ctx context.Context, kc client.Client, log *logger.Logger, LVMLogicalVolumeName string) error { var err error llv, err := GetLVMLogicalVolume(ctx, kc, LVMLogicalVolumeName, "") @@ -86,10 +86,15 @@ func DeleteLVMLogicalVolume(ctx context.Context, kc client.Client, LVMLogicalVol return fmt.Errorf("get LVMLogicalVolume %s: %w", LVMLogicalVolumeName, err) } - err = removeLLVFinalizerIfExist(ctx, kc, llv, SDSLocalVolumeCSIFinalizer) + removed, err := removeLLVFinalizerIfExist(ctx, kc, llv, SDSLocalVolumeCSIFinalizer) if err != nil { return fmt.Errorf("remove finalizers from LVMLogicalVolume %s: %w", LVMLogicalVolumeName, err) } + if removed { + log.Trace(fmt.Sprintf("[DeleteLVMLogicalVolume] finalizer %s removed from LVMLogicalVolume %s", SDSLocalVolumeCSIFinalizer, LVMLogicalVolumeName)) + } else { + log.Warning(fmt.Sprintf("[DeleteLVMLogicalVolume] finalizer %s not found in LVMLogicalVolume %s", SDSLocalVolumeCSIFinalizer, LVMLogicalVolumeName)) + } for attempt := 0; attempt < KubernetesApiRequestLimit; attempt++ { err = kc.Delete(ctx, llv) @@ -105,7 +110,7 @@ func DeleteLVMLogicalVolume(ctx context.Context, kc client.Client, LVMLogicalVol return nil } -func WaitForStatusUpdate(ctx context.Context, kc client.Client, log logger.Logger, LVMLogicalVolumeName, namespace string, llvSize, delta resource.Quantity) (int, error) { +func WaitForStatusUpdate(ctx context.Context, kc client.Client, log *logger.Logger, LVMLogicalVolumeName, namespace string, llvSize, delta resource.Quantity) (int, error) { var attemptCounter int sizeEquals := false for { @@ -166,7 +171,7 @@ func AreSizesEqualWithinDelta(leftSize, rightSize, allowedDelta resource.Quantit return math.Abs(leftSizeFloat-rightSizeFloat) < float64(allowedDelta.Value()) } -func GetNodeWithMaxFreeSpace(log logger.Logger, lvgs []v1alpha1.LvmVolumeGroup, storageClassLVGParametersMap map[string]string, lvmType string) (nodeName string, freeSpace resource.Quantity, err error) { +func GetNodeWithMaxFreeSpace(log *logger.Logger, lvgs []v1alpha1.LvmVolumeGroup, storageClassLVGParametersMap map[string]string, lvmType string) (nodeName string, freeSpace resource.Quantity, err error) { var maxFreeSpace int64 for _, lvg := range lvgs { @@ -320,7 +325,7 @@ func UpdateLVMLogicalVolume(ctx context.Context, kc client.Client, llv *v1alpha1 return nil } -func GetStorageClassLVGsAndParameters(ctx context.Context, kc client.Client, log logger.Logger, storageClassLVGParametersString string) (storageClassLVGs []v1alpha1.LvmVolumeGroup, storageClassLVGParametersMap map[string]string, err error) { +func GetStorageClassLVGsAndParameters(ctx context.Context, kc client.Client, log *logger.Logger, storageClassLVGParametersString string) (storageClassLVGs []v1alpha1.LvmVolumeGroup, storageClassLVGParametersMap map[string]string, err error) { var storageClassLVGParametersList LVMVolumeGroups err = yaml.Unmarshal([]byte(storageClassLVGParametersString), &storageClassLVGParametersList) if err != nil { @@ -377,7 +382,7 @@ func GetLVGList(ctx context.Context, kc client.Client) (*v1alpha1.LvmVolumeGroup return nil, fmt.Errorf("after %d attempts of getting LvmVolumeGroupList, last error: %w", KubernetesApiRequestLimit, err) } -func GetLLVSpec(log logger.Logger, lvName string, selectedLVG v1alpha1.LvmVolumeGroup, storageClassLVGParametersMap map[string]string, nodeName, lvmType string, llvSize resource.Quantity) v1alpha1.LVMLogicalVolumeSpec { +func GetLLVSpec(log *logger.Logger, lvName string, selectedLVG v1alpha1.LvmVolumeGroup, storageClassLVGParametersMap map[string]string, nodeName, lvmType string, llvSize resource.Quantity) v1alpha1.LVMLogicalVolumeSpec { var llvThin *v1alpha1.ThinLogicalVolumeSpec if lvmType == internal.LLMTypeThin { llvThin = &v1alpha1.ThinLogicalVolumeSpec{} @@ -402,7 +407,7 @@ func SelectLVG(storageClassLVGs []v1alpha1.LvmVolumeGroup, storageClassLVGParame return v1alpha1.LvmVolumeGroup{}, fmt.Errorf("[SelectLVG] no LVMVolumeGroup found for node %s", nodeName) } -func removeLLVFinalizerIfExist(ctx context.Context, kc client.Client, llv *v1alpha1.LVMLogicalVolume, finalizer string) error { +func removeLLVFinalizerIfExist(ctx context.Context, kc client.Client, llv *v1alpha1.LVMLogicalVolume, finalizer string) (bool, error) { removed := false for i, val := range llv.Finalizers { @@ -414,7 +419,11 @@ func removeLLVFinalizerIfExist(ctx context.Context, kc client.Client, llv *v1alp } if removed { - return UpdateLVMLogicalVolume(ctx, kc, llv) + err := UpdateLVMLogicalVolume(ctx, kc, llv) + if err != nil { + return false, err + } + return true, nil } - return nil + return false, nil }