Fix nvidia-cdi-refresh systemd packaging#1827
Conversation
d476654 to
7bd9cf2
Compare
Coverage Report for CI Build 25912014995Coverage remained the same at 43.342%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Signed-off-by: Evan Lezar <elezar@nvidia.com>
7bd9cf2 to
c29c81f
Compare
cdesiniotis
left a comment
There was a problem hiding this comment.
Thanks @elezar. Overall lgtm, left some general questions.
| %if 0%{?rhel} == 7 || 0%{?amzn} == 2 | ||
| BuildRequires: systemd | ||
| %else | ||
| BuildRequires: systemd-rpm-macros | ||
| %endif |
There was a problem hiding this comment.
Question -- these are build-time constraints that only apply when building the RPM packages, correct?
There was a problem hiding this comment.
Yes, that is my understanding. We build the packages on centos7, so the systemd-rpm-macros packages are not available.
| systemctl start nvidia-cdi-refresh.path >/dev/null 2>&1 || : | ||
| systemctl start nvidia-cdi-refresh.service >/dev/null 2>&1 || : |
There was a problem hiding this comment.
Question -- what's the rationale behind not running systemctl enabled --now anymore before this? Is it because our systemd service always gets enabled upon installation (due to the newly added .preset file)?
There was a problem hiding this comment.
The way I understand it, this does not always work as expected in cases where systemd is not yet in a valid state. For example, when building a system image -- which is why this is being triggered for DGX Spark.
| rm_conffile /etc/systemd/system/nvidia-cdi-refresh.service @VERSION@~ nvidia-container-toolkit-base | ||
| rm_conffile /etc/systemd/system/nvidia-cdi-refresh.path @VERSION@~ nvidia-container-toolkit-base |
There was a problem hiding this comment.
IIUC this will remove the old nvidia-cdi-refresh.service / nvidia-cdi-refresh.path files if they happen to still exist in /etc/systemd/system/ from a prior installation of nvidia-container-toolkit-base. Should we also do this in our rpm packaging (I don't see any equivalent logic added there)?
This PR updates the installation and configuration of the
nvidia-cdi-refreshsystemd units. This should address issues when installing the packages on non-running systems (e.g. during machine image builds).Changes made using Codex.