From e56fd98cfae5314d6e1c48363811f2e7005147a2 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Thu, 6 Apr 2023 14:59:53 +0100 Subject: [PATCH] instancetype: Handle VirtualMachineInstancetypeSpecRevision without APIVersion As set out in issue #9261 there have been reports of older VirtualMachineInstancetypeSpecRevision objects being stored in ControllerRevisions without APIVersion set. This is likely due to a known client-go issue [1] stripping TypeMeta from returned objects. Given the limited support for v1alpha1 and the fact that we moved away from VirtualMachineInstancetypeSpecRevision to complete objects being captured in ControllerRevisions by v1alpah2 this change simply works around this issue by ignoring the value of APIVersion. This should allow the conversion to complete correctly until we upgrade these ControllerRevisions in the future. [1] https://github.com/kubernetes/client-go/issues/541 Signed-off-by: Lee Yarwood --- pkg/instancetype/compatibility.go | 8 -------- pkg/instancetype/compatibility_test.go | 19 +++++++++++++------ pkg/instancetype/instancetype_test.go | 22 ++++++++++++++-------- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/pkg/instancetype/compatibility.go b/pkg/instancetype/compatibility.go index e3a51798bb64..c78e5b684c6f 100644 --- a/pkg/instancetype/compatibility.go +++ b/pkg/instancetype/compatibility.go @@ -41,10 +41,6 @@ func decodeOldInstancetypeRevisionObject(data []byte) (*instancetypev1alpha1.Vir return nil, nil } - if revision.APIVersion != instancetypev1alpha1.SchemeGroupVersion.String() { - return nil, nil - } - instancetypeSpec := &instancetypev1alpha1.VirtualMachineInstancetypeSpec{} err = json.Unmarshal(revision.Spec, instancetypeSpec) if err != nil { @@ -71,10 +67,6 @@ func decodeOldPreferenceRevisionObject(data []byte) (*instancetypev1alpha1.Virtu return nil, nil } - if revision.APIVersion != instancetypev1alpha1.SchemeGroupVersion.String() { - return nil, nil - } - preferenceSpec := &instancetypev1alpha1.VirtualMachinePreferenceSpec{} err = json.Unmarshal(revision.Spec, preferenceSpec) if err != nil { diff --git a/pkg/instancetype/compatibility_test.go b/pkg/instancetype/compatibility_test.go index 8cb5110fc4c5..ab5861860ed1 100644 --- a/pkg/instancetype/compatibility_test.go +++ b/pkg/instancetype/compatibility_test.go @@ -16,7 +16,7 @@ import ( var _ = Describe("instancetype compatibility", func() { Context("reading old ControllerRevision", func() { - It("should decode v1alpha1 instancetype from ControllerRevision", func() { + DescribeTable("should decode v1alpha1 instancetype from ControllerRevision", func(apiVersion string) { instancetypeSpec := v1alpha1.VirtualMachineInstancetypeSpec{ CPU: v1alpha1.CPUInstancetype{ Guest: 4, @@ -27,7 +27,7 @@ var _ = Describe("instancetype compatibility", func() { Expect(err).ToNot(HaveOccurred()) revision := v1alpha1.VirtualMachineInstancetypeSpecRevision{ - APIVersion: v1alpha1.SchemeGroupVersion.String(), + APIVersion: apiVersion, Spec: specBytes, } @@ -38,9 +38,12 @@ var _ = Describe("instancetype compatibility", func() { Expect(err).ToNot(HaveOccurred()) Expect(decoded).ToNot(BeNil()) Expect(decoded.Spec.CPU).To(Equal(instancetypeSpec.CPU)) - }) + }, + Entry("with APIVersion", v1alpha1.SchemeGroupVersion.String()), + Entry("without APIVersion", ""), + ) - It("should decode v1alpha1 preference from ControllerRevision", func() { + DescribeTable("should decode v1alpha1 preference from ControllerRevision", func(apiVersion string) { preferredTopology := v1alpha1.PreferCores preferenceSpec := v1alpha1.VirtualMachinePreferenceSpec{ CPU: &v1alpha1.CPUPreferences{ @@ -52,7 +55,7 @@ var _ = Describe("instancetype compatibility", func() { Expect(err).ToNot(HaveOccurred()) revision := v1alpha1.VirtualMachinePreferenceSpecRevision{ - APIVersion: v1alpha1.SchemeGroupVersion.String(), + APIVersion: apiVersion, Spec: specBytes, } @@ -63,7 +66,11 @@ var _ = Describe("instancetype compatibility", func() { Expect(err).ToNot(HaveOccurred()) Expect(decoded).ToNot(BeNil()) Expect(decoded.Spec).To(Equal(preferenceSpec)) - }) + + }, + Entry("with APIVersion", v1alpha1.SchemeGroupVersion.String()), + Entry("without APIVersion", ""), + ) }) Context("instancetype conversion", func() { diff --git a/pkg/instancetype/instancetype_test.go b/pkg/instancetype/instancetype_test.go index 5f0dd0b42531..1db63146ad78 100644 --- a/pkg/instancetype/instancetype_test.go +++ b/pkg/instancetype/instancetype_test.go @@ -163,6 +163,9 @@ var _ = Describe("Instancetype and Preferences", func() { CPU: instancetypev1alpha2.CPUInstancetype{ Guest: uint32(2), }, + Memory: instancetypev1alpha2.MemoryInstancetype{ + Guest: resource.MustParse("128Mi"), + }, }, } @@ -283,7 +286,7 @@ var _ = Describe("Instancetype and Preferences", func() { Expect(instancetypeMethods.StoreControllerRevisions(vm)).To(MatchError(ContainSubstring("VM field conflicts with selected Instancetype"))) }) - It("find fails to decode v1alpha1 VirtualMachineInstancetypeSpecRevision ControllerRevision without APIVersion set - bug #9261", func() { + It("find successfully decodes v1alpha1 VirtualMachineInstancetypeSpecRevision ControllerRevision without APIVersion set - bug #9261", func() { specData, err := json.Marshal(clusterInstancetype.Spec) Expect(err).ToNot(HaveOccurred()) @@ -313,9 +316,9 @@ var _ = Describe("Instancetype and Preferences", func() { Kind: apiinstancetype.ClusterSingularResourceName, } - _, err = instancetypeMethods.FindInstancetypeSpec(vm) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("Object 'Kind' is missing in")) + foundInstancetypeSpec, err := instancetypeMethods.FindInstancetypeSpec(vm) + Expect(err).ToNot(HaveOccurred()) + Expect(*foundInstancetypeSpec).To(Equal(clusterInstancetype.Spec)) }) }) @@ -342,6 +345,9 @@ var _ = Describe("Instancetype and Preferences", func() { CPU: instancetypev1alpha2.CPUInstancetype{ Guest: uint32(2), }, + Memory: instancetypev1alpha2.MemoryInstancetype{ + Guest: resource.MustParse("128Mi"), + }, }, } @@ -461,7 +467,7 @@ var _ = Describe("Instancetype and Preferences", func() { Expect(instancetypeMethods.StoreControllerRevisions(vm)).To(MatchError(ContainSubstring("VM field conflicts with selected Instancetype"))) }) - It("find fails to decode v1alpha1 VirtualMachineInstancetypeSpecRevision ControllerRevision without APIVersion set - bug #9261", func() { + It("find successfully decodes v1alpha1 VirtualMachineInstancetypeSpecRevision ControllerRevision without APIVersion set - bug #9261", func() { specData, err := json.Marshal(fakeInstancetype.Spec) Expect(err).ToNot(HaveOccurred()) @@ -491,9 +497,9 @@ var _ = Describe("Instancetype and Preferences", func() { Kind: apiinstancetype.SingularResourceName, } - _, err = instancetypeMethods.FindInstancetypeSpec(vm) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("Object 'Kind' is missing in")) + foundInstancetypeSpec, err := instancetypeMethods.FindInstancetypeSpec(vm) + Expect(err).ToNot(HaveOccurred()) + Expect(*foundInstancetypeSpec).To(Equal(fakeInstancetype.Spec)) }) }) })