Skip to content

Commit

Permalink
disk: cleanup NodeExpandVolume
Browse files Browse the repository at this point in the history
remove unnessesary statfs
remove float math
remove the unused parameter FILE_SYSTEM_LOSE_PERCENT
optimize error message
fix false error if req.CapacityRange is not specified
  • Loading branch information
huww98 committed Apr 5, 2024
1 parent a75e2a5 commit 4b735f0
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 58 deletions.
11 changes: 0 additions & 11 deletions pkg/disk/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ type GlobalConfig struct {
DetachBeforeDelete bool
DiskBdfEnable bool
ClientSet *kubernetes.Clientset
FilesystemLosePercent float64
ClusterID string
DiskPartitionEnable bool
ControllerService bool
Expand Down Expand Up @@ -201,15 +200,6 @@ func GlobalConfigSet(m metadata.MetadataProvider) *restclient.Config {
}
}

// fileSystemLosePercent ...
fileSystemLosePercent := float64(0.90)
if fileSystemLoseCapacityPercent := os.Getenv(FileSystemLoseCapacityPercent); fileSystemLoseCapacityPercent != "" {
percent, err := strconv.ParseFloat(fileSystemLoseCapacityPercent, 64)
if err == nil {
fileSystemLosePercent = percent
}
}

nodeName := os.Getenv(kubeNodeName)
runtimeValue := "runc"
nodeInfo, err := kubeClient.CoreV1().Nodes().Get(context.Background(), nodeName, metav1.GetOptions{})
Expand Down Expand Up @@ -252,7 +242,6 @@ func GlobalConfigSet(m metadata.MetadataProvider) *restclient.Config {
DetachBeforeAttach: csiCfg.GetBool("disk-detach-before-attach", "DISK_FORCE_DETACHED", true),
ClientSet: kubeClient,
SnapClient: snapClient,
FilesystemLosePercent: fileSystemLosePercent,
ClusterID: clustID,
DiskPartitionEnable: csiCfg.GetBool("disk-partition-enable", "DISK_PARTITION_ENABLE", true),
ControllerService: controllerServerType,
Expand Down
48 changes: 16 additions & 32 deletions pkg/disk/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ import (
"bytes"
"context"
"crypto/sha256"
"errors"
"fmt"
"math"
"os"
"os/exec"
"path/filepath"
Expand All @@ -39,6 +37,7 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -93,8 +92,6 @@ const (
RunvRunTimeMode = "runv"
// InputOutputErr tag
InputOutputErr = "input/output error"
// FileSystemLoseCapacityPercent is the env of container
FileSystemLoseCapacityPercent = "FILE_SYSTEM_LOSE_PERCENT"
// DiskMultiTenantEnable Enable disk multi-tenant mode
DiskMultiTenantEnable = "DISK_MULTI_TENANT_ENABLE"
// TenantUserUID tag
Expand Down Expand Up @@ -889,8 +886,7 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV
*csi.NodeExpandVolumeResponse, error) {
log.Infof("NodeExpandVolume: node expand volume: %v", req)

volExpandBytes := float64(req.GetCapacityRange().GetRequiredBytes())
requestGB := math.Floor(volExpandBytes / float64(1024*1024*1024))
requestBytes := req.GetCapacityRange().GetRequiredBytes()

volumePath := req.GetVolumePath()
diskID := req.GetVolumeId()
Expand Down Expand Up @@ -928,7 +924,7 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV
devicePaths := GetVolumeDeviceName(diskID)
if len(devicePaths) == 0 {
log.Errorf("NodeExpandVolume:: can't get devicePath: %s", diskID)
return nil, status.Error(codes.InvalidArgument, "can't get devicePath for "+diskID)
return nil, status.Errorf(codes.NotFound, "can't get devicePath for: %s", diskID)
}
rootDevice, subDevice, err := GetRootSubDevicePath(devicePaths)
if err != nil {
Expand All @@ -937,16 +933,6 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV
}
devicePath := ChooseDevice(rootDevice, subDevice)

log.Infof("NodeExpandVolume:: volumeId: %s, devicePath: %s, volumePath: %s", diskID, devicePaths, volumePath)
beforeResizeDiskCapacity, err := getDiskCapacity(volumePath)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return nil, status.Error(codes.NotFound, fmt.Sprintf("volumePath does not exist: %v", err))
}
log.Errorf("NodeExpandVolume:: get diskCapacity error %+v", err)
return nil, status.Error(codes.Internal, err.Error())
}

if GlobalConfigVar.DiskPartitionEnable && !IsDeviceNvme(devicePath) && isDevicePartition(devicePath) {
rootPath, index, err := getDeviceRootAndIndex(devicePath)
if err != nil {
Expand All @@ -959,7 +945,8 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV
if bytes.Contains(output, []byte("it cannot be grown")) || bytes.Contains(output, []byte("could only be grown by")) {
deviceCapacity := getBlockDeviceCapacity(devicePath)
rootCapacity := getBlockDeviceCapacity(rootPath)
log.Infof("NodeExpandVolume: Volume %s with Device Partition %s no need to grown, with requestSize: %v, rootBlockSize: %v, partition BlockDevice size: %v", diskID, devicePath, requestGB, rootCapacity, deviceCapacity)
log.Infof("NodeExpandVolume: Volume %s with Device Partition %s no need to grown, with requestSize: %v, rootBlockSize: %v, partition BlockDevice size: %v",
diskID, devicePath, requestBytes, rootCapacity, deviceCapacity)
return &csi.NodeExpandVolumeResponse{}, nil
}
}
Expand Down Expand Up @@ -988,21 +975,18 @@ func (ns *nodeServer) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandV
}
return nil, status.Error(codes.Internal, "Fail to resize volume fs")
}
diskCapacity, err := getDiskCapacity(volumePath)
if err != nil {
log.Errorf("NodeExpandVolume:: get diskCapacity error %+v", err)
return nil, status.Error(codes.Internal, err.Error())
}

deviceCapacity := getBlockDeviceCapacity(devicePath)
if deviceCapacity < requestGB {
// After calling openapi to expansion cloud disk, the size of the underlying block device may not change. need to retry
log.Errorf("NodeExpandVolume:: Actual block size: %v is smaller than expected block size: %v, need to retry waiting", deviceCapacity, requestGB)
return nil, status.Errorf(codes.Aborted, "deviceCapacity: %v, requestGB: %v not in range", deviceCapacity, requestGB)
}
log.Infof(
"NodeExpandVolume:: filesystem resize context device capacity: %v, before resize fs capacity: %v resize fs capacity: %v, requestGB: %v. file system lose percent: %v",
deviceCapacity, beforeResizeDiskCapacity, diskCapacity, requestGB, GlobalConfigVar.FilesystemLosePercent)
return &csi.NodeExpandVolumeResponse{}, nil
if requestBytes > 0 && deviceCapacity < requestBytes {
// After calling OpenAPI to expansion cloud disk, the size of the underlying block device may not change immediately.
// return error and CO will retry later.
return nil, status.Errorf(codes.Aborted, "requested %v, but actual block size %v is smaller. Not updated yet?",
resource.NewQuantity(requestBytes, resource.BinarySI), resource.NewQuantity(deviceCapacity, resource.BinarySI))
}
log.Infof("NodeExpandVolume:: Expand %s to %dB successful", diskID, deviceCapacity)
return &csi.NodeExpandVolumeResponse{
CapacityBytes: deviceCapacity,
}, nil
}

// NodeGetVolumeStats used for csi metrics
Expand Down
20 changes: 5 additions & 15 deletions pkg/disk/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import (
"github.com/kubernetes-sigs/alibaba-cloud-csi-driver/pkg/version"
perrors "github.com/pkg/errors"
log "github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -1080,6 +1079,9 @@ func GetVolumeDeviceName(diskID string) []string {
devices, err := GetDeviceByVolumeID(diskID)
if err != nil {
deviceName := getVolumeConfig(diskID)
if deviceName == "" {
return nil
}
devices = []string{deviceName}
log.Infof("GetVolumeDeviceName, Get Device Name by Config File %s, DeviceName: %s", diskID, deviceName)
}
Expand Down Expand Up @@ -1110,19 +1112,7 @@ func deleteEmpty(s []string) []string {
return r
}

func getDiskCapacity(devicePath string) (float64, error) {
statfs := &unix.Statfs_t{}
err := unix.Statfs(devicePath, statfs)

if err != nil {
log.Errorf("getDiskCapacity:: get device error: %+v", err)
return 0, fmt.Errorf("getDiskCapacity:: get device error: %w", err)
}

return float64(statfs.Blocks*uint64(statfs.Bsize)) / GBSIZE, nil
}

func getBlockDeviceCapacity(devicePath string) float64 {
func getBlockDeviceCapacity(devicePath string) int64 {

file, err := os.Open(devicePath)
if err != nil {
Expand All @@ -1134,7 +1124,7 @@ func getBlockDeviceCapacity(devicePath string) float64 {
log.Errorf("getBlockDeviceCapacity:: failed to read devicePath: %v", devicePath)
return 0
}
return float64(pos) / GBSIZE
return pos
}

func GetAvailableDiskTypes(ctx context.Context, c cloud.ECSInterface, m metadata.MetadataProvider) (types []string, err error) {
Expand Down

0 comments on commit 4b735f0

Please sign in to comment.