Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configure: relax flux-security version check #6673

Merged
merged 4 commits into from
Mar 1, 2025

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Feb 28, 2025

This PR relaxes the flux-security version check from v0.14.0 back to v0.13.0 since v0.14.0 is only required for the sdexec module when used to launch jobs in a system instance.

A test is added to the sdexec module to abort if a sufficient flux-security version isn't found at runtime, and a version check added to affected tests.

This allows flux-core to continue to be built on TOSS systems with flux-security v0.13.0 installed. We'll probably have to run with this version for a month at least, and it is nice to be able to build test versions of flux --with-flux-security.

Fixes #6672

Copy link
Member

@chu11 chu11 left a comment

Choose a reason for hiding this comment

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

LGTM!

@grondo
Copy link
Contributor Author

grondo commented Feb 28, 2025

Yay! Thanks!

Problem: Tests in the testsuite may need to check for a minimum
flux-security version, but there's no helper to do that.

Add test_flux_security_version() to sharness.d/flux-sharness.sh
that checks for a minimum flux-security verison if flux is configured
with flux-security.
Problem: The sdexec module launches work as systemd transient
units with Type=notify, which implies flux-security >= 0.14.0 is
being used at runtime. However, we don't want to make this version
of flux-security a hard requirement at this time.

Add a runtime version check to the sdexec module and exit with an
error if an incompatible flux-security version is found.
Problem: Tests that load the sdexec module require flux-security
v0.14.0 or later, but this is not checked.

Add checks to the sdexec tests for the correct flux-security version.
Problem: configure currently requires flux-security v0.14.0 or later,
but this is only required for the sdexec module, only used in system
instances and a few tests.

Now that the sdexec module and tests have runtime checks for the
flux-security version, relax the required version of flux-security
back to v0.13.0.

Note that v0.14.0 is still installed in CI and required in the debian
control file.
@grondo grondo force-pushed the downgrade-flux-security branch from 84375af to 8d61172 Compare February 28, 2025 21:07
@grondo
Copy link
Contributor Author

grondo commented Mar 1, 2025

@Mergifyio requeue

Copy link
Contributor

mergify bot commented Mar 1, 2025

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@grondo
Copy link
Contributor Author

grondo commented Mar 1, 2025

@Mergifyio refresh

Copy link
Contributor

mergify bot commented Mar 1, 2025

refresh

✅ Pull request refreshed

@mergify mergify bot merged commit ff486ca into flux-framework:master Mar 1, 2025
34 of 35 checks passed
Copy link

codecov bot commented Mar 1, 2025

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Project coverage is 83.86%. Comparing base (f15140e) to head (8d61172).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/modules/sdexec/sdexec.c 55.55% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6673   +/-   ##
=======================================
  Coverage   83.85%   83.86%           
=======================================
  Files         533      533           
  Lines       88686    88695    +9     
=======================================
+ Hits        74369    74381   +12     
+ Misses      14317    14314    -3     
Files with missing lines Coverage Δ
src/modules/sdexec/sdexec.c 67.73% <55.55%> (-0.18%) ⬇️

... and 6 files with indirect coverage changes

@grondo grondo deleted the downgrade-flux-security branch March 1, 2025 02:31
@garlick
Copy link
Member

garlick commented Mar 1, 2025

Oh duh, I should have anticipated that problem, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flux-core can't be built with flux-security < 0.14.0, but this is only necessary for a system instance
3 participants