From 46124b2fbc665184078f8b2f1fc7d793a9cd99a4 Mon Sep 17 00:00:00 2001 From: Tim Serong Date: Mon, 18 Mar 2024 21:17:10 +1100 Subject: [PATCH] Expose virtual size of VM images This requires: - https://github.com/longhorn/backing-image-manager/pull/208 - https://github.com/longhorn/longhorn-manager/pull/2680 With this change, we can do this: # kubectl get vmimages NAME DISPLAY-NAME SIZE VIRTUALSIZE AGE image-26r4s sles15-sp5-minimal-vm.x86_64-kvm-and-xen-qu1.qcow2 267845632 25769803776 6m36s image-bgnhb sle-micro.x86_64-5.5.0-default-qcow-gm.qcow2 1001652224 21474836480 20m image-nmmvj sle-15-sp5-full-x86_64-gm-media1.iso 14548992000 14548992000 19m Related issue: https://github.com/harvester/harvester/issues/4905 Signed-off-by: Tim Serong --- .../harvesterhci.io_virtualmachineimages.yaml | 6 ++++ pkg/apis/harvesterhci.io/v1beta1/image.go | 4 +++ .../master/image/backing_image_controller.go | 32 ++++++++++++++++--- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/deploy/charts/harvester-crd/templates/harvesterhci.io_virtualmachineimages.yaml b/deploy/charts/harvester-crd/templates/harvesterhci.io_virtualmachineimages.yaml index 75856097eb0..29ffd49faf9 100644 --- a/deploy/charts/harvester-crd/templates/harvesterhci.io_virtualmachineimages.yaml +++ b/deploy/charts/harvester-crd/templates/harvesterhci.io_virtualmachineimages.yaml @@ -26,6 +26,9 @@ spec: - jsonPath: .status.size name: SIZE type: integer + - jsonPath: .status.virtualSize + name: VIRTUALSIZE + type: integer - jsonPath: .metadata.creationTimestamp name: AGE type: date @@ -123,6 +126,9 @@ spec: type: integer storageClassName: type: string + virtualSize: + format: int64 + type: integer type: object required: - spec diff --git a/pkg/apis/harvesterhci.io/v1beta1/image.go b/pkg/apis/harvesterhci.io/v1beta1/image.go index 34bb4528f70..5bc9a1c07d3 100644 --- a/pkg/apis/harvesterhci.io/v1beta1/image.go +++ b/pkg/apis/harvesterhci.io/v1beta1/image.go @@ -23,6 +23,7 @@ const ( // +kubebuilder:resource:shortName=vmimage;vmimages,scope=Namespaced // +kubebuilder:printcolumn:name="DISPLAY-NAME",type=string,JSONPath=`.spec.displayName` // +kubebuilder:printcolumn:name="SIZE",type=integer,JSONPath=`.status.size` +// +kubebuilder:printcolumn:name="VIRTUALSIZE",type=integer,JSONPath=`.status.virtualSize` // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=`.metadata.creationTimestamp` type VirtualMachineImage struct { @@ -77,6 +78,9 @@ type VirtualMachineImageStatus struct { // +optional Size int64 `json:"size,omitempty"` + // +optional + VirtualSize int64 `json:"virtualSize,omitempty"` + // +optional StorageClassName string `json:"storageClassName,omitempty"` diff --git a/pkg/controller/master/image/backing_image_controller.go b/pkg/controller/master/image/backing_image_controller.go index 9e851c454c9..36b22fd8a81 100644 --- a/pkg/controller/master/image/backing_image_controller.go +++ b/pkg/controller/master/image/backing_image_controller.go @@ -37,7 +37,26 @@ func (h *backingImageHandler) OnChanged(_ string, backingImage *lhv1beta2.Backin } else if err != nil { return nil, err } - if !harvesterv1beta1.ImageInitialized.IsTrue(vmImage) || !harvesterv1beta1.ImageImported.IsUnknown(vmImage) { + // There are two states that we care about here: + // - ImageInitialized + // - ImageImported + // If ImageInitialized isn't yet true, it means there's no backing + // image or storage class, so we've got nothing to work with yet and + // should return immediately. + if !harvesterv1beta1.ImageInitialized.IsTrue(vmImage) { + return nil, nil + } + // If ImageImported is not unknown, it means the backing image has + // been imported, and we think we know everything about it, i.e. we've + // now been through a series of progress updates during image download, + // and those are finally done, so let's not worry about further updates. + // The problem with this logic is that when we add new fields (e.g. + // VirtualSize), existing images won't pick up those newly added fields + // if we return here immediately. So, now there's an additional check + // for that new field. Another, simpler, alternative would be to just + // drop the ImageImported.IsUnknown check entirely, and let the following + // loop run through on every OnChanged event. + if !harvesterv1beta1.ImageImported.IsUnknown(vmImage) && vmImage.Status.VirtualSize == backingImage.Status.VirtualSize { return nil, nil } toUpdate := vmImage.DeepCopy() @@ -46,11 +65,16 @@ func (h *backingImageHandler) OnChanged(_ string, backingImage *lhv1beta2.Backin toUpdate = handleFail(toUpdate, condition.Cond(harvesterv1beta1.ImageImported), fmt.Errorf(status.Message)) toUpdate.Status.Progress = status.Progress } else if status.State == lhv1beta2.BackingImageStateReady || status.State == lhv1beta2.BackingImageStateReadyForTransfer { - harvesterv1beta1.ImageImported.True(toUpdate) - harvesterv1beta1.ImageImported.Reason(toUpdate, "Imported") - harvesterv1beta1.ImageImported.Message(toUpdate, status.Message) toUpdate.Status.Progress = status.Progress toUpdate.Status.Size = backingImage.Status.Size + toUpdate.Status.VirtualSize = backingImage.Status.VirtualSize + if toUpdate.Status.VirtualSize > 0 { + // We can't set ImageImported to True until we know the VirtualSize, + // which may happen one update after the image becomes ready. + harvesterv1beta1.ImageImported.True(toUpdate) + harvesterv1beta1.ImageImported.Reason(toUpdate, "Imported") + harvesterv1beta1.ImageImported.Message(toUpdate, status.Message) + } } else if status.Progress != toUpdate.Status.Progress { harvesterv1beta1.ImageImported.Unknown(toUpdate) harvesterv1beta1.ImageImported.Reason(toUpdate, "Importing")