fix create nodeclaim issue (#290)

* Add namespace level vGPU resource quotas

* Add namespace level vGPU resource quotas

* add webhook and fix type convert issue

* fix make lint issue

* fix webhook validate

* rollback webhook validate

* remove duplicated webhook

* #207  in memory check gpuquota and  allocate/deallocate/init

* quota design

* refactor code

* rollback webhook suit-test

* rollback test suit

* Karpenter node claim design

* design refactor

* remove design

* fix load karpenter scheme issue

* fix rbac create node claim no permission and gpuresourcename null

* fix zone/region nodeclaim requirement key bug and add the role

* Mapping capacity type for capacity type

* fix nodeclaim capacity type value issue

---------

Co-authored-by: 0x5457 <[email protected]>

Author: wangqianqianjun

config/rbac/role.yaml

@@ -164,3 +164,23 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - karpenter.sh
+  resources:
+  - nodeclaims
+  verbs:
+  - get
+  - list
+  - watch
+  - update
+  - delete
+  - create
+  - patch
+- apiGroups:
+  - karpenter.k8s.aws
+  resources:
+  - ec2nodeclasses
+  verbs:
+  - get
+  - list
+  - watch

internal/cloudprovider/karpenter/nodeclaim.go

@@ -25,9 +25,22 @@ import (
 )
 
 var (
-	KarpenterGroup   = "karpenter.sh"
-	KarpenterVersion = "v1"
-	kindNodeClaim    = "NodeClaim"
+	KarpenterGroup         = "karpenter.sh"
+	KarpenterVersion       = "v1"
+	kindNodeClaim          = "NodeClaim"
+	DefaultGPUResourceName = "nvidia.com/gpu"
+	EC2NodeClassGroup      = "karpenter.k8s.aws"
+	// AWS capacity type: https://karpenter.sh/docs/concepts/scheduling/#well-known-labels
+	AWSOnDemandType = "on-demand"
+	AWSReservedType = "reserved"
+	AWSSpotType     = "spot"
+
+	// CapacityTypeMapping maps the CapacityTypeEnum to the corresponding Karpenter capacity type
+	CapacityTypeMapping = map[tfv1.CapacityTypeEnum]string{
+		tfv1.CapacityTypeOnDemand: AWSOnDemandType,
+		tfv1.CapacityTypeSpot:     AWSReservedType,
+		tfv1.CapacityTypeReserved: AWSSpotType,
+	}
 )
 
 // KarpenterExtraConfig holds Karpenter-specific configuration parsed from ExtraParams
@@ -83,20 +96,12 @@ func (p KarpenterGPUNodeProvider) TestConnection() error {
 	if p.client == nil {
 		return fmt.Errorf("kubernetes client is not initialized")
 	}
-	exist := true
 	// check if NodeClaim CRD exists
 	if err := p.client.List(context.Background(), &karpv1.NodeClaimList{}); err != nil {
 		log.FromContext(p.ctx).Error(err, "karpenter NodeClaim CRD not found.")
-		exist = false
-	}
-	if exist {
-		return nil
-	}
-	// check if NodePool CRD exists
-	if err := p.client.List(context.Background(), &karpv1.NodePoolList{}); err != nil {
-		log.FromContext(p.ctx).Error(err, "karpenter NodePool CRD not found.")
-		return fmt.Errorf("karpenter CRD not found or not accessible")
+		return fmt.Errorf("karpenter nodeclaim CRD not found or not accessible")
 	}
+	log.FromContext(p.ctx).Info("Test connection to Karpenter nodeclaim succeeded.")
 	return nil
 }
 
@@ -224,7 +229,9 @@ func (p KarpenterGPUNodeProvider) GetGPUNodeInstanceTypeInfo(region string) []ty
 
 // parseKarpenterConfig extracts Karpenter-specific configuration from ExtraParams
 func (p KarpenterGPUNodeProvider) parseKarpenterConfig(param *tfv1.GPUNodeClaimSpec) *KarpenterExtraConfig {
-	karpenterConfig := &KarpenterExtraConfig{}
+	karpenterConfig := &KarpenterExtraConfig{
+		GPUResourceName: corev1.ResourceName(DefaultGPUResourceName),
+	}
 	extraParams := param.ExtraParams
 	if extraParams == nil {
 		return karpenterConfig
@@ -261,7 +268,7 @@ func (p KarpenterGPUNodeProvider) parseKarpenterConfig(param *tfv1.GPUNodeClaimS
 	}
 
 	if karpenterConfig.GPUResourceName == "" {
-		karpenterConfig.GPUResourceName = "nvidia.com/gpu"
+		karpenterConfig.GPUResourceName = corev1.ResourceName(DefaultGPUResourceName)
 	}
 
 	return karpenterConfig
@@ -375,44 +382,77 @@ func (p KarpenterGPUNodeProvider) queryAndBuildNodeClassRef(ctx context.Context,
 func (p KarpenterGPUNodeProvider) buildRequirements(nodeClaim *karpv1.NodeClaim, param *tfv1.GPUNodeClaimSpec) {
 	// Build node selector requirements using Karpenter's NodeSelectorRequirement
 	requirements := []karpv1.NodeSelectorRequirementWithMinValues{}
+	seen := make(map[string]struct{})
 	// 1. instance type
 	if param.InstanceType != "" {
+		key := string(tfv1.NodeRequirementKeyInstanceType)
 		requirements = append(requirements, karpv1.NodeSelectorRequirementWithMinValues{
 			NodeSelectorRequirement: corev1.NodeSelectorRequirement{
-				Key:      string(tfv1.NodeRequirementKeyInstanceType),
+				Key:      key,
 				Operator: corev1.NodeSelectorOpIn,
 				Values:   []string{param.InstanceType},
 			},
 		})
+		seen[key] = struct{}{}
 	}
 	// 2. zone
 	if param.Zone != "" {
+		key := string(tfv1.NodeRequirementKeyZone)
 		requirements = append(requirements, karpv1.NodeSelectorRequirementWithMinValues{
 			NodeSelectorRequirement: corev1.NodeSelectorRequirement{
-				Key:      string(tfv1.NodeRequirementKeyZone),
+				Key:      key,
 				Operator: corev1.NodeSelectorOpIn,
 				Values:   []string{param.Zone},
 			},
 		})
+		seen[key] = struct{}{}
 	}
 
 	// 3. region
 	if param.Region != "" {
+		key := string(tfv1.NodeRequirementKeyRegion)
 		requirements = append(requirements, karpv1.NodeSelectorRequirementWithMinValues{
 			NodeSelectorRequirement: corev1.NodeSelectorRequirement{
-				Key:      string(tfv1.NodeRequirementKeyRegion),
+				Key:      key,
 				Operator: corev1.NodeSelectorOpIn,
 				Values:   []string{param.Region},
 			},
 		})
+		seen[key] = struct{}{}
+	}
+	// 4. capacity type
+	if param.CapacityType != "" {
+		key := string(tfv1.NodeRequirementKeyCapacityType)
+		value := string(param.CapacityType)
+		// if capacity type is not in the mapping, use the original value
+		// otherwise, use the mapping value
+		// https://github.com/kubernetes-sigs/karpenter/blob/f8da711d7e72b678e77f4758bc73a34ba34286d2/pkg/apis/v1/labels.go#L35
+		if _, exists := CapacityTypeMapping[param.CapacityType]; exists {
+			value = CapacityTypeMapping[param.CapacityType]
+		}
+
+		requirements = append(requirements, karpv1.NodeSelectorRequirementWithMinValues{
+			NodeSelectorRequirement: corev1.NodeSelectorRequirement{
+				Key:      key,
+				Operator: corev1.NodeSelectorOpIn,
+				Values:   []string{value},
+			},
+		})
+		seen[key] = struct{}{}
 	}
 
 	// 4. custom GPU requirements
 	if p.nodeManagerConfig.NodeProvisioner.GPURequirements != nil {
 		for _, requirement := range p.nodeManagerConfig.NodeProvisioner.GPURequirements {
+			key := string(requirement.Key)
+			// remove duplicate requirements
+			if _, exists := seen[key]; exists {
+				continue
+			}
+			seen[key] = struct{}{}
 			requirements = append(requirements, karpv1.NodeSelectorRequirementWithMinValues{
 				NodeSelectorRequirement: corev1.NodeSelectorRequirement{
-					Key:      string(requirement.Key),
+					Key:      key,
 					Operator: requirement.Operator,
 					Values:   requirement.Values,
 				},

internal/cloudprovider/karpenter/nodeclaim_test.go

@@ -121,7 +121,12 @@ func TestKarpenterGPUNodeProvider_CreateNode(t *testing.T) {
 				{
 					Key:      "karpenter.sh/capacity-type",
 					Operator: corev1.NodeSelectorOpIn,
-					Values:   []string{"on-demand"},
+					Values:   []string{string(tfv1.CapacityTypeOnDemand)},
+				},
+				{
+					Key:      "node.kubernetes.io/instance-type",
+					Operator: corev1.NodeSelectorOpIn,
+					Values:   []string{"x6g.large"},
 				},
 			},
 			GPULabels: map[string]string{
@@ -167,10 +172,70 @@ func TestKarpenterGPUNodeProvider_CreateNode(t *testing.T) {
 				TFlopsOffered:    resource.MustParse("125"),
 				VRAMOffered:      resource.MustParse("64Gi"),
 				GPUDeviceOffered: 4,
-				ExtraParams: map[string]string{
-					"karpenter.nodeClaim.terminationGracePeriod": "30s",
-					"karpenter.gpuResource":                      "nvidia.com/gpu",
+				ExtraParams:      map[string]string{},
+			},
+			expectError: false,
+		},
+		{
+			name: "successful node creation with GPU",
+			param: &tfv1.GPUNodeClaimSpec{
+				NodeName:     "test-gpu-node",
+				Region:       "us-west-2",
+				Zone:         "us-west-2a",
+				InstanceType: "p3.8xlarge",
+				CapacityType: tfv1.CapacityTypeReserved,
+				NodeClassRef: tfv1.GroupKindName{
+					Name:    "test-ec2-node-class",
+					Group:   "karpenter.k8s.aws",
+					Kind:    "EC2NodeClass",
+					Version: "v1",
+				},
+				TFlopsOffered:    resource.MustParse("125"),
+				VRAMOffered:      resource.MustParse("64Gi"),
+				GPUDeviceOffered: 4,
+				ExtraParams:      map[string]string{},
+			},
+			expectError: false,
+		},
+		{
+			name: "successful node creation with GPU",
+			param: &tfv1.GPUNodeClaimSpec{
+				NodeName:     "test-gpu-node",
+				Region:       "us-west-2",
+				Zone:         "us-west-2a",
+				InstanceType: "p3.8xlarge",
+				CapacityType: tfv1.CapacityTypeSpot,
+				NodeClassRef: tfv1.GroupKindName{
+					Name:    "test-ec2-node-class",
+					Group:   "karpenter.k8s.aws",
+					Kind:    "EC2NodeClass",
+					Version: "v1",
 				},
+				TFlopsOffered:    resource.MustParse("125"),
+				VRAMOffered:      resource.MustParse("64Gi"),
+				GPUDeviceOffered: 4,
+				ExtraParams:      map[string]string{},
+			},
+			expectError: false,
+		},
+		{
+			name: "successful node creation with GPU",
+			param: &tfv1.GPUNodeClaimSpec{
+				NodeName:     "test-gpu-node",
+				Region:       "us-west-2",
+				Zone:         "us-west-2a",
+				InstanceType: "p3.8xlarge",
+				CapacityType: "unexpect-capacity-type",
+				NodeClassRef: tfv1.GroupKindName{
+					Name:    "test-ec2-node-class",
+					Group:   "karpenter.k8s.aws",
+					Kind:    "EC2NodeClass",
+					Version: "v1",
+				},
+				TFlopsOffered:    resource.MustParse("125"),
+				VRAMOffered:      resource.MustParse("64Gi"),
+				GPUDeviceOffered: 4,
+				ExtraParams:      map[string]string{},
 			},
 			expectError: false,
 		},
@@ -317,18 +382,29 @@ func TestKarpenterGPUNodeProvider_parseKarpenterConfig(t *testing.T) {
 			param: &tfv1.GPUNodeClaimSpec{
 				ExtraParams: nil,
 			},
-			expected: "", // Early return when extraParams is nil
+			expected: "nvidia.com/gpu", // Early return when extraParams is nil
 		},
 		{
 			name: "empty extra params",
 			param: &tfv1.GPUNodeClaimSpec{
 				ExtraParams: map[string]string{
-					"karpenter.nodeClassRef.kind":                "EC2NodeClass",
 					"karpenter.nodeClaim.terminationGracePeriod": "30s",
 				},
 			},
 			expected: "30s", // Default value should be set
 		},
+		{
+			name: "empty extra params",
+			param: &tfv1.GPUNodeClaimSpec{
+				ExtraParams: map[string]string{},
+			},
+			expected: "nvidia.com/gpu", // Default value should be set
+		},
+		{
+			name:     "empty extra params",
+			param:    &tfv1.GPUNodeClaimSpec{},
+			expected: "nvidia.com/gpu", // Default value should be set
+		},
 	}
 
 	t.Run(tests[0].name, func(t *testing.T) {
@@ -341,6 +417,14 @@ func TestKarpenterGPUNodeProvider_parseKarpenterConfig(t *testing.T) {
 		assert.Equal(t, tests[1].expected, result.NodeClaim.TerminationGracePeriod)
 	})
 
+	t.Run(tests[2].name, func(t *testing.T) {
+		result := provider.parseKarpenterConfig(tests[2].param)
+		assert.Equal(t, tests[2].expected, string(result.GPUResourceName))
+	})
+	t.Run(tests[3].name, func(t *testing.T) {
+		result := provider.parseKarpenterConfig(tests[3].param)
+		assert.Equal(t, tests[3].expected, string(result.GPUResourceName))
+	})
 }
 
 func TestSetNestedValue(t *testing.T) {

internal/controller/fake_node_claim_contoller_test.go

@@ -56,7 +56,12 @@ var _ = Describe("FakeNodeClaimController", func() {
 						{
 							Key:      "karpenter.sh/capacity-type",
 							Operator: corev1.NodeSelectorOpIn,
-							Values:   []string{"on-demand"},
+							Values:   []string{"OnDemand"},
+						},
+						{
+							Key:      "node.kubernetes.io/instance-type",
+							Operator: corev1.NodeSelectorOpIn,
+							Values:   []string{"x6g.large"},
 						},
 					},
 					GPULabels: map[string]string{