fix server side apply

kubernetes-sigs · Sep 19, 2024 · 141d4fa · 141d4fa
1 parent c170562
commit 141d4fa
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 26 deletions.
diff --git a/kwok/apis/crds/kwok.karpenter.sh_kwoknodeclasses.yaml b/kwok/apis/crds/kwok.karpenter.sh_kwoknodeclasses.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.16.2
+    controller-gen.kubebuilder.io/version: v0.16.3
   name: kwoknodeclasses.kwok.karpenter.sh
 spec:
   group: kwok.karpenter.sh

diff --git a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.16.2
+    controller-gen.kubebuilder.io/version: v0.16.3
   name: nodeclaims.karpenter.sh
 spec:
   group: karpenter.sh

diff --git a/pkg/apis/crds/karpenter.sh_nodepools.yaml b/pkg/apis/crds/karpenter.sh_nodepools.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.16.2
+    controller-gen.kubebuilder.io/version: v0.16.3
   name: nodepools.karpenter.sh
 spec:
   group: karpenter.sh

diff --git a/pkg/apis/v1/nodeclaim_conversion.go b/pkg/apis/v1/nodeclaim_conversion.go
@@ -63,13 +63,15 @@ func (in *NodeClaimSpec) convertTo(v1beta1nc *v1beta1.NodeClaimSpec, kubeletAnno
 	})
 	// Convert the NodeClassReference depending on whether the annotation exists
 	v1beta1nc.NodeClassRef = &v1beta1.NodeClassReference{}
-	if nodeClassReferenceAnnotation != "" {
-		if err := json.Unmarshal([]byte(nodeClassReferenceAnnotation), v1beta1nc.NodeClassRef); err != nil {
-			return fmt.Errorf("unmarshaling nodeClassRef annotation, %w", err)
+	if in.NodeClassRef != nil {
+		if nodeClassReferenceAnnotation != "" {
+			if err := json.Unmarshal([]byte(nodeClassReferenceAnnotation), v1beta1nc.NodeClassRef); err != nil {
+				return fmt.Errorf("unmarshaling nodeClassRef annotation, %w", err)
+			}
+		} else {
+			v1beta1nc.NodeClassRef.Name = in.NodeClassRef.Name
+			v1beta1nc.NodeClassRef.Kind = in.NodeClassRef.Kind
 		}
-	} else {
-		v1beta1nc.NodeClassRef.Name = in.NodeClassRef.Name
-		v1beta1nc.NodeClassRef.Kind = in.NodeClassRef.Kind
 	}
 	if kubeletAnnotation != "" {
 		v1beta1kubelet := &v1beta1.KubeletConfiguration{}
@@ -151,11 +153,14 @@ func (in *NodeClaimSpec) convertFrom(ctx context.Context, v1beta1nc *v1beta1.Nod
 		}
 	})
 
-	defaultNodeClassGVK := injection.GetNodeClasses(ctx)[0]
-	in.NodeClassRef = &NodeClassReference{
-		Name:  v1beta1nc.NodeClassRef.Name,
-		Kind:  lo.Ternary(v1beta1nc.NodeClassRef.Kind == "", defaultNodeClassGVK.Kind, v1beta1nc.NodeClassRef.Kind),
-		Group: lo.Ternary(v1beta1nc.NodeClassRef.APIVersion == "", defaultNodeClassGVK.Group, strings.Split(v1beta1nc.NodeClassRef.APIVersion, "/")[0]),
+	in.NodeClassRef = &NodeClassReference{}
+	if v1beta1nc.NodeClassRef != nil {
+		defaultNodeClassGVK := injection.GetNodeClasses(ctx)[0]
+		in.NodeClassRef = &NodeClassReference{
+			Name:  v1beta1nc.NodeClassRef.Name,
+			Kind:  lo.Ternary(v1beta1nc.NodeClassRef.Kind == "", defaultNodeClassGVK.Kind, v1beta1nc.NodeClassRef.Kind),
+			Group: lo.Ternary(v1beta1nc.NodeClassRef.APIVersion == "", defaultNodeClassGVK.Group, strings.Split(v1beta1nc.NodeClassRef.APIVersion, "/")[0]),
+		}
 	}
 
 	if v1beta1nc.Kubelet != nil {

diff --git a/pkg/apis/v1/nodeclaim_conversion_test.go b/pkg/apis/v1/nodeclaim_conversion_test.go
@@ -210,6 +210,13 @@ var _ = Describe("Convert v1 to v1beta1 NodeClaim API", func() {
 				Expect(v1beta1nodeclaim.Spec.NodeClassRef.Kind).To(Equal(nodeClassReference.Kind))
 				Expect(v1beta1nodeclaim.Spec.NodeClassRef.APIVersion).To(Equal(nodeClassReference.APIVersion))
 			})
+			It("should not panic when the v1 nodeclassRef is not defined", func() {
+				v1nodeclaim.Spec.NodeClassRef = nil
+				Expect(v1nodeclaim.ConvertTo(ctx, v1beta1nodeclaim)).To(Succeed())
+				Expect(v1beta1nodeclaim.Spec.NodeClassRef.Kind).To(Equal(""))
+				Expect(v1beta1nodeclaim.Spec.NodeClassRef.Name).To(Equal(""))
+				Expect(v1beta1nodeclaim.Spec.NodeClassRef.APIVersion).To(Equal(""))
+			})
 		})
 	})
 	Context("NodeClaim Status", func() {
@@ -487,6 +494,13 @@ var _ = Describe("Convert V1beta1 to V1 NodeClaim API", func() {
 				Expect(v1nodeclaim.Spec.NodeClassRef.Group).To(Equal(cloudProvider.NodeClassGroupVersionKind[0].Group))
 				Expect(v1nodeclaim.Annotations).To(HaveKeyWithValue(NodeClassReferenceAnnotationKey, string(nodeClassReferenceAnnotation)))
 			})
+			It("should not panic when the v1beta1 nodeclassRef is not defined", func() {
+				v1beta1nodeclaim.Spec.NodeClassRef = nil
+				Expect(v1nodeclaim.ConvertFrom(ctx, v1beta1nodeclaim)).To(Succeed())
+				Expect(v1nodeclaim.Spec.NodeClassRef.Kind).To(Equal(""))
+				Expect(v1nodeclaim.Spec.NodeClassRef.Name).To(Equal(""))
+				Expect(v1nodeclaim.Spec.NodeClassRef.Group).To(Equal(""))
+			})
 		})
 	})
 	Context("NodeClaim Status", func() {

diff --git a/pkg/apis/v1/nodepool_conversion.go b/pkg/apis/v1/nodepool_conversion.go
@@ -88,13 +88,15 @@ func (in *NodeClaimTemplate) convertTo(v1beta1np *v1beta1.NodeClaimTemplate, kub
 	})
 	// Convert the NodeClassReference depending on whether the annotation exists
 	v1beta1np.Spec.NodeClassRef = &v1beta1.NodeClassReference{}
-	if nodeClassReferenceAnnotation != "" {
-		if err := json.Unmarshal([]byte(nodeClassReferenceAnnotation), v1beta1np.Spec.NodeClassRef); err != nil {
-			return fmt.Errorf("unmarshaling nodeClassRef annotation, %w", err)
+	if in.Spec.NodeClassRef != nil {
+		if nodeClassReferenceAnnotation != "" {
+			if err := json.Unmarshal([]byte(nodeClassReferenceAnnotation), v1beta1np.Spec.NodeClassRef); err != nil {
+				return fmt.Errorf("unmarshaling nodeClassRef annotation, %w", err)
+			}
+		} else {
+			v1beta1np.Spec.NodeClassRef.Name = in.Spec.NodeClassRef.Name
+			v1beta1np.Spec.NodeClassRef.Kind = in.Spec.NodeClassRef.Kind
 		}
-	} else {
-		v1beta1np.Spec.NodeClassRef.Name = in.Spec.NodeClassRef.Name
-		v1beta1np.Spec.NodeClassRef.Kind = in.Spec.NodeClassRef.Kind
 	}
 	if kubeletAnnotation != "" {
 		v1beta1kubelet := &v1beta1.KubeletConfiguration{}
@@ -173,11 +175,14 @@ func (in *NodeClaimTemplate) convertFrom(ctx context.Context, v1beta1np *v1beta1
 		}
 	})
 
-	defaultNodeClassGVK := injection.GetNodeClasses(ctx)[0]
-	in.Spec.NodeClassRef = &NodeClassReference{
-		Name:  v1beta1np.Spec.NodeClassRef.Name,
-		Kind:  lo.Ternary(v1beta1np.Spec.NodeClassRef.Kind == "", defaultNodeClassGVK.Kind, v1beta1np.Spec.NodeClassRef.Kind),
-		Group: lo.Ternary(v1beta1np.Spec.NodeClassRef.APIVersion == "", defaultNodeClassGVK.Group, strings.Split(v1beta1np.Spec.NodeClassRef.APIVersion, "/")[0]),
+	in.Spec.NodeClassRef = &NodeClassReference{}
+	if v1beta1np.Spec.NodeClassRef != nil {
+		defaultNodeClassGVK := injection.GetNodeClasses(ctx)[0]
+		in.Spec.NodeClassRef = &NodeClassReference{
+			Name:  v1beta1np.Spec.NodeClassRef.Name,
+			Kind:  lo.Ternary(v1beta1np.Spec.NodeClassRef.Kind == "", defaultNodeClassGVK.Kind, v1beta1np.Spec.NodeClassRef.Kind),
+			Group: lo.Ternary(v1beta1np.Spec.NodeClassRef.APIVersion == "", defaultNodeClassGVK.Group, strings.Split(v1beta1np.Spec.NodeClassRef.APIVersion, "/")[0]),
+		}
 	}
 	if v1beta1np.Spec.Kubelet != nil {
 		kubelet, err := json.Marshal(v1beta1np.Spec.Kubelet)

diff --git a/pkg/apis/v1/nodepool_conversion_test.go b/pkg/apis/v1/nodepool_conversion_test.go
@@ -208,6 +208,13 @@ var _ = Describe("Convert V1 to V1beta1 NodePool API", func() {
 					Expect(v1beta1nodepool.Spec.Template.Spec.NodeClassRef.Kind).To(Equal(nodeClassReference.Kind))
 					Expect(v1beta1nodepool.Spec.Template.Spec.NodeClassRef.APIVersion).To(Equal(nodeClassReference.APIVersion))
 				})
+				It("should not panic when the v1 nodeclassRef is not defined", func() {
+					v1nodepool.Spec.Template.Spec.NodeClassRef = nil
+					Expect(v1nodepool.ConvertTo(ctx, v1beta1nodepool)).To(Succeed())
+					Expect(v1beta1nodepool.Spec.Template.Spec.NodeClassRef.Kind).To(Equal(""))
+					Expect(v1beta1nodepool.Spec.Template.Spec.NodeClassRef.Name).To(Equal(""))
+					Expect(v1beta1nodepool.Spec.Template.Spec.NodeClassRef.APIVersion).To(Equal(""))
+				})
 			})
 		})
 		Context("Disruption", func() {
@@ -515,6 +522,13 @@ var _ = Describe("Convert V1beta1 to V1 NodePool API", func() {
 					Expect(v1nodepool.Spec.Template.Spec.NodeClassRef.Group).To(Equal(cloudProvider.NodeClassGroupVersionKind[0].Group))
 					Expect(v1nodepool.Annotations).To(HaveKeyWithValue(NodeClassReferenceAnnotationKey, string(nodeClassReferenceAnnotation)))
 				})
+				It("should not panic when the v1beta1 nodeclassRef is not defined", func() {
+					v1beta1nodepool.Spec.Template.Spec.NodeClassRef = nil
+					Expect(v1nodepool.ConvertFrom(ctx, v1beta1nodepool)).To(Succeed())
+					Expect(v1nodepool.Spec.Template.Spec.NodeClassRef.Kind).To(Equal(""))
+					Expect(v1nodepool.Spec.Template.Spec.NodeClassRef.Name).To(Equal(""))
+					Expect(v1nodepool.Spec.Template.Spec.NodeClassRef.Group).To(Equal(""))
+				})
 			})
 		})
 		Context("Disruption", func() {

diff --git a/pkg/test/v1alpha1/crds/karpenter.test.sh_testnodeclasses.yaml b/pkg/test/v1alpha1/crds/karpenter.test.sh_testnodeclasses.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.16.2
+    controller-gen.kubebuilder.io/version: v0.16.3
   name: testnodeclasses.karpenter.test.sh
 spec:
   group: karpenter.test.sh