From dff5a42081d678239b461a64659b1cd32bb9442d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9iy=C3=AC=20Zhang?= Date: Mon, 26 Jun 2023 14:50:03 -0700 Subject: [PATCH] 80 share support for stateful driver --- pkg/csi_driver/multishare_controller.go | 5 ++- pkg/csi_driver/multishare_controller_test.go | 2 +- .../multishare_stateful_controller.go | 33 ++++++++++++++++--- pkg/csi_driver/reconciler.go | 28 ++++++++++++---- 4 files changed, 54 insertions(+), 14 deletions(-) diff --git a/pkg/csi_driver/multishare_controller.go b/pkg/csi_driver/multishare_controller.go index e951a9a11..14f65c0a7 100644 --- a/pkg/csi_driver/multishare_controller.go +++ b/pkg/csi_driver/multishare_controller.go @@ -131,7 +131,7 @@ func (m *MultishareController) CreateVolume(ctx context.Context, req *csi.Create return nil, status.Error(codes.InvalidArgument, err.Error()) } - maxSharesPerInstance, maxShareSizeSizeBytes, err := m.parseMaxVolumeSizeParam(req) + maxSharesPerInstance, maxShareSizeSizeBytes, err := m.parseMaxVolumeSizeParam(req.GetParameters()) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -712,8 +712,7 @@ func generateInstanceDescFromEcfsDesc(desc string) string { return d } -func (m *MultishareController) parseMaxVolumeSizeParam(req *csi.CreateVolumeRequest) (int, int64, error) { - params := req.GetParameters() +func (m *MultishareController) parseMaxVolumeSizeParam(params map[string]string) (int, int64, error) { v, ok := params[paramMaxVolumeSize] if !m.featureMaxSharePerInstance && ok { return 0, 0, fmt.Errorf("configurable max shares per instance feature not enabled") diff --git a/pkg/csi_driver/multishare_controller_test.go b/pkg/csi_driver/multishare_controller_test.go index fb779640a..15c8656e5 100644 --- a/pkg/csi_driver/multishare_controller_test.go +++ b/pkg/csi_driver/multishare_controller_test.go @@ -2035,7 +2035,7 @@ func TestParseMaxVolumeSizeParam(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { m := initTestMultishareControllerWithFeatureOpts(t, tc.features) - sharePerInstance, maxShareCapacity, err := m.parseMaxVolumeSizeParam(tc.req) + sharePerInstance, maxShareCapacity, err := m.parseMaxVolumeSizeParam(tc.req.GetParameters()) if tc.expectError && err == nil { t.Errorf("failed") } diff --git a/pkg/csi_driver/multishare_stateful_controller.go b/pkg/csi_driver/multishare_stateful_controller.go index fca05ba5e..ac93017e4 100644 --- a/pkg/csi_driver/multishare_stateful_controller.go +++ b/pkg/csi_driver/multishare_stateful_controller.go @@ -72,7 +72,18 @@ func (m *MultishareStatefulController) CreateVolume(ctx context.Context, req *cs return nil, status.Error(codes.InvalidArgument, err.Error()) } - reqBytes, err := getShareRequestCapacity(req.GetCapacityRange(), util.MinShareSizeBytes, util.MaxShareSizeBytes) + _, maxShareSizeBytes, err := m.mc.parseMaxVolumeSizeParam(req.GetParameters()) + if err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + + var reqBytes int64 + if m.mc.featureMaxSharePerInstance { + reqBytes, err = getShareRequestCapacity(req.GetCapacityRange(), util.ConfigurablePackMinShareSizeBytes, maxShareSizeBytes) + } else { + reqBytes, err = getShareRequestCapacity(req.GetCapacityRange(), util.MinShareSizeBytes, util.MaxShareSizeBytes) + } + if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -139,7 +150,7 @@ func (m *MultishareStatefulController) CreateVolume(ctx context.Context, req *cs if err != nil { return nil, err } - return m.mc.getShareAndGenerateCSICreateVolumeResponse(ctx, instanceSCLabel, share, util.MaxShareSizeBytes) + return m.mc.getShareAndGenerateCSICreateVolumeResponse(ctx, instanceSCLabel, share, maxShareSizeBytes) } func (m *MultishareStatefulController) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { @@ -234,7 +245,21 @@ func (m *MultishareStatefulController) deleteShareInfo(ctx context.Context, siNa } func (m *MultishareStatefulController) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) { - reqBytes, err := getShareRequestCapacity(req.GetCapacityRange(), util.MinShareSizeBytes, util.MaxShareSizeBytes) + volumeId := req.GetVolumeId() + if len(volumeId) == 0 { + return nil, status.Error(codes.InvalidArgument, "ControllerExpandVolume volume ID must be provided") + } + + maxShareSizeBytes := util.MaxShareSizeBytes + if m.mc.featureMaxSharePerInstance { + var err error + maxShareSizeBytes, err = m.mc.GetShareMaxSizeFromPV(ctx, volumeId) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) + } + klog.Infof("maxShareSizeBytes %d", maxShareSizeBytes) + } + reqBytes, err := getShareRequestCapacity(req.GetCapacityRange(), util.ConfigurablePackMinShareSizeBytes, maxShareSizeBytes) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -275,7 +300,7 @@ func (m *MultishareStatefulController) ControllerExpandVolume(ctx context.Contex } if shareInfo.Status.CapacityBytes >= reqBytes && shareInfo.Status.ShareStatus == v1.READY { - klog.Infof("Controller expand volume succeeded for volume %v, size(bytes): %v", req.VolumeId, shareInfo.Status.CapacityBytes) + klog.Infof("Controller expand volume succeeded for volume %v, size(bytes): %v", volumeId, shareInfo.Status.CapacityBytes) share, err := generateFileShareFromShareInfo(shareInfo) if err != nil { diff --git a/pkg/csi_driver/reconciler.go b/pkg/csi_driver/reconciler.go index 4fed74640..d636ad87d 100644 --- a/pkg/csi_driver/reconciler.go +++ b/pkg/csi_driver/reconciler.go @@ -505,6 +505,10 @@ func (recon *MultishareReconciler) generateNewMultishareInstance(instanceInfo *v Description: generateInstanceDescFromEcfsDesc(recon.config.EcfsDescription), } + if recon.controllerServer.config.multiShareController.featureMaxSharePerInstance { + instance.MaxShareCount = recon.parseMaxSharePerInstance(instanceInfo.Spec.Parameters) + } + // reserve ip range var reservedIPRange string if connectMode == privateServiceAccess { @@ -591,7 +595,7 @@ func (recon *MultishareReconciler) fixTwoWayPointers(shareInfos map[string]*v1.S var err error if !ok { klog.Errorf("Share %q is assigned to instance %q but instanceInfo does not exist. Trying to create one", shareName, shareInfo.Status.InstanceHandle) - actualAssigned, err = recon.generateInstanceInfo(shareInfo.Status.InstanceHandle, shareInfo.Spec.InstancePoolTag) + actualAssigned, err = recon.generateInstanceInfo(shareInfo.Status.InstanceHandle, shareInfo.Spec.InstancePoolTag, shareInfo.Spec.Parameters) if err != nil { klog.Errorf("Failed to create instanceInfo %q: %v", shareInfo.Status.InstanceHandle, err) continue @@ -649,7 +653,7 @@ func (recon *MultishareReconciler) assignSharesToEligibleOrNewInstances(shareInf // if InstanceStatus is not empty but instance is no longer present, instance might have been manually deleted by user and can no longer be used klog.Warningf("instanceInfo %s has non empty InstanceStatus but underlying instance does not exist. Skip assignment to that instance", instanceInfo.Name) } - if instanceFitShare(instanceInfo, shareInfo) { + if recon.instanceFitShare(instanceInfo, shareInfo) { instanceURI = util.InstanceInfoNameToInstanceURI(instanceInfo.Name) instanceInfo, err = recon.assignShareToInstanceInfo(instanceInfo, shareInfo.Name) if err != nil { @@ -674,7 +678,7 @@ func (recon *MultishareReconciler) assignSharesToEligibleOrNewInstances(shareInf Name: util.NewMultishareInstancePrefix + string(uuid.NewUUID()), }) - instanceInfo, err := recon.generateInstanceInfo(instanceURI, shareInfo.Spec.InstancePoolTag) + instanceInfo, err := recon.generateInstanceInfo(instanceURI, shareInfo.Spec.InstancePoolTag, shareInfo.Spec.Parameters) if err != nil { klog.Errorf("Failed to create new instanceInfo %q: %v", instanceURI, err) continue @@ -732,7 +736,7 @@ func (recon *MultishareReconciler) deleteOrResizeInstances(instanceInfos map[str } // generateInstanceInfo generates and creates a new instanceInfo object based on instanceURI and storage class tag. -func (recon *MultishareReconciler) generateInstanceInfo(instanceURI string, scTag string) (*v1.InstanceInfo, error) { +func (recon *MultishareReconciler) generateInstanceInfo(instanceURI string, scTag string, shareParams map[string]string) (*v1.InstanceInfo, error) { storageClass, err := recon.storageClassFromTag(scTag) if err != nil { return nil, err @@ -748,6 +752,7 @@ func (recon *MultishareReconciler) generateInstanceInfo(instanceURI string, scTa Spec: v1.InstanceInfoSpec{ CapacityBytes: util.MinMultishareInstanceSizeBytes, StorageClassName: storageClass.Name, + Parameters: shareParams, }, } return recon.createInstanceInfo(context.TODO(), newInstanceInfo) @@ -1256,7 +1261,7 @@ func (recon *MultishareReconciler) listMultishareResourceOps(ctx context.Context } // instanceFitShare returns true if shareInfo can be assigned to instanceInfo. -func instanceFitShare(instanceInfo *v1.InstanceInfo, shareInfo *v1.ShareInfo) bool { +func (recon *MultishareReconciler) instanceFitShare(instanceInfo *v1.InstanceInfo, shareInfo *v1.ShareInfo) bool { // Instance needs to be: // 1. not up for delete 2.of the same storage class and 3. has less than max number of shares assigned already. if instanceInfo.DeletionTimestamp != nil || @@ -1264,7 +1269,9 @@ func instanceFitShare(instanceInfo *v1.InstanceInfo, shareInfo *v1.ShareInfo) bo return false } - if instanceInfo.Status != nil && len(instanceInfo.Status.ShareNames) >= util.MaxSharesPerInstance { + maxSharePerInstance := recon.parseMaxSharePerInstance(instanceInfo.Spec.Parameters) + + if instanceInfo.Status != nil && len(instanceInfo.Status.ShareNames) >= maxSharePerInstance { return false } @@ -1277,6 +1284,15 @@ func instanceFitShare(instanceInfo *v1.InstanceInfo, shareInfo *v1.ShareInfo) bo return true } +// parseMaxSharePerInstance assumes that params has valid parameter of max volume size and returns max share per instance +func (recon *MultishareReconciler) parseMaxSharePerInstance(params map[string]string) int { + maxSharePerInstance, _, _ := recon.controllerServer.config.multiShareController.parseMaxVolumeSizeParam(params) + if maxSharePerInstance == 0 { + maxSharePerInstance = util.MaxSharesPerInstance + } + return maxSharePerInstance +} + // instanceEmpty returns true if instanceInfo.Status.ShareNames has zero entries. func instanceEmpty(instanceInfo *v1.InstanceInfo) bool { if instanceInfo.Status == nil || len(instanceInfo.Status.ShareNames) != 0 {