Merge pull request #216 from jwcesign/vs

fix: fix volumesize modification

Author: gitautomator[bot]

charts/karpenter/crds/karpenter.k8s.alibabacloud_ecsnodeclasses.yaml

@@ -61,17 +61,13 @@ spec:
                     device:
                       description: Mount point of the data disk.
                       type: string
-                    size:
+                    volumeSize:
+                      default: 20Gi
                       description: |-
-                        The size of the data disk. Unit: GiB.
-                        Valid values:
-                          * If you set Category to cloud: 20 to 500.
-                          * If you set Category to other disk categories: 20 to 2048.
-                      format: int32
-                      type: integer
-                      x-kubernetes-validations:
-                      - message: size invalid
-                        rule: self >= 20
+                        Size in `Gi`, `G`, `Ti`, or `T`. You must specify either a snapshot ID or
+                        a volume size.
+                      pattern: ^((?:[1-9][0-9]{0,3}|[1-4][0-9]{4}|[5][0-8][0-9]{3}|59000)Gi|(?:[1-9][0-9]{0,3}|[1-5][0-9]{4}|[6][0-3][0-9]{3}|64000)G|([1-9]||[1-5][0-7]|58)Ti|([1-9]||[1-5][0-9]|6[0-3]|64)T)$
+                      type: string
                   type: object
                 type: array
               imageSelectorTerms:
@@ -321,7 +317,6 @@ spec:
                     - PL3
                     type: string
                   size:
-                    default: 20
                     description: |-
                       The size of the system disk. Unit: GiB.
                       Valid values:
@@ -335,16 +330,7 @@ spec:
                   volumeSize:
                     description: |-
                       Size in `Gi`, `G`, `Ti`, or `T`. You must specify either a snapshot ID or
-                      a volume size. The following are the supported volumes sizes for each volume
-                      type:
-
-                         * gp2 and gp3: 1-16,384
-
-                         * io1 and io2: 4-16,384
-
-                         * st1 and sc1: 125-16,384
-
-                         * standard: 1-1,024
+                      a volume size.
                     pattern: ^((?:[1-9][0-9]{0,3}|[1-4][0-9]{4}|[5][0-8][0-9]{3}|59000)Gi|(?:[1-9][0-9]{0,3}|[1-5][0-9]{4}|[6][0-3][0-9]{3}|64000)G|([1-9]||[1-5][0-7]|58)Ti|([1-9]||[1-5][0-9]|6[0-3]|64)T)$
                     type: string
                 type: object

examples/nodepool/ecsnodeclass.yaml

@@ -12,9 +12,12 @@ spec:
   systemDisk:
     categories:
       - cloud
+      - cloud_efficiency
       - cloud_ssd
+      - cloud_essd
       - cloud_auto
-    size: 60
+      - cloud_essd_entry
+    volumeSize: 60Gi
   imageSelectorTerms:
     # ContainerOS only support x86_64 linux nodes, and it's faster to initialize
     - alias: ContainerOS

pkg/apis/v1alpha1/ecsnodeclass.go

