Created attachment 3806 [details] [PATCH 1/3] plugin mcs : add mcs_label in db The goal of this development (as explained in a SUG 2016 talk) is to put the mcs_label of a job in the database (in the job_table). 3 patchs : - the first to put this new information in the database - the second to modify sacct command in order to show mcs-label with a new format parameter mcslabel - the last to adapt the testsuite test17.51 and test17.52 to test this new option of sacct. I tested the add of this new field from scratch (no database at the beginning of the test) and from a version 17_02 without mcs_label into a version 17_02 with label. In those 2 cases, it works.
Created attachment 3807 [details] [PATCH 2/3] plugin mcs : add mcs_label in sacct output format
Created attachment 3808 [details] [PATCH 1/3] plugin mcs : add mcs_label in db : adapt tests
I'll be reviewing this further for inclusion in 17.02; looks good so far at a quick glance but I need to look at it a bit more before committing. Thanks for your patience. - Tim
I've been discussing this internally, and I won't be in 17.02 at this point. From what I can tell, most of this information already exists when using mcs/user or mcs/account - the user matches the user, and the mcs/account needs to match the job account. AFAICT, the only circumstance in which the mcs label would not be equivalent to a different field is for the mcs/group plugin; but I don't want to introduce any additional motivation for sites to rely on the Unix groups. As a potential workaround, if you do need to capture data that is not already stored in some fashion, 17.02 adds an 'AdminComment' field that is intended to be used alongside a job_submit plugin. That plugin could pack whatever additional data is required in whatever format is suitable into that field, and that will be captured in the database. I'm also noticing some apparent problems (although this may just be some issues with the documentation) where mcs/account does not appear to enforce isolation as expected - a job with --exclusive=mcs ended up running alongside a job from an unrelated account on the same node; I'll open a separate bug and look into that next week. Just as a note - one idea that came up in discussion was to tie the existing WCKey implementation in with the MCS logic - I'm not sure if that something that had been considered before? It looks like adding an mcs/wckey plugin would be straightforward, and give sites a way to build a set of security policies that (a) can take advantage of WCKey's reporting and accounting data, and (b) allow for the MCS security groups to be defined in a way divorced from the fairshare/association limits, and not tied to a Unix group. - Tim
- About your proposition to use 'AdminComment' field : Exact. This is a solution, but as you said, it's a workaround. We prefer to keep this column for a real comment on a job. We don't want to use it for another purpose, that's why we propose a new field in the database. We need this field to filter access to sacct results on mcs (with use of privatedata). - About your proposition to create a mcs/wckey plugin : That's a good idea ! Aline
Hi Tim, We really need this new feature for our accounting (use of sacct). How can we work on that ? What do you propose ? Regards Aline
Hi, That is also the only proper way we found to then have the capability to filter sacct results based on MCS label, thus providing private data to sacct logic. Regards Le jeu. 16 févr. 2017 10:27, <bugs@schedmd.com> a écrit : > *Comment # 6 <https://bugs.schedmd.com/show_bug.cgi?id=3321#c6> on bug > 3321 <https://bugs.schedmd.com/show_bug.cgi?id=3321> from aline.roy@cea.fr > <aline.roy@cea.fr> * > > Hi Tim, > > We really need this new feature for our accounting (use of sacct). How can we > work on that ? What do you propose ? > > Regards > Aline > > ------------------------------ > You are receiving this mail because: > > - You are on the CC list for the bug. > >
Reclassifying as Sev5 targeting 17.11. I believe the issues in bug 3428 have been addressed, and we can't reconsider the addition of this to the database ahead of the next release. - Tim
Matthieu/Aline, this has been added in commits de0dd49aecfe 168fea013016 5606ccb7f500 7553af4b9be3 The code only needed to massaged a bit with 17.11, thanks for the good code.
