plugin: add project information to Association information in plugin #434
Conversation
This file contains 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
cmoussa1
added
new feature
cmoussa1
force-pushed
the
projects-plugin
branch
from
ffdab94 to
cd3d659
Compare
cmoussa1
changed the title
jameshcorbett
approved these changes
Mar 27, 2024
| || flux_jobtap_service_register (p, "rec_q_update", rec_q_cb, p) < 0) | ||
| || flux_jobtap_service_register (p, "rec_q_update", rec_q_cb, p) | ||
| || flux_jobtap_service_register (p, "rec_proj_update", rec_proj_cb, p) | ||
| < 0) |
There was a problem hiding this comment.
Why the < 0 check only for the last one in this || chain?
There was a problem hiding this comment.
Oops, I guess I forgot to include a < 0 check for the other calls here. Thanks for catching this! I added another commit to this PR that adds checks for all of the flux_jobtap_service_register () calls here
src/plugins/mf_priority.cpp
Outdated
| // clear the projects vector | ||
| projects.clear (); | ||
|
|
||
| for (int i = 0; i < num_data; i++) { |
There was a problem hiding this comment.
This looks like a good place for json_array_foreach?
There was a problem hiding this comment.
I think you're right! I converted this for-loop to use json_array_foreach (). Thanks for pointing that out
cmoussa1
force-pushed
the
projects-plugin
branch
from
cd3d659 to
45b06d9
Compare
|
Thanks for approving @jameshcorbett - will set MWP here shortly |
Problem: The priority plugin has no way to unpack project information from the flux-accounting database. Add two new members to the Association class: projects and def_project, which will store the projects an association is allowed to charge as well as their default project. Add an unpack of the "projects" and "def_project" columns for each association when unpacking them in the rec_update_cb () function. Add a new callback to unpack information from the project_table and store it in a vector of projects to be used for project validation and annotation.
Problem: flux-accounting does not send any information regarding projects to the priority plugin. Add information from the project_table and the projects column in the association_table to the bulk update script that sends user, bank, and queue information to the multi-factor priority plugin.
Problem: Some tests in flux-accounting's testsuite don't contain active, project, or default project information in the payloads that get sent to the priority plugin. Add "active", "projects", and "def_project" key-value pairs to the fake payloads in a number of tests in the testsuite.
Problem: There exists no way to validate a project for an association in the priority plugin. Add a new helper function get_project_info () which will validate a passed-in project for an association. If the project is unknown to flux-accounting or is an invalid one for an association to submit their job under, an integer < 0 will be returned, otherwise 0.
Problem: There exists no unit tests for get_project_info (). Add some unit tests.
Problem: A number of the flux_jobtap_service_register () calls in the initialization of the plugin don't actually check the result of the call for < 0. Add these checks when registering the callbacks for the plugin.
cmoussa1
force-pushed
the
projects-plugin
branch
from
45b06d9 to
bf90b45
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #434 +/- ##
==========================================
+ Coverage 83.39% 83.47% +0.08%
==========================================
Files 23 23
Lines 1457 1525 +68
==========================================
+ Hits 1215 1273 +58
- Misses 242 252 +10
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Background
flux-accounting has the ability to define and list projects for associations in the database, but the plugin does not know about or interact with them. Eventually, the plugin will need to support associating jobs with projects. This is the first step to getting there.
This PR looks to start the process of adding project support to the priority plugin by just adding project information to the flux-accounting information that gets sent from the database to the plugin. Each
Associationobject now has two new member fields:projectsanddef_project, fields to store their list of projects and their default project, respectively. This info, along with the list of all registered projects, is sent and unpacked and stored in the plugin. At the moment, it is not utilized anywhere in the plugin, it is simply just unpacked and added to eachAssociationobject. Future PRs will look to integrate actual support for associating jobs with projects.The other piece of this PR is to add a helper function to
accounting.cppto help validate projects for a given association. The plan with this function is to use it when validating a job that charges a certain project. Unit tests are also added for this helper function.