rfsmngr: Consider two knobs when checking free space

We have two knobs for the same parameter - reserved storage in the volume where the sysroot's ostree repo is hosted. One of the knobs is used by libostree during ostree pull. This change takes this knob value into account before pulling ostree commit if static delta statistic is available. Thus we don't start ostree pull if we know in advance that the "ostree" knob is going to be violate is going to be violated. Signed-off-by: Mike Sul <[email protected]>
foundriesio · Sep 1, 2023 · cda5dc8 · cda5dc8
1 parent 3bd3326
commit cda5dc8
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 9 deletions.
diff --git a/src/rootfstreemanager.cc b/src/rootfstreemanager.cc
@@ -35,8 +35,16 @@ std::tuple<bool, std::string> RootfsTreeManager::prePullCheck(const DeltaStat& d
   bool ok{true};
 
   try {
-    storage::Volume::getUsageInfo(sysroot_->repoPath(), sysroot_->reservedStorageSpacePercentageDelta(),
-                                  OSTree::Sysroot::Config::ReservedStorageSpacePercentageDeltaParamName, usage_info);
+    std::pair<unsigned int, std::string> free_space_reservation{
+        sysroot_->reservedStorageSpacePercentageDelta(),
+        OSTree::Sysroot::Config::ReservedStorageSpacePercentageDeltaParamName};
+    // Use the max of the two knobs for the available free storage verification.
+    if (free_space_reservation.first < sysroot_->reservedStorageSpacePercentageOstree()) {
+      free_space_reservation = {sysroot_->reservedStorageSpacePercentageOstree(),
+                                OSTree::Sysroot::Config::ReservedStorageSpacePercentageOstreeParamName};
+    }
+    storage::Volume::getUsageInfo(sysroot_->repoPath(), free_space_reservation.first, free_space_reservation.second,
+                                  usage_info);
     ok = usage_info.available.first >= delta_stat.uncompressedSize;
     msg << "required: " << usage_info.toRelativeType(delta_stat.uncompressedSize) << ", " << usage_info;
   } catch (const std::exception& exc) {

diff --git a/tests/nospace_test.cc b/tests/nospace_test.cc
@@ -310,17 +310,22 @@ TEST_F(NoSpaceTest, OstreeUpdateNoSpaceIfStaticDeltaStats) {
     ASSERT_TRUE(targetsMatch(client->getCurrent(), getInitialTarget()));
   }
   {
-    // (21 - 11 = 10) - 10% of blocks will be free, need 10% so, it's supposed to succeed.
-    // But, the commit function checks if it will be more than 15% of storage capacity free after commit.
-    // Obviously it's not since only 10% will be available.
-    SetFreeBlockNumb(21, 100);
-    sys_repo_.setMinFreeSpacePercent("15");
+    // 11% is required for update; 16% is free;
+    // 5% (default) is reserved by the "delta" knob;
+    // 6% (set in the test) is reserved by the "ostree" knob.
+    // 16% - 11% = 5% - the reservation made by the "ostree" knob is violated.
+    // We used to allow ostree pulling in this case and expecting the error from the libostree
+    // once it detects that the threshold (6%) is crossed.
+    // Now, if delta stats are available, we are more pro-active and don't allow ostree pull start
+    // if it violates the max of the two knobs.
+    SetFreeBlockNumb(16, 100);
+    sys_repo_.setMinFreeSpacePercent("6");
     update(*client, getInitialTarget(), new_target, data::ResultCode::Numeric::kDownloadFailed,
            {DownloadResult::Status::DownloadFailed_NoSpace, "Insufficient storage available"});
     const auto events{device_gateway_.getEvents()};
     const std::string event_err_msg{events[events.size() - 1]["event"]["details"].asString()};
-    ASSERT_TRUE(std::string::npos != event_err_msg.find("opcode close: min-free-space-percent '15%' would be exceeded"))
-        << event_err_msg;
+    ASSERT_TRUE(std::string::npos != event_err_msg.find("required: 42397B 11%")) << event_err_msg;
+    ASSERT_TRUE(std::string::npos != event_err_msg.find("available: 40960B 10%")) << event_err_msg;
     ASSERT_TRUE(targetsMatch(client->getCurrent(), getInitialTarget()));
   }
   {