Closed haytok closed 6 months ago
Hi, @vsiravar
Thanks for comments !!! I have added e2e test for this fix, so could you please review when you have time ?
Hi, @vsiravar
Thanks for comments !!! The following parts have been corrected in accordance with the review.
Could you review this fix when you have time ?
Thanks @haytok , the changes look good. Just had one clarifying question that I put in the comment.
@vsiravar Thanks for comments. I have added comments, so could you check when you have time?
Hi, Team
My response to this comments seem invisible, so add my comments.
will users lose their persistent data if they have to resize their disk?
Yes, users will lose their persistent data when they have to resize their disk using finchDiskSize
.
In the current implementation, finchDiskSize
will not take effect unless users delete the persistent disk (~/.finch/.disks
) and execute finch vm init
.
What is the resolution for this? Can this be run without resetDisks with a test that checks the containers prior to resize are available after resize. As mentioned in the ticket ".After I update the value, stop the VM, and then start the VM, the value should take effect."
No, the current implementation requires that there be no persistent disks in order to test whether the disk of the finchDiskSize
value is created. Therefore, resetDisks(o, installed)
need to be executed before running this test.
This is because the value of finchDiskSize
is effective only when the persistent disk dose not exit and finch vm init
has been executed.
Therefore, at this time, after updating finchDiskSize
in ~/.finch/finch.yaml
and running finch vm start
, the allocated size for finch dose not change.
This behavior can be seen in the following steps.
```bash haytok finch > cat ~/.finch/finch.yaml | grep finchDiskSize finchDiskSize: 3GiB haytok finch > _output/bin/finch vm init INFO[0000] Initializing and starting Finch virtual machine... INFO[0052] Finch virtual machine started successfully haytok finch > LIMA_HOME=/Users/haytok/finch/_output/lima/data/ /Users/haytok/finch/_output/lima/bin/limactl shell finch df -h Filesystem Size Used Avail Use% Mounted on ... /dev/vdb1 2.9G 136K 2.8G 1% /mnt/lima-finch ... haytok finch > _output/bin/finch vm stop INFO[0000] Stopping existing Finch virtual machine... INFO[0004] Finch virtual machine stopped successfully # updated finchDiskSize in finch.yaml haytok finch > cat ~/.finch/finch.yaml | grep finchDiskSize finchDiskSize: 4GiB # The persistent data has not been deleted. haytok finch > _output/bin/finch vm start INFO[0000] Starting existing Finch virtual machine... INFO[0041] Finch virtual machine started successfully haytok finch > LIMA_HOME=/Users/haytok/finch/_output/lima/data/ /Users/haytok/finch/_output/lima/bin/limactl shell finch df -h Filesystem Size Used Avail Use% Mounted on ... /dev/vdb1 2.9G 136K 2.8G 1% /mnt/lima-finch ... ```
The size of the allocated space for finch dose not change because of below implementations.
As described below, when disk exists in ~/.finch/.disks
and finchDiskSize
has changed, the size for finch dose not resize because disk dose not recreated.
haytok finch
> LIMA_HOME=/Users/haytok/finch/_output/lima/data/ /Users/haytok/finch/_output/lima/bin/limactl disk ls
NAME SIZE DIR IN-USE-BY
finch 3GiB /Users/haytok/finch/_output/lima/data/_disks/finch finch
In the current implementation, when we update finchDiskSize
and then finch vm stop
and finch vm start
, we need to delete ~/.finch/.disks
to change the disk size for finch.
Or, when the disk exists in ~/.finch/.disks
, we need to add a feature to resize the disk size on Lima side as described in the following issue, and use the feature when running finch vm init
and finch vm start
. However, the following feature has not been implemented at this time.
From the above, in the implementation at this time, the size allocated for finch space dose not change when finchDiskSize
has been updated and, finch vm stop
and finch vm start
. Therefore, in order to pass this test, resetDisks(o, installed)
have to be run before finch vm init
.
I think it is useful to set the size allocated for finch when running finch vm init
, but this PR dose not fully resolve issue#275, so I'll close this PR if the maintainers determine that this PR is no longer needed. I'll add the feature to resize disk size for finch on the Lima size and recreate the PR to resolve issue#275.
@haytok Closing this pr as per above communications. Feel free to re-open it if you needs to reviewed again.
In the current implementation, the disk space allocated for finch is hard-coded with a fixed value of 50G. Therefore, when users pull large docker images and use more than 50G of the allocated finch space, "no space left on device" error may occur.
As mentioned above, the disk size available for finch is fixed at 50GB and cannot be set by users. To work around this setting, they need to rewrite the hard-coded parts and use their own build of finch, but this is very time-consuming.
Alternatively, they can use the following method to change the disk size.
Thus, although there is some workarounds to adjust the disk size for finch, users will be able to use finch more flexibly if they are able to specify the disk size that finch can use at the time of vm init.
Therefore, this fix improves usability by allowing users to run "finch vm init" and flexibly set the size of the disk created for finch based on the finchDiskSize in ~/.finch/finch.yaml.
Note that for finch at present, the size of finchDiskSize becomes the disk size of the finch area only when the vm is initialized ("finch vm init").
Issue #, if available: No
Description of changes: Detail are described in this commit message.
Testing done: Yes
License Acceptance
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.