restorableappengine: Use new storage usage func

- Apply the new functionality for obtaining storage usage statistic to the restorable apps - the check if there is enough free storage before pulling App. - This fixes the math issue in the previous usage function used by the pre-pull check. The previous math applied the "watermark" (max allowed storage space that can be utilized by apps) to the free space not to the all storage capacity. For example, if there is already less than <capacity> - <capacity*watermark (0.8)> = <reserved storage> of free storage the math would allow update as long as <free space> * 0.8 is enough to fit App. - Adjust tests to the new math. - One of the tests had dos/ms line ending, so the whole file was changed. Signed-off-by: Mike Sul <[email protected]>
foundriesio · Sep 6, 2023 · e600fb0 · e600fb0
1 parent e3e1d8e
commit e600fb0
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 94 deletions.
diff --git a/src/docker/restorableappengine.cc b/src/docker/restorableappengine.cc
@@ -42,16 +42,12 @@ RestorableAppEngine::StorageSpaceFunc RestorableAppEngine::GetDefStorageSpaceFun
   }
 
   return [watermark](const boost::filesystem::path& path) {
-    boost::system::error_code ec;
-    const boost::filesystem::space_info store_info{boost::filesystem::space(path, ec)};
-    if (ec.failed()) {
-      throw std::runtime_error("Failed to get an available storage size: " + ec.message());
+    storage::Volume::UsageInfo usage_info{storage::Volume::getUsageInfo(
+        path.string(), (100 > watermark) ? static_cast<unsigned int>(100 - watermark) : 0, "pacman:storage_watermark")};
+    if (!usage_info.isOk()) {
+      LOG_ERROR << "Failed to obtain storage usage statistic: " << usage_info.err;
     }
-
-    boost::uintmax_t allowed_for_usage{static_cast<boost::uintmax_t>((float(watermark) / 100) * store_info.available)};
-    return std::tuple<boost::uintmax_t, boost::uintmax_t>(
-        store_info.available /* overall available */,
-        allowed_for_usage /* available for usage taking into account a highe level watermark*/);
+    return usage_info;
   };
 }
 
