Expose virtual size of backing images (backport #2680) #2719
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![mergify[bot]](https://avatars.githubusercontent.com/in/10562?s=60&v=4){kind=small}
When doing local builds (i.e. without $DRONE_REPO and $DRONE_PULL_REQUEST,
or $DRONE_COMMIT_REF set), the build fails with "fatal: Needed a single
revision" in `scripts/validate`:
```
> make REPO=tserong
[...]
Running validation
Running: go vet
refs/heads/wip-add-virtualSize
fatal: Needed a single revision
FATA[0032] exit status 128
make: *** [Makefile:11: ci] Error 1
```
If I run the commands in `scripts/validate` by hand, I see this:
```
> git symbolic-ref -q HEAD && REV="origin/HEAD" || REV="HEAD^"
refs/heads/wip-add-virtualSize
> echo $REV
origin/HEAD
> headSHA=$(git rev-parse --short=12 ${REV})
fatal: Needed a single revision
```
I don't know if this is just something weird about my environment, but
if I change "origin/HEAD" to "HEAD", it works fine:
```
> headSHA=$(git rev-parse --short=12 HEAD)
> echo $headSHA
eb27fbf6e678
```
Signed-off-by: Tim Serong <tserong@suse.com>
(cherry picked from commit 9707ff0)
Signed-off-by: Tim Serong <tserong@suse.com> (cherry picked from commit ffe9a83) # Conflicts: # go.mod # go.sum # vendor/modules.txt
This commit adds virtualSize to the BackingImageStatus and BackingImageFileInfo structs, and thus to the BackingImage and BackingImageManager CRDs. We can see how this works in practice by creating a new backing image downloaded from a URL, then periodically running `kubectl -n longhorn-system get lhbi`. I'm using https://download.opensuse.org/distribution/leap/15.5/appliances/openSUSE-Leap-15.5-Minimal-VM.x86_64-Cloud.qcow2 in the example below. Initially, when the download is just started, before the file size is known: ``` > kubectl -n longhorn-system get lhbi/default-image-7mplj NAME UUID SOURCETYPE SIZE VIRTUALSIZE AGE default-image-7mplj 6d5a98b0 download 0 0 9s ``` A little later, while the download is running: ``` > kubectl -n longhorn-system get lhbi/default-image-7mplj NAME UUID SOURCETYPE SIZE VIRTUALSIZE AGE default-image-7mplj 6d5a98b0 download 255701504 0 3m33s ``` Finally, once the download is complete: ``` > kubectl -n longhorn-system get lhbi/default-image-7mplj NAME UUID SOURCETYPE SIZE VIRTUALSIZE AGE default-image-7mplj 6d5a98b0 download 255701504 821035008 4m26s ``` Compare size and virtualSize above with the image downloaded manually: ``` > wget https://download.opensuse.org/distribution/leap/15.5/appliances/openSUSE-Leap-15.5-Minimal-VM.x86_64-Cloud.qcow2 [...] > ls -l openSUSE-Leap-15.5-Minimal-VM.x86_64-Cloud.qcow2 -rw-r--r-- 1 tserong users 255701504 Dec 19 23:26 openSUSE-Leap-15.5-Minimal-VM.x86_64-Cloud.qcow2 > qemu-img info openSUSE-Leap-15.5-Minimal-VM.x86_64-Cloud.qcow2 | grep virtual virtual size: 783 MiB (821035008 bytes) ``` Related issue: longhorn/longhorn#7923 Signed-off-by: Tim Serong <tserong@suse.com> (cherry picked from commit d438e9b)
…atch Doing this means if there's somehow a mismatch between backing image size or virtual size reported by backing image manager vs. what's currently in the backing image resource's status, the error will be made visible in status.diskFileStatusMap.$DISKUUID.message, i.e. the user should be able to can see the problem by running `kubectl -n longhorn-system get lhbi -o yaml`. This is the same logic as is already used in updateStatusWithFileInfo(). The one thing I'm struggling with here is how to inject such a failure into a running system in order to prove that this change works correctly. Signed-off-by: Tim Serong <tserong@suse.com> (cherry picked from commit 6df0510)
|
Cherry-pick of ffe9a83 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
|
Close this since the conflict is resolved in #2723 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue(s) this PR fixes:
longhorn/longhorn#7923
What this PR does / why we need it:
This commit adds virtualSize to the BackingImageStatus and BackingImageFileInfo structs, and thus to the BackingImage and BackingImageManager CRDs. We can see how this works in practice by creating a new backing image downloaded from a URL, then periodically running
kubectl -n longhorn-system get lhbi. I'm using https://download.opensuse.org/distribution/leap/15.5/appliances/openSUSE-Leap-15.5-Minimal-VM.x86_64-Cloud.qcow2 in the example below.Initially, when the download is just started, before the file size is known:
A little later, while the download is running:
Finally, once the download is complete:
Compare size and virtualSize above with the image downloaded manually:
Special notes for your reviewer:
I've pulled in the latest backing-image-manager by updating go.mod to directly reference the latest commit from the master branch (longhorn/backing-image-manager@c5288d7). This can be seen as
github.com/longhorn/backing-image-manager v1.7.0-dev.0.20240326182459-c5288d745f4ain therequiresection in go.mod. I'm not sure if that's how this sort of thing is usually handled - would it be better to make a v 1.7.0-dev tag in backing-image-manager and reference that instead?Additional documentation or context
N/A
This is an automatic backport of pull request #2680 done by Mergify.