Merge pull request #875 from bryanv/bryanv/vmi-dup-append-too-large

🐛 Fix duplicate append of Image.Status fields

controllers/contentlibrary/utils/controller_builder.go

@@ -462,6 +462,12 @@ func (r *Reconciler) syncImageContent(
 	vmiStatus *vmopv1.VirtualMachineImageStatus) error {
 
 	latestVersion := cliStatus.ContentVersion
+	if vmiStatus.ProviderContentVersion == latestVersion {
+		// Hack: populate Disks fields during version upgrade.
+		if len(vmiStatus.Disks) != 0 {
+			return nil
+		}
+	}
 
 	err := r.VMProvider.SyncVirtualMachineImage(ctx, cliObj, vmiObj)
 	if err != nil {

controllers/contentlibrary/utils/controller_builder_test.go

@@ -433,6 +433,40 @@ var _ = Describe("Reconcile",
 						})
 					})
 				})
+
+				When("Image resource is created and already up-to-date and Status.Disk is not empty", func() {
+
+					JustBeforeEach(func() {
+						newVMI(
+							ctx,
+							req.Namespace,
+							vmiName,
+							vmopv1.VirtualMachineImageStatus{
+								ProviderContentVersion: cliStatus.ContentVersion,
+								Disks:                  make([]vmopv1.VirtualMachineImageDiskInfo, 1),
+								Firmware:               "should-not-be-updated",
+							})
+					})
+
+					BeforeEach(func() {
+						fakeVMProvider.SyncVirtualMachineImageFn = func(_ context.Context, _, vmiObj client.Object) error {
+							// This should not be called since the content versions match and disks isn't empty.
+							vmi := vmiObj.(*vmopv1.VirtualMachineImage)
+							vmi.Status.Firmware = firmwareValue
+							return fmt.Errorf("sync-error")
+						}
+					})
+
+					It("should skip updating the ClusterVirtualMachineImage with library item", func() {
+						_, err := reconciler.Reconcile(context.Background(), req)
+						Expect(err).ToNot(HaveOccurred())
+						cliObj, cliSpec, cliStatus = getCLI(ctx, req.Namespace, req.Name)
+
+						vmiObj, vmiSpec, vmiStatus := getVMI(ctx, req.Namespace, vmiName)
+						assertVMImageFromCLItem(cliObj, *cliSpec, *cliStatus, vmiObj, *vmiSpec, *vmiStatus)
+						Expect(vmiStatus.Firmware).To(Equal("should-not-be-updated"))
+					})
+				})
 			})
 
 			Context("ReconcileDelete", func() {

pkg/providers/vsphere/contentlibrary/content_library_utils.go

@@ -65,6 +65,8 @@ func UpdateVmiWithOvfEnvelope(obj client.Object, ovfEnvelope ovf.Envelope) {
 
 	if ovfEnvelope.Disk != nil && len(ovfEnvelope.Disk.Disks) > 0 {
 		populateImageStatusFromOVFDiskSection(status, ovfEnvelope.Disk)
+	} else {
+		status.Disks = nil
 	}
 }
 
@@ -107,6 +109,7 @@ func initImageStatusFromOVFVirtualSystem(
 		}
 	}
 
+	imageStatus.OVFProperties = nil
 	for _, product := range ovfVirtualSystem.Product {
 		for _, prop := range product.Property {
 			// Only show user configurable properties
@@ -121,6 +124,7 @@ func initImageStatusFromOVFVirtualSystem(
 		}
 	}
 
+	imageStatus.VMwareSystemProperties = nil
 	ovfSystemProps := getVmwareSystemPropertiesFromOvf(ovfVirtualSystem)
 	if len(ovfSystemProps) > 0 {
 		for k, v := range ovfSystemProps {

pkg/providers/vsphere/contentlibrary/content_library_utils_test.go

@@ -177,6 +177,16 @@ var _ = Describe("UpdateVmiWithOvfEnvelope", func() {
 		Expect(image.Status.Disks[1].Capacity.String()).To(Equal("10Gi"))
 	})
 
+	It("Repeated UpdateVmiWithOvfEnvelope should not duplicated items", func() {
+		Expect(image.Status.Disks).ToNot(BeEmpty())
+		Expect(image.Status.OVFProperties).ToNot(BeEmpty())
+		Expect(image.Status.VMwareSystemProperties).ToNot(BeEmpty())
+
+		savedImage := image.DeepCopy()
+		contentlibrary.UpdateVmiWithOvfEnvelope(image, ovfEnvelope)
+		Expect(image).To(Equal(savedImage))
+	})
+
 	Context("Image is V1Alpha1Compatible", func() {
 		BeforeEach(func() {
 			ovfEnvelope.VirtualSystem.VirtualHardware[0].ExtraConfig = append(ovfEnvelope.VirtualSystem.VirtualHardware[0].ExtraConfig,