@@ -226,16 +226,7 @@ type SystemDisk struct {
 	// +optional
 	Categories []string `json:"categories,omitempty"`
 	// Size in `Gi`, `G`, `Ti`, or `T`. You must specify either a snapshot ID or
-	// a volume size. The following are the supported volumes sizes for each volume
-	// type:
-	//
-	//    * gp2 and gp3: 1-16,384
-	//
-	//    * io1 and io2: 4-16,384
-	//
-	//    * st1 and sc1: 125-16,384
-	//
-	//    * standard: 1-1,024
+	// a volume size.
 	// + TODO: Add the CEL resources.quantity type after k8s 1.29
 	// + https://github.com/kubernetes/apiserver/commit/b137c256373aec1c5d5810afbabb8932a19ecd2a#diff-838176caa5882465c9d6061febd456397a3e2b40fb423ed36f0cabb1847ecb4dR190
 	// +kubebuilder:validation:Pattern:="^((?:[1-9][0-9]{0,3}|[1-4][0-9]{4}|[5][0-8][0-9]{3}|59000)Gi|(?:[1-9][0-9]{0,3}|[1-5][0-9]{4}|[6][0-3][0-9]{3}|64000)G|([1-9]||[1-5][0-7]|58)Ti|([1-9]||[1-5][0-9]|6[0-3]|64)T)$"
@@ -249,7 +240,6 @@ type SystemDisk struct {
 	//   * If you set Category to other disk categories: 20 to 2048.
 	//
 	// +kubebuilder:validation:XValidation:message="size invalid",rule="self >= 20"
-	// +kubebuilder:default:=20
 	// +optional
 	Size *int32 `json:"size,omitempty"`
 	// The performance level of the ESSD to use as the system disk. Default value: PL0.
@@ -265,13 +255,16 @@ type SystemDisk struct {
 }
 
 type DataDisk struct {
-	// The size of the data disk. Unit: GiB.
-	// Valid values:
-	//   * If you set Category to cloud: 20 to 500.
-	//   * If you set Category to other disk categories: 20 to 2048.
-	//
-	// +kubebuilder:validation:XValidation:message="size invalid",rule="self >= 20"
-	Size *int32 `json:"size,omitempty"`
+	// Size in `Gi`, `G`, `Ti`, or `T`. You must specify either a snapshot ID or
+	// a volume size.
+	// + TODO: Add the CEL resources.quantity type after k8s 1.29
+	// + https://github.com/kubernetes/apiserver/commit/b137c256373aec1c5d5810afbabb8932a19ecd2a#diff-838176caa5882465c9d6061febd456397a3e2b40fb423ed36f0cabb1847ecb4dR190
+	// +kubebuilder:validation:Pattern:="^((?:[1-9][0-9]{0,3}|[1-4][0-9]{4}|[5][0-8][0-9]{3}|59000)Gi|(?:[1-9][0-9]{0,3}|[1-5][0-9]{4}|[6][0-3][0-9]{3}|64000)G|([1-9]||[1-5][0-7]|58)Ti|([1-9]||[1-5][0-9]|6[0-3]|64)T)$"
+	// +kubebuilder:validation:Schemaless
+	// +kubebuilder:validation:Type:=string
+	// +kubebuilder:default:="20Gi"
+	// +optional
+	VolumeSize *resource.Quantity `json:"volumeSize,omitempty" hash:"string"`
 	// Mount point of the data disk.
 	// +optional
 	Device *string `json:"device,omitempty"`
@@ -331,3 +324,20 @@ type ECSNodeClassList struct {
 func ImageFamilyFromAlias(alias string) string {
 	return alias
 }
+
+func (sd *SystemDisk) GetGiBSize() int32 {
+	if sd.VolumeSize != nil {
+		return int32(sd.VolumeSize.Value() / (1024 * 1024 * 1024)) // #nosec G115
+	}
+	if sd.Size != nil {
+		return *sd.Size
+	}
+	return 0
+}
+
+func (dd *DataDisk) GetGiBSize() int32 {
+	if dd.VolumeSize != nil {
+		return int32(dd.VolumeSize.Value() / (1024 * 1024 * 1024)) // #nosec G115
+	}
+	return 0
+}

pkg/apis/v1alpha1/zz_generated.deepcopy.go

@@ -49,8 +49,8 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1alpha1.ECSNodeC
 	stored := nodeClass.DeepCopy()
 
 	if nodeClass.Spec.SystemDisk != nil &&
-		nodeClass.Spec.SystemDisk.Size != nil {
-		nodeClass.Spec.SystemDisk.VolumeSize = resources.Quantity(fmt.Sprintf("%dG", *nodeClass.Spec.SystemDisk.Size))
+		nodeClass.Spec.SystemDisk.Size != nil && nodeClass.Spec.SystemDisk.VolumeSize == nil {
+		nodeClass.Spec.SystemDisk.VolumeSize = resources.Quantity(fmt.Sprintf("%dGi", *nodeClass.Spec.SystemDisk.Size))
 	}
 	if !equality.Semantic.DeepEqual(stored, nodeClass) {
 		if err := c.kubeClient.Patch(ctx, nodeClass, client.MergeFrom(stored)); err != nil {

pkg/controllers/nodeclass/volumesize/controller.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/client-go/util/workqueue"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/karpenter/pkg/cloudprovider"
+	"sigs.k8s.io/karpenter/pkg/utils/resources"
 
 	"github.com/cloudpilot-ai/karpenter-provider-alibabacloud/pkg/apis/v1alpha1"
 	"github.com/cloudpilot-ai/karpenter-provider-alibabacloud/pkg/utils/alierrors"
@@ -38,7 +39,7 @@ import (
 var DefaultSystemDisk = v1alpha1.SystemDisk{
 	// TODO: Change me, comprehensive ranking based on the pricing
 	Categories: []string{"cloud", "cloud_ssd", "cloud_efficiency", "cloud_essd", "cloud_auto", "cloud_essd_entry"},
-	Size:       tea.Int32(20),
+	VolumeSize: resources.Quantity("20Gi"),
 }
 
 // Options define the static launch template parameters

pkg/providers/imagefamily/resolver.go

@@ -53,9 +53,10 @@ import (
 
 const (
 	// TODO: After that open up the configuration options
-	instanceTypeFlexibilityThreshold = 5 // falling back to on-demand without flexibility risks insufficient capacity errors
-	maxInstanceTypes                 = 20
-	instanceCacheExpiration          = 15 * time.Second
+	instanceTypeFlexibilityThreshold       = 5 // falling back to on-demand without flexibility risks insufficient capacity errors
+	maxInstanceTypes                       = 20
+	instanceCacheExpiration                = 15 * time.Second
+	defaultDataDiskSize              int32 = 20
 )
 
 type Provider interface {
@@ -554,12 +555,16 @@ func (p *DefaultProvider) getProvisioningGroup(ctx context.Context, nodeClass *v
 	if systemDisk == nil {
 		systemDisk = imagefamily.DefaultSystemDisk.DeepCopy()
 	}
+	if systemDisk.VolumeSize == nil {
+		systemDisk.VolumeSize = imagefamily.DefaultSystemDisk.VolumeSize
+	}
 
 	imageID, ok := mappedImages[instanceTypes[0].Name]
 	if !ok {
 		return nil, errors.New("matching image not found")
 	}
 
+	// Convert bytes to GiB
 	createAutoProvisioningGroupRequest := &ecsclient.CreateAutoProvisioningGroupRequest{
 		RegionId:                        tea.String(p.region),
 		TotalTargetCapacity:             tea.String("1"),
@@ -574,7 +579,7 @@ func (p *DefaultProvider) getProvisioningGroup(ctx context.Context, nodeClass *v
 			UserData:         tea.String(userData),
 			ResourceGroupId:  tea.String(nodeClass.Spec.ResourceGroupID),
 			SecurityGroupIds: securityGroupIDs,
-			SystemDiskSize:   systemDisk.Size,
+			SystemDiskSize:   tea.Int32(systemDisk.GetGiBSize()),
 			Tag:              reqTags,
 		},
 		// Add this tag to auto-provisioning-group, alibabacloud will monitor the requests and enhance the stability
@@ -592,7 +597,7 @@ func (p *DefaultProvider) getProvisioningGroup(ctx context.Context, nodeClass *v
 	if nodeClass.Spec.DataDisks != nil && nodeClass.Spec.DataDisksCategories != nil {
 		createAutoProvisioningGroupRequest.LaunchConfiguration.DataDisk = lo.Map(nodeClass.Spec.DataDisks, func(dataDisk v1alpha1.DataDisk, index int) *ecsclient.CreateAutoProvisioningGroupRequestLaunchConfigurationDataDisk {
 			return &ecsclient.CreateAutoProvisioningGroupRequestLaunchConfigurationDataDisk{
-				Size:   dataDisk.Size,
+				Size:   tea.Int32(max(defaultDataDiskSize, dataDisk.GetGiBSize())),
 				Device: dataDisk.Device,
 			}
 		})

pkg/providers/instance/instance.go

@@ -334,11 +334,11 @@ func getCPUModel(cpuName string) string {
 }
 
 func ephemeralStorage(systemDisk *v1alpha1.SystemDisk) *resource.Quantity {
-	if systemDisk == nil {
-		return resources.Quantity(fmt.Sprintf("%dG", *imagefamily.DefaultSystemDisk.Size))
+	if systemDisk == nil || systemDisk.VolumeSize == nil {
+		return imagefamily.DefaultSystemDisk.VolumeSize
 	}
 
-	return resources.Quantity(fmt.Sprintf("%dG", *systemDisk.Size))
+	return systemDisk.VolumeSize
 }
 
 func privateIPv4Address(info *ecsclient.DescribeInstanceTypesResponseBodyInstanceTypesInstanceType) *resource.Quantity {