Project

General

Profile

Feature #5345

Expand attribute expansion capabilities

Added by Igor Sfiligoi about 7 years ago. Updated 13 days ago.

Status:
Assigned
Priority:
Normal
Category:
Configuration Management
Target version:
Start date:
02/04/2014
Due date:
% Done:

0%

Estimated time:
Stakeholders:

CMS, OSG

Duration:

Description

Right now, the only place where we allow for attribute expansion is in the FE expressions.

This functionality would instead be useful everywhere else.
Including nested/recursive (but no loops) expansion.


Related issues

Related to GlideinWMS - Feature #2452: Access to config defined attributes in the match_exprClosed02/06/2012

History

#1 Updated by Igor Sfiligoi about 7 years ago

  • Priority changed from Normal to High

#2 Updated by Igor Sfiligoi about 7 years ago

Looking into the details of $$ expansion, I see it honors the attribute type.
Which means, strings get quoted during expansion.

This makes is useless for generic expansion;
e.g. could not be used to propagate the host name in the factory config.

Moreover, it is also hard to imagine doing recursive expansions this way.

We need a way to get "plain expansion" semantics.
I see three possible ways:
  1. just change the $$ semantics, and break backwards compatibility
  2. having different semantics depending where it is used, so current use cases are covered in backwards-compatible way
  3. use a different symbol for plain expansion, and keep $$ semantics as is (just extended to all strings)

I don't really like any of the above solutions, so if you can come up with a better idea, please let me know ASAP.

#3 Updated by Igor Sfiligoi about 7 years ago

Looking at the XML files I have access to, I see that single $ is only used in
GLIDEIN_CPUS=$(DETECTED_CORES)

I think we can deal with this backward incompatibility.

So I am proposing to use single $() expansion for straight replacement, and keep $$() for quoted replacement.

We also define $(DOLLAR) as being expanded to $.

If anyone objects to this plan, please let me know ASAP.

#4 Updated by Igor Sfiligoi about 7 years ago

Looking in more details, it looks like this is going to be harder than I thought, unless we make some semantic changes.

The main problem is that we may have entry/group attributes influence the global values.
Since we write (a subset of) the global values to disk, and reuse them for all the entries/groups,
we would need to do the expansion at runtime.
Which would include both the python processes AND in the glideins.
And in the glideins we actually currently lacking some of the info, too.

I don't think that is really reasonable, so I am proposing to "promote" any global value that needs expansion into the entry/group level.
This way I can do the expansion at (re)config time.

This may slightly increase the disc space used, but I think the benefits way outweigh the drawbacks.

Let me know if you disagree with this plan.

#5 Updated by Parag Mhashilkar about 7 years ago

I am in favor of putting everything in the leaf node (directory). This way the entry directory can be populated at reconfig time and will be functional by itself. Might as well simplify the code.

#6 Updated by Igor Sfiligoi about 7 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from Igor Sfiligoi to Parag Mhashilkar
  • Priority changed from High to Normal
  • Target version changed from v3_x to v3_2_x

The proposed semantics was implemented both for factory and frontend.
I have committed the code in
v3/5345

Please review.

PS: I am selectively promoting only attributes than need expansion.
As mentioned on the phone call, we can extend that in a separate ticket.

#7 Updated by Igor Sfiligoi almost 7 years ago

  • Status changed from Feedback to Assigned
  • Assignee changed from Parag Mhashilkar to Igor Sfiligoi

Further testing showed some bugs.
Re-taking ownership of it.

#8 Updated by Igor Sfiligoi almost 7 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from Igor Sfiligoi to Parag Mhashilkar

Fixed the discovered bugs (still in v3/5345).

Should be ready to be reviewed.

#9 Updated by Igor Sfiligoi almost 7 years ago

  • Target version changed from v3_2_x to v3_2_6

Merged with v3_2_4 and fixed the conflicts.

Now in v3/5345_v2.

#10 Updated by Parag Mhashilkar almost 7 years ago

  • Assignee changed from Parag Mhashilkar to Igor Sfiligoi

Reviewed v3/5345_v2 and sent feedback separately.

#11 Updated by Burt Holzman over 6 years ago

  • Target version changed from v3_2_6 to v3_2_x

#12 Updated by Igor Sfiligoi over 6 years ago

  • Assignee changed from Igor Sfiligoi to Parag Mhashilkar
  • Target version changed from v3_2_x to v3_2_7

I think I have addressed all the points, including adding the documentation.

The new code is now in v3/5345_v3.

Please review.

#13 Updated by Burt Holzman over 6 years ago

  • Target version changed from v3_2_7 to v3_2_8

#14 Updated by Parag Mhashilkar over 6 years ago

  • Target version changed from v3_2_8 to v3_2_x

#15 Updated by Marco Mambelli almost 3 years ago

  • Target version changed from v3_2_x to v3_4_x

#16 Updated by Marco Mambelli over 2 years ago

  • Target version changed from v3_4_x to v3_5_x

#17 Updated by Marco Mambelli over 1 year ago

  • Target version changed from v3_5_x to v3_6_x

#18 Updated by Marco Mascheroni 6 months ago

Marco Mambelli, this is in Feedback, what should we do with this? I don't think CMS needs it, but code was already developed..

#19 Updated by Marco Mambelli 16 days ago

  • Target version changed from v3_6_x to v3_7_3
  • Assignee changed from Parag Mhashilkar to Marco Mambelli

#20 Updated by Marco Mambelli 14 days ago

  • Target version changed from v3_7_3 to v3_6_7
  • Status changed from Feedback to Assigned

changes are in branch v36/5345_v5
This is a new branch starting from master. Reviewed, reapplied, and modified all the changes in v3/5345_v3.
A 3-way comparison was used (base 63e06efb33ba0bdbd2df6509e50c6e02d42c482c, v3/5345_v3 and master):

    new file:   creation/lib/cWExpand.py
    modified:   creation/lib/cWDictFile.py
    modified:   creation/lib/cgWCreate.py
    modified:   creation/lib/cgWParamDict.py
    modified:   creation/lib/cvWDictFile.py
    modified:   creation/lib/cvWParamDict.py
    modified:   creation/lib/cvWParams.py
    modified:   doc/factory/configuration.html
    modified:   doc/frontend/configuration.html
    modified:   frontend/glideinFrontendElement.py
    modified:   unittests/test_creation_lib_cgWDictFile.py

Commits suggested from rebase (original commits):
pick 5473cd46 Move all attributes and description items that need to be expanded from main into group
pick 759018b2 Move parameters around slightly
pick b2206ffe Move FE consts from StrWWorkTypeDictFile to StrDictFile. The simpler class works just as well
pick f5761a8e Add attribute expansion for the Frontend
pick 097701ac Move the validation of the match string in cvWParamDict, after the attribute expansion
pick 3cd23cb3 Correct docs; attributes are always strings in the frontend process
pick 4ed09842 Remove expand_DD from frontend process, since it is now being done at reconfig time
pick 1d3f4842 Move factory attributes that need expansion from global to entry
pick 8fdca775 Add attribute expansion to factory as well
pick d7f1b96c Fix error reporting string
pick 27fcf682 Fix ordering
pick 9e0a2ddb Not all values in a dict are strings, so expansion testwas not working. Also, unlike the FE, the GF params are a normal dictionary
pick 1d5a5182 Fix typo
pick 0871fd4c Use for x in dict instead of for x in dict.keys()
pick a45774bd Get rid of duplicate code
pick bafa3090 Slightly change derive_and_validate_match invocation arguments. Document validate_match and derive_and_validate_match.
pick d92255c2 Revert "Use for x in dict instead of for x in dict.keys()", since the Params objects don't support the "for in dict" syntax.
pick 65bcd50a 5345# Document the attribute substitution in the Frontend
pick 5c8857ed 5345# Slight improvement to the documentation
pick 0e03586f Add the get method to the DictFile class
pick ce813760 5345# Attributes that do not end in the glideins are still useful as reference in other attributes
pick 2534817d 5345# Document attribute substitution in the Factory docs

# Rebase 34d12e0a..2534817d onto 34d12e0a (22 commands)

#21 Updated by Marco Mambelli 13 days ago

  • Assignee changed from Marco Mambelli to Marco Mascheroni

Hi Marco,
please take over this ticket and test this with a reasonable configuration.
I'm not sure if the current problems are w/ the code or the test I added.

If there are problems you can decide that it is not worth to pursue and we'll drop the feature

Thanks,
Marco

Also available in: Atom PDF