From 7d9444480403389138cbf0c6f66bef775d6f58f1 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Thu, 19 Sep 2024 09:23:16 -0700 Subject: [PATCH] fix server side apply --- pkg/apis/crds/karpenter.sh_nodeclaims.yaml | 2 +- pkg/apis/crds/karpenter.sh_nodepools.yaml | 2 +- pkg/apis/v1/nodeclaim_conversion.go | 27 +++++++++++-------- pkg/apis/v1/nodeclaim_conversion_test.go | 14 ++++++++++ pkg/apis/v1/nodepool_conversion.go | 27 +++++++++++-------- pkg/apis/v1/nodepool_conversion_test.go | 14 ++++++++++ .../karpenter.test.sh_testnodeclasses.yaml | 2 +- 7 files changed, 63 insertions(+), 25 deletions(-) diff --git a/pkg/apis/crds/karpenter.sh_nodeclaims.yaml b/pkg/apis/crds/karpenter.sh_nodeclaims.yaml index e62e764fbd..66ec8382e7 100644 --- 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 index 57ecfe118f..068d961fd1 100644 --- 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 index 4b96b41248..bc0c626efe 100644 --- a/pkg/apis/v1/nodeclaim_conversion.go +++ b/pkg/apis/v1/nodeclaim_conversion.go @@ -66,13 +66,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{} @@ -162,11 +164,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 index 7f4fe24672..6b69fa2241 100644 --- 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() { @@ -497,6 +504,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 index cc8b1fe4c7..d2fed0f021 100644 --- 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 index 414b8e1af7..9d2ad584f4 100644 --- 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() { @@ -519,6 +526,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 index fb3c15c292..b8684d22e0 100644 --- 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