rootfsmanager: Do pre-pull check correctly

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]>

src/rootfstreemanager.cc

@@ -7,7 +7,6 @@
 #include "http/httpclient.h"
 #include "ostree/repo.h"
 #include "storage/invstorage.h"
-#include "storage/stat.h"
 #include "target.h"
 
 RootfsTreeManager::Config::Config(const PackageConfig& pconfig) {
@@ -53,9 +52,7 @@ DownloadResult RootfsTreeManager::Download(const TufTarget& target) {
       setRemote(remote.name, remote.baseUrl, remote.keys);
     }
 
-    storage::Volume::UsageInfo pre_pull_usage_info{
-        storage::Volume::getUsageInfo(sysroot_->repoPath(), sysroot_->reservedStorageSpacePercentageDelta(),
-                                      OSTree::Sysroot::Config::ReservedStorageSpacePercentageDeltaParamName)};
+    storage::Volume::UsageInfo pre_pull_usage_info{getUsageInfo()};
     if (!pre_pull_usage_info.isOk()) {
       LOG_ERROR << "Failed to obtain storage usage statistic: " << pre_pull_usage_info.err;
     }
@@ -393,3 +390,15 @@ bool RootfsTreeManager::findDeltaStatForUpdate(const Json::Value& delta_stats, c
   found_delta_stat = {found_delta["size"].asUInt64(), found_delta["u_size"].asUInt64()};
   return true;
 }
+
+storage::Volume::UsageInfo RootfsTreeManager::getUsageInfo() const {
+  unsigned int reserved_percentage{sysroot_->reservedStorageSpacePercentageDelta()};
+  std::string reserved_by{OSTree::Sysroot::Config::ReservedStorageSpacePercentageDeltaParamName};
+
+  const unsigned int reserved_by_ostree{sysroot_->reservedStorageSpacePercentageOstree()};
+  if (reserved_percentage < reserved_by_ostree) {
+    reserved_percentage = reserved_by_ostree;
+    reserved_by = OSTree::Sysroot::Config::ReservedStorageSpacePercentageOstreeParamName;
+  }
+  return storage::Volume::getUsageInfo(sysroot_->repoPath(), reserved_percentage, reserved_by);
+}

src/rootfstreemanager.h

@@ -6,6 +6,7 @@
 #include "http/httpinterface.h"
 #include "ostree/sysroot.h"
 #include "package_manager/ostreemanager.h"
+#include "storage/stat.h"
 
 class RootfsTreeManager : public OstreeManager, public Downloader {
  public:
@@ -70,6 +71,7 @@ class RootfsTreeManager : public OstreeManager, public Downloader {
   void setRemote(const std::string& name, const std::string& url, const boost::optional<const KeyManager*>& keys);
   data::InstallationResult verifyBootloaderUpdate(const Uptane::Target& target) const;
   bool getDeltaStatIfAvailable(const TufTarget& target, const Remote& remote, DeltaStat& delta_stat) const;
+  storage::Volume::UsageInfo getUsageInfo() const;
 
   static bool getDeltaStatsRef(const Json::Value& json, DeltaStatsRef& ref);
   static Json::Value downloadDeltaStats(const DeltaStatsRef& ref, const Remote& remote);

tests/nospace_test.cc

@@ -308,7 +308,7 @@ TEST_F(NoSpaceTest, OstreeUpdateNoSpaceIfStaticDeltaStats) {
     ASSERT_TRUE(targetsMatch(client->getCurrent(), getInitialTarget()));
   }
   {
-    // required 11%, free 15%, reserved 5% (default) -> available 10% < 11%
+    // required 11%, free 15%, reserved 5% (default, by the delta knob) -> available 10% < 11%
     SetFreeBlockNumb(15, 100);
     const auto expected_available{((15 > OSTree::Sysroot::Config::DefaultReservedStorageSpacePercentageDelta))
                                       ? (15 - OSTree::Sysroot::Config::DefaultReservedStorageSpacePercentageDelta)
@@ -323,19 +323,29 @@ TEST_F(NoSpaceTest, OstreeUpdateNoSpaceIfStaticDeltaStats) {
     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(expected_msg.str())) << event_err_msg;
+    ASSERT_TRUE(std::string::npos !=
+                event_err_msg.find(OSTree::Sysroot::Config::ReservedStorageSpacePercentageDeltaParamName))
+        << event_err_msg;
     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");
+    // required 11%, free 16%, reserved 5% by the delta knob, reserved 6% by the ostree knob
+    // -> available 10% < 11%
+    SetFreeBlockNumb(16, 100);
+    sys_repo_.setMinFreeSpacePercent("6");
+    const auto expected_available{16 - 6};
+    storage::Volume::UsageInfo usage_info{.size = {100 * 4096, 100},
+                                          .available = {expected_available * 4096, expected_available}};
+    std::stringstream expected_msg;
+    expected_msg << "required: " << usage_info.withRequired(delta_size).required
+                 << ", available: " << usage_info.available;
     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"))
+    ASSERT_TRUE(std::string::npos != event_err_msg.find(expected_msg.str())) << event_err_msg;
+    ASSERT_TRUE(std::string::npos !=
+                event_err_msg.find(OSTree::Sysroot::Config::ReservedStorageSpacePercentageOstreeParamName))
         << event_err_msg;
     ASSERT_TRUE(targetsMatch(client->getCurrent(), getInitialTarget()));
   }