Refactoring the repo manager code

viniciusferrao · Feb 12, 2025 · 7a96b66 · 7a96b66
1 parent 0487532
commit 7a96b66
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 27 deletions.
diff --git a/include/cloysterhpc/services/repos.h b/include/cloysterhpc/services/repos.h
@@ -29,7 +29,7 @@ class RepoManager {
     void loadFiles(const std::filesystem::path& basedir = "/etc/yum.repos.d");
     void commitStatus();
     void enable(const std::string& repo);
-    void enableMultiple(std::vector<std::string> repos);
+    void enable(const std::vector<std::string>& repos);
     void disable(const std::string& repo);
     [[nodiscard]] const std::vector<BaseRepository>& listRepos() const;
     [[nodiscard]] std::vector<std::string> getxCATOSImageRepos() const;

diff --git a/include/cloysterhpc/services/shell.h b/include/cloysterhpc/services/shell.h
@@ -21,11 +21,10 @@ using cloyster::models::Cluster;
  * This class provides functionalities for configuring various system settings,
  * installing required packages, and setting up cluster-specific services.
  */
-class Shell : public Execution {
+class Shell final : public Execution {
 private:
-    const std::unique_ptr<Cluster>& m_cluster;
+    const std::unique_ptr<Cluster> m_cluster;
 
-private:
     /**
      * @brief Configures SELinux mode.
      *
@@ -142,6 +141,7 @@ class Shell : public Execution {
      */
     void configureQueueSystem();
 
+
     /**
      * @brief Configures the InfiniBand settings.
      *
@@ -172,16 +172,14 @@ class Shell : public Execution {
     static void disableSELinux();
 
 public:
-    // FIXME: Guideline: Don’t use a const unique_ptr& as a parameter;
-    //  use widget* instead.
     /**
      * @brief Constructs a Shell object.
      *
      * Initializes the Shell object with a reference to a Cluster object.
      *
      * @param cluster A reference to a unique pointer managing a Cluster object.
      */
-    explicit Shell(const std::unique_ptr<Cluster>& cluster);
+    explicit Shell(const std::unique_ptr<Cluster> cluster);
     /**
      * @brief Installs and configures the system.
      *

diff --git a/src/main.cpp b/src/main.cpp
@@ -176,7 +176,7 @@ int main(int argc, const char** argv)
         }
 
         std::unique_ptr<Execution> executionEngine
-            = std::make_unique<cloyster::services::Shell>(model);
+            = std::make_unique<cloyster::services::Shell>(std::move(model));
 
         executionEngine->install();
 

diff --git a/src/services/repos.cpp b/src/services/repos.cpp
@@ -178,9 +178,9 @@ void RepoManager::setEnableState(const std::string& id, bool value)
 void RepoManager::enable(const std::string& id) { setEnableState(id, true); }
 
 
-void RepoManager::enableMultiple(std::vector<std::string> ids)
+void RepoManager::enable(const std::vector<std::string>& ids)
 {
-    std::ranges::for_each(ids, [&](const auto& id) { this->enable(id); });
+    std::ranges::for_each(ids, [&](const auto& id) { enable(id); });
 }
 
 void RepoManager::disable(const std::string& id) { setEnableState(id, false); }

diff --git a/src/services/shell.cpp b/src/services/shell.cpp
@@ -24,13 +24,14 @@
 #include <cloysterhpc/models/slurm.h>
 
 #include <cloysterhpc/dbus_client.h>
+#include <ranges>
 
 using cloyster::runCommand;
 
 namespace cloyster::services {
 
-Shell::Shell(const std::unique_ptr<Cluster>& cluster)
-    : m_cluster(cluster)
+Shell::Shell(std::unique_ptr<Cluster> cluster)
+    : m_cluster(std::move(cluster))
 {
     // Initialize directory tree
     cloyster::createDirectory(installPath);
@@ -385,6 +386,31 @@ void Shell::installDevelopmentComponents()
     runCommand("dnf -y install lmod-defaults-gnu12-openmpi4-ohpc");
 }
 
+namespace {
+auto getToEnableRepoNames()
+{
+    return std::vector<std::string>({ "-beegfs", "-elrepo", "-epel", "-openhpc",
+        "-openhpc-updates", "-rpmfusion-free-updates" })
+    | std::views::transform([](const std::string& repo) {
+        return fmt::format("{}{}", cloyster::productName, repo);
+    })
+    | std::ranges::to<std::vector<std::string>>();
+}
+
+}
+
+void Shell::configureRepositories()
+{
+    auto repos = cloyster::getRepoManager(m_cluster->getHeadnode().getOS());
+    // 1. Install files into /etc, these files are the templates
+    //    at include/cloysterhpc/repos/el*/*.repo
+    repos->loadFiles();
+    // 2. Enable the repositories
+    repos->enable(getToEnableRepoNames());
+    // 3. Commit data to disk
+    repos->commitStatus();
+}
+
 /* This method is the entrypoint of shell based cluster install
  * The first session of the method will configure and install services on the
  * headnode. The last part will do provisioner related settings and image
@@ -411,21 +437,7 @@ void Shell::install()
 
     installRequiredPackages();
 
-    // TODO: This is the repos entrypoint. It should be replaced.
-    auto repos = cloyster::getRepoManager(m_cluster->getHeadnode().getOS());
-    repos->loadFiles();
-
-    const auto toEnable = std::vector<std::string>();
-        // @TODO Fix this
-        // std::vector({ "-beegfs", "-elrepo", "-epel", "-openhpc", "-openhpc-updates", "-rpmfusion-free-updates" })
-        // | std::views::transform([&](const std::string& pkg) {
-        //     return cloyster::productName + pkg;
-        // }) 
-        // | std::ranges::to<std::vector<std::string>>();
-
-    repos->enableMultiple(toEnable);
-    repos->commitStatus();
-    // End of Repos entrypoint
+    configureRepositories();
 
     runSystemUpdate();