Unify and improve free space verification #266
Merged
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
Contributor
mike-sul
commented
Sep 4, 2023
mike-sul
commented
- Add functionality to obtain storage usage that can be used across all use-cases.
- Apply the new storage usage obtaining functionality to all ostree pull use-cases, with and without delta, negative and positive cases.
- Extend and improve the unit tests to verify the new and modified functionality.
- Change the meaning of the storage space "watermark" for delta to the same as the meaning of the storage space "watermark" for ostree objects. Now, both of the parameters indicate amount of reserved storage in percentage of the overall storage capacity. - Rename these two params according to their meaning, what they actually set/define. Use the term "reserved" instead of "min" or "watermark". - Adjust existing code and tests to the changes. Signed-off-by: Mike Sul <[email protected]>
Add functionality to get storage usage statistic and represent it in a nice and convenient way. It allows us using the same storage stat function across many use-cases. The functionality includes a way to identify storage reservation type. We have three knobs to control storage reservation: 1. The knob/param to set storage reservation for apps. The same value for both the skopeo and docker storage even if they are located on different volumes. 2. The knob to set `min-free-space-percent` parameter of the sysroot's ostree repo. This value is used by libostree during committing/adding an object to a repo. The check occurs per each object, just before adding it. 3. The knob to set the reserved space value that is used before pulling an ostree commit if static delta statistic is available. Therefore, we need a way to identify these three types of knobs. For example, it's useful to see in the storage usage statistic which of the knobs is applied in a given case. Signed-off-by: Mike Sul <[email protected]>
- Utilize the new storage usage functionality in `rootfsmanager`. - Remove the previously used storage usage functionality. - Use the path to the sysroot's ostree repo as the path to get storage usage statistic for. Signed-off-by: Mike Sul <[email protected]>
A size of a new ostree commit may differ on different hosts due to some randomness in some of additional files added to the sysroot filesystem. Therefore, the delta size between a current and a new ostree commits is equal <requested block number * block size> + <random size less than one block>. Therefore, to make the tests pass on any host we need to determine the delta size exactly and check that the corresponding numbers are present in the resultant update events. Signed-off-by: Mike Sul <[email protected]>
There are two knobs to configure the same thing - a reserved amount of storage capacity. One is used by libostree during ostree object committing to a repo, another one is intended for the delta-based pull use-case. If the "ostree" knob reserves more storage than the "delta" knob then it makes sense to take into account the "ostree" reservation during the pre-pull update size verification, not the "delta" one. Otherwise, libostree will yield an error during ostree pull once the storage usage reaches the reserved threshold. So, no point in starting an ostree pull if it will fail. Ideally, we need to get rid of the "delta" know and use just the "ostree" one. Signed-off-by: Mike Sul <[email protected]>
Get, print, and attach to the `EcuDownloadCompleted` event the storage usage info if an ostree pull is successful. Signed-off-by: Mike Sul <[email protected]>
- Print and add usage statistic to the "download complete" event if the "no space" error occurs during ostree pull (the pre-pull check is successful). - Correct corresponding tests. - Extend the tests to cover the new functionality. Signed-off-by: Mike Sul <[email protected]>
- Print and add storage usage statistic to the "download complete" event if a generic download error occurs, e.g. 404. - Add unit tests to make sure that the stat is added to the update event context. Signed-off-by: Mike Sul <[email protected]>
mike-sul
force-pushed
the
unify-and-improve-free-space-verification
branch
from
ca49515 to
37ee7c0
Compare
doanac
approved these changes
Sep 5, 2023
| ", err: " + std::strerror(errno); | ||
| } | ||
| stat.blockNumb = fs_stat.f_blocks; | ||
| stat.freeBlockNumber = (getuid() != 0 ? fs_stat.f_bavail : fs_stat.f_bfree); |
There was a problem hiding this comment.
is this for unit tests? Don't think we can run as non-root? maybe this could use a comment.
There was a problem hiding this comment.
Yes. This makes code user permission agnostic, actually I copy-pasted it from the libostree :).
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.