From 79f6cdc0015e832054c4870b1094c6a3384b7e7b 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 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 This in turn means we can update the UI to take image virtual size into account when creating VMs. Note that this requires the following Longhorn PRs, which have been backported to longhorn 1.5.5 and 1.6.2: - https://github.com/longhorn/backing-image-manager/pull/208 - https://github.com/longhorn/longhorn-manager/pull/2680 - https://github.com/longhorn/longhorn/pull/8267 Related issue: https://github.com/harvester/harvester/issues/4905 Signed-off-by: Tim Serong --- api/openapi-spec/swagger.json | 4 +++ .../harvesterhci.io_virtualmachineimages.yaml | 6 ++++ pkg/apis/harvesterhci.io/v1beta1/image.go | 4 +++ .../v1beta1/openapi_generated.go | 6 ++++ .../master/image/backing_image_controller.go | 32 ++++++++++++++++--- 5 files changed, 48 insertions(+), 4 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 70a6709270..ca84d7051c 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -9547,6 +9547,10 @@ }, "storageClassName": { "type": "string" + }, + "virtualSize": { + "type": "integer", + "format": "int64" } } }, diff --git a/deploy/charts/harvester-crd/templates/harvesterhci.io_virtualmachineimages.yaml b/deploy/charts/harvester-crd/templates/harvesterhci.io_virtualmachineimages.yaml index 75856097eb..29ffd49faf 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 708708c834..097132b61c 100644 --- a/pkg/apis/harvesterhci.io/v1beta1/image.go +++ b/pkg/apis/harvesterhci.io/v1beta1/image.go @@ -17,6 +17,7 @@ var ( // +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 { @@ -80,6 +81,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/apis/harvesterhci.io/v1beta1/openapi_generated.go b/pkg/apis/harvesterhci.io/v1beta1/openapi_generated.go index 90cb0e9b62..e1dc2c5c5c 100644 --- a/pkg/apis/harvesterhci.io/v1beta1/openapi_generated.go +++ b/pkg/apis/harvesterhci.io/v1beta1/openapi_generated.go @@ -3967,6 +3967,12 @@ func schema_pkg_apis_harvesterhciio_v1beta1_VirtualMachineImageStatus(ref common Format: "int64", }, }, + "virtualSize": { + SchemaProps: spec.SchemaProps{ + Type: []string{"integer"}, + Format: "int64", + }, + }, "storageClassName": { SchemaProps: spec.SchemaProps{ Type: []string{"string"}, diff --git a/pkg/controller/master/image/backing_image_controller.go b/pkg/controller/master/image/backing_image_controller.go index 9e851c454c..36b22fd8a8 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")