Project

General

Profile

Feature #2452

Access to config defined attributes in the match_expr

Added by Igor Sfiligoi almost 8 years ago. Updated over 7 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Douglas Strain
Category:
Frontend
Target version:
Start date:
02/06/2012
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Spent time:
Stakeholders:
Duration:

Description

Hi all.

I just noticed that I am not able to access the attributes defined in the configuration file while evaluating the match_expr.
This is annoying, since I am instead able to access them in the start_expr.

I am thus proposing to add this capability.
Any objections?


Subtasks

Bug #2461: Review 2452ClosedDouglas Strain


Related issues

Related to GlideinWMS - Feature #5345: Expand attribute expansion capabilitiesFeedback02/04/2014

History

#1 Updated by Igor Sfiligoi almost 8 years ago

Small hiccup... the frontend currently has no clue what the values of the constants are!
While they are known at (re)config time, they are currently not loaded into the frontend process memory.

So, they need to be loaded... which brings the next issue...
where is the constants file!
The frontend process currently has no direct way to know where the stage area is.
Although it has the Web address to access it.

Now, there actually is a good reason why this is so:
since the Web area may need to be on a remote server (due to limits on who can operate Web servers at some locations),
the stage area must be locally accessible at reconfig time, but not at run time.
(e.g. AFS and tokens)

So the frontend will need to actually use the Web server to download the consts file; which implies also doing the integrity checks.
Annoying, but I don't think we can do better than this.
As a side bonus, though, the frontend will test the Web server at startup (instead asking for glideins that will fail)

#2 Updated by Igor Sfiligoi almost 8 years ago

A word why I need this capability:
In order to use the same values in both the python match_expr and in Condor start_expr.

If comparing against a list, right now I have to copy and past the long string in both parts, which is error prone; especially as the string evolves in time.

PS: I need this both for AAA Overflow and CMS AnaOps setups.

#3 Updated by Igor Sfiligoi almost 8 years ago

  • % Done changed from 0 to 70

Implemented in
branch_v2_5_5plus_igor_attrs

The new attribute is called
attr_dict

Is the name acceptable?

The only thing I am not sure if it is complete is the type of the values; the constants are all strings right now, while the parameters have the right type.
The constants file does not contain the type; I would need to correlate with the vars file; it is doable, but I ran out of time.

#4 Updated by Igor Sfiligoi almost 8 years ago

  • Status changed from New to Assigned

Added the proper type handling.

#5 Updated by Igor Sfiligoi almost 8 years ago

I have added the feature of using $$(attr) in both the factory and job query_expr.
I found I needed such functionality in my CMS frontend.

$$(attr) is the standard way Condor usually does attribute substitution in the requirements, but has no semantics in the constraints for condor_q and condor_status. So this seemed the right way forward.

Committed on
branch_v2_5_5plus_igor_attrs2
which fully includes
branch_v2_5_5plus_igor_attrs

Igor

#6 Updated by Parag Mhashilkar almost 8 years ago

Name attr_dict is too generic to convey the real meaning. We should change this to something more meaningful.

Also, while reviewing the code I found that in frontend/glideinFrontendLib.py in functions 'countMatch' and 'countRealRunning' you are missing the attr_dict but never using it. Why change the function signature?

#7 Updated by Igor Sfiligoi almost 8 years ago

The attr_dict is used when evaluating the expression.
While you don't see it explicitly in the code, this is exactly the new feature.

As for the name, I am not particularly attached to it;
I asked on a phone call if it was OK, and everyone seemed OK with it.
If you want to propose a better name, I am all ears.

Igor

#8 Updated by Parag Mhashilkar almost 8 years ago

Maybe I missed the call for attr name. Ir respectively, we should change it but I don't have any good suggestion at this moment.

#9 Updated by Igor Sfiligoi almost 8 years ago

Here is a (simplifies) example of how I use it:

   <match match_expr='job["Owner"] in attr_dict["ITB_Users"].split(",")) and (attr_dict["ITB_Foo"]&gt;0)' 
          start_expr='(stringListMember(Owner,ITB_Users,",")=?=True)&amp;&amp;(ITB_Foo&gt;0)'>

#10 Updated by Douglas Strain almost 8 years ago

Ok, I have "reviewed" this branch, but, just from the code, its not 100% clear to me how this is all working.

attr_dicts is a very ambiguous name. How about "config_attrs" since these are attributes from the config xml files (they are from the config file, right?)?

Also, its not clear to me why we need to access the staging area. Is it because the attributes are not loaded into descript files? I guess that accessing the staging area via http works, but I am not convinced (yet) of its necessity. Isn't the xml loaded into the work directory? Couldn't we just access the xml file there?

#11 Updated by Igor Sfiligoi almost 8 years ago

Yes, not all the attributes are in "descript files";
the constants are available only in the stage area.
They have to be in the stage area, because they are delivered to the glideins.
While we could make a copy in the work_dir, it would be redundant.

As for the XML file, it is never read from the daemons;
the XML file is only the input for create_frontend and reconfig_frontend to create the other files.

#12 Updated by Igor Sfiligoi almost 8 years ago

As agreed, I changed the code to not use the Web server anymore.
The reconfig now creates a file called attrs.cfg, and the daemons read it.

It has a positive side effect that the types are now always correct;
before, all constants were strings, since this is how they go to the WN.

The code is committed in branch
branch_v2_5_5plus_igor_attrs3

#13 Updated by Douglas Strain almost 8 years ago

Looking over the patch: looks fine to me. I haven't tested it, but assuming Igor has, I have no more objections to merging.

#14 Updated by Igor Sfiligoi over 7 years ago

Merged branch_v2_5_5plus_igor_attrs3 into branch_v2plus:
commit:f29511ce4f94d47e106f5b3d83da2951340d6f22

And also into master, via rebase:
commit:f3f2ac4692588bba3fe1c63b196a313daf63b138 - commit:bd37ac3d9e8e322e5d8d84231bcadb92c13b9a15

I have verified that pylint works, but have not tested either branch after the merge.

PS: Documentation still needs to be written.

#15 Updated by Parag Mhashilkar over 7 years ago

  • Target version changed from v2_5_6 to v2_5_7

#16 Updated by Krista Larson over 7 years ago

  • Assignee changed from Igor Sfiligoi to Douglas Strain

#17 Updated by Douglas Strain over 7 years ago

  • Status changed from Assigned to Resolved

I have tested this with an attribute defined in the frontend.xml that contains a user list (similar to the ITB_USERS Igor has in the example). It prints out in attrs.cfg, and only submits glideins for those users as well as prints out in the glidein pilot requirements and selects jobs only of those users. I think this is enough of a test to qualify this as "resolved".

I have also added documentation for attr_dict to the frontend configuration page and committed to both branch_v2plus and master.

#18 Updated by Parag Mhashilkar over 7 years ago

  • Target version changed from v2_5_7 to v2_7_x

#19 Updated by Parag Mhashilkar over 7 years ago

  • Target version changed from v2_7_x to v2_6

#20 Updated by Parag Mhashilkar over 7 years ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF