From 7c89f7576768353470df601f21bd0cb77c63210a 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 9cd50521e5bf..2bcb8992c948 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()) @@ -314,9 +317,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)) }) }) @@ -343,6 +346,9 @@ var _ = Describe("Instancetype and Preferences", func() { CPU: instancetypev1alpha2.CPUInstancetype{ Guest: uint32(2), }, + Memory: instancetypev1alpha2.MemoryInstancetype{ + Guest: resource.MustParse("128Mi"), + }, }, } @@ -462,7 +468,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()) @@ -493,9 +499,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)) }) }) })