Conversation
dhilst
force-pushed
the
hotfix-20250814
branch
from
fd2c6cc to
4b835c7
Compare
| "chkconfig", | ||
| "initscripts", |
There was a problem hiding this comment.
For now this seems OK. But this is only a requirement for xCAT.
| namespace cloyster::services::runner { | ||
|
|
||
| int shell(std::string_view cmd) { return shellfmt("{}", cmd); } | ||
| void shell(std::string_view cmd) { shellfmt("{}", cmd); } |
There was a problem hiding this comment.
The idea here was to return the execution code, since it's always an integer it could have some usage. If we change to path to thrown an exception it may start to be a problem if we use the return code in other place of the code and I think we use it.
There was a problem hiding this comment.
Yes, I have two variants, unsafe does not check the exit code, but has [[nodiscard]] so that the compiler enforces that we check it, and the safe version aborts if the exit code is non-zero. We need that because some commands are intended to fail, like when I check if the image exists or if a package is installed the exit code is non-zero
| "mtu {} ipv4.method manual ipv4.address {}/{} " | ||
| "ipv4.dns \"{}\" " | ||
| // "ipv4.gateway {} ipv4.dns \"{}\" " | ||
| // @FIXME: This will break Confluent, is it required by xCAT? |
There was a problem hiding this comment.
I think the IPv6 disablement was a leftover.
| void XCAT::installPackages() | ||
| { | ||
| auto osservice = cloyster::Singleton<IOSService>::get(); | ||
| osservice->install("initscripts"); |
There was a problem hiding this comment.
Maybe this should stay here.
The thing is... it should be one of the firsts. Because xCAT and only xCAT requires that. Modern software don't care for initscripts at all. For instance, Confluent does not need it.
There was a problem hiding this comment.
This was moved to base.cpp, it is in among the first packages installed, that's why I removed this line here
There was a problem hiding this comment.
This was moved to
base.cpp, it is in among the first packages installed, that's why I removed this line here
Yes, I got it. But this package is only needed due to xCAT. I'm not sure if we must ship it by default.
There was a problem hiding this comment.
I'm going to address this during the Confluent integration
https://github.com/viniciusferrao/cloysterhpc/pull/101/files#r2282713249
| kernel=5.14.0-427.13.1.el9_4.x86_64 | ||
| version=9.6 | ||
| # kernel=5.14.0-427.13.1.el9_4.x86_64 | ||
| kernel=5.14.0-503.21.1.el9_5.x86_64 |
There was a problem hiding this comment.
In the future we should have a compatibility bitmap to map the supported versions.
dhilst
force-pushed
the
hotfix-20250814
branch
4 times, most recently
from
96b0b5e to
c0cf12a
Compare
|
dhilst
changed the base branch from
ansible-roles-migration-part-1
to
master
dhilst
changed the base branch from
master
to
ansible-roles-migration-part-1
- Fix exit code unhandling bug - Get kernel version from answerfile - Fix OFED version in repos.conf - Fix image generation with custom kernel - Fix spack idempotency - Fix diskImage use before set - Run clang-format - Enable keepcache in dnf - Fix from where the kernel version is fetched - Use running kernel when kernel option is ommited in the answerfile - Fix slurld error in the computing node - Comment kernel in rocky9-base.ini - Fix makedns command & restart chronyd
dhilst
force-pushed
the
hotfix-20250814
branch
from
c0cf12a to
06682dc
Compare
|
|
Merged at #102 |
2 participants
Fixes for the deployment issues captured on 2025-08-13