Skip to content

Fix nvidia-cdi-refresh systemd packaging#1827

Open
elezar wants to merge 1 commit into
NVIDIA:mainfrom
elezar:fix-cdi-refresh-systemd-packaging
Open

Fix nvidia-cdi-refresh systemd packaging#1827
elezar wants to merge 1 commit into
NVIDIA:mainfrom
elezar:fix-cdi-refresh-systemd-packaging

Conversation

@elezar
Copy link
Copy Markdown
Member

@elezar elezar commented May 15, 2026

This PR updates the installation and configuration of the nvidia-cdi-refresh systemd units. This should address issues when installing the packages on non-running systems (e.g. during machine image builds).

Changes made using Codex.

@elezar elezar force-pushed the fix-cdi-refresh-systemd-packaging branch 3 times, most recently from d476654 to 7bd9cf2 Compare May 15, 2026 10:02
@coveralls
Copy link
Copy Markdown

Coverage Report for CI Build 25912014995

Coverage remained the same at 43.342%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 14861
Covered Lines: 6441
Line Coverage: 43.34%
Coverage Strength: 0.48 hits per line

💛 - Coveralls

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar elezar force-pushed the fix-cdi-refresh-systemd-packaging branch from 7bd9cf2 to c29c81f Compare May 15, 2026 12:48
@elezar elezar marked this pull request as ready for review May 17, 2026 07:48
@elezar elezar requested review from cdesiniotis and tariq1890 May 20, 2026 07:34
Copy link
Copy Markdown
Contributor

@cdesiniotis cdesiniotis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @elezar. Overall lgtm, left some general questions.

Comment on lines +25 to +29
%if 0%{?rhel} == 7 || 0%{?amzn} == 2
BuildRequires: systemd
%else
BuildRequires: systemd-rpm-macros
%endif
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question -- these are build-time constraints that only apply when building the RPM packages, correct?

Comment on lines +111 to +112
systemctl start nvidia-cdi-refresh.path >/dev/null 2>&1 || :
systemctl start nvidia-cdi-refresh.service >/dev/null 2>&1 || :
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)?

@cdesiniotis cdesiniotis self-assigned this May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants