Skip to content
This repository was archived by the owner on Apr 16, 2026. It is now read-only.

Commit 4213823

Browse files
dhilstdhilst
committed
Fix review comments
1 parent 0c07c28 commit 4213823

9 files changed

Lines changed: 112 additions & 81 deletions

File tree

‎include/cloysterhpc/functions.h‎

Lines changed: 1 addition & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include <cloysterhpc/models/cluster.h>
55
#include <cloysterhpc/services/log.h>
6+
#include <cloysterhpc/patterns/singleton.h>
67
#include <boost/process/child.hpp>
78
#include <boost/process/pipe.hpp>
89
#include <cloysterhpc/services/repos.h>
@@ -26,49 +27,6 @@ constexpr std::unique_ptr<B> makeUniqueDerived(Args... args)
2627
return static_cast<std::unique_ptr<B>>(std::make_unique<T>(args...));
2728
}
2829

29-
template <typename T>
30-
class Singleton {
31-
// Private constructor to prevent direct instantiation
32-
Singleton() = default;
33-
34-
// Static members for singleton management
35-
static std::unique_ptr<T> instance;
36-
static std::once_flag initFlag;
37-
public:
38-
Singleton(Singleton&&) = delete;
39-
Singleton& operator=(Singleton&&) = delete;
40-
Singleton(const Singleton&) = delete;
41-
Singleton& operator=(const Singleton&) = delete;
42-
~Singleton() = delete;
43-
44-
static void init(std::unique_ptr<T> value)
45-
{
46-
std::call_once(initFlag, [&](){
47-
instance = std::move(value);
48-
});
49-
}
50-
51-
static void init(const auto& factory)
52-
{
53-
std::call_once(initFlag, [&](){
54-
instance = std::move(factory());
55-
});
56-
}
57-
58-
static gsl::not_null<T*> get() {
59-
if (!instance) {
60-
throw std::runtime_error("Singleton read before initialization");
61-
}
62-
return gsl::not_null<T*>(instance.get());
63-
}
64-
};
65-
66-
template <typename T>
67-
std::unique_ptr<T> Singleton<T>::instance = nullptr;
68-
69-
template <typename T>
70-
std::once_flag Singleton<T>::initFlag;
71-
7230
using cloyster::services::BaseRunner;
7331
using cloyster::models::OS;
7432

‎include/cloysterhpc/models/answerfile.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ class AnswerFile {
160160

161161
struct AFOFED {
162162
std::string kind = cloyster::utils::enumToString(OFED::Kind::Inbox);
163-
std::string version = "latest";
163+
std::optional<std::string> version = "latest";
164164
bool enabled = false;
165165
};
166166

‎include/cloysterhpc/models/os.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ class OS {
106106
void setPlatform(std::string_view platform);
107107

108108
[[nodiscard]] Distro getDistro() const;
109+
[[nodiscard]] std::string getDistroString() const;
109110
void setDistro(Distro distro);
110111
void setDistro(std::string_view distro);
111112

‎include/cloysterhpc/patterns/singleton.h‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
#ifndef CLOYSTER_SINGLETON_H
2+
#define CLOYSTER_SINGLETON_H
3+
4+
#include <memory>
5+
#include <mutex>
6+
#include <gsl/gsl-lite.hpp>
7+
8+
namespace cloyster {
9+
10+
/**
11+
* @brief This class implement the Singleton pattern.
12+
* @details The user initialize a class C with Singleton<C>::init(value),
13+
* and then get the instance with Singleton<C>::get().
14+
*/
15+
template <typename T>
16+
class Singleton final {
17+
// Private constructor to prevent direct instantiation
18+
Singleton() = default;
19+
20+
// Static members for singleton management
21+
static std::unique_ptr<T> instance;
22+
static std::once_flag initFlag;
23+
public:
24+
Singleton(Singleton&&) = delete;
25+
Singleton& operator=(Singleton&&) = delete;
26+
Singleton(const Singleton&) = delete;
27+
Singleton& operator=(const Singleton&) = delete;
28+
~Singleton() = delete;
29+
30+
static void init(std::unique_ptr<T> value)
31+
{
32+
std::call_once(initFlag, [&](){
33+
instance = std::move(value);
34+
});
35+
}
36+
37+
static void init(const auto& factory)
38+
{
39+
std::call_once(initFlag, [&](){
40+
instance = std::move(factory());
41+
});
42+
}
43+
44+
static gsl::not_null<T*> get() {
45+
if (!instance) {
46+
throw std::runtime_error("Singleton read before initialization");
47+
}
48+
return gsl::not_null<T*>(instance.get());
49+
}
50+
};
51+
52+
template <typename T>
53+
std::unique_ptr<T> Singleton<T>::instance = nullptr;
54+
55+
template <typename T>
56+
std::once_flag Singleton<T>::initFlag;
57+
58+
}
59+
60+
61+
#endif

‎src/models/answerfile.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ void AnswerFile::loadOFED()
624624
ofed.version = "latest"; // use as default
625625
}
626626

627-
LOG_DEBUG("OFED enabled, {} {}", ofed.kind, ofed.version)
627+
LOG_DEBUG("OFED enabled, {} {}", ofed.kind, ofed.version.value())
628628
}
629629
}
630630

‎src/models/cluster.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ void Cluster::fillData(const std::filesystem::path& answerfilePath)
603603
answerfil.ofed.kind
604604
));
605605
}
606-
setOFED(kind.value(), answerfil.ofed.version);
606+
setOFED(kind.value(), answerfil.ofed.version.value());
607607
} else {
608608
// Install Inbox OFED by default
609609
setOFED(OFED::Kind::Inbox);

‎src/models/os.cpp‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,30 @@ OS::Distro OS::getDistro() const
145145
return std::get<OS::Distro>(m_distro);
146146
}
147147

148+
std::string OS::getDistroString() const
149+
{
150+
std::string distro;
151+
switch (getDistro())
152+
{
153+
case OS::Distro::RHEL:
154+
distro = "rhel";
155+
break;
156+
case OS::Distro::AlmaLinux:
157+
distro = "almalinux";
158+
break;
159+
case OS::Distro::Rocky:
160+
distro = "rockylinux";
161+
break;
162+
case OS::Distro::OL:
163+
distro = "ol";
164+
break;
165+
default:
166+
std::unreachable();
167+
}
168+
169+
return fmt::format("{}{}.{}", distro, m_minorVersion, m_majorVersion);
170+
}
171+
148172
OS::PackageType OS::getPackageType() const
149173
{
150174
switch (getDistro()) {

‎src/ofed.cpp‎

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -27,32 +27,6 @@ gpgkey=https://linux.mellanox.com/public/repo/doca/{0}/{1}/{2}/GPG-KEY-Mellanox.
2727
return data;
2828
}
2929

30-
// Return distro to match repositories at
31-
// https://linux.mellanox.com/public/repo/doca/latest/
32-
std::string headnodeDistroName()
33-
{
34-
using cloyster::models::Cluster;
35-
auto cluster = cloyster::Singleton<Cluster>::get();
36-
switch (cluster->getHeadnode().getOS().getDistro()) {
37-
// Assuming we'll be using the last distro version
38-
case cloyster::OS::Distro::RHEL:
39-
return "rhel9.5";
40-
case cloyster::OS::Distro::OL:
41-
return "ol9.4";
42-
case cloyster::OS::Distro::Rocky:
43-
return "rockylinux9.2";
44-
case cloyster::OS::Distro::AlmaLinux:
45-
return "alinux3.2";
46-
default:
47-
std::unreachable();
48-
};
49-
50-
std::unreachable();
51-
}
52-
53-
void installMellanoxDoca(const OFED& ofed)
54-
{
55-
}
5630
};
5731

5832
void OFED::setKind(Kind kind) { m_kind = kind; }
@@ -99,12 +73,23 @@ void OFED::install() const
9973
case OFED::Kind::Mellanox:
10074
{
10175
auto cluster = cloyster::Singleton<cloyster::models::Cluster>::get();
76+
auto hnOs = cluster->getHeadnode().getOS();
10277
auto runner = cloyster::Singleton<cloyster::services::BaseRunner>::get();
10378
auto repoManager = cloyster::Singleton<cloyster::services::repos::RepoManager>::get();
79+
// distroName denotes a folder in the remote repository:
80+
// https://linux.mellanox.com/public/repo/doca/latest/. All
81+
// distributions except Oracle Linux use rhelX.Y folder. Oracle
82+
// Linux does not uses rhelX.Y package because it uses a distinct
83+
// kernel
84+
auto distroName = fmt::format(
85+
"{}{}",
86+
(hnOs.getDistro() == cloyster::OS::Distro::OL ? "ol" : "rhel"),
87+
hnOs.getVersion());
10488

10589
auto repoData = docaRepoTemplate(
106-
getVersion(), headnodeDistroName(),
107-
cloyster::utils::enumToString(cluster->getHeadnode().getOS().getArch()));
90+
getVersion(),
91+
distroName,
92+
cloyster::utils::enumToString(hnOs.getArch()));
10893
std::filesystem::path path = "/etc/yum.repos.d/mlx-doca.repo";
10994

11095
// Install the repository and enable it
@@ -126,20 +111,21 @@ void OFED::install() const
126111
// The driver may support weak updates modules and load without
127112
// need for reboot.
128113
if (cloyster::getEnvironmentVariable("CATTUS_SKIP_INFINIBAND_COMPILE_DOCA_DRIVER") != "1") {
129-
runner->checkCommand("bash -c \"/opt/mellanox/doca/tools/doca-kernel-support -k $(rpm -q --qf \"%{VERSION}-%{RELEASE}.%{ARCH}\n\" kernel-devel)\"");
114+
runner->checkCommand(
115+
"bash -c \"/opt/mellanox/doca/tools/doca-kernel-support -k "
116+
"$(rpm -q --qf \"%{VERSION}-%{RELEASE}.%{ARCH}\n\" kernel-devel)\"");
130117
}
131118

132119
// Get the last rpm in /tmp/DOCA*/ folder
133120
auto rpm = runner->checkOutput("bash -c \"find /tmp/DOCA*/ -name '*.rpm' -printf '%T@ %p\n' | sort -nk1 | tail -1 | awk '{print $2}'\"");
134121
assert(rpm.size() > 0); // at last one line
135122

136123
// Install the (last) generated rpm
137-
runner->executeCommand(fmt::format("rpm -vih {}", rpm[0]));
124+
runner->executeCommand(fmt::format("dnf install -y {}", rpm[0]));
138125

139126
runner->checkCommand("dnf makecache");
140-
// @NOTE: Are these packages correct/good default?
141127
runner->checkCommand("dnf install -y doca-ofed mlnx-fw-updater");
142-
runner->checkCommand("modprobe mlx5_core");
128+
runner->checkCommand("systemctl restart openibd");
143129
}
144130
break;
145131

‎src/services/xcat.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ void XCAT::configureInfiniband()
227227

228228

229229
// Configure Apache to serve the RPM repository
230-
const std::string_view repoName = "rpmrepo";
230+
std::string_view repoName = "repos";
231231
const auto repoFolder = fmt::format("/var/www/html/{}", repoName);
232232
cloyster::createHTTPRepo(repoName);
233233

@@ -246,8 +246,9 @@ void XCAT::configureInfiniband()
246246

247247
// Add the local repository to the stateless image
248248
runner->checkCommand(
249-
fmt::format("bash -c \"chdef -t osimage {} --plus otherpkgdir=http://localhost/rpmrepo/\"",
250-
m_stateless.osimage));
249+
fmt::format(
250+
"bash -c \"chdef -t osimage {} --plus otherpkgdir=http://localhost/{}/\"",
251+
m_stateless.osimage, repoName));
251252

252253
}
253254
break;

0 commit comments

Comments
 (0)