From d96418805d24a5f6a4777ce92b47c257f376fa1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Wed, 8 Jan 2025 13:29:33 -0300 Subject: [PATCH 01/23] Fixes an outragous number of bugs and warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com> --- include/cloysterhpc/diskImage.h | 2 +- include/cloysterhpc/node.h | 2 +- include/cloysterhpc/os.h | 19 ++++++----- include/cloysterhpc/queuesystem/slurm.h | 6 ++-- include/cloysterhpc/repos.h | 5 +-- include/cloysterhpc/repos/el9/cloyster.repo | 2 +- include/cloysterhpc/services/shell.h | 21 ++++++------ include/cloysterhpc/services/xcat.h | 18 +++++----- include/cloysterhpc/view/newt.h | 4 +-- src/cluster.cpp | 6 ++-- src/diskImage.cpp | 1 + src/functions.cpp | 6 ++++ src/network.cpp | 16 +++++---- src/os.cpp | 36 +++++++++++++------ src/repos.cpp | 13 ++++--- src/services/IService.cpp | 35 +++++++++++++++++++ src/services/shell.cpp | 38 +++++++++++++-------- src/services/xcat.cpp | 6 +++- 18 files changed, 158 insertions(+), 78 deletions(-) diff --git a/include/cloysterhpc/diskImage.h b/include/cloysterhpc/diskImage.h index a20dc5b9..09bf7caf 100644 --- a/include/cloysterhpc/diskImage.h +++ b/include/cloysterhpc/diskImage.h @@ -37,7 +37,7 @@ class DiskImage { * @param path Filesystem path to the disk image to check. * @return True if the disk image is known, false otherwise. */ - bool isKnownImage(const std::filesystem::path& path); + static bool isKnownImage(const std::filesystem::path& path); /** * @brief Checks if the given disk image has a verified checksum. diff --git a/include/cloysterhpc/node.h b/include/cloysterhpc/node.h index 4b980615..46d4c569 100644 --- a/include/cloysterhpc/node.h +++ b/include/cloysterhpc/node.h @@ -55,7 +55,7 @@ class Node : public Server { const std::string& getMACAddress() const; void setMACAddress(const std::string& macAddress); const std::optional& getNodeRootPassword() const; - void setNodeRootPassword(const std::optional& ndeRootPassword); + void setNodeRootPassword(const std::optional& nodeRootPassword); }; #endif // CLOYSTERHPC_NODE_H_ diff --git a/include/cloysterhpc/os.h b/include/cloysterhpc/os.h index 662bec94..702ef034 100644 --- a/include/cloysterhpc/os.h +++ b/include/cloysterhpc/os.h @@ -12,6 +12,7 @@ #include #include #include +#include /** * @class OS @@ -50,10 +51,10 @@ class OS { enum class Distro { RHEL, OL, Rocky, AlmaLinux }; private: - Arch m_arch; - Family m_family; - Platform m_platform; - Distro m_distro; + std::variant m_arch; + std::variant m_family; + std::variant m_platform; + std::variant m_distro; std::string m_kernel; unsigned m_majorVersion {}; unsigned m_minorVersion {}; @@ -61,7 +62,7 @@ class OS { // however repos.h needs to be rewritten to support it. // 'OS::os()' is implicitly deleted because the default definition // would be ill-formed - std::shared_ptr m_packageManager {}; + std::shared_ptr m_packageManager; private: void setMajorVersion(unsigned int majorVersion); @@ -74,7 +75,10 @@ class OS { * @param line The key-value pair string. * @return The value extracted from the key-value pair string. */ - std::string getValueFromKey(const std::string& line); + static std::string getValueFromKey(const std::string& line); + + std::shared_ptr factoryPackageManager( + OS::Platform platform); public: OS(); @@ -119,8 +123,7 @@ class OS { [[nodiscard]] unsigned int getMajorVersion() const; [[nodiscard]] unsigned int getMinorVersion() const; - std::shared_ptr createPackageManager( - OS::Platform platform); + package_manager* packageManager() const; #ifndef NDEBUG /** diff --git a/include/cloysterhpc/queuesystem/slurm.h b/include/cloysterhpc/queuesystem/slurm.h index c5ba8c24..c01b84ab 100644 --- a/include/cloysterhpc/queuesystem/slurm.h +++ b/include/cloysterhpc/queuesystem/slurm.h @@ -22,7 +22,7 @@ class SLURM : public QueueSystem { /** * @brief Installs the SLURM server package on the system. */ - void installServer(); + static void installServer(); /** * @brief Configures the SLURM server. @@ -32,12 +32,12 @@ class SLURM : public QueueSystem { /** * @brief Enables the SLURM server to start at boot. */ - void enableServer(); + static void enableServer(); /** * @brief Starts the SLURM server. */ - void startServer(); + static void startServer(); }; #endif // CLOYSTERHPC_SLURM_H_ diff --git a/include/cloysterhpc/repos.h b/include/cloysterhpc/repos.h index 3e2bee98..819fabca 100644 --- a/include/cloysterhpc/repos.h +++ b/include/cloysterhpc/repos.h @@ -52,6 +52,7 @@ class RepoManager { void loadFiles(const std::filesystem::path& basedir = "/etc/yum.repos.d"); void loadCustom(inifile& file, const std::filesystem::path& path); + // BUG: Enable and EnableMultiple are the same method. Overload it. void enable(const std::string& id); void enableMultiple(std::vector ids); void disable(const std::string& id); @@ -65,11 +66,11 @@ class RepoManager { private: std::vector m_repos; BaseRunner& m_runner; - OS m_os; + const OS& m_os; void createFileFor(std::filesystem::path path); - std::vector buildCloysterTree( + const std::vector buildCloysterTree( const std::filesystem::path& basedir); void loadSingleFile(std::filesystem::path source); diff --git a/include/cloysterhpc/repos/el9/cloyster.repo b/include/cloysterhpc/repos/el9/cloyster.repo index c5255fb2..5c5b88b1 100644 --- a/include/cloysterhpc/repos/el9/cloyster.repo +++ b/include/cloysterhpc/repos/el9/cloyster.repo @@ -7,7 +7,7 @@ gpgkey=https://yum.oracle.com/RPM-GPG-KEY-oracle-ol9 [{0}-Rocky-BaseOS] name=Rocky Linux $releasever - BaseOS -baseurl=http://dl.rockylinux.org/$contentdir/$releasever/BaseOS/$basearch/os/ +baseurl=http://dl.rockylinux.org/pub/rocky/$releasever/BaseOS/$basearch/os/ enabled=0 gpgcheck=1 gpgkey=https://mirror.versatushpc.com.br/rocky/linux/RPM-GPG-KEY-Rocky-9 diff --git a/include/cloysterhpc/services/shell.h b/include/cloysterhpc/services/shell.h index 6a1a0c50..337d4661 100644 --- a/include/cloysterhpc/services/shell.h +++ b/include/cloysterhpc/services/shell.h @@ -10,7 +10,6 @@ #include #include -#include /** * @class Shell @@ -71,9 +70,9 @@ class Shell : public Execution { * * This function disables the DNS override feature of NetworkManager. */ - void disableNetworkManagerDNSOverride(); // This should be on Network + static void disableNetworkManagerDNSOverride(); // This should be on Network - void deleteConnectionIfExists(std::string_view connectionName); + static void deleteConnectionIfExists(std::string_view connectionName); /** * @brief Configures network connections. @@ -83,7 +82,7 @@ class Shell : public Execution { * * @param connections A list of network connections to configure. */ - void configureNetworks(const std::list&); + static void configureNetworks(const std::list&); /** * @brief Runs a system update. @@ -98,7 +97,7 @@ class Shell : public Execution { * This function installs all the necessary packages for the system to * operate. */ - void installRequiredPackages(); + static void installRequiredPackages(); /** * @brief Disallows SSH root password login. @@ -106,14 +105,14 @@ class Shell : public Execution { * This function configures SSH to disallow root login using password * authentication. */ - void disallowSSHRootPasswordLogin(); + static void disallowSSHRootPasswordLogin(); /** * @brief Installs the OpenHPC base packages. * * This function installs the base packages required for OpenHPC. */ - void installOpenHPCBase(); + static void installOpenHPCBase(); /** * @brief Configure repositories @@ -131,7 +130,7 @@ class Shell : public Execution { * @param connections A list of network connections to use for time service * configuration. */ - void configureTimeService(const std::list&); + static void configureTimeService(const std::list&); /** * @brief Configures the queue system. @@ -153,13 +152,13 @@ class Shell : public Execution { * * This function removes any limits on memory locking. */ - void removeMemlockLimits(); + static void removeMemlockLimits(); /** * @brief Installs development components. * * This function installs the necessary development tools and libraries. */ - void installDevelopmentComponents(); + static void installDevelopmentComponents(); /* Ancillary functions */ /** @@ -167,7 +166,7 @@ class Shell : public Execution { * * This function completely disables SELinux enforcement. */ - void disableSELinux(); + static void disableSELinux(); public: // FIXME: Guideline: Don’t use a const unique_ptr& as a parameter; diff --git a/include/cloysterhpc/services/xcat.h b/include/cloysterhpc/services/xcat.h index 2c60c03f..1c95b8a4 100644 --- a/include/cloysterhpc/services/xcat.h +++ b/include/cloysterhpc/services/xcat.h @@ -55,15 +55,15 @@ class XCAT : public Provisioner { } m_stateless; private: - void setDHCPInterfaces(std::string_view interface); - void setDomain(std::string_view domain); + static void setDHCPInterfaces(std::string_view interface); + static void setDomain(std::string_view domain); /** * @brief Copies installation media to the disk image. * * @param diskImage The path to the disk image. */ - void copycds(const std::filesystem::path& diskImage); + static void copycds(const std::filesystem::path& diskImage); /** * @brief Generates the OS image. @@ -91,7 +91,7 @@ class XCAT : public Provisioner { * * This function sets up the directory structure required for provisioning. */ - void createDirectoryTree(); + static void createDirectoryTree(); /** * @brief Configures OpenHPC settings. @@ -140,7 +140,7 @@ class XCAT : public Provisioner { * * This function creates the synchronization list file. */ - void generateSynclistsFile(); + static void generateSynclistsFile(); /** * @brief Configures the OS image definition. @@ -161,7 +161,7 @@ class XCAT : public Provisioner { * * @param node The node to add. */ - void addNode(const Node& node); + static void addNode(const Node& node); /** * @brief Generates the OS image name based on type and node. @@ -184,7 +184,7 @@ class XCAT : public Provisioner { * * This function sets up configurations for EL9. */ - void configureEL9(); + static void configureEL9(); public: /** @@ -234,14 +234,14 @@ class XCAT : public Provisioner { * * This function configures the nodes to boot using the assigned OS image. */ - void setNodesBoot(); + static void setNodesBoot(); /** * @brief Resets the nodes. * * This function resets the nodes. */ - void resetNodes(); + static void resetNodes(); explicit XCAT(const std::unique_ptr& cluster); }; diff --git a/include/cloysterhpc/view/newt.h b/include/cloysterhpc/view/newt.h index bb4b49b4..b34c257b 100644 --- a/include/cloysterhpc/view/newt.h +++ b/include/cloysterhpc/view/newt.h @@ -64,7 +64,7 @@ class Newt : public View { void abort() override; void helpMessage(const char*) override; - bool hasEmptyField(const struct newtWinEntry*); + static bool hasEmptyField(const struct newtWinEntry*); public: Newt(); @@ -129,7 +129,7 @@ class Newt : public View { okCancelMessage(nullptr, message, pairs); } - std::vector convertToNewtList( + static std::vector convertToNewtList( const std::vector& s); // TODO: diff --git a/src/cluster.cpp b/src/cluster.cpp index aae18e0a..a8672ccd 100644 --- a/src/cluster.cpp +++ b/src/cluster.cpp @@ -109,7 +109,7 @@ std::list>& Cluster::getNetworks() void Cluster::initRepoManager() { - m_repos.emplace(*m_runner, this->getHeadnode().getOS()); + m_repos.emplace(*m_runner, m_headnode.getOS()); } RepoManager& Cluster::getRepoManager() @@ -251,6 +251,7 @@ const std::filesystem::path& Cluster::getDiskImage() const void Cluster::setDiskImage(const std::filesystem::path& diskImagePath) { + // BUG: This does not hanble ~ expansion for userdir if (std::filesystem::exists(diskImagePath)) { m_diskImage.setPath(diskImagePath); } else { @@ -682,7 +683,8 @@ void Cluster::fillData(const std::string& answerfilePath) // System setUpdateSystem(true); setProvisioner(Provisioner::xCAT); - m_headnode.setOS(nodeOS); + // BUG: Headnode OS may not be the same as the node OS + //m_headnode.setOS(nodeOS); for (const auto& tool : answerfile.getTools()) { tool->install(); diff --git a/src/diskImage.cpp b/src/diskImage.cpp index 14ea4d43..42fe9580 100644 --- a/src/diskImage.cpp +++ b/src/diskImage.cpp @@ -40,6 +40,7 @@ bool DiskImage::isKnownImage(const std::filesystem::path& path) return false; } +// BUG: Consider removing bool DiskImage::hasVerifiedChecksum(const std::filesystem::path& path) { if (!isKnownImage(path)) { diff --git a/src/functions.cpp b/src/functions.cpp index 4be874fa..0c661f2b 100644 --- a/src/functions.cpp +++ b/src/functions.cpp @@ -174,6 +174,12 @@ void writeConfig(const std::string& filename) void touchFile(const std::filesystem::path& path) { + if (cloyster::dryRun) { + LOG_INFO("Would touch the file {}", path.string()) + return; + } + + // BUG: I don't have to comment why this is a BUG, right? FILE* f = fopen(path.c_str(), "ab"); (void)fflush(f); (void)fclose(f); diff --git a/src/network.cpp b/src/network.cpp index 73719987..b08e86d0 100644 --- a/src/network.cpp +++ b/src/network.cpp @@ -110,9 +110,9 @@ void Network::setAddress(const std::string& ip) address Network::fetchAddress(const std::string& interface) { - struct in_addr addr { }; - struct in_addr netmask { }; - struct in_addr network { }; + struct in_addr addr {}; + struct in_addr netmask {}; + struct in_addr network {}; if (inet_aton( Connection::fetchAddress(interface).to_string().c_str(), &addr) @@ -208,8 +208,8 @@ address Network::calculateAddress(const address& connectionAddress) "calculate the address")); } - struct in_addr ip_addr { }; - struct in_addr subnet_addr { }; + struct in_addr ip_addr {}; + struct in_addr subnet_addr {}; inet_aton(connectionAddress.to_string().c_str(), &ip_addr); inet_aton(m_subnetMask.to_string().c_str(), &subnet_addr); @@ -299,12 +299,15 @@ const std::string& Network::getDomainName() const { return m_domainName; } void Network::setDomainName(const std::string& domainName) { + if (domainName.empty()) + throw std::invalid_argument("Domain name cannot be empty."); + if (domainName.size() > 255) throw std::length_error("Domain name exceeds the maximum allowed " "length of 255 characters."); if (domainName.starts_with('-') or domainName.ends_with('-')) - throw std::runtime_error("Invalid hostname"); + throw std::runtime_error("Hostname cannot start or end with a hyphen."); /* Check if string has only digits */ if (std::regex_match(domainName, std::regex("^[0-9]+$"))) @@ -329,6 +332,7 @@ std::string Network::fetchDomainName() LOG_TRACE("Got domain name {}", domainName) + // BUG: This is a bug, we must return the domain name auto ret = std::string(domainName); if (ret == "(none)") { ret = ""; diff --git a/src/os.cpp b/src/os.cpp index 259c1a16..02950164 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -3,6 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include "cloysterhpc/services/package_manager.h" #include #include #include @@ -12,6 +13,7 @@ #endif #include +#include #include #include @@ -89,6 +91,8 @@ OS::OS() fmt::format("Error while reading file: {}", filename)); } } + + factoryPackageManager(getPlatform()); } OS::OS(OS::Arch arch, OS::Family family, OS::Platform platform, @@ -98,14 +102,14 @@ OS::OS(OS::Arch arch, OS::Family family, OS::Platform platform, , m_family(family) , m_platform(platform) , m_distro(distro) + , m_packageManager(factoryPackageManager(platform)) { setKernel(kernel); setMajorVersion(majorVersion); setMinorVersion(minorVersion); - createPackageManager(platform); } -OS::Arch OS::getArch() const { return m_arch; } +OS::Arch OS::getArch() const { return std::get(m_arch); } void OS::setArch(Arch arch) { m_arch = arch; } @@ -119,7 +123,7 @@ void OS::setArch(std::string_view arch) setArch(OS::Arch::x86_64); } -OS::Family OS::getFamily() const { return m_family; } +OS::Family OS::getFamily() const { return std::get(m_family); } void OS::setFamily(Family family) { m_family = family; } @@ -131,7 +135,10 @@ void OS::setFamily(std::string_view family) throw std::runtime_error(fmt::format("Unsupported OS: {}", family)); } -OS::Platform OS::getPlatform() const { return m_platform; } +OS::Platform OS::getPlatform() const +{ + return std::get(m_platform); +} void OS::setPlatform(OS::Platform platform) { m_platform = platform; } @@ -151,7 +158,7 @@ void OS::setPlatform(std::string_view platform) throw std::runtime_error(fmt::format("Unsupported Platform: {}", platform)); } -OS::Distro OS::getDistro() const { return m_distro; } +OS::Distro OS::getDistro() const { return std::get(m_distro); } void OS::setDistro(OS::Distro distro) { m_distro = distro; } @@ -218,6 +225,7 @@ void OS::setVersion(const std::string& version) * fetchValueFromKey() and setOS(); an those methods should really be on OS * class and not here. */ + std::string OS::getValueFromKey(const std::string& line) { std::string value; @@ -232,23 +240,29 @@ std::string OS::getValueFromKey(const std::string& line) return value; } -std::shared_ptr OS::createPackageManager(OS::Platform platform) +std::shared_ptr OS::factoryPackageManager( + OS::Platform platform) { if (platform == OS::Platform::el8 || platform == OS::Platform::el9) { - return std::make_shared(); + m_packageManager = std::make_shared(); + return m_packageManager; } else { throw std::runtime_error("Unsupported OS platform"); } } +package_manager* OS::packageManager() const { return m_packageManager.get(); } + #ifndef NDEBUG void OS::printData() const { - LOG_DEBUG("Architecture: {}", magic_enum::enum_name(m_arch)) - LOG_DEBUG("Family: {}", magic_enum::enum_name(m_family)) + LOG_DEBUG("Architecture: {}", magic_enum::enum_name(std::get(m_arch))) + LOG_DEBUG("Family: {}", magic_enum::enum_name(std::get(m_family))) LOG_DEBUG("Kernel Release: {}", m_kernel) - LOG_DEBUG("Platform: {}", magic_enum::enum_name(m_platform)) - LOG_DEBUG("Distribution: {}", magic_enum::enum_name(m_distro)) + LOG_DEBUG( + "Platform: {}", magic_enum::enum_name(std::get(m_platform))) + LOG_DEBUG( + "Distribution: {}", magic_enum::enum_name(std::get(m_distro))) LOG_DEBUG("Major Version: {}", m_majorVersion) LOG_DEBUG("Minor Version: {}", m_minorVersion) } diff --git a/src/repos.cpp b/src/repos.cpp index 1c5c031f..4b8bb6d3 100644 --- a/src/repos.cpp +++ b/src/repos.cpp @@ -23,6 +23,7 @@ constexpr std::string_view CLOYSTER_REPO_EL8 { constexpr std::string_view CLOYSTER_REPO_EL9 = { #include "cloysterhpc/repos/el9/cloyster.repo" + }; static repository loadSection(const std::filesystem::path& source, @@ -79,6 +80,7 @@ static void loadFromINI(const std::filesystem::path& source, inifile& file, } } +// BUG: Why? #define NOSONAR(code) code void RepoManager::loadSingleFile(std::filesystem::path source) @@ -234,7 +236,7 @@ void RepoManager::commitStatus() #define FORMAT_TEMPLATE(src) fmt::format(src, cloyster::productName) -std::vector RepoManager::buildCloysterTree( +const std::vector RepoManager::buildCloysterTree( const std::filesystem::path& basedir) { @@ -278,8 +280,9 @@ void RepoManager::createFileFor(std::filesystem::path path) inifile file; - auto filtered = m_repos - | std::views::filter([&path](auto& r) { return path == r.source; }); + auto filtered = m_repos | std::views::filter([&path](const auto& r) { + return path == r.source; + }); for (const auto& repo : filtered) { writeSection(file, repo); @@ -317,8 +320,8 @@ void RepoManager::configureXCAT(const std::filesystem::path& repofile_dest) LOG_INFO("Setting up XCAT repositories"); // TODO: we need to download these files in a sort of temporary directory - m_runner.downloadFile("https://xcat.org/files/xcat/repos/yum/latest/" - "xcat-core/xcat-core.repo", + m_runner.downloadFile("https://xcat.org/files/xcat/repos/yum/devel/" + "core-snap/xcat-core.repo", repofile_dest.string()); switch (m_os.getPlatform()) { diff --git a/src/services/IService.cpp b/src/services/IService.cpp index 2835c4f8..79684c49 100644 --- a/src/services/IService.cpp +++ b/src/services/IService.cpp @@ -1,7 +1,17 @@ +#include #include #include #include +/* BUG: Refactor: + * Legacy casting. + * Dry run is a band-aid solution. + * Variables could be const. + * Variables name are not the best ones. + * Check grammar. + * Warnings during compilation. + */ + using EnableRType = std::vector>; @@ -17,6 +27,11 @@ bool IService::handleException(const sdbus::Error& e, const std::string_view fn) void IService::enable() { + if (cloyster::dryRun) { + LOG_INFO("Would have enabled the service {}", m_name) + return; + } + LOG_TRACE("service: enabling {}", m_name); auto ret = callObjectFunctionArray("EnableUnitFiles", false, true) @@ -30,6 +45,11 @@ void IService::enable() void IService::disable() { + if (cloyster::dryRun) { + LOG_INFO("Would have disabled the service {}", m_name) + return; + } + LOG_TRACE("service: disabling {}", m_name); auto ret = callObjectFunctionArray("DisableUnitFiles", false, true) @@ -42,18 +62,33 @@ void IService::disable() void IService::start() { + if (cloyster::dryRun) { + LOG_INFO("Would have started the service {}", m_name) + return; + } + LOG_TRACE("service: starting {}", m_name); (void)callObjectFunction("StartUnit", "replace"); } void IService::restart() { + if (cloyster::dryRun) { + LOG_INFO("Would have restarted the service {}", m_name) + return; + } + LOG_TRACE("service: restarting {}", m_name); (void)callObjectFunction("RestartUnit", "replace"); } void IService::stop() { + if (cloyster::dryRun) { + LOG_INFO("Would have stopped the service {}", m_name) + return; + } + LOG_TRACE("service: stopping {}", m_name); (void)callObjectFunction("StopUnit", "replace"); } diff --git a/src/services/shell.cpp b/src/services/shell.cpp index ec7a2210..e880c43c 100644 --- a/src/services/shell.cpp +++ b/src/services/shell.cpp @@ -10,9 +10,9 @@ #include #include -#include #include #include +#include #include #include @@ -115,7 +115,7 @@ void Shell::configureHostsFile() { LOG_INFO("Setting up additional entries on hosts file") - auto& headnode = m_cluster->getHeadnode(); + const auto& headnode = m_cluster->getHeadnode(); const auto& ip = headnode.getConnection(Network::Profile::Management) .getAddress() @@ -164,6 +164,7 @@ void Shell::disableNetworkManagerDNSOverride() runCommand("systemctl restart NetworkManager"); } +// BUG: Why this method exists? The name does not do what it says. void Shell::deleteConnectionIfExists(std::string_view connectionName) { runCommand(fmt::format("nmcli connection delete \"{}\"", connectionName)); @@ -214,6 +215,13 @@ void Shell::configureNetworks(const std::list& connections) connection.getNetwork()->getGateway().to_string(), fmt::join(formattedNameservers, " "), connection.getNetwork()->getDomainName())); + + /* Give network manage some time to settle thing up + * Avoids: Error: Connection activation failed: IP configuration could + * not be reserved (no available address, timeout, etc.). + */ + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + runCommand(fmt::format("nmcli device connect {}", interface)); } @@ -239,8 +247,8 @@ void Shell::disallowSSHRootPasswordLogin() { LOG_INFO("Allowing root login only through public key authentication (SSH)") - runCommand("sed -i 's/PermitRootLogin\\ yes/PermitRootLogin\\ " - "without-password/g' /etc/ssh/sshd_config"); + cloyster::addStringToFile( + "/etc/ssh/sshd_config", "PermitRootLogin without-password\n"); } void Shell::installOpenHPCBase() @@ -389,17 +397,17 @@ void Shell::install() installRequiredPackages(); - auto repos = m_cluster->getRepoManager(); - repos.loadFiles(); - - std::vector toEnable = { "-beegfs", "-elrepo", "-epel", - "-openhpc", "-rpmfusion-free-updates" }; - for (auto& package : toEnable) { - package = cloyster::productName + package; - } - - repos.enableMultiple(toEnable); - repos.commitStatus(); + // auto repos = m_cluster->getRepoManager(); + // repos.loadFiles(); + // + // std::vector toEnable = { "-beegfs", "-elrepo", "-epel", + // "-openhpc", "-rpmfusion-free-updates" }; + // for (auto& package : toEnable) { + // package = cloyster::productName + package; + // } + // + // repos.enableMultiple(toEnable); + // repos.commitStatus(); runSystemUpdate(); diff --git a/src/services/xcat.cpp b/src/services/xcat.cpp index 7f046a0b..3e1fe82f 100644 --- a/src/services/xcat.cpp +++ b/src/services/xcat.cpp @@ -31,7 +31,11 @@ XCAT::XCAT(const std::unique_ptr& cluster) setenv("PERL_BADLANG", "0", false); } -void XCAT::installPackages() { cloyster::runCommand("dnf -y install xCAT"); } +void XCAT::installPackages() +{ + m_cluster->getHeadnode().getOS().packageManager()->install(std::string{"initscripts"}); + cloyster::runCommand("dnf -y install xCAT"); +} void XCAT::setup() { From 233c439f312d68a36021cb7c014478ce69693fd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Thu, 9 Jan 2025 03:33:47 -0300 Subject: [PATCH 02/23] Fixed more outrageous bugs. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The program is now able to at least boot a compute node again Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com> --- include/cloysterhpc/os.h | 7 +- include/cloysterhpc/services/xcat.h | 8 +++ src/answerfile.cpp | 6 +- src/diskImage.cpp | 4 +- src/hardware.cpp | 4 +- src/main.cpp | 2 +- src/os.cpp | 35 ++++------ .../PresenterNodesOperationalSystem.cpp | 7 +- src/repos.cpp | 8 ++- src/services/locale.cpp | 4 +- src/services/shell.cpp | 35 ++++++---- src/services/xcat.cpp | 68 +++++++++++++------ src/tools/nvhpc.cpp | 2 +- 13 files changed, 116 insertions(+), 74 deletions(-) diff --git a/include/cloysterhpc/os.h b/include/cloysterhpc/os.h index 702ef034..d4fc14e2 100644 --- a/include/cloysterhpc/os.h +++ b/include/cloysterhpc/os.h @@ -6,10 +6,9 @@ #ifndef CLOYSTERHPC_OS_H_ #define CLOYSTERHPC_OS_H_ -#include "services/dnf.h" - #include #include +#include #include #include #include @@ -78,7 +77,7 @@ class OS { static std::string getValueFromKey(const std::string& line); std::shared_ptr factoryPackageManager( - OS::Platform platform); + OS::Platform platform); public: OS(); @@ -123,7 +122,7 @@ class OS { [[nodiscard]] unsigned int getMajorVersion() const; [[nodiscard]] unsigned int getMinorVersion() const; - package_manager* packageManager() const; + gsl::not_null packageManager() const; #ifndef NDEBUG /** diff --git a/include/cloysterhpc/services/xcat.h b/include/cloysterhpc/services/xcat.h index 1c95b8a4..337eae96 100644 --- a/include/cloysterhpc/services/xcat.h +++ b/include/cloysterhpc/services/xcat.h @@ -195,6 +195,14 @@ class XCAT : public Provisioner { */ void installPackages(); + /** + * @brief Patches xCAT to resolve bugs that aren't addressed upstream. + * + * This function applies patches to xCAT to fix issues during installation + * on recent versions of the OS. + */ + static void patchInstall(); + /** * @brief Sets up the provisioning environment. * diff --git a/src/answerfile.cpp b/src/answerfile.cpp index 19fe9a2b..cbeebe55 100644 --- a/src/answerfile.cpp +++ b/src/answerfile.cpp @@ -3,9 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "cloysterhpc/answerfile.h" -#include "cloysterhpc/services/log.h" -#include "cloysterhpc/tools/nvhpc.h" +#include +#include +#include #include #include #include diff --git a/src/diskImage.cpp b/src/diskImage.cpp index 42fe9580..8ffdd5eb 100644 --- a/src/diskImage.cpp +++ b/src/diskImage.cpp @@ -4,8 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "cloysterhpc/os.h" -#include "cloysterhpc/services/log.h" +#include +#include #include #include #include diff --git a/src/hardware.cpp b/src/hardware.cpp index 3b71f336..1fccfd5a 100644 --- a/src/hardware.cpp +++ b/src/hardware.cpp @@ -3,7 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "cloysterhpc/hardware.h" +// BUG: File Marked for removal + +#include #include std::vector Hardware::getCPU() const { return m_cpu; } diff --git a/src/main.cpp b/src/main.cpp index 92800fcc..cf74e038 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6,7 +6,7 @@ #include #include -#include "cloysterhpc/hardware.h" +#include #include #include #include diff --git a/src/os.cpp b/src/os.cpp index 02950164..dd76e70d 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "cloysterhpc/services/package_manager.h" +#include #include #include #include @@ -20,10 +20,7 @@ #include #include - -#if __cpp_lib_starts_ends_with < 201711L -#include -#endif +#include OS::OS() { @@ -55,11 +52,7 @@ OS::OS() while (std::getline(file, line)) { /* TODO: Refactor the next three conditions */ -#if __cpp_lib_starts_ends_with >= 201711L if (line.starts_with("PLATFORM_ID=")) { -#else - if (boost::algorithm::starts_with(line, "PLATFORM_ID=")) { -#endif auto value = getValueFromKey(line); if (value.starts_with("platform:")) { setPlatform(value.substr(9)); // Skip the 'platform:' prefix @@ -68,19 +61,11 @@ OS::OS() } } -#if __cpp_lib_starts_ends_with >= 201711L if (line.starts_with("ID=")) { -#else - if (boost::algorithm::starts_with(line, "ID=")) { -#endif setDistro(getValueFromKey(line)); } -#if __cpp_lib_starts_ends_with >= 201711L if (line.starts_with("VERSION=")) { -#else - if (boost::algorithm::starts_with(line, "VERSION=")) { -#endif setVersion(getValueFromKey(line)); } } @@ -168,13 +153,15 @@ void OS::setDistro(std::string_view distro) // comparison in magic_enum would be implemented it may easily replace the // lambda block. Reference: https://github.com/Neargye/magic_enum/pull/139 -#if 0 - if (const auto& rv = magic_enum::enum_cast(distro, magic_enum::case_insensitive)) -#endif +#if 1 + if (const auto& rv + = magic_enum::enum_cast(distro, magic_enum::case_insensitive)) +#else if (const auto &rv = magic_enum::enum_cast(distro, [](char lhs, char rhs) { return std::tolower(lhs) == std::tolower(rhs); })) +#endif setDistro(rv.value()); else throw std::runtime_error( @@ -247,11 +234,15 @@ std::shared_ptr OS::factoryPackageManager( m_packageManager = std::make_shared(); return m_packageManager; } else { - throw std::runtime_error("Unsupported OS platform"); + throw std::runtime_error(fmt::format( + "Unsupported OS platform: {}", magic_enum::enum_name(platform))); } } -package_manager* OS::packageManager() const { return m_packageManager.get(); } +gsl::not_null OS::packageManager() const +{ + return m_packageManager.get(); +} #ifndef NDEBUG void OS::printData() const diff --git a/src/presenter/PresenterNodesOperationalSystem.cpp b/src/presenter/PresenterNodesOperationalSystem.cpp index 2b0d0fa0..9106d6ed 100644 --- a/src/presenter/PresenterNodesOperationalSystem.cpp +++ b/src/presenter/PresenterNodesOperationalSystem.cpp @@ -3,13 +3,14 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "cloysterhpc/functions.h" -#include "cloysterhpc/services/log.h" +#include +#include +#include + #include #include #include #include -#include #include #include #include diff --git a/src/repos.cpp b/src/repos.cpp index 4b8bb6d3..79dee95e 100644 --- a/src/repos.cpp +++ b/src/repos.cpp @@ -350,13 +350,19 @@ std::vector RepoManager::getxCATOSImageRepos() const std::vector repos; - std::vector latestEL = { "8.9", "9.3" }; + /* BUG: This is a very bad implementation; it should find out the latest + * version and not be hardcoded. Also the directory formation does not work + * that way. We should support finding out the repository paths by parsing + * /etc/yum.repos.d + */ + std::vector latestEL = { "8.10", "9.5" }; std::string crb = "CRB"; std::string rockyBranch = "linux"; // To check if Rocky mirror directory points to 'linux' // (latest version) or 'vault' + // BUG: Really? A string? std::string OpenHPCVersion = "3"; if (osMajorVersion < 9) { diff --git a/src/services/locale.cpp b/src/services/locale.cpp index a1e3920b..db55da10 100644 --- a/src/services/locale.cpp +++ b/src/services/locale.cpp @@ -3,8 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "cloysterhpc/services/locale.h" -#include "cloysterhpc/functions.h" +#include +#include #include Locale::Locale() diff --git a/src/services/shell.cpp b/src/services/shell.cpp index e880c43c..a5c405d6 100644 --- a/src/services/shell.cpp +++ b/src/services/shell.cpp @@ -10,9 +10,9 @@ #include #include +#include #include #include -#include #include #include @@ -247,8 +247,9 @@ void Shell::disallowSSHRootPasswordLogin() { LOG_INFO("Allowing root login only through public key authentication (SSH)") - cloyster::addStringToFile( - "/etc/ssh/sshd_config", "PermitRootLogin without-password\n"); + runCommand( + "sed -i \"/^#\\?PermitRootLogin/c\\PermitRootLogin without-password\"" + " /etc/ssh/sshd_config"); } void Shell::installOpenHPCBase() @@ -397,17 +398,19 @@ void Shell::install() installRequiredPackages(); - // auto repos = m_cluster->getRepoManager(); - // repos.loadFiles(); - // - // std::vector toEnable = { "-beegfs", "-elrepo", "-epel", - // "-openhpc", "-rpmfusion-free-updates" }; - // for (auto& package : toEnable) { - // package = cloyster::productName + package; - // } - // - // repos.enableMultiple(toEnable); - // repos.commitStatus(); + // TODO: This is the repos entrypoint. It should be replaced. + auto repos = m_cluster->getRepoManager(); + repos.loadFiles(); + + std::vector toEnable = { "-beegfs", "-elrepo", "-epel", + "-openhpc", "-openhpc-updates", "-rpmfusion-free-updates" }; + for (auto& package : toEnable) { + package = cloyster::productName + package; + } + + repos.enableMultiple(toEnable); + repos.commitStatus(); + // End of Repos entrypoint runSystemUpdate(); @@ -415,6 +418,7 @@ void Shell::install() configureInfiniband(); + // BUG: Broken. Compute nodes does not mount anything. NFS networkFileSystem = NFS(systemdBus, "pub", "/opt/ohpc", m_cluster->getHeadnode() .getConnection(Network::Profile::Management) @@ -448,6 +452,9 @@ void Shell::install() LOG_INFO("[{}] Installing packages", provisionerName) provisioner->installPackages(); + LOG_INFO("[{}] Patching the provisioner", provisionerName) + provisioner->patchInstall(); + LOG_INFO("[{}] Setting up the provisioner", provisionerName) provisioner->setup(); diff --git a/src/services/xcat.cpp b/src/services/xcat.cpp index 3e1fe82f..5ba87020 100644 --- a/src/services/xcat.cpp +++ b/src/services/xcat.cpp @@ -33,16 +33,28 @@ XCAT::XCAT(const std::unique_ptr& cluster) void XCAT::installPackages() { - m_cluster->getHeadnode().getOS().packageManager()->install(std::string{"initscripts"}); - cloyster::runCommand("dnf -y install xCAT"); + m_cluster->getHeadnode().getOS().packageManager()->install("initscripts"); + m_cluster->getHeadnode().getOS().packageManager()->install("xCAT"); +} + +void XCAT::patchInstall() +{ + /* Required for EL 9.5 + * Upstream PR: https://github.com/xcat2/xcat-core/pull/7489 + */ + cloyster::runCommand("sed -i \"s/\-extensions\ usr_cert\ //g\" " + "/opt/xcat/share/xcat/scripts/setup-local-client.sh"); + cloyster::runCommand("sed -i \"s/\-extensions\ server //g\" " + "/opt/xcat/share/xcat/scripts/setup-server-cert.sh"); + cloyster::runCommand("xcatconfig -f"); } void XCAT::setup() { setDHCPInterfaces(m_cluster->getHeadnode() - .getConnection(Network::Profile::Management) - .getInterface() - .value()); + .getConnection(Network::Profile::Management) + .getInterface() + .value()); setDomain(m_cluster->getDomainName()); } @@ -82,7 +94,7 @@ void XCAT::nodeset(std::string_view nodes) void XCAT::createDirectoryTree() { if (cloyster::dryRun) { - LOG_INFO("Would create the directory CHROOT/install/custom/netboot") + LOG_INFO("Would create the directory /install/custom/netboot") return; } @@ -193,6 +205,17 @@ void XCAT::generatePostinstallFile() cloyster::removeFile(filename); + // TODO: Should be replaced with autofs + m_stateless.postinstall.emplace_back( + fmt::format("cat << END >> $IMG_ROOTIMGDIR/etc/fstab\n" + "{0}:/home /home nfs nfsvers=3,nodev,nosuid 0 0\n" + "{0}:/opt/ohpc/pub /opt/ohpc/pub nfs nfsvers=3,nodev 0 0\n" + "END\n\n", + m_cluster->getHeadnode() + .getConnection(Network::Profile::Management) + .getAddress() + .to_string())); + m_stateless.postinstall.emplace_back( "perl -pi -e 's/# End of file/\\* soft memlock unlimited\\n$&/s' " "$IMG_ROOTIMGDIR/etc/security/limits.conf\n" @@ -210,7 +233,6 @@ void XCAT::generatePostinstallFile() LOG_INFO("Would change file {} permissions", filename) return; } - std::filesystem::permissions(filename, std::filesystem::perms::owner_exec | std::filesystem::perms::group_exec | std::filesystem::perms::others_exec, @@ -257,19 +279,25 @@ void XCAT::configureOSImageDefinition() void XCAT::customizeImage() { - // Bugfixes for Munge - // Not needed if we sync /etc/passwd and /etc/groups -#if 0 - cloyster::runCommand(fmt::format( - "/bin/bash -c \"chroot {} chown munge:munge /var/lib/munge\"", - m_stateless.chroot.string())); - cloyster::runCommand(fmt::format( - "/bin/bash -c \"chroot {} chown munge:munge /var/log/munge\"", - m_stateless.chroot.string())); - cloyster::runCommand(fmt::format( - "/bin/bash -c \"chroot {} chown munge:munge /etc/munge\"", - m_stateless.chroot.string())); -#endif + // Permission fixes for munge + if (m_cluster->getQueueSystem().value()->getKind() + == QueueSystem::Kind::SLURM) { + cloyster::runCommand( + fmt::format("cp -f /etc/passwd /etc/group /etc/shadow {}/etc", + m_stateless.chroot.string())); + cloyster::runCommand( + fmt::format("mkdir -p {0}/var/lib/munge {0}/var/log/munge " + "{0}/etc/munge {0}/run/munge", + m_stateless.chroot.string())); + cloyster::runCommand(fmt::format( + "chown munge:munge {}/var/lib/munge", m_stateless.chroot.string())); + cloyster::runCommand(fmt::format( + "chown munge:munge {}/var/log/munge", m_stateless.chroot.string())); + cloyster::runCommand(fmt::format( + "chown munge:munge {}/etc/munge", m_stateless.chroot.string())); + cloyster::runCommand(fmt::format( + "chown munge:munge {}/run/munge", m_stateless.chroot.string())); + } } /* This is necessary to avoid problems with EL9-based distros. diff --git a/src/tools/nvhpc.cpp b/src/tools/nvhpc.cpp index 23111e40..70c33ae0 100644 --- a/src/tools/nvhpc.cpp +++ b/src/tools/nvhpc.cpp @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include "cloysterhpc/tools/nvhpc.h" +#include void NVhpc::install() { From d7dcb4969829569b5bc0fc81c2f59fe4ce00b319 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 13:21:38 -0300 Subject: [PATCH 03/23] Disable cosmetic warnings --- cmake/CompilerWarnings.cmake | 3 +++ cmake/StaticAnalyzers.cmake | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/cmake/CompilerWarnings.cmake b/cmake/CompilerWarnings.cmake index 0ec6e94b..e08007e7 100644 --- a/cmake/CompilerWarnings.cmake +++ b/cmake/CompilerWarnings.cmake @@ -67,6 +67,9 @@ function( -Wduplicated-branches # warn if if / else branches have duplicated code -Wlogical-op # warn about logical operations being used where bitwise were probably wanted -Wuseless-cast # warn if you perform a cast to the same type + -Wno-unused-variable + -Wno-unused-function + -Wno-unused-parameter ) endif() diff --git a/cmake/StaticAnalyzers.cmake b/cmake/StaticAnalyzers.cmake index a039b516..fac44913 100644 --- a/cmake/StaticAnalyzers.cmake +++ b/cmake/StaticAnalyzers.cmake @@ -13,6 +13,7 @@ macro(cloysterhpc_enable_cppcheck WARNINGS_AS_ERRORS CPPCHECK_OPTIONS) # style should enable the other 3, but we'll be explicit just in case set(CMAKE_CXX_CPPCHECK ${CPPCHECK} + --quiet --template=${CPPCHECK_TEMPLATE} --enable=style,performance,warning,portability --inline-suppr @@ -26,6 +27,21 @@ macro(cloysterhpc_enable_cppcheck WARNINGS_AS_ERRORS CPPCHECK_OPTIONS) # ignores code that cppcheck thinks is invalid C++ --suppress=syntaxError --suppress=preprocessorErrorDirective + # FIXME: The below warnings were disabled to underflow the devopment + # they should be enabled again + --suppress=functionStatic + --suppress=functionConst + --suppress=funcArgNamesDifferent + --suppress=unusedPrivateFunction + --suppress=constVariable + --suppress=missingOverride + --suppress=useStlAlgorithm + --suppress=constParameter + --suppress=noExplicitConstructor + --suppress=duplicateBreak + --suppress=variableScope + --suppress=unreadVariable + --suppress=shadowFunction --inconclusive) else() # if the user provides a CPPCHECK_OPTIONS with a template specified, it will override this template From 61a01f359bfcdb804db03f57963fb7e10769a1ee Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 13:21:59 -0300 Subject: [PATCH 04/23] Pin spdlog version --- conanfile.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conanfile.txt b/conanfile.txt index 596f09c3..6db9b400 100644 --- a/conanfile.txt +++ b/conanfile.txt @@ -2,7 +2,7 @@ cli11/2.3.2 # libfmt version will be automatically selected by spdlog fmt/10.2.1 -spdlog/[>=1.14.0] +spdlog/[=1.14.0] boost/1.82.0 magic_enum/0.9.2 gsl-lite/0.40.0 From 527bd42ac42b18542af9a85fbd6aba57092fce0f Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 13:22:25 -0300 Subject: [PATCH 05/23] Fix warning --- include/cloysterhpc/messagebus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/cloysterhpc/messagebus.h b/include/cloysterhpc/messagebus.h index 7812cd75..d52dbfc4 100644 --- a/include/cloysterhpc/messagebus.h +++ b/include/cloysterhpc/messagebus.h @@ -107,7 +107,7 @@ class MessageBusMethod { virtual MessageReply callMethod() = 0; public: - void addParams() + static void addParams() { // end case for the variadic template below } From 2de5a13a4970f4531d70c48e76a9ce00404e2e0c Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 13:25:03 -0300 Subject: [PATCH 06/23] Run clang-format --- include/cloysterhpc/dbus_client.h | 6 +++--- include/cloysterhpc/diskImage.h | 2 +- include/cloysterhpc/messagebus.h | 4 ++-- include/cloysterhpc/node.h | 3 ++- include/cloysterhpc/tests.h | 4 +++- src/answerfile.cpp | 6 +++--- src/cluster.cpp | 2 +- src/cpu.cpp | 4 ++-- src/diskImage.cpp | 2 +- src/main.cpp | 2 +- src/network.cpp | 10 +++++----- src/os.cpp | 4 ++-- src/services/locale.cpp | 2 +- src/services/xcat.cpp | 6 +++--- 14 files changed, 30 insertions(+), 27 deletions(-) diff --git a/include/cloysterhpc/dbus_client.h b/include/cloysterhpc/dbus_client.h index c8611d4f..95cfac37 100644 --- a/include/cloysterhpc/dbus_client.h +++ b/include/cloysterhpc/dbus_client.h @@ -27,9 +27,9 @@ class DBusClient : public MessageBus { public: DBusClient(std::string bus, std::string object) : m_proxy( - sdbus::createProxy(std::move(sdbus::createSystemBusConnection()), - std::move(sdbus::ServiceName { bus }), - std::move(sdbus::ObjectPath { object }))) + sdbus::createProxy(std::move(sdbus::createSystemBusConnection()), + std::move(sdbus::ServiceName { bus }), + std::move(sdbus::ObjectPath { object }))) { } diff --git a/include/cloysterhpc/diskImage.h b/include/cloysterhpc/diskImage.h index 09bf7caf..727f7c8e 100644 --- a/include/cloysterhpc/diskImage.h +++ b/include/cloysterhpc/diskImage.h @@ -38,7 +38,7 @@ class DiskImage { * @return True if the disk image is known, false otherwise. */ static bool isKnownImage(const std::filesystem::path& path); - + /** * @brief Checks if the given disk image has a verified checksum. * diff --git a/include/cloysterhpc/messagebus.h b/include/cloysterhpc/messagebus.h index d52dbfc4..5aaba949 100644 --- a/include/cloysterhpc/messagebus.h +++ b/include/cloysterhpc/messagebus.h @@ -14,8 +14,8 @@ * tell this to the user */ template -concept MessageReturnable = std::is_nothrow_default_constructible_v< - T> && std::is_nothrow_move_constructible_v; +concept MessageReturnable = std::is_nothrow_default_constructible_v + && std::is_nothrow_move_constructible_v; /** * A more or less "generic"-ish way to refer to a message reply diff --git a/include/cloysterhpc/node.h b/include/cloysterhpc/node.h index 46d4c569..6543d4be 100644 --- a/include/cloysterhpc/node.h +++ b/include/cloysterhpc/node.h @@ -55,7 +55,8 @@ class Node : public Server { const std::string& getMACAddress() const; void setMACAddress(const std::string& macAddress); const std::optional& getNodeRootPassword() const; - void setNodeRootPassword(const std::optional& nodeRootPassword); + void setNodeRootPassword( + const std::optional& nodeRootPassword); }; #endif // CLOYSTERHPC_NODE_H_ diff --git a/include/cloysterhpc/tests.h b/include/cloysterhpc/tests.h index dac2bd1e..6dad6560 100644 --- a/include/cloysterhpc/tests.h +++ b/include/cloysterhpc/tests.h @@ -10,7 +10,9 @@ #include namespace tests { -const std::filesystem::path sampleDirectory { TEST_SAMPLE_DIR }; // current_path corresponds to '//test' +const std::filesystem::path sampleDirectory { + TEST_SAMPLE_DIR +}; // current_path corresponds to '//test' } #endif // CLOYSTERHPC_TESTS_H_ diff --git a/src/answerfile.cpp b/src/answerfile.cpp index cbeebe55..85686d54 100644 --- a/src/answerfile.cpp +++ b/src/answerfile.cpp @@ -3,12 +3,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include -#include #include #include #include +#include +#include +#include #include #include #include diff --git a/src/cluster.cpp b/src/cluster.cpp index a8672ccd..42c9183f 100644 --- a/src/cluster.cpp +++ b/src/cluster.cpp @@ -684,7 +684,7 @@ void Cluster::fillData(const std::string& answerfilePath) setUpdateSystem(true); setProvisioner(Provisioner::xCAT); // BUG: Headnode OS may not be the same as the node OS - //m_headnode.setOS(nodeOS); + // m_headnode.setOS(nodeOS); for (const auto& tool : answerfile.getTools()) { tool->install(); diff --git a/src/cpu.cpp b/src/cpu.cpp index 1ff31e59..018dc79e 100644 --- a/src/cpu.cpp +++ b/src/cpu.cpp @@ -13,8 +13,8 @@ CPU::CPU() CPU::CPU( std::size_t sockets, std::size_t coresPerSocket, std::size_t threadsPerCore) : CPU(sockets, coresPerSocket * sockets, - threadsPerCore * coresPerSocket * sockets, coresPerSocket, - threadsPerCore) + threadsPerCore * coresPerSocket * sockets, coresPerSocket, + threadsPerCore) { } diff --git a/src/diskImage.cpp b/src/diskImage.cpp index 8ffdd5eb..e50d05b1 100644 --- a/src/diskImage.cpp +++ b/src/diskImage.cpp @@ -4,9 +4,9 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include #include #include -#include #include #include #include diff --git a/src/main.cpp b/src/main.cpp index cf74e038..1f103fe0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6,11 +6,11 @@ #include #include -#include #include #include #include #include +#include #include #include #include diff --git a/src/network.cpp b/src/network.cpp index b08e86d0..4cf09037 100644 --- a/src/network.cpp +++ b/src/network.cpp @@ -110,9 +110,9 @@ void Network::setAddress(const std::string& ip) address Network::fetchAddress(const std::string& interface) { - struct in_addr addr {}; - struct in_addr netmask {}; - struct in_addr network {}; + struct in_addr addr { }; + struct in_addr netmask { }; + struct in_addr network { }; if (inet_aton( Connection::fetchAddress(interface).to_string().c_str(), &addr) @@ -208,8 +208,8 @@ address Network::calculateAddress(const address& connectionAddress) "calculate the address")); } - struct in_addr ip_addr {}; - struct in_addr subnet_addr {}; + struct in_addr ip_addr { }; + struct in_addr subnet_addr { }; inet_aton(connectionAddress.to_string().c_str(), &ip_addr); inet_aton(m_subnetMask.to_string().c_str(), &subnet_addr); diff --git a/src/os.cpp b/src/os.cpp index dd76e70d..db9c518c 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -3,9 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include #include #include +#include #include #ifndef NDEBUG @@ -24,7 +24,7 @@ OS::OS() { - struct utsname system {}; + struct utsname system { }; uname(&system); setArch(system.machine); diff --git a/src/services/locale.cpp b/src/services/locale.cpp index db55da10..727c099e 100644 --- a/src/services/locale.cpp +++ b/src/services/locale.cpp @@ -3,8 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include #include +#include #include Locale::Locale() diff --git a/src/services/xcat.cpp b/src/services/xcat.cpp index 5ba87020..c9c9871c 100644 --- a/src/services/xcat.cpp +++ b/src/services/xcat.cpp @@ -52,9 +52,9 @@ void XCAT::patchInstall() void XCAT::setup() { setDHCPInterfaces(m_cluster->getHeadnode() - .getConnection(Network::Profile::Management) - .getInterface() - .value()); + .getConnection(Network::Profile::Management) + .getInterface() + .value()); setDomain(m_cluster->getDomainName()); } From b2c1bb63efc183d0ca5e47197a662f2c83049bd4 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 15:30:39 -0300 Subject: [PATCH 07/23] Handle copy to temporary file error --- src/repos.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/repos.cpp b/src/repos.cpp index 79dee95e..9a1839d6 100644 --- a/src/repos.cpp +++ b/src/repos.cpp @@ -207,7 +207,13 @@ void RepoManager::commitStatus() for (auto const& dir_entry : std::filesystem::directory_iterator { tmpdir }) { - std::filesystem::copy(dir_entry, "/etc/yum.repos.d"); + try { + std::filesystem::copy(dir_entry, "/etc/yum.repos.d"); + } catch (std::filesystem::filesystem_error const& ex) { + if (ex.code().message().starts_with("File exists")) { + continue; + } + } } std::vector to_enable; From 75c03aff7fb375bc079e586dc8fbe5e58739062b Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 15:33:25 -0300 Subject: [PATCH 08/23] Fix message in CMakeLists.txt --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a3b76788..00ed58a4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,7 +25,7 @@ endif() # Set a default build type if none was specified # This is required for Conan 2.0 to work properly if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) - message(STATUS "Setting build type to 'Release' as none was specified.") + message(STATUS "Setting build type to 'Debug' as none was specified.") set(CMAKE_BUILD_TYPE Debug CACHE STRING "Choose the type of build." FORCE) From a892e849f112c601c3e7dfdbbc5e9cc87acf58d6 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 15:49:18 -0300 Subject: [PATCH 09/23] Handle error in answerfile minor version --- include/cloysterhpc/services/log.h | 5 +++++ src/os.cpp | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/include/cloysterhpc/services/log.h b/include/cloysterhpc/services/log.h index 17cdc32a..d59bb71b 100644 --- a/include/cloysterhpc/services/log.h +++ b/include/cloysterhpc/services/log.h @@ -24,6 +24,11 @@ * * @param __VA_ARGS__ The message to log and its format arguments. */ +#define LOG_ABORT(...) \ + if (spdlog::get(productName) != nullptr) { \ + spdlog::get(productName)->critical(__VA_ARGS__); \ + std::exit(-1); \ + } #define LOG_CRITICAL(...) \ if (spdlog::get(productName) != nullptr) { \ spdlog::get(productName)->critical(__VA_ARGS__); \ diff --git a/src/os.cpp b/src/os.cpp index db9c518c..2efdd61e 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -204,6 +204,11 @@ void OS::setVersion(const std::string& version) setMajorVersion(static_cast( std::stoul(version.substr(0, version.find('.'))))); + + if (version.find('.') != std::string::npos) { + LOG_ABORT("system version (in the answerfile.yml) must follow the format M.N, where M is the major version number and N is the minor version number."); + } + setMinorVersion( static_cast(stoul(version.substr(version.find('.') + 1)))); } From cc06a6ce3e3956327a47a8aefbef99ba6881b736 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 15:49:50 -0300 Subject: [PATCH 10/23] Fix routing error disabling the VM network --- src/services/shell.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/services/shell.cpp b/src/services/shell.cpp index a5c405d6..2701cd4a 100644 --- a/src/services/shell.cpp +++ b/src/services/shell.cpp @@ -204,7 +204,8 @@ void Shell::configureNetworks(const std::list& connections) runCommand( fmt::format("nmcli connection add con-name {} ifname {} type {} " "mtu {} ipv4.method manual ipv4.address {}/{} " - "ipv4.gateway {} ipv4.dns \"{}\" " + "ipv4.dns \"{}\" " + // "ipv4.gateway {} ipv4.dns \"{}\" " "ipv4.dns-search {} ipv6.method disabled", magic_enum::enum_name(connection.getNetwork()->getProfile()), interface, @@ -212,7 +213,7 @@ void Shell::configureNetworks(const std::list& connections) connection.getMTU(), connection.getAddress().to_string(), connection.getNetwork()->cidr.at( connection.getNetwork()->getSubnetMask().to_string()), - connection.getNetwork()->getGateway().to_string(), + //connection.getNetwork()->getGateway().to_string(), fmt::join(formattedNameservers, " "), connection.getNetwork()->getDomainName())); @@ -222,7 +223,8 @@ void Shell::configureNetworks(const std::list& connections) */ std::this_thread::sleep_for(std::chrono::milliseconds(200)); - runCommand(fmt::format("nmcli device connect {}", interface)); + // Breaking my ssh connection during development + // runCommand(fmt::format("nmcli device connect {}", interface)); } disableNetworkManagerDNSOverride(); From 1f55fdfdf132e6e8b341f54ead1acce816cae249 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 15:59:22 -0300 Subject: [PATCH 11/23] Enable Managed network again --- src/services/shell.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/shell.cpp b/src/services/shell.cpp index 2701cd4a..0a0ec71a 100644 --- a/src/services/shell.cpp +++ b/src/services/shell.cpp @@ -224,7 +224,7 @@ void Shell::configureNetworks(const std::list& connections) std::this_thread::sleep_for(std::chrono::milliseconds(200)); // Breaking my ssh connection during development - // runCommand(fmt::format("nmcli device connect {}", interface)); + runCommand(fmt::format("nmcli device connect {}", interface)); } disableNetworkManagerDNSOverride(); From 871219806a527e01862782b2fa16d7000020ccbe Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Thu, 9 Jan 2025 16:55:33 -0300 Subject: [PATCH 12/23] Fix answerfile version handling --- include/cloysterhpc/services/log.h | 12 ++++++------ src/os.cpp | 7 +++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/include/cloysterhpc/services/log.h b/include/cloysterhpc/services/log.h index d59bb71b..d8648faa 100644 --- a/include/cloysterhpc/services/log.h +++ b/include/cloysterhpc/services/log.h @@ -24,11 +24,6 @@ * * @param __VA_ARGS__ The message to log and its format arguments. */ -#define LOG_ABORT(...) \ - if (spdlog::get(productName) != nullptr) { \ - spdlog::get(productName)->critical(__VA_ARGS__); \ - std::exit(-1); \ - } #define LOG_CRITICAL(...) \ if (spdlog::get(productName) != nullptr) { \ spdlog::get(productName)->critical(__VA_ARGS__); \ @@ -45,7 +40,12 @@ if (spdlog::get(productName) != nullptr) { \ spdlog::get(productName)->info(__VA_ARGS__); \ } - +#define LOG_ABORT_IF(cnd, msg) \ + if ((cnd) && spdlog::get(productName) != nullptr) { \ + LOG_CRITICAL("ABORT - {}\n\t{}\n\tin file: {}\n\ton line: {}", #cnd, \ + msg, __FILE__, __LINE__); \ + LOG_BREAK; \ + } // Available only with DEBUG builds #ifndef NDEBUG #define LOG_DEBUG(...) \ diff --git a/src/os.cpp b/src/os.cpp index 2efdd61e..1cbfa89e 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -204,10 +204,9 @@ void OS::setVersion(const std::string& version) setMajorVersion(static_cast( std::stoul(version.substr(0, version.find('.'))))); - - if (version.find('.') != std::string::npos) { - LOG_ABORT("system version (in the answerfile.yml) must follow the format M.N, where M is the major version number and N is the minor version number."); - } + LOG_ABORT_IF(version.find('.') == std::string::npos, + "system version (in the answerfile.yml) must follow the format M.N, " + "where M is the major version number and N is the minor version number."); setMinorVersion( static_cast(stoul(version.substr(version.find('.') + 1)))); From fb6231ad4c27f59827de601b5e4058563b025199 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Fri, 10 Jan 2025 15:37:35 -0300 Subject: [PATCH 13/23] Fix copyFile implementation --- include/cloysterhpc/functions.h | 8 ++++++++ include/cloysterhpc/services/log.h | 7 +------ src/functions.cpp | 20 ++++++++++++++++++++ src/os.cpp | 13 ++++++++++--- src/repos.cpp | 8 +------- 5 files changed, 40 insertions(+), 16 deletions(-) diff --git a/include/cloysterhpc/functions.h b/include/cloysterhpc/functions.h index 31bdc533..57efa82b 100644 --- a/include/cloysterhpc/functions.h +++ b/include/cloysterhpc/functions.h @@ -156,6 +156,14 @@ std::string getCurrentTimestamp(); std::string findAndReplace(const std::string_view& source, const std::string_view& find, const std::string_view& replace); +/** + * @brief Copies a file, ignore if it exists + * + * @param dir_entry + * @param string The string to add to the file. + */ +void copyFile(std::filesystem::path from, std::filesystem::path to); + } /* namespace cloyster */ #endif // CLOYSTERHPC_FUNCTIONS_H_ diff --git a/include/cloysterhpc/services/log.h b/include/cloysterhpc/services/log.h index d8648faa..17cdc32a 100644 --- a/include/cloysterhpc/services/log.h +++ b/include/cloysterhpc/services/log.h @@ -40,12 +40,7 @@ if (spdlog::get(productName) != nullptr) { \ spdlog::get(productName)->info(__VA_ARGS__); \ } -#define LOG_ABORT_IF(cnd, msg) \ - if ((cnd) && spdlog::get(productName) != nullptr) { \ - LOG_CRITICAL("ABORT - {}\n\t{}\n\tin file: {}\n\ton line: {}", #cnd, \ - msg, __FILE__, __LINE__); \ - LOG_BREAK; \ - } + // Available only with DEBUG builds #ifndef NDEBUG #define LOG_DEBUG(...) \ diff --git a/src/functions.cpp b/src/functions.cpp index 0c661f2b..b5026955 100644 --- a/src/functions.cpp +++ b/src/functions.cpp @@ -330,4 +330,24 @@ std::string findAndReplace(const std::string_view& source, return result; } +/// Copy a file, ignore if file exists +void copyFile(std::filesystem::path from, std::filesystem::path to) +{ + if (cloyster::dryRun) { + LOG_INFO("Would copy file {}, to {}", from.string(), to.string()) + return; + } + + try { + std::filesystem::copy(from, to); + } catch (std::filesystem::filesystem_error const& ex) { + if (ex.code().default_error_condition() == std::errc::file_exists) { + LOG_WARN("File {} exists, skiping copying", to.string()); + return; + } + + throw ex; + } +} + } // namespace cloyster diff --git a/src/os.cpp b/src/os.cpp index 1cbfa89e..0956b619 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -204,9 +204,16 @@ void OS::setVersion(const std::string& version) setMajorVersion(static_cast( std::stoul(version.substr(0, version.find('.'))))); - LOG_ABORT_IF(version.find('.') == std::string::npos, - "system version (in the answerfile.yml) must follow the format M.N, " - "where M is the major version number and N is the minor version number."); + // FIXME: Read the value from /etc/os-release intead of + // expecting it to be explicit in answerfile.ini + + // We expect the system.version in the answerfile + // to be in the format M.N, and abort if it is not valid + if (version.find('.') == std::string::npos) { + LOG_CRITICAL( + "Unexpected value for system.version (in asnwerfile.ini). Expected M.N format, e.g., 9.5, value found instead: {}", version); + std::exit(-1); + } setMinorVersion( static_cast(stoul(version.substr(version.find('.') + 1)))); diff --git a/src/repos.cpp b/src/repos.cpp index 9a1839d6..df230f10 100644 --- a/src/repos.cpp +++ b/src/repos.cpp @@ -207,13 +207,7 @@ void RepoManager::commitStatus() for (auto const& dir_entry : std::filesystem::directory_iterator { tmpdir }) { - try { - std::filesystem::copy(dir_entry, "/etc/yum.repos.d"); - } catch (std::filesystem::filesystem_error const& ex) { - if (ex.code().message().starts_with("File exists")) { - continue; - } - } + cloyster::copyFile(dir_entry, "/etc/yum.repos.d"); } std::vector to_enable; From 3dfb88992acf56cff5275dcc1f1931a8874ada37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Fri, 10 Jan 2025 19:07:08 -0300 Subject: [PATCH 14/23] Fix typos, change variables names and fix throw. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix typos in the documentation. * Fix typos on LOG messages. * Change variable names to be more accurate with the context. * Rethrow should also follow the golden rule of: throw by value and catch by reference. Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com> --- include/cloysterhpc/functions.h | 8 ++++---- src/functions.cpp | 15 ++++++++------- src/os.cpp | 11 ++++++----- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/include/cloysterhpc/functions.h b/include/cloysterhpc/functions.h index 57efa82b..0f20cd33 100644 --- a/include/cloysterhpc/functions.h +++ b/include/cloysterhpc/functions.h @@ -157,12 +157,12 @@ std::string findAndReplace(const std::string_view& source, const std::string_view& find, const std::string_view& replace); /** - * @brief Copies a file, ignore if it exists + * @brief Copies a file, skip copying if it exists * - * @param dir_entry - * @param string The string to add to the file. + * @param source The source file to copy. + * @param destination The path where the source file will be copied. */ -void copyFile(std::filesystem::path from, std::filesystem::path to); +void copyFile(std::filesystem::path source, std::filesystem::path destination); } /* namespace cloyster */ diff --git a/src/functions.cpp b/src/functions.cpp index b5026955..178091f7 100644 --- a/src/functions.cpp +++ b/src/functions.cpp @@ -331,22 +331,23 @@ std::string findAndReplace(const std::string_view& source, } /// Copy a file, ignore if file exists -void copyFile(std::filesystem::path from, std::filesystem::path to) +void copyFile(std::filesystem::path source, std::filesystem::path destination) { if (cloyster::dryRun) { - LOG_INFO("Would copy file {}, to {}", from.string(), to.string()) + LOG_INFO( + "Would copy file {} to {}", source.string(), destination.string()) return; } try { - std::filesystem::copy(from, to); - } catch (std::filesystem::filesystem_error const& ex) { + std::filesystem::copy(source, destination); + } catch (const std::filesystem::filesystem_error& ex) { if (ex.code().default_error_condition() == std::errc::file_exists) { - LOG_WARN("File {} exists, skiping copying", to.string()); + LOG_WARN("File {} already exists, skip copying", source.string()); return; } - - throw ex; + + throw; } } diff --git a/src/os.cpp b/src/os.cpp index 0956b619..24017c6e 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -24,7 +24,7 @@ OS::OS() { - struct utsname system { }; + struct utsname system {}; uname(&system); setArch(system.machine); @@ -204,14 +204,15 @@ void OS::setVersion(const std::string& version) setMajorVersion(static_cast( std::stoul(version.substr(0, version.find('.'))))); - // FIXME: Read the value from /etc/os-release intead of + // FIXME: Read the value from the ISO file intead of // expecting it to be explicit in answerfile.ini - + // We expect the system.version in the answerfile // to be in the format M.N, and abort if it is not valid if (version.find('.') == std::string::npos) { - LOG_CRITICAL( - "Unexpected value for system.version (in asnwerfile.ini). Expected M.N format, e.g., 9.5, value found instead: {}", version); + LOG_CRITICAL("Unexpected value for system.version (in answerfile.ini). " + "Expected M.N format, e.g., 9.5. Value found instead: {}", + version); std::exit(-1); } From 0bfe8ee9ee672570d8e8813d47e1d82b6bbe932d Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Fri, 10 Jan 2025 19:24:43 -0300 Subject: [PATCH 15/23] Clearning more warnings --- cmake/CompilerWarnings.cmake | 1 + include/cloysterhpc/answerfile.h | 1 - include/cloysterhpc/dbus_client.h | 4 ++-- include/cloysterhpc/view/newt.h | 2 +- src/answerfile.cpp | 2 +- src/hardware.cpp | 10 +++++----- src/presenter/PresenterNodesOperationalSystem.cpp | 3 ++- 7 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cmake/CompilerWarnings.cmake b/cmake/CompilerWarnings.cmake index e08007e7..d5ba22aa 100644 --- a/cmake/CompilerWarnings.cmake +++ b/cmake/CompilerWarnings.cmake @@ -70,6 +70,7 @@ function( -Wno-unused-variable -Wno-unused-function -Wno-unused-parameter + -Wno-shadow ) endif() diff --git a/include/cloysterhpc/answerfile.h b/include/cloysterhpc/answerfile.h index 6f1a0018..b38d53d5 100644 --- a/include/cloysterhpc/answerfile.h +++ b/include/cloysterhpc/answerfile.h @@ -12,7 +12,6 @@ #include #include #include -#include #include using boost::asio::ip::address; diff --git a/include/cloysterhpc/dbus_client.h b/include/cloysterhpc/dbus_client.h index 95cfac37..d147bd23 100644 --- a/include/cloysterhpc/dbus_client.h +++ b/include/cloysterhpc/dbus_client.h @@ -28,8 +28,8 @@ class DBusClient : public MessageBus { DBusClient(std::string bus, std::string object) : m_proxy( sdbus::createProxy(std::move(sdbus::createSystemBusConnection()), - std::move(sdbus::ServiceName { bus }), - std::move(sdbus::ObjectPath { object }))) + sdbus::ServiceName { bus }, + sdbus::ObjectPath { object })) { } diff --git a/include/cloysterhpc/view/newt.h b/include/cloysterhpc/view/newt.h index b34c257b..cc256e27 100644 --- a/include/cloysterhpc/view/newt.h +++ b/include/cloysterhpc/view/newt.h @@ -200,7 +200,7 @@ class Newt : public View { break; } case 4: // remove - if (selector >= 0 && selector < cStrings.size()) { + if (selector >= 0 && static_cast(selector) < cStrings.size()) { tempStrings.erase(tempStrings.begin() + selector); cStrings = convertToNewtList(tempStrings); } diff --git a/src/answerfile.cpp b/src/answerfile.cpp index 85686d54..aeaeac68 100644 --- a/src/answerfile.cpp +++ b/src/answerfile.cpp @@ -19,7 +19,7 @@ AnswerFile::AnswerFile(const std::filesystem::path& path) loadFile(m_path); } -AnswerFile::AnswerFile() {}; +//AnswerFile::AnswerFile() {}; void AnswerFile::loadFile(const std::filesystem::path& path) { diff --git a/src/hardware.cpp b/src/hardware.cpp index 1fccfd5a..3cfdf06a 100644 --- a/src/hardware.cpp +++ b/src/hardware.cpp @@ -20,11 +20,11 @@ hwinfo::Memory Hardware::getMemory() const { return m_memory; } hwinfo::OS Hardware::getOS() const { return m_os; } -Hardware::Hardware() +Hardware::Hardware() : + m_cpu(hwinfo::getAllCPUs()), + m_disk(hwinfo::getAllDisks()), + m_gpu(hwinfo::getAllGPUs()) { - m_cpu = hwinfo::getAllCPUs(); - m_disk = hwinfo::getAllDisks(); - m_gpu = hwinfo::getAllGPUs(); } void Hardware::printOverview() @@ -92,4 +92,4 @@ int64_t Hardware::convertByteToGB(int64_t bytes) { return bytes / 1073741824; // 1073741824 is the same as static_cast(std::pow(1024,3)) -} \ No newline at end of file +} diff --git a/src/presenter/PresenterNodesOperationalSystem.cpp b/src/presenter/PresenterNodesOperationalSystem.cpp index 9106d6ed..35bc891b 100644 --- a/src/presenter/PresenterNodesOperationalSystem.cpp +++ b/src/presenter/PresenterNodesOperationalSystem.cpp @@ -105,8 +105,9 @@ PresenterNodesOperationalSystem::selectVersion(OS::Distro distro) findit != versions.end()) { const auto currentver = std::distance(versions.begin(), findit); const auto& vdistro = version_map.at(distro); + LOG_ASSERT(currentver >= 0, "currentver is negative"); return std::make_optional( - vdistro[currentver]); + vdistro[static_cast(currentver)]); } return std::nullopt; From 121594c30e3ad37c9310299b1e2f5055c9a04831 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Fri, 10 Jan 2025 23:17:51 -0300 Subject: [PATCH 16/23] Fix the last warnings --- include/cloysterhpc/services/IService.h | 6 ++++-- src/cpu.cpp | 5 +++++ src/services/IService.cpp | 24 +++++++++++++++--------- src/services/xcat.cpp | 4 ++-- src/tempdir.cpp | 2 -- src/view/newtListMenu.cpp | 2 +- 6 files changed, 27 insertions(+), 16 deletions(-) diff --git a/include/cloysterhpc/services/IService.h b/include/cloysterhpc/services/IService.h index cb68f3db..dcb7bdea 100644 --- a/include/cloysterhpc/services/IService.h +++ b/include/cloysterhpc/services/IService.h @@ -31,7 +31,7 @@ class IService { bool handleException(const sdbus::Error& e, const std::string_view fn); template - sdbus::ObjectPath callObjectFunction( + std::optional callObjectFunction( const std::string function, Ts... params) { try { @@ -42,11 +42,12 @@ class IService { } catch (sdbus::Error& e) { if (!handleException(e, function)) throw; + return {}; } } template - MessageReply callObjectFunctionArray( + std::optional callObjectFunctionArray( const std::string function, Ts... params) { try { @@ -58,6 +59,7 @@ class IService { } catch (sdbus::Error& e) { if (!handleException(e, function)) throw; + return {}; } } diff --git a/src/cpu.cpp b/src/cpu.cpp index 018dc79e..a09893ce 100644 --- a/src/cpu.cpp +++ b/src/cpu.cpp @@ -6,6 +6,11 @@ #include CPU::CPU() + : m_sockets(0) + , m_cores(0) + , m_threads(0) + , m_coresPerSocket(0) + , m_threadsPerCore(0) { // TODO: Implement a CPU detection algorithm } diff --git a/src/services/IService.cpp b/src/services/IService.cpp index 79684c49..d4b52f85 100644 --- a/src/services/IService.cpp +++ b/src/services/IService.cpp @@ -34,9 +34,12 @@ void IService::enable() LOG_TRACE("service: enabling {}", m_name); - auto ret = callObjectFunctionArray("EnableUnitFiles", false, true) - .getPair(); - const auto& [_install, retvec] = ret; + auto ret = callObjectFunctionArray("EnableUnitFiles", false, true); + if (!ret.has_value()) { + LOG_ERROR("callObjectFunctionArray returned none for service {}", m_name); + return; + } + const auto& [_install, retvec] = (*ret).getPair(); if (retvec.empty()) { LOG_WARN("service {} already enabled", m_name); @@ -52,10 +55,13 @@ void IService::disable() LOG_TRACE("service: disabling {}", m_name); - auto ret = callObjectFunctionArray("DisableUnitFiles", false, true) - .get(); + auto ret = callObjectFunctionArray("DisableUnitFiles", false, true); + if (!ret.has_value()) { + LOG_ERROR("callObjectFunctionArray returned none, service {}", m_name); + return; + }; - if (ret.empty()) { + if ((*ret).get().empty()) { LOG_WARN("service {} already disabled", m_name); } } @@ -68,7 +74,7 @@ void IService::start() } LOG_TRACE("service: starting {}", m_name); - (void)callObjectFunction("StartUnit", "replace"); + callObjectFunction("StartUnit", "replace"); } void IService::restart() @@ -79,7 +85,7 @@ void IService::restart() } LOG_TRACE("service: restarting {}", m_name); - (void)callObjectFunction("RestartUnit", "replace"); + callObjectFunction("RestartUnit", "replace"); } void IService::stop() @@ -90,5 +96,5 @@ void IService::stop() } LOG_TRACE("service: stopping {}", m_name); - (void)callObjectFunction("StopUnit", "replace"); + callObjectFunction("StopUnit", "replace"); } diff --git a/src/services/xcat.cpp b/src/services/xcat.cpp index c9c9871c..87790cee 100644 --- a/src/services/xcat.cpp +++ b/src/services/xcat.cpp @@ -42,9 +42,9 @@ void XCAT::patchInstall() /* Required for EL 9.5 * Upstream PR: https://github.com/xcat2/xcat-core/pull/7489 */ - cloyster::runCommand("sed -i \"s/\-extensions\ usr_cert\ //g\" " + cloyster::runCommand("sed -i \"s/-extensions usr_cert //g\" " "/opt/xcat/share/xcat/scripts/setup-local-client.sh"); - cloyster::runCommand("sed -i \"s/\-extensions\ server //g\" " + cloyster::runCommand("sed -i \"s/-extensions server //g\" " "/opt/xcat/share/xcat/scripts/setup-server-cert.sh"); cloyster::runCommand("xcatconfig -f"); } diff --git a/src/tempdir.cpp b/src/tempdir.cpp index 8a492ae9..ee0baa59 100644 --- a/src/tempdir.cpp +++ b/src/tempdir.cpp @@ -19,7 +19,6 @@ static char value_to_char(std::uint64_t value) #define NOSONAR(code) code -#pragma warning disable S2245 static std::filesystem::path create_temporary_filename() { static std::random_device dev; @@ -35,7 +34,6 @@ static std::filesystem::path create_temporary_filename() std::string basename = fmt::format("temp{}", values.data()); return std::filesystem::path { "/tmp" } / basename; // NOSONAR } -#pragma warning restore S2245 TempDir::TempDir() { diff --git a/src/view/newtListMenu.cpp b/src/view/newtListMenu.cpp index a907421a..fd903cfc 100644 --- a/src/view/newtListMenu.cpp +++ b/src/view/newtListMenu.cpp @@ -32,7 +32,7 @@ std::pair> Newt::multipleSelectionMenu( auto* list = newtListbox(1, 3, m_maxListHeight, NEWT_FLAG_MULTIPLE); for (const auto& [key, item, enabled] : items) { - newtListboxAppendEntry(list, item.c_str(), (void*)key.c_str()); + newtListboxAppendEntry(list, item.c_str(), key.c_str()); if (enabled) newtListboxSelectItem(list, key.c_str(), NEWT_FLAGS_SET); From 5c3012ab6c780d948655b6ff94de7ee033a59fc9 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Sat, 11 Jan 2025 00:05:54 -0300 Subject: [PATCH 17/23] Remove std::exit call, and raise an exception instead --- src/os.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/os.cpp b/src/os.cpp index 24017c6e..6218e947 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -210,10 +210,9 @@ void OS::setVersion(const std::string& version) // We expect the system.version in the answerfile // to be in the format M.N, and abort if it is not valid if (version.find('.') == std::string::npos) { - LOG_CRITICAL("Unexpected value for system.version (in answerfile.ini). " - "Expected M.N format, e.g., 9.5. Value found instead: {}", - version); - std::exit(-1); + throw std::runtime_error(fmt::format( + "Unexpected value for system.version (in answerfile.ini). " + "Expected M.N format, e.g., 9.5. Value found instead: {}", version)); } setMinorVersion( From 66ce55b62425851dd616de4ea61894375ab8c700 Mon Sep 17 00:00:00 2001 From: Daniel Hilst Date: Mon, 13 Jan 2025 09:52:41 -0300 Subject: [PATCH 18/23] Run clang-format --- include/cloysterhpc/dbus_client.h | 3 +-- include/cloysterhpc/view/newt.h | 3 ++- src/answerfile.cpp | 2 +- src/hardware.cpp | 8 ++++---- src/os.cpp | 5 +++-- src/services/IService.cpp | 3 ++- src/services/shell.cpp | 2 +- 7 files changed, 14 insertions(+), 12 deletions(-) diff --git a/include/cloysterhpc/dbus_client.h b/include/cloysterhpc/dbus_client.h index d147bd23..d69602d7 100644 --- a/include/cloysterhpc/dbus_client.h +++ b/include/cloysterhpc/dbus_client.h @@ -28,8 +28,7 @@ class DBusClient : public MessageBus { DBusClient(std::string bus, std::string object) : m_proxy( sdbus::createProxy(std::move(sdbus::createSystemBusConnection()), - sdbus::ServiceName { bus }, - sdbus::ObjectPath { object })) + sdbus::ServiceName { bus }, sdbus::ObjectPath { object })) { } diff --git a/include/cloysterhpc/view/newt.h b/include/cloysterhpc/view/newt.h index cc256e27..5394f388 100644 --- a/include/cloysterhpc/view/newt.h +++ b/include/cloysterhpc/view/newt.h @@ -200,7 +200,8 @@ class Newt : public View { break; } case 4: // remove - if (selector >= 0 && static_cast(selector) < cStrings.size()) { + if (selector >= 0 + && static_cast(selector) < cStrings.size()) { tempStrings.erase(tempStrings.begin() + selector); cStrings = convertToNewtList(tempStrings); } diff --git a/src/answerfile.cpp b/src/answerfile.cpp index aeaeac68..1cb7e30f 100644 --- a/src/answerfile.cpp +++ b/src/answerfile.cpp @@ -19,7 +19,7 @@ AnswerFile::AnswerFile(const std::filesystem::path& path) loadFile(m_path); } -//AnswerFile::AnswerFile() {}; +// AnswerFile::AnswerFile() {}; void AnswerFile::loadFile(const std::filesystem::path& path) { diff --git a/src/hardware.cpp b/src/hardware.cpp index 3cfdf06a..41786dee 100644 --- a/src/hardware.cpp +++ b/src/hardware.cpp @@ -20,10 +20,10 @@ hwinfo::Memory Hardware::getMemory() const { return m_memory; } hwinfo::OS Hardware::getOS() const { return m_os; } -Hardware::Hardware() : - m_cpu(hwinfo::getAllCPUs()), - m_disk(hwinfo::getAllDisks()), - m_gpu(hwinfo::getAllGPUs()) +Hardware::Hardware() + : m_cpu(hwinfo::getAllCPUs()) + , m_disk(hwinfo::getAllDisks()) + , m_gpu(hwinfo::getAllGPUs()) { } diff --git a/src/os.cpp b/src/os.cpp index 6218e947..92f346ee 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -24,7 +24,7 @@ OS::OS() { - struct utsname system {}; + struct utsname system { }; uname(&system); setArch(system.machine); @@ -212,7 +212,8 @@ void OS::setVersion(const std::string& version) if (version.find('.') == std::string::npos) { throw std::runtime_error(fmt::format( "Unexpected value for system.version (in answerfile.ini). " - "Expected M.N format, e.g., 9.5. Value found instead: {}", version)); + "Expected M.N format, e.g., 9.5. Value found instead: {}", + version)); } setMinorVersion( diff --git a/src/services/IService.cpp b/src/services/IService.cpp index d4b52f85..d2e7efad 100644 --- a/src/services/IService.cpp +++ b/src/services/IService.cpp @@ -36,7 +36,8 @@ void IService::enable() auto ret = callObjectFunctionArray("EnableUnitFiles", false, true); if (!ret.has_value()) { - LOG_ERROR("callObjectFunctionArray returned none for service {}", m_name); + LOG_ERROR( + "callObjectFunctionArray returned none for service {}", m_name); return; } const auto& [_install, retvec] = (*ret).getPair(); diff --git a/src/services/shell.cpp b/src/services/shell.cpp index 0a0ec71a..e0d3b01e 100644 --- a/src/services/shell.cpp +++ b/src/services/shell.cpp @@ -213,7 +213,7 @@ void Shell::configureNetworks(const std::list& connections) connection.getMTU(), connection.getAddress().to_string(), connection.getNetwork()->cidr.at( connection.getNetwork()->getSubnetMask().to_string()), - //connection.getNetwork()->getGateway().to_string(), + // connection.getNetwork()->getGateway().to_string(), fmt::join(formattedNameservers, " "), connection.getNetwork()->getDomainName())); From a659471182bee08afc5f49e4782d672ca74d3ca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 13 Jan 2025 12:22:19 -0300 Subject: [PATCH 19/23] Added version ranges to conanfile.txt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change will keep the project more reliable while still getting updates from upstream on minor versions. Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com> --- conanfile.txt | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/conanfile.txt b/conanfile.txt index 6db9b400..85b00f62 100644 --- a/conanfile.txt +++ b/conanfile.txt @@ -1,14 +1,13 @@ [requires] -cli11/2.3.2 -# libfmt version will be automatically selected by spdlog -fmt/10.2.1 -spdlog/[=1.14.0] -boost/1.82.0 -magic_enum/0.9.2 -gsl-lite/0.40.0 -doctest/2.4.11 -cryptopp/8.9.0 -sdbus-cpp/2.0.0 +cli11/[>=2.4.0 <2.5.0] +spdlog/[>=1.14.0 <1.15.0] +fmt/[>=10.0.0 <12.0.0] +boost/[>=1.83.0 <1.84.0] +magic_enum/[>=0.9.0 <0.10.0] +gsl-lite/[>=0.41.0 <0.42.0] +doctest/[>=2.4.0 <2.5.0] +cryptopp/[>=8.9.0 <8.10.0] +sdbus-cpp/[>=2.0.0 <2.1.0] [layout] cmake_layout From 69d6dace58039afdad86e94b8ae7f66adf4b748d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 13 Jan 2025 12:32:30 -0300 Subject: [PATCH 20/23] Tries to update the clang-format.yml workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This workflow is broken, let's see if an update will fix it. Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com> --- .github/workflows/clang-format-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang-format-check.yml b/.github/workflows/clang-format-check.yml index 81a8e14f..122bdcb5 100644 --- a/.github/workflows/clang-format-check.yml +++ b/.github/workflows/clang-format-check.yml @@ -7,7 +7,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: Run clang-format style check for C/C++ programs. - uses: jidicula/clang-format-action@v4.6.2 + uses: jidicula/clang-format-action@v4.14.0 with: clang-format-version: '14' check-path: 'src' From 39ba2c14872fd363eb8d72d8c28aad69a6f576d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 13 Jan 2025 13:00:07 -0300 Subject: [PATCH 21/23] Try to fix clang-format again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com> --- .github/workflows/clang-format-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang-format-check.yml b/.github/workflows/clang-format-check.yml index 122bdcb5..1c8ad931 100644 --- a/.github/workflows/clang-format-check.yml +++ b/.github/workflows/clang-format-check.yml @@ -9,6 +9,6 @@ jobs: - name: Run clang-format style check for C/C++ programs. uses: jidicula/clang-format-action@v4.14.0 with: - clang-format-version: '14' + clang-format-version: '19' check-path: 'src' From bb0eae7178f77fca7700a1d4cf1997062474b828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 13 Jan 2025 13:12:08 -0300 Subject: [PATCH 22/23] Last try to fix clang-format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com> --- CMakeLists.txt | 118 +++++++++++++++++++-------------------- CMakePresets.json | 4 +- fuzz_test/CMakeLists.txt | 16 +++--- src/CMakeLists.txt | 48 ++++++++-------- src/cluster.cpp | 12 ++-- src/os.cpp | 2 +- test/CMakeLists.txt | 102 ++++++++++++++++----------------- 7 files changed, 151 insertions(+), 151 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 00ed58a4..2eb9d6af 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,8 +8,8 @@ cmake_minimum_required(VERSION 3.26.5) # Only set the cxx_standard if it is not set by someone else if (NOT DEFINED CMAKE_CXX_STANDARD) - set(CMAKE_CXX_STANDARD 23) -endif() + set(CMAKE_CXX_STANDARD 23) +endif () # strongly encouraged to enable this globally to avoid conflicts between # -Wpedantic being enabled and -std=c++20 and -std=gnu++20 for example @@ -18,34 +18,34 @@ set(CMAKE_CXX_EXTENSIONS OFF) # Enable Conan 2.0 support option(cloysterhpc_ENABLE_CONAN "Use Conan 2 to manage dependencies" ON) -if(cloysterhpc_ENABLE_CONAN) - set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES cmake/conan_provider.cmake) -endif() +if (cloysterhpc_ENABLE_CONAN) + set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES cmake/conan_provider.cmake) +endif () # Set a default build type if none was specified # This is required for Conan 2.0 to work properly -if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) - message(STATUS "Setting build type to 'Debug' as none was specified.") - set(CMAKE_BUILD_TYPE - Debug - CACHE STRING "Choose the type of build." FORCE) - # Set the possible values of build type for cmake-gui, ccmake - set_property( - CACHE CMAKE_BUILD_TYPE - PROPERTY STRINGS - "Debug" - "Release" - "MinSizeRel" - "RelWithDebInfo") -endif() +if (NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) + message(STATUS "Setting build type to 'Debug' as none was specified.") + set(CMAKE_BUILD_TYPE + Debug + CACHE STRING "Choose the type of build." FORCE) + # Set the possible values of build type for cmake-gui, ccmake + set_property( + CACHE CMAKE_BUILD_TYPE + PROPERTY STRINGS + "Debug" + "Release" + "MinSizeRel" + "RelWithDebInfo") +endif () # Set the project name and language project( - CloysterHPC - VERSION 0.1.1 - DESCRIPTION "Cloyster HPC is a turnkey HPC cluster solution with an user-friendly installer" - HOMEPAGE_URL "https://github.com/viniciusferrao/cloysterhpc" - LANGUAGES CXX C) + CloysterHPC + VERSION 0.1.1 + DESCRIPTION "Cloyster HPC is a turnkey HPC cluster solution with an user-friendly installer" + HOMEPAGE_URL "https://github.com/viniciusferrao/cloysterhpc" + LANGUAGES CXX C) include(cmake/PreventInSourceBuilds.cmake) include(ProjectOptions.cmake) @@ -71,16 +71,16 @@ execute_process( ) # Check if GIT_SHA is empty and set to "Unknown" if it is -if("${GIT_SHA}" STREQUAL "") - set(GIT_SHA "Unknown") -endif() +if ("${GIT_SHA}" STREQUAL "") + set(GIT_SHA "Unknown") +endif () # Get the short SHA (first 8 characters) if GIT_SHA is not "Unknown" -if(NOT "${GIT_SHA}" STREQUAL "Unknown") - string(SUBSTRING "${GIT_SHA}" 0 8 GIT_SHORT_SHA) -else() - set(GIT_SHORT_SHA "Unknown") -endif() +if (NOT "${GIT_SHA}" STREQUAL "Unknown") + string(SUBSTRING "${GIT_SHA}" 0 8 GIT_SHORT_SHA) +else () + set(GIT_SHORT_SHA "Unknown") +endif () # Cache the variables for future use set(GIT_SHA "${GIT_SHA}" CACHE STRING "SHA this build was generated from") @@ -105,51 +105,51 @@ add_subdirectory(configured_files) add_subdirectory(src) # Don't even look at tests if we're not top level -if(NOT PROJECT_IS_TOP_LEVEL) - return() -endif() +if (NOT PROJECT_IS_TOP_LEVEL) + return() +endif () # Adding the tests: include(CTest) -if(BUILD_TESTING) - add_subdirectory(test) -endif() +if (BUILD_TESTING) + add_subdirectory(test) +endif () -if(cloysterhpc_BUILD_FUZZ_TESTS) - message(AUTHOR_WARNING "Building Fuzz Tests, using fuzzing sanitizer https://www.llvm.org/docs/LibFuzzer.html") - if (NOT cloysterhpc_ENABLE_ADDRESS_SANITIZER AND NOT cloysterhpc_ENABLE_THREAD_SANITIZER) - message(WARNING "You need asan or tsan enabled for meaningful fuzz testing") - endif() - add_subdirectory(fuzz_test) +if (cloysterhpc_BUILD_FUZZ_TESTS) + message(AUTHOR_WARNING "Building Fuzz Tests, using fuzzing sanitizer https://www.llvm.org/docs/LibFuzzer.html") + if (NOT cloysterhpc_ENABLE_ADDRESS_SANITIZER AND NOT cloysterhpc_ENABLE_THREAD_SANITIZER) + message(WARNING "You need asan or tsan enabled for meaningful fuzz testing") + endif () + add_subdirectory(fuzz_test) -endif() +endif () # If MSVC is being used, and ASAN is enabled, we need to set the debugger environment # so that it behaves well with MSVC's debugger, and we can run the target from visual studio -if(MSVC) - get_all_installable_targets(all_targets) - message("all_targets=${all_targets}") - set_target_properties(${all_targets} PROPERTIES VS_DEBUGGER_ENVIRONMENT "PATH=$(VC_ExecutablePath_x64);%PATH%") -endif() +if (MSVC) + get_all_installable_targets(all_targets) + message("all_targets=${all_targets}") + set_target_properties(${all_targets} PROPERTIES VS_DEBUGGER_ENVIRONMENT "PATH=$(VC_ExecutablePath_x64);%PATH%") +endif () # set the startup project for the "play" button in MSVC set_property(DIRECTORY PROPERTY VS_STARTUP_PROJECT ${cloysterhpc_BINARY_NAME}) -if(CMAKE_SKIP_INSTALL_RULES) - return() -endif() +if (CMAKE_SKIP_INSTALL_RULES) + return() +endif () include(cmake/PackageProject.cmake) # Add other targets that you want installed here, by default we just package the one executable # we know we want to ship cloysterhpc_package_project( - TARGETS - ${cloysterhpc_BINARY_NAME} - cloysterhpc_options - cloysterhpc_warnings - # FIXME: this does not work! CK - # PRIVATE_DEPENDENCIES_CONFIGURED project_options project_warnings + TARGETS + ${cloysterhpc_BINARY_NAME} + cloysterhpc_options + cloysterhpc_warnings + # FIXME: this does not work! CK + # PRIVATE_DEPENDENCIES_CONFIGURED project_options project_warnings ) diff --git a/CMakePresets.json b/CMakePresets.json index 21ca411d..54d62c3b 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -22,7 +22,9 @@ "condition": { "type": "inList", "string": "${hostSystemName}", - "list": ["Linux"] + "list": [ + "Linux" + ] }, "vendor": { "microsoft.com/VisualStudioRemoteSettings/CMake/1.0": { diff --git a/fuzz_test/CMakeLists.txt b/fuzz_test/CMakeLists.txt index a7e3d5ef..674d0fd4 100644 --- a/fuzz_test/CMakeLists.txt +++ b/fuzz_test/CMakeLists.txt @@ -5,17 +5,17 @@ find_package(fmt) add_executable(fuzz_tester fuzz_tester.cpp) target_link_libraries( - fuzz_tester - PRIVATE cmake_conan2_template_options - cmake_conan2_template_warnings - fmt::fmt - -coverage - -fsanitize=fuzzer) + fuzz_tester + PRIVATE cmake_conan2_template_options + cmake_conan2_template_warnings + fmt::fmt + -coverage + -fsanitize=fuzzer) target_compile_options(fuzz_tester PRIVATE -fsanitize=fuzzer) # Allow short runs during automated testing to see if something new breaks set(FUZZ_RUNTIME - 10 - CACHE STRING "Number of seconds to run fuzz tests during ctest run") # Default of 10 seconds + 10 + CACHE STRING "Number of seconds to run fuzz tests during ctest run") # Default of 10 seconds add_test(NAME fuzz_tester_run COMMAND fuzz_tester -max_total_time=${FUZZ_RUNTIME}) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6b15648b..1206c48d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -12,45 +12,45 @@ add_executable(${cloysterhpc_BINARY_NAME} main.cpp) include(../cmake/CommonLibraries.cmake) target_link_libraries( - ${cloysterhpc_BINARY_NAME} - PRIVATE - cloysterhpc_static - cloysterhpc::cloysterhpc_options - cloysterhpc::cloysterhpc_warnings) + ${cloysterhpc_BINARY_NAME} + PRIVATE + cloysterhpc_static + cloysterhpc::cloysterhpc_options + cloysterhpc::cloysterhpc_warnings) target_link_system_libraries( - ${cloysterhpc_BINARY_NAME} - PRIVATE - ${COMMON_LIBS}) + ${cloysterhpc_BINARY_NAME} + PRIVATE + ${COMMON_LIBS}) target_link_libraries( - cloysterhpc_object - PRIVATE - cloysterhpc::cloysterhpc_options - cloysterhpc::cloysterhpc_warnings) + cloysterhpc_object + PRIVATE + cloysterhpc::cloysterhpc_options + cloysterhpc::cloysterhpc_warnings) target_link_system_libraries( - cloysterhpc_object - PRIVATE - ${COMMON_LIBS}) + cloysterhpc_object + PRIVATE + ${COMMON_LIBS}) target_include_directories(cloysterhpc_static PRIVATE - "${CMAKE_BINARY_DIR}/configured_files/include") + "${CMAKE_BINARY_DIR}/configured_files/include") target_include_directories(cloysterhpc_static ${WARNING_GUARD} PRIVATE - $ - $) + $ + $) target_include_directories(cloysterhpc_object PRIVATE - "${CMAKE_BINARY_DIR}/configured_files/include") + "${CMAKE_BINARY_DIR}/configured_files/include") target_include_directories(cloysterhpc_object ${WARNING_GUARD} PRIVATE - $ - $) + $ + $) target_include_directories(${cloysterhpc_BINARY_NAME} PRIVATE - "${CMAKE_BINARY_DIR}/configured_files/include") + "${CMAKE_BINARY_DIR}/configured_files/include") target_include_directories(${cloysterhpc_BINARY_NAME} ${WARNING_GUARD} PRIVATE - $ - $) + $ + $) # Set target locations to the root folder of the project in /bin and /lib. # WARNING: This will break multiple target compile, disable it if necessary diff --git a/src/cluster.cpp b/src/cluster.cpp index 42c9183f..6c779b14 100644 --- a/src/cluster.cpp +++ b/src/cluster.cpp @@ -551,8 +551,7 @@ void Cluster::fillData(const std::string& answerfilePath) externalConnection.setAddress(answerfile.external.con_ip_addr.value()); getNetwork(Network::Profile::External) - .setAddress( - getNetwork(Network::Profile::External) + .setAddress(getNetwork(Network::Profile::External) .calculateAddress(answerfile.external.con_ip_addr.value())); } else { auto externalNetworkIpAddress = externalConnection.fetchAddress( @@ -561,7 +560,7 @@ void Cluster::fillData(const std::string& answerfilePath) getNetwork(Network::Profile::External) .setAddress(getNetwork(Network::Profile::External) - .calculateAddress(externalNetworkIpAddress)); + .calculateAddress(externalNetworkIpAddress)); } if (!answerfile.external.con_mac_addr->empty()) { @@ -619,8 +618,7 @@ void Cluster::fillData(const std::string& answerfilePath) serviceConnection.setAddress(answerfile.service.con_ip_addr.value()); getNetwork(Network::Profile::Service) - .setAddress( - getNetwork(Network::Profile::Service) + .setAddress(getNetwork(Network::Profile::Service) .calculateAddress(answerfile.service.con_ip_addr.value())); m_headnode.addConnection(std::move(serviceConnection)); @@ -674,8 +672,8 @@ void Cluster::fillData(const std::string& answerfilePath) getNetwork(Network::Profile::Application) .setAddress(getNetwork(Network::Profile::Application) - .calculateAddress( - answerfile.application.con_ip_addr.value())); + .calculateAddress( + answerfile.application.con_ip_addr.value())); m_headnode.addConnection(std::move(applicationConnection)); } diff --git a/src/os.cpp b/src/os.cpp index 92f346ee..16b899e4 100644 --- a/src/os.cpp +++ b/src/os.cpp @@ -24,7 +24,7 @@ OS::OS() { - struct utsname system { }; + struct utsname system {}; uname(&system); setArch(system.machine); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c070ee67..60f36f40 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -4,16 +4,16 @@ project(CmakeConfigPackageTests LANGUAGES CXX) # ---- Test as standalone project the exported config package ---- -if(PROJECT_IS_TOP_LEVEL OR TEST_INSTALLED_VERSION) - enable_testing() +if (PROJECT_IS_TOP_LEVEL OR TEST_INSTALLED_VERSION) + enable_testing() - find_package(cloysterhpc CONFIG REQUIRED) # for intro, project_options, ... + find_package(cloysterhpc CONFIG REQUIRED) # for intro, project_options, ... - if(NOT TARGET cloysterhpc_options) - message(FATAL_ERROR "Required config package not found!") - return() # be strictly paranoid for Template Janitor github action! CK - endif() -endif() + if (NOT TARGET cloysterhpc_options) + message(FATAL_ERROR "Required config package not found!") + return() # be strictly paranoid for Template Janitor github action! CK + endif () +endif () # Provide a simple smoke test to make sure that the CLI works and can display a --help message add_test(NAME cli.has_help COMMAND main --help) @@ -42,47 +42,47 @@ target_compile_definitions(cloysterhpc_test_object PRIVATE BUILD_TESTING=1) include(../cmake/CommonLibraries.cmake) target_link_libraries( - ${PROJECT_NAME} - PRIVATE - cloysterhpc_test_object - cloysterhpc::cloysterhpc_warnings - cloysterhpc::cloysterhpc_options) + ${PROJECT_NAME} + PRIVATE + cloysterhpc_test_object + cloysterhpc::cloysterhpc_warnings + cloysterhpc::cloysterhpc_options) target_link_system_libraries( - ${PROJECT_NAME} - PRIVATE - ${COMMON_LIBS}) + ${PROJECT_NAME} + PRIVATE + ${COMMON_LIBS}) target_include_directories(${PROJECT_NAME} ${WARNING_GUARD} PRIVATE - $ - $) + $ + $) target_link_libraries( - cloysterhpc_test_object - PRIVATE - cloysterhpc::cloysterhpc_options - cloysterhpc::cloysterhpc_warnings) + cloysterhpc_test_object + PRIVATE + cloysterhpc::cloysterhpc_options + cloysterhpc::cloysterhpc_warnings) target_link_system_libraries( - cloysterhpc_test_object - PRIVATE - ${COMMON_LIBS}) + cloysterhpc_test_object + PRIVATE + ${COMMON_LIBS}) target_include_directories(cloysterhpc_test_object PRIVATE - "${CMAKE_BINARY_DIR}/../configured_files/include") + "${CMAKE_BINARY_DIR}/../configured_files/include") target_include_directories(cloysterhpc_test_object ${WARNING_GUARD} PRIVATE - $ - $) + $ + $) set_target_properties(${PROJECT_NAME} PROPERTIES CXX_STANDARD ${CMAKE_CXX_STANDARD}) -if(WIN32 AND BUILD_SHARED_LIBS) - add_custom_command( - TARGET ${PROJECT_NAME} - PRE_BUILD - COMMAND ${CMAKE_COMMAND} -E copy $ $ - COMMAND_EXPAND_LISTS) -endif() +if (WIN32 AND BUILD_SHARED_LIBS) + add_custom_command( + TARGET ${PROJECT_NAME} + PRE_BUILD + COMMAND ${CMAKE_COMMAND} -E copy $ $ + COMMAND_EXPAND_LISTS) +endif () # ---- Add Tests ---- @@ -91,26 +91,26 @@ endif() # ${PROJECT_NAME}) if (cloysterhpc_ENABLE_CONAN) - string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER) - string(TOUPPER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_UPPER) - string(TOLOWER "${CMAKE_SYSTEM_PROCESSOR}" CMAKE_SYSTEM_PROCESSOR_LOWER) + string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER) + string(TOUPPER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_UPPER) + string(TOLOWER "${CMAKE_SYSTEM_PROCESSOR}" CMAKE_SYSTEM_PROCESSOR_LOWER) - # Hack to fix wrong architecture name for aarch64 during Conan install - if (CMAKE_SYSTEM_PROCESSOR_LOWER STREQUAL "aarch64") - set(CMAKE_SYSTEM_PROCESSOR_LOWER "armv8") - endif() + # Hack to fix wrong architecture name for aarch64 during Conan install + if (CMAKE_SYSTEM_PROCESSOR_LOWER STREQUAL "aarch64") + set(CMAKE_SYSTEM_PROCESSOR_LOWER "armv8") + endif () - include(${doctest_DIR}/doctest-${CMAKE_BUILD_TYPE_LOWER}-${CMAKE_SYSTEM_PROCESSOR_LOWER}-data.cmake) - include(${doctest_BUILD_DIRS_${CMAKE_BUILD_TYPE_UPPER}}/doctest.cmake) -else() - include(${doctest_SOURCE_DIR}/scripts/cmake/doctest.cmake) -endif() + include(${doctest_DIR}/doctest-${CMAKE_BUILD_TYPE_LOWER}-${CMAKE_SYSTEM_PROCESSOR_LOWER}-data.cmake) + include(${doctest_BUILD_DIRS_${CMAKE_BUILD_TYPE_UPPER}}/doctest.cmake) +else () + include(${doctest_SOURCE_DIR}/scripts/cmake/doctest.cmake) +endif () doctest_discover_tests(${PROJECT_NAME}) # ---- code coverage ---- -if(ENABLE_TEST_COVERAGE) - target_compile_options(${PROJECT_NAME} PUBLIC -O0 -g -fprofile-arcs -ftest-coverage) - target_link_options(${PROJECT_NAME} PUBLIC -fprofile-arcs -ftest-coverage) -endif() +if (ENABLE_TEST_COVERAGE) + target_compile_options(${PROJECT_NAME} PUBLIC -O0 -g -fprofile-arcs -ftest-coverage) + target_link_options(${PROJECT_NAME} PUBLIC -fprofile-arcs -ftest-coverage) +endif () From b6a6fa0a014422fb605d6db0993586be25be91b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 13 Jan 2025 13:22:30 -0300 Subject: [PATCH 23/23] Final try to fix clang-format. I promise. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com> --- src/headnode.cpp | 2 +- src/network.cpp | 10 +++++----- src/presenter/PresenterInfiniband.cpp | 2 +- src/presenter/PresenterMailSystem.cpp | 2 +- src/presenter/PresenterQueueSystem.cpp | 2 +- src/services/bmc.cpp | 2 +- src/services/xcat.cpp | 6 +++--- src/tools/nvhpc.cpp | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/headnode.cpp b/src/headnode.cpp index 4dfd8084..5c839075 100644 --- a/src/headnode.cpp +++ b/src/headnode.cpp @@ -32,7 +32,7 @@ Headnode::Headnode() void Headnode::discoverNames() { - struct utsname system { }; + struct utsname system {}; uname(&system); std::string fqdn = system.nodename; diff --git a/src/network.cpp b/src/network.cpp index 4cf09037..b08e86d0 100644 --- a/src/network.cpp +++ b/src/network.cpp @@ -110,9 +110,9 @@ void Network::setAddress(const std::string& ip) address Network::fetchAddress(const std::string& interface) { - struct in_addr addr { }; - struct in_addr netmask { }; - struct in_addr network { }; + struct in_addr addr {}; + struct in_addr netmask {}; + struct in_addr network {}; if (inet_aton( Connection::fetchAddress(interface).to_string().c_str(), &addr) @@ -208,8 +208,8 @@ address Network::calculateAddress(const address& connectionAddress) "calculate the address")); } - struct in_addr ip_addr { }; - struct in_addr subnet_addr { }; + struct in_addr ip_addr {}; + struct in_addr subnet_addr {}; inet_aton(connectionAddress.to_string().c_str(), &ip_addr); inet_aton(m_subnetMask.to_string().c_str(), &subnet_addr); diff --git a/src/presenter/PresenterInfiniband.cpp b/src/presenter/PresenterInfiniband.cpp index 4f055d69..f2d47d65 100644 --- a/src/presenter/PresenterInfiniband.cpp +++ b/src/presenter/PresenterInfiniband.cpp @@ -26,7 +26,7 @@ PresenterInfiniband::PresenterInfiniband(std::unique_ptr& model, m_model->setOFED(magic_enum::enum_cast( m_view->listMenu(Messages::title, Messages::OFED::question, magic_enum::enum_names(), Messages::OFED::help)) - .value()); + .value()); LOG_DEBUG("Set OFED stack as: {}", magic_enum::enum_name(m_model->getOFED()->getKind())); diff --git a/src/presenter/PresenterMailSystem.cpp b/src/presenter/PresenterMailSystem.cpp index 41ff866a..8d6b46df 100644 --- a/src/presenter/PresenterMailSystem.cpp +++ b/src/presenter/PresenterMailSystem.cpp @@ -17,7 +17,7 @@ PresenterMailSystem::PresenterMailSystem( m_view->listMenu(Messages::title, Messages::Profile::question, magic_enum::enum_names(), Messages::Profile::help)) - .value()); + .value()); auto& mailSystem = m_model->getMailSystem().value(); const auto& mailSystemProfile = mailSystem.getProfile(); diff --git a/src/presenter/PresenterQueueSystem.cpp b/src/presenter/PresenterQueueSystem.cpp index d5e8535b..5dbb0bad 100644 --- a/src/presenter/PresenterQueueSystem.cpp +++ b/src/presenter/PresenterQueueSystem.cpp @@ -13,7 +13,7 @@ PresenterQueueSystem::PresenterQueueSystem( m_model->setQueueSystem(magic_enum::enum_cast( m_view->listMenu(Messages::title, Messages::question, magic_enum::enum_names(), Messages::help)) - .value()); + .value()); // TODO: Placeholder data auto fieldsSLURM = std::to_array>( diff --git a/src/services/bmc.cpp b/src/services/bmc.cpp index 3f704d44..232e910f 100644 --- a/src/services/bmc.cpp +++ b/src/services/bmc.cpp @@ -16,7 +16,7 @@ BMC::BMC(const std::string& address, const std::string& username, { } -BMC::BMC() {}; +BMC::BMC() { }; const std::string& BMC::getAddress() const { return m_address; } void BMC::setAddress(const std::string& address) { m_address = address; } diff --git a/src/services/xcat.cpp b/src/services/xcat.cpp index 87790cee..50b46ab7 100644 --- a/src/services/xcat.cpp +++ b/src/services/xcat.cpp @@ -52,9 +52,9 @@ void XCAT::patchInstall() void XCAT::setup() { setDHCPInterfaces(m_cluster->getHeadnode() - .getConnection(Network::Profile::Management) - .getInterface() - .value()); + .getConnection(Network::Profile::Management) + .getInterface() + .value()); setDomain(m_cluster->getDomainName()); } diff --git a/src/tools/nvhpc.cpp b/src/tools/nvhpc.cpp index 70c33ae0..8b806b23 100644 --- a/src/tools/nvhpc.cpp +++ b/src/tools/nvhpc.cpp @@ -11,4 +11,4 @@ void NVhpc::install() cloyster::runCommand("dnf -y install nvhpc-24.3"); } -void NVhpc::configure() {}; +void NVhpc::configure() { };