@@ -963,15 +959,12 @@ void RestorableAppEngine::checkAvailableStorageInStores(const std::string& app_n
                                                         const uint64_t& docker_required_storage) const {
   auto checkRoomInStore = [&](const std::string& store_name, const uint64_t& required_storage,
                               const boost::filesystem::path& store_path) {
-    boost::uintmax_t capacity;
-    boost::uintmax_t available;
-
-    std::tie(capacity, available) = storage_space_func_(store_path);
+    storage::Volume::UsageInfo usage_info{storage_space_func_(store_path)};
     LOG_INFO << app_name << " -> " << store_name << " store total update size: " << required_storage
-             << " bytes; available: " << available << " (out of " << capacity << ") "
+             << " bytes; available: " << usage_info.available << " (out of " << usage_info.size << ") "
              << ", path: " << store_path.string();
-    if (required_storage > available) {
-      throw InsufficientSpaceError(store_name, store_path.string(), required_storage, available);
+    if (required_storage > usage_info.available.first) {
+      throw InsufficientSpaceError(store_name, store_path.string(), required_storage, usage_info.available.first);
     }
   };
 

diff --git a/src/docker/restorableappengine.h b/src/docker/restorableappengine.h
@@ -7,6 +7,7 @@
 
 #include "docker/docker.h"
 #include "docker/dockerclient.h"
+#include "storage/stat.h"
 
 namespace Docker {
 
@@ -74,8 +75,7 @@ namespace Docker {
 class RestorableAppEngine : public AppEngine {
  public:
   static const std::string ComposeFile;
-  using StorageSpaceFunc =
-      std::function<std::tuple<boost::uintmax_t, boost::uintmax_t>(const boost::filesystem::path&)>;
+  using StorageSpaceFunc = std::function<storage::Volume::UsageInfo(const boost::filesystem::path&)>;
   using ClientImageSrcFunc = std::function<std::string(const Docker::Uri&, const std::string&)>;
 
   static const int LowWatermarkLimit{20};

diff --git a/tests/fixtures/aklitetest.cc b/tests/fixtures/aklitetest.cc
@@ -1,54 +1,46 @@
-#include "fixtures/composeappenginetest.cc"
-#include "fixtures/liteclienttest.cc"
-
-class AkliteTest : public fixtures::ClientTest,
-                   public fixtures::AppEngineTest,
-                   public ::testing::WithParamInterface<std::string> {
- protected:
-  void SetUp() override {
-    fixtures::AppEngineTest::SetUp();
-
-    const auto app_engine_type{GetParam()};
-
-    if (app_engine_type == "ComposeAppEngine") {
-      app_engine =
-          std::make_shared<Docker::ComposeAppEngine>(apps_root_dir, compose_cmd, docker_client_, registry_client_);
-    } else if (app_engine_type == "RestorableAppEngine") {
-      setAvailableStorageSpace(std::numeric_limits<boost::uintmax_t>::max());
-      app_engine = std::make_shared<Docker::RestorableAppEngine>(
-          fixtures::ClientTest::test_dir_.Path() / "apps-store", apps_root_dir, daemon_.dataRoot(), registry_client_,
-          docker_client_, registry.getSkopeoClient(), daemon_.getUrl(), compose_cmd,
-          [this](const boost::filesystem::path& path) {
-            return std::tuple<boost::uintmax_t, boost::uintmax_t>{this->available_storage_space_,
-                                                                  (this->watermark_ * this->available_storage_space_)};
-          });
-    } else {
-      throw std::invalid_argument("Unsupported AppEngine type: " + app_engine_type);
-    }
-  }
-
-  std::shared_ptr<LiteClient> createLiteClient(InitialVersion initial_version = InitialVersion::kOn,
-                                               boost::optional<std::vector<std::string>> apps = boost::none,
-                                               bool finalize = true) override {
-    const auto app_engine_type{GetParam()};
-
-    if (app_engine_type == "ComposeAppEngine") {
-      return ClientTest::createLiteClient(app_engine, initial_version, apps, apps_root_dir.string(), boost::none,
-                                          create_containers_before_reboot_, finalize);
-    } else if (app_engine_type == "RestorableAppEngine") {
-      return ClientTest::createLiteClient(app_engine, initial_version, apps, apps_root_dir.string(),
-                                          !!apps ? apps : std::vector<std::string>{""},
-                                          create_containers_before_reboot_, finalize);
-    } else {
-      throw std::invalid_argument("Unsupported AppEngine type: " + app_engine_type);
-    }
-  }
-
-  void setAvailableStorageSpace(const boost::uintmax_t& space_size) { available_storage_space_ = space_size; }
-  void setCreateContainersBeforeReboot(bool value) { create_containers_before_reboot_ = value; }
-
- private:
-  boost::uintmax_t available_storage_space_;
-  float watermark_{0.8};
-  bool create_containers_before_reboot_{true};
-};
+#include "fixtures/composeappenginetest.cc"
+#include "fixtures/liteclienttest.cc"
+
+class AkliteTest : public fixtures::ClientTest,
+                   public fixtures::AppEngineTest,
+                   public ::testing::WithParamInterface<std::string> {
+ protected:
+  void SetUp() override {
+    fixtures::AppEngineTest::SetUp();
+
+    const auto app_engine_type{GetParam()};
+
+    if (app_engine_type == "ComposeAppEngine") {
+      app_engine =
+          std::make_shared<Docker::ComposeAppEngine>(apps_root_dir, compose_cmd, docker_client_, registry_client_);
+    } else if (app_engine_type == "RestorableAppEngine") {
+      app_engine = std::make_shared<Docker::RestorableAppEngine>(
+          fixtures::ClientTest::test_dir_.Path() / "apps-store", apps_root_dir, daemon_.dataRoot(), registry_client_,
+          docker_client_, registry.getSkopeoClient(), daemon_.getUrl(), compose_cmd, getTestStorageSpaceFunc());
+    } else {
+      throw std::invalid_argument("Unsupported AppEngine type: " + app_engine_type);
+    }
+  }
+
+  std::shared_ptr<LiteClient> createLiteClient(InitialVersion initial_version = InitialVersion::kOn,
+                                               boost::optional<std::vector<std::string>> apps = boost::none,
+                                               bool finalize = true) override {
+    const auto app_engine_type{GetParam()};
+
+    if (app_engine_type == "ComposeAppEngine") {
+      return ClientTest::createLiteClient(app_engine, initial_version, apps, apps_root_dir.string(), boost::none,
+                                          create_containers_before_reboot_, finalize);
+    } else if (app_engine_type == "RestorableAppEngine") {
+      return ClientTest::createLiteClient(app_engine, initial_version, apps, apps_root_dir.string(),
+                                          !!apps ? apps : std::vector<std::string>{""},
+                                          create_containers_before_reboot_, finalize);
+    } else {
+      throw std::invalid_argument("Unsupported AppEngine type: " + app_engine_type);
+    }
+  }
+
+  void setCreateContainersBeforeReboot(bool value) { create_containers_before_reboot_ = value; }
+
+ private:
+  bool create_containers_before_reboot_{true};
+};
diff --git a/tests/fixtures/composeappenginetest.cc b/tests/fixtures/composeappenginetest.cc
@@ -2,14 +2,16 @@
 #include "fixtures/composeapp.cc"
 #include "fixtures/dockerdaemon.cc"
 #include "fixtures/dockerregistry.cc"
+#include "docker/restorableappengine.h"
 
 
 namespace fixtures {
 
 
 class AppEngineTest : virtual public ::testing::Test {
  protected:
-  AppEngineTest() : registry{test_dir_.Path() / "registry"}, daemon_{test_dir_.Path() / "daemon"} {}
+  AppEngineTest() : registry{test_dir_.Path() / "registry"}, daemon_{test_dir_.Path() / "daemon"},
+                    available_storage_space_{std::numeric_limits<boost::uintmax_t>::max()} {}
 
   void SetUp() override {
     auto env{boost::this_process::environment()};
@@ -22,6 +24,28 @@ class AppEngineTest : virtual public ::testing::Test {
     docker_client_ = std::make_shared<Docker::DockerClient>(daemon_.getClient());
   }
 
+  void setAvailableStorageSpace(const boost::uintmax_t& space_size) {
+    available_storage_space_ = space_size/this->watermark_;
+  }
+  void setAvailableStorageSpaceWithoutWatermark(const boost::uintmax_t& space_size) {
+    available_storage_space_ = space_size;
+  }
+
+  Docker::RestorableAppEngine::StorageSpaceFunc getTestStorageSpaceFunc() {
+    return [this](const boost::filesystem::path& path) {
+      // this->available_storage_space_ * this->watermark_ is available in the new math
+      // let's assume that storage size is twice as the amount of free storage for sake of simplicity
+      auto avail{this->available_storage_space_ * this->watermark_};
+      return storage::Volume::UsageInfo{
+          .path = path.string(),
+          .size = {this->available_storage_space_ * 2, 100},
+          .free = {this->available_storage_space_, 50},
+          .reserved = {this->available_storage_space_ - avail, 50 * (1 - this->watermark_)},
+          .available = {avail, 50 * this->watermark_},
+      };
+    };
+  }
+
  protected:
   TemporaryDirectory test_dir_;
   fixtures::DockerRegistry registry;
@@ -32,6 +56,8 @@ class AppEngineTest : virtual public ::testing::Test {
   std::string compose_cmd;
   boost::filesystem::path apps_root_dir;
   std::shared_ptr<AppEngine> app_engine;
+  boost::uintmax_t available_storage_space_;
+  float watermark_{0.8};
 
  private:
   std::shared_ptr<HttpInterface> http_client_;

diff --git a/tests/restorableappengine_test.cc b/tests/restorableappengine_test.cc
@@ -17,19 +17,14 @@
 
 class RestorableAppEngineTest : public fixtures::AppEngineTest {
  protected:
-  RestorableAppEngineTest()
-      : AppEngineTest(),
-        skopeo_store_root_{test_dir_ / "apps-store"},
-        available_storage_space_{std::numeric_limits<boost::uintmax_t>::max()} {}
+  RestorableAppEngineTest() : AppEngineTest(), skopeo_store_root_{test_dir_ / "apps-store"} {}
+
   void SetUp() override {
     fixtures::AppEngineTest::SetUp();
 
     app_engine = std::make_shared<Docker::RestorableAppEngine>(
         skopeo_store_root_, apps_root_dir, daemon_.dataRoot(), registry_client_, docker_client_,
-        registry.getSkopeoClient(), daemon_.getUrl(), compose_cmd, [this](const boost::filesystem::path& path) {
-          return std::tuple<boost::uintmax_t, boost::uintmax_t>{this->available_storage_space_,
-                                                                (this->watermark_ * this->available_storage_space_)};
-        });
+        registry.getSkopeoClient(), daemon_.getUrl(), compose_cmd, getTestStorageSpaceFunc());
   }
 
   void removeAppManifest(const AppEngine::App& app) {
@@ -55,19 +50,10 @@ class RestorableAppEngineTest : public fixtures::AppEngineTest {
         app_dir / (Docker::HashedDigest(manifest.archiveDigest()).hash() + Docker::Manifest::ArchiveExt)};
     Utils::writeFile(archive_full_path, std::string("foo bar"));
   }
-
-  void setAvailableStorageSpace(const boost::uintmax_t& space_size) {
-    available_storage_space_ = space_size / watermark_;
-  }
-  void setAvailableStorageSpaceWithoutWatermark(const boost::uintmax_t& space_size) {
-    available_storage_space_ = space_size;
-  }
   const boost::filesystem::path& storeRoot() const { return skopeo_store_root_; }
 
  protected:
   const boost::filesystem::path skopeo_store_root_;
-  boost::uintmax_t available_storage_space_;
-  float watermark_{0.8};
 };
 
 class RestorableAppEngineTestParameterized : public RestorableAppEngineTest,
@@ -80,10 +66,7 @@ class RestorableAppEngineTestParameterized : public RestorableAppEngineTest,
     app_engine = std::make_shared<Docker::RestorableAppEngine>(
         skopeo_store_root_, apps_root_dir, docker_data_root_path.empty() ? daemon_.dataRoot() : docker_data_root_path,
         registry_client_, docker_client_, registry.getSkopeoClient(), daemon_.getUrl(), compose_cmd,
-        [this](const boost::filesystem::path& path) {
-          return std::tuple<boost::uintmax_t, boost::uintmax_t>{this->available_storage_space_,
-                                                                (this->watermark_ * this->available_storage_space_)};
-        });
+        getTestStorageSpaceFunc());
   }
 };