client: Use stat to decide whether do pull attempt

- Extend the download result context with storage usage info and free space required by an ostree update. - Use the storage usage info and the required value obtained in the "no space" download result to decide whether to do a pull attempt again or not. If there is still not enough free storage available to fit the given ostree update then skip an update attempt. Signed-off-by: Mike Sul <[email protected]>
foundriesio · Sep 4, 2023 · e8a8885 · e8a8885
1 parent 37ee7c0
commit e8a8885
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 44 deletions.
diff --git a/src/composeappmanager.cc b/src/composeappmanager.cc
@@ -242,13 +242,13 @@ bool ComposeAppManager::checkForAppsToUpdate(const Uptane::Target& target) {
   return cur_apps_to_fetch_and_update_.empty() && cur_apps_to_fetch_.empty();
 }
 
-DownloadResult ComposeAppManager::Download(const TufTarget& target) {
+DownloadResultWithStat ComposeAppManager::Download(const TufTarget& target) {
   auto ostree_download_res{RootfsTreeManager::Download(target)};
   if (!ostree_download_res) {
     return ostree_download_res;
   }
 
-  DownloadResult res{ostree_download_res};
+  DownloadResultWithStat res{ostree_download_res};
   const Uptane::Target uptane_target{Target::fromTufTarget(target)};
 
   if (cfg_.force_update) {

diff --git a/src/composeappmanager.h b/src/composeappmanager.h
@@ -43,7 +43,7 @@ class ComposeAppManager : public RootfsTreeManager {
                     AppEngine::Ptr app_engine = nullptr);
 
   std::string name() const override { return Name; }
-  DownloadResult Download(const TufTarget& target) override;
+  DownloadResultWithStat Download(const TufTarget& target) override;
   bool fetchTarget(const Uptane::Target& target, Uptane::Fetcher& fetcher, const KeyManager& keys,
                    const FetcherProgressCb& progress_cb, const api::FlowControlToken* token) override;
 

diff --git a/src/downloader.h b/src/downloader.h
@@ -2,10 +2,16 @@
 #define AKTUALIZR_LITE_DOWNLOADER_H_
 
 #include "aktualizr-lite/api.h"
+#include "storage/stat.h"
+
+class DownloadResultWithStat : public DownloadResult {
+ public:
+  storage::Volume::UsageInfo stat;
+};
 
 class Downloader {
  public:
-  virtual DownloadResult Download(const TufTarget& target) = 0;
+  virtual DownloadResultWithStat Download(const TufTarget& target) = 0;
 
   virtual ~Downloader() = default;
   Downloader(const Downloader&) = delete;

diff --git a/src/liteclient.cc b/src/liteclient.cc
@@ -513,10 +513,10 @@ bool LiteClient::checkImageMetaOffline() {
   return true;
 }
 
-DownloadResult LiteClient::downloadImage(const Uptane::Target& target, const api::FlowControlToken* token) {
+DownloadResultWithStat LiteClient::downloadImage(const Uptane::Target& target, const api::FlowControlToken* token) {
   key_manager_->loadKeys();
 
-  DownloadResult download_result{DownloadResult::Status::DownloadFailed, ""};
+  DownloadResultWithStat download_result{DownloadResult::Status::DownloadFailed, ""};
   {
     const int max_tries = 3;
     int tries = 0;
@@ -632,7 +632,7 @@ void LiteClient::reportAppsState() {
   }
 }
 
-DownloadResult LiteClient::download(const Uptane::Target& target, const std::string& reason) {
+DownloadResultWithStat LiteClient::download(const Uptane::Target& target, const std::string& reason) {
   notifyDownloadStarted(target, reason);
   auto download_result{downloadImage(target)};
   notifyDownloadFinished(target, download_result, download_result.description);

diff --git a/src/liteclient.h b/src/liteclient.h
@@ -1,6 +1,7 @@
 #ifndef AKTUALIZR_LITE_CLIENT_H_
 #define AKTUALIZR_LITE_CLIENT_H_
 
+#include "downloader.h"
 #include "gtest/gtest_prod.h"
 #include "libaktualizr/config.h"
 #include "libaktualizr/packagemanagerinterface.h"
@@ -41,7 +42,7 @@ class LiteClient {
   void checkForUpdatesEndWithFailure(const std::string& err);
   bool finalizeInstall(data::InstallationResult* ir = nullptr);
   Uptane::Target getRollbackTarget();
-  DownloadResult download(const Uptane::Target& target, const std::string& reason);
+  DownloadResultWithStat download(const Uptane::Target& target, const std::string& reason);
   data::ResultCode::Numeric install(const Uptane::Target& target);
   void notifyInstallFinished(const Uptane::Target& t, data::InstallationResult& ir);
   std::pair<bool, std::string> isRebootRequired() const {
@@ -91,7 +92,7 @@ class LiteClient {
   void writeCurrentTarget(const Uptane::Target& t) const;
 
   data::InstallationResult installPackage(const Uptane::Target& target);
-  DownloadResult downloadImage(const Uptane::Target& target, const api::FlowControlToken* token = nullptr);
+  DownloadResultWithStat downloadImage(const Uptane::Target& target, const api::FlowControlToken* token = nullptr);
   static void add_apps_header(std::vector<std::string>& headers, PackageConfig& config);
   data::InstallationResult finalizePendingUpdate(boost::optional<Uptane::Target>& target);
   void initRequestHeaders(std::vector<std::string>& headers) const;

diff --git a/src/main.cc b/src/main.cc
@@ -178,9 +178,9 @@ static std::pair<bool, std::unique_ptr<Uptane::Target>> find_target(LiteClient&
   return {false, std_::make_unique<Uptane::Target>(Uptane::Target::Unknown())};
 }
 
-static std::tuple<data::ResultCode::Numeric, DownloadResult, std::string> do_update(LiteClient& client,
-                                                                                    Uptane::Target target,
-                                                                                    const std::string& reason) {
+static std::tuple<data::ResultCode::Numeric, DownloadResultWithStat, std::string> do_update(LiteClient& client,
+                                                                                            Uptane::Target target,
+                                                                                            const std::string& reason) {
   client.logTarget("Updating Active Target: ", client.getCurrent());
   client.logTarget("To New Target: ", target);
 
@@ -198,7 +198,7 @@ static std::tuple<data::ResultCode::Numeric, DownloadResult, std::string> do_upd
     return {res.result_code.num_code, download_res, target.correlation_id()};
   }
 
-  return {client.install(target), {DownloadResult::Status::Ok}, target.correlation_id()};
+  return {client.install(target), download_res, target.correlation_id()};
 }
 
 static data::ResultCode::Numeric do_app_sync(LiteClient& client) {
@@ -245,7 +245,7 @@ static int update_main(LiteClient& client, const bpo::variables_map& variables_m
   if (find_target_res.first) {
     std::string reason = "Manual update to " + version;
     data::ResultCode::Numeric rc;
-    DownloadResult dr;
+    DownloadResultWithStat dr;
     std::string cor_id;
     std::tie(rc, dr, cor_id) = do_update(client, *find_target_res.second, reason);
 
@@ -258,12 +258,6 @@ static int update_main(LiteClient& client, const bpo::variables_map& variables_m
   }
 }
 
-static std::tuple<boost::uintmax_t, bool> get_available_space(const std::string& path) {
-  boost::system::error_code ec;
-  const boost::filesystem::space_info store_info{boost::filesystem::space(path, ec)};
-  return {ec.failed() ? 0 : store_info.available, !ec.failed()};
-}
-
 static int daemon_main(LiteClient& client, const bpo::variables_map& variables_map) {
   FileLock lock;
   if (client.config.uptane.repo_server.empty()) {
@@ -290,10 +284,9 @@ static int daemon_main(LiteClient& client, const bpo::variables_map& variables_m
 
   struct NoSpaceDownloadState {
     Hash ostree_commit_hash;
-    boost::uintmax_t free_space;
-    std::string dst_path;
     std::string cor_id;
-  } state_when_download_failed{Hash{"", ""}, 0, "", ""};
+    storage::Volume::UsageInfo stat;
+  } state_when_download_failed{Hash{"", ""}, 0, "", {}};
 
   while (true) {
     LOG_INFO << "Active Target: " << current.filename() << ", sha256: " << current.sha256Hash();
@@ -376,15 +369,20 @@ static int daemon_main(LiteClient& client, const bpo::variables_map& variables_m
         // New Target is available, try to update a device with it.
         // But prior to performing the update, check if aklite haven't tried to fetch the target ostree before,
         // and it failed due to lack of space, and the space has not increased since that time.
-        if (state_when_download_failed.free_space != 0 &&
+        if (state_when_download_failed.stat.required.first > 0 && state_when_download_failed.stat.isOk() &&
             target_to_install.MatchHash(state_when_download_failed.ostree_commit_hash)) {
-          const auto available_space_res{get_available_space(state_when_download_failed.dst_path)};
-          const auto required_space{state_when_download_failed.free_space + 1024 /* at least 1K should be freed */};
-          if (std::get<1>(available_space_res) && std::get<0>(available_space_res) < required_space) {
-            const auto err_msg{boost::str(
-                boost::format("No space to download Target's ostree; hash: %s, required: %d, available: %d") %
-                target_to_install.sha256Hash() % required_space % std::get<0>(available_space_res))};
+          storage::Volume::UsageInfo current_usage_info{storage::Volume::getUsageInfo(
+              state_when_download_failed.stat.path, state_when_download_failed.stat.reserved.second,
+              state_when_download_failed.stat.reserved_by)};
+          if (!current_usage_info.isOk()) {
+            LOG_ERROR << "Failed to obtain storage usage statistic: " << current_usage_info.err;
+          }
 
+          if (current_usage_info.isOk() &&
+              current_usage_info.available.first < state_when_download_failed.stat.required.first) {
+            const std::string err_msg{
+                "Insufficient storage available to download Target's ostree; hash: " + target_to_install.sha256Hash() +
+                ", " + current_usage_info.withRequired(state_when_download_failed.stat.required.first).str()};
             LOG_ERROR << err_msg;
             target_to_install.setCorrelationId(state_when_download_failed.cor_id);
             client.notifyDownloadFinished(target_to_install, false, err_msg);
@@ -394,12 +392,12 @@ static int daemon_main(LiteClient& client, const bpo::variables_map& variables_m
           }
         }
 
-        state_when_download_failed = {Hash{"", ""}, 0, "", ""};
+        state_when_download_failed = {Hash{"", ""}, "", {}};
         std::string reason = std::string(rollback ? "Rolling back" : "Updating") + " from " + current.filename() +
                              " to " + target_to_install.filename();
 
         data::ResultCode::Numeric rc;
-        DownloadResult dr;
+        DownloadResultWithStat dr;
         std::string cor_id;
         std::tie(rc, dr, cor_id) = do_update(client, target_to_install, reason);
         if (rc == data::ResultCode::Numeric::kOk) {
@@ -418,12 +416,8 @@ static int daemon_main(LiteClient& client, const bpo::variables_map& variables_m
           // changes in the latest failing Target.
           client.setAppsNotChecked();
           continue;
-        } else if (rc == data::ResultCode::Numeric::kDownloadFailed && !dr.destination_path.empty()) {
-          const auto available_space_res{get_available_space(dr.destination_path)};
-          if (std::get<1>(available_space_res)) {
-            state_when_download_failed = {Hash{Hash::Type::kSha256, target_to_install.sha256Hash()},
-                                          std::get<0>(available_space_res), dr.destination_path, cor_id};
-          }
+        } else if (rc == data::ResultCode::Numeric::kDownloadFailed && dr.noSpace()) {
+          state_when_download_failed = {Hash{Hash::Type::kSha256, target_to_install.sha256Hash()}, cor_id, dr.stat};
         }
 
       } else {

diff --git a/src/rootfstreemanager.cc b/src/rootfstreemanager.cc
@@ -28,7 +28,7 @@ RootfsTreeManager::RootfsTreeManager(const PackageConfig& pconfig, const Bootloa
       keys_{keys},
       cfg_{pconfig} {}
 
-DownloadResult RootfsTreeManager::Download(const TufTarget& target) {
+DownloadResultWithStat RootfsTreeManager::Download(const TufTarget& target) {
   auto prog_cb = [this](const Uptane::Target& t, const std::string& description, unsigned int progress) {
     // report_progress_cb(events_channel.get(), t, description, progress);
     // TODO: consider make use of it for download progress reporting
@@ -44,7 +44,7 @@ DownloadResult RootfsTreeManager::Download(const TufTarget& target) {
     getAdditionalRemotes(remotes, target.Name());
   }
 
-  DownloadResult res{DownloadResult::Status::Ok, ""};
+  DownloadResultWithStat res{DownloadResult::Status::Ok, ""};
   data::InstallationResult pull_err{data::ResultCode::Numeric::kUnknown, ""};
   std::string error_desc;
   for (const auto& remote : remotes) {
@@ -67,7 +67,7 @@ DownloadResult RootfsTreeManager::Download(const TufTarget& target) {
           return {
               DownloadResult::Status::DownloadFailed_NoSpace,
               "Insufficient storage available; " + pre_pull_usage_info.withRequired(delta_stat.uncompressedSize).str(),
-              sysroot_->repoPath()};
+              sysroot_->repoPath(), pre_pull_usage_info};
         }
         LOG_INFO << "Sufficient free storage available; "
                  << pre_pull_usage_info.withRequired(delta_stat.uncompressedSize);
@@ -111,7 +111,11 @@ DownloadResult RootfsTreeManager::Download(const TufTarget& target) {
       res = {DownloadResult::Status::DownloadFailed_NoSpace,
              "Insufficient storage available; " + pull_err.description + "\nbefore ostree pull; " +
                  pre_pull_usage_info.str() + "\nafter ostree pull; " + post_pull_usage_info.str(),
-             sysroot_->repoPath()};
+             sysroot_->repoPath(),
+             delta_stat_avail
+                 ? post_pull_usage_info.withRequired(delta_stat.uncompressedSize)
+                 : post_pull_usage_info.withRequired(
+                       4096 * 10)} /* we don't know how much more storage is needed, so just require 10 blocks */;
       break;
     }
     error_desc += pull_err.description + "\nbefore ostree pull; " + pre_pull_usage_info.str() +

diff --git a/src/rootfstreemanager.h b/src/rootfstreemanager.h
@@ -34,7 +34,7 @@ class RootfsTreeManager : public OstreeManager, public Downloader {
                     const std::shared_ptr<INvStorage>& storage, const std::shared_ptr<HttpInterface>& http,
                     std::shared_ptr<OSTree::Sysroot> sysroot, const KeyManager& keys);
 
-  DownloadResult Download(const TufTarget& target) override;
+  DownloadResultWithStat Download(const TufTarget& target) override;
 
   bool fetchTarget(const Uptane::Target& target, Uptane::Fetcher& fetcher, const KeyManager& keys,
                    const FetcherProgressCb& progress_cb, const api::FlowControlToken* token) override;

diff --git a/src/storage/stat.h b/src/storage/stat.h
@@ -20,7 +20,7 @@ struct Volume {
 
     Type required;
 
-    std::string err;
+    std::string err{"undefined"};
     bool isOk() const { return err.empty(); }
 
     UsageInfo& withRequired(const uint64_t& val);