instancetype: Handle VirtualMachineInstancetypeSpecRevision without A…

…PIVersion As set out in issue kubevirt#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] kubernetes/client-go#541 Signed-off-by: Lee Yarwood <lyarwood@redhat.com>
lyarwood · Apr 11, 2023 · e56fd98 · e56fd98
1 parent 54cb4bb
commit e56fd98
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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))
 			})
 		})
 	})