This repository was archived by the owner on Apr 16, 2026. It is now read-only.
OFED doca + Revam repos + Remove deps#97
Merged
dhilst merged 43 commits intorepo-classfrom Jul 2, 2025
Merged
Conversation
100725b to
dc21b8e
Compare
dhilst
commented
Mar 12, 2025
dhilst
commented
Mar 12, 2025
| * specified kind. | ||
| */ | ||
| void install() const; | ||
| void install(cloyster::services::repos::RepoManager& repoManager) const; |
Collaborator
Author
There was a problem hiding this comment.
now that cluster has a global getter this parameter can be removed
dhilst
commented
Mar 12, 2025
dhilst
commented
Mar 12, 2025
dhilst
commented
Mar 12, 2025
dhilst
commented
Mar 12, 2025
| // Return true if the repository should not be loaded | ||
| constexpr auto blacklisted = [](const std::string& repo) { | ||
| if (repo.starts_with("doca-kernel-")) { | ||
| // @FIXME: This is the repositories created by the doca scripts |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| // @FIXME: This is the repositories created by the doca scripts | |
| // @FIXME: This is the repository created by the doca scripts |
dhilst
commented
Mar 12, 2025
| } | ||
| } | ||
|
|
||
| Cluster& cluster() { return getClusterSingleton(); } |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| Cluster& cluster() { return getClusterSingleton(); } | |
| inline Cluster& cluster() { return getClusterSingleton(); } |
dhilst
commented
Mar 17, 2025
Comment on lines
76
to
85
| static std::unique_ptr<Cluster> clusterSingleton; | ||
| void initClusterSingleton(std::unique_ptr<Cluster> cluster) | ||
| { | ||
| assert(!clusterSingleton); | ||
| clusterSingleton = std::move(cluster); | ||
| Singleton<Cluster>::init(std::move(cluster)); | ||
| } | ||
|
|
||
| Cluster& getClusterSingleton() | ||
| gsl::not_null<Cluster*> getClusterSingleton() | ||
| { | ||
| assert(clusterSingleton); | ||
| return *clusterSingleton; | ||
| return Singleton<Cluster>::get(); | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
This will be removed in favor of Singleton<Cluster>::get() where required
f004300 to
064d3a3
Compare
dhilst
commented
Mar 19, 2025
Comment on lines
+377
to
+387
| const char* what() const noexcept override { return message.c_str(); } | ||
| [[nodiscard]] const char* what() const noexcept override { return message.c_str(); } |
Collaborator
Author
There was a problem hiding this comment.
I couldn't change that to std::string or std::string_view it wouldn't type check
Collaborator
Author
There was a problem hiding this comment.
I double-checked this the const char* is inherited from std::exception, there is not much we can do to get rid of this char*
Owner
There was a problem hiding this comment.
I think that's OK, because it's how exceptions works. With primitive types.
f25d299 to
3605def
Compare
There is no libstdc++exp.a in Rocklinux repositories, stacktrace requires it.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


(AI generated)
Here’s a summary of the changes made in your project diff, organized into up to 10 general topics. For each topic, I’ll describe what was changed, provide a general evaluation of the affected files, and derive a potential reason for the change, based on your stated goals of removing dependencies, implementing OFED installation, and revamping repository management.
Dependency Removal
What Changed: Several external dependencies were removed from Dependencies.cmake and CommonLibraries.cmake, including CLI11 (replaced with Boost::program_options), hwinfo (and its associated hardware.h/hardware.cpp), and SimpleIni (replaced with Glib's KeyFile). The hardware.h and hardware.cpp files were deleted entirely.
Evaluation:
Dependencies.cmake: Removed lines fetching CLI11, hwinfo, and SimpleIni via CPM, simplifying the dependency management.
CommonLibraries.cmake: Updated the COMMON_LIBS list to reflect the new dependency structure, removing CLI11, SimpleIni, and hwinfo, and adding Boost::program_options.
hardware.h/hardware.cpp: Deletion indicates these files are no longer needed, reducing the codebase size.
Reason: This aligns with your goal of removing dependencies to make the project more lightweight and self-contained, reducing reliance on external libraries and potentially easing maintenance and build processes.
Configuration Handling Revamp
What Changed: The inifile.h/inifile.cpp (using SimpleIni) were replaced with services/files.h/services/files.cpp (using Glib's KeyFile). Many files, such as mailsystem/postfix.cpp, models/answerfile.cpp, and services/repos.cpp, were updated to use KeyFile instead of inifile.
Evaluation:
services/files.h/files.cpp: Enhanced KeyFile class with new methods (e.g., listAllPrefixedEntries, hasGroup), improving configuration handling capabilities.
mailsystem/postfix.cpp: Switched configuration file operations from inifile to KeyFile, simplifying the logic.
models/answerfile.cpp: Adapted to use KeyFile, removing inifile dependencies and adjusting data loading/saving.
Reason: The shift to KeyFile likely aims to leverage Glib’s robust configuration parsing, reducing external dependencies (SimpleIni) and improving integration with existing libraries, enhancing maintainability and consistency.
Command-Line Parsing Switch
What Changed: Replaced CLI11 with Boost::program_options for command-line parsing, notably in main.cpp and options.cpp. Dependencies.cmake and CommonLibraries.cmake reflect this shift.
Evaluation:
main.cpp: Removed CLI11-based parsing and introduced a new Options singleton initialized via options.cpp.
options.cpp: New file implementing command-line parsing with Boost::program_options, defining an Options struct with various flags and options.
Reason: This change supports dependency removal by using Boost (already a dependency) instead of CLI11, reducing the number of external libraries while maintaining robust command-line functionality.
Singleton Pattern Introduction
What Changed: Introduced a Singleton template in patterns/singleton.h, used across files like main.cpp, services/shell.cpp, and functions.cpp to manage global instances (e.g., Options, MessageBus, IRunner, Cluster).
Evaluation:
patterns/singleton.h: Provides a thread-safe singleton implementation using std::unique_ptr and std::once_flag.
main.cpp: Initializes singletons for Options, MessageBus, and IRunner during startup.
services/shell.cpp: Uses singletons to access Cluster and IRunner, removing direct constructor dependencies.
Reason: This centralizes access to key components, reducing coupling and supporting dependency removal by eliminating the need to pass objects around, aligning with a more modular architecture.
Repository Management Overhaul
What Changed: Revamped repository management in services/repos.h/repos.cpp, removing static .repo files (e.g., repos/el8/cloyster.repo) and generating them dynamically. Added air-gap and mirror URL support in functions.cpp.
Evaluation:
services/repos.h/repos.cpp: Shifted from static .repo files to dynamic generation using ELConfig, with new methods like repo() and repoFile().
functions.cpp: Added createHTTPRepo and makeAirGapUrl for dynamic repository creation and URL handling.
Deleted repos/el*/cloyster.repo: No longer needed due to runtime generation.
Reason: This fulfills your goal of revamping repository management, making it more flexible and adaptable to different environments (e.g., air-gapped setups), reducing hardcoded configurations.
OFED Installation Implementation
What Changed: Added OFED installation support in ofed.h/ofed.cpp and models/answerfile.h/answerfile.cpp, with specific handling for Mellanox OFED. Integrated into services/xcat.cpp for image configuration.
Evaluation:
ofed.h/ofed.cpp: Implemented install() for Inbox and Mellanox OFED, adding version support and repository setup.
models/answerfile.h/answerfile.cpp: Added AFOFED struct and loading logic for OFED settings.
services/xcat.cpp: Configures Mellanox OFED in stateless images using dynamic repository setup.
Reason: This directly addresses your goal of implementing OFED installation, enhancing HPC capabilities with flexible Infiniband support tailored to different vendors.
Service Abstraction Enhancement
What Changed: Introduced services/osservice.h/osservice.cpp as an abstraction for OS-specific operations, replacing package_manager.h/dnf.h. Updated files like NFS.cpp, mailsystem/postfix.cpp, and models/slurm.cpp to use it.
Evaluation:
services/osservice.h/osservice.cpp: Defines IOSService interface with methods like install(), enableService(), implemented for Enterprise Linux.
NFS.cpp, mailsystem/postfix.cpp, models/slurm.cpp: Replaced direct command execution with IOSService calls.
Reason: This abstraction removes dependency on package_manager, improving modularity and maintainability by centralizing OS interactions, supporting your broader refactoring goals.
Dry-Run Functionality Improvements
What Changed: Enhanced dry-run handling across files like functions.cpp, services/IService.cpp, and ofed.cpp, using the Options singleton instead of global variables.
Evaluation:
functions.cpp: Updated functions like touchFile, createDirectory to check Options::dryRun via singleton.
services/IService.cpp: Service methods (e.g., enable, start) now respect dry-run mode consistently.
ofed.cpp: Added dry-run checks to skip actual installation.
Reason: Improves simulation capabilities without execution, enhancing testing and debugging, and aligns with a singleton-based configuration approach.
Enum Handling Refactor
What Changed: Replaced magic_enum with a custom utils/enums.h, used in files like connection.cpp, presenter/Presenter*.cpp, and models/answerfile.cpp.
Evaluation:
utils/enums.h: Provides toString, toStrings, and ofStringOpt for enum manipulation, with case-sensitive/insensitive options.
connection.cpp, presenter/Presenter*.cpp: Updated enum-to-string conversions to use new utility.
Reason: Removes dependency on magic_enum, offering a tailored solution for enum handling, improving control and reducing external library reliance.
Testing Adjustments
What Changed: Removed test-related code (test/, tests.h, CMakeLists.txt modifications) and adjusted files like CMakeLists.txt to reflect major refactoring breaking existing tests.
Evaluation:
CMakeLists.txt: Commented out test subdirectory addition with a note about refactoring breaking tests.
Deleted test/: Removed dedicated test files, indicating a shift in testing strategy.
Reason: The significant refactoring (e.g., dependency removal, singleton usage) likely invalidated existing tests, requiring updates or a new testing approach, temporarily deferred to focus on core changes.
Conclusion
These changes collectively aim to streamline the project by reducing external dependencies, enhancing modularity with singletons and service abstractions, and improving flexibility in repository and OFED management. The refactoring aligns with your stated objectives, making the codebase more maintainable, adaptable to HPC environments, and easier to build and deploy. Each file change reflects a move toward a more self-contained, efficient architecture, though some areas (e.g., testing) remain to be revisited post-refactoring.