Adopt CMS_PATH and SITECONFIG_PATH to locate the site catalog #11481

amaltaro · 2023-02-08T17:28:22Z

Status

not-tested

Description

In light of the migration from storage.xml to storage.json, CMSSW went through some modifications to support those catalogs, in short:

any releases made before 12_6_0_pre2 will support storage.xml (through CMS_PATH environment variable)
any releases starting in 12_6_0_pre3 (exception to 12_6_3) will support only storage.json (through SITECONF_PATH environment variable).

That means, our Scram module needs to keep explicitly defining those variables in the CMSSW environment. As confirmed by Matti below, it should not be a problem to have both variables defined.

Is it backward compatible (if not, which system it affects?)

YES (to the best of my knowledge)

Related PRs

Somehow related to #11472

External dependencies / deployment changes

None

cmsdmwmbot · 2023-02-08T17:45:43Z

Jenkins results:

Python3 Unit tests: failed
- 1 new failures
- 3 tests no longer failing
- 2 changes in unstable tests
Python3 Pylint check: failed
- 23 warnings and errors that must be fixed
- 12 warnings
- 176 comments to review
Pylint py3k check: succeeded
Pycodestyle check: succeeded
- 38 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/13992/artifact/artifacts/PullRequestReport.html

makortel · 2023-02-08T18:44:28Z

@amaltaro I think WM should keep the propagation of CMS_PATH. CMSSW versions before 12_6_0_pre2 still rely on CMS_PATH for the location of site-local-config.xml. It's ok to have both defined (cmsset_default.sh anyway defines both if they are not already defined)

cmsdmwmbot · 2023-02-08T19:02:42Z

Jenkins results:

Python3 Unit tests: failed
- 1 new failures
- 4 tests no longer failing
Python3 Pylint check: failed
- 23 warnings and errors that must be fixed
- 12 warnings
- 177 comments to review
Pylint py3k check: succeeded
Pycodestyle check: succeeded
- 38 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/13994/artifact/artifacts/PullRequestReport.html

amaltaro · 2023-02-08T19:12:10Z

Noted and code updated in the Scram module. Thanks Matti.

cmsdmwmbot · 2023-02-08T21:31:34Z

Jenkins results:

Python3 Unit tests: failed
- 1 new failures
- 4 tests no longer failing
Python3 Pylint check: failed
- 23 warnings and errors that must be fixed
- 12 warnings
- 177 comments to review
Pylint py3k check: succeeded
Pycodestyle check: succeeded
- 38 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/13999/artifact/artifacts/PullRequestReport.html

todor-ivanov

Thanks @amaltaro
I am only worried about backwards compatibility here. though, If we are sure the $SITECONFIG_PATH is defined regardless of the CMSSW version, we seem to be safe.

todor-ivanov · 2023-02-09T17:12:04Z

src/python/WMCore/Storage/SiteLocalConfig.py

@@ -39,11 +39,11 @@ def loadSiteLocalConfig():
            msg = "%s env. var. provided but not pointing to an existing file, ignoring." % overVarName
            logging.log(logging.ERROR, msg)

-    defaultPath = "$CMS_PATH/SITECONF/local/JobConfig/site-local-config.xml"
+    defaultPath = "$SITECONFIG_PATH/JobConfig/site-local-config.xml"


IIRC the ../local.. part from this path here is usually a sym. link related to the site name/setup. Are we sure it has been properly expanded in the $SITECONFIG_PATH env. variable? Or should we even care to check, actually?... Maybe we just take it for granted and use what ever provided inside the environment? (just a thought here, no request for a change).

@todor-ivanov we can take it from granted, it is handled in the cms-common script below:

https://github.com/cms-sw/cms-common/blob/9c8203bf637b153697b855dfa522d6741379bcc9/cmsset_default.sh#L40-L44

thanks @khurtado, indeed we can.

amaltaro · 2023-02-09T17:24:00Z

The way I understand it is that SITECONFIG_PATH is actually defined in the CMS pilot (the same as it is for CMS_PATH). So I'd simply take it for granted as we used to do for CMS_PATH.

Regarding the local symlink, it should be just a special file name that is part of the absolute path. I fail to see any problems with that.

makortel · 2023-02-09T18:14:19Z

The way I understand it is that SITECONFIG_PATH is actually defined in the CMS pilot (the same as it is for CMS_PATH). So I'd simply take it for granted as we used to do for CMS_PATH.

This is correct.

There is one detail in the conceptual difference between SITECONFIG_PATH and CMS_PATH that may or may not be relevant here. By default SITECONFIG_PATH is defined as $CMS_PATH/SITECONF/local, but sites are allowed to override the SITECONFIG_PATH to be something else for sub-sites. This works fine for CMSSW releases >= 12_6_0_pre2 that use the SITECONFIG_PATH.

But I wonder if there is anything to be concerned about for CMSSW releases <= 12_6_0_pre1 that still use CMS_PATH? (mainly because I don't know what information exactly WM uses from the site-local-config.xml and for what) My understanding is that sites ought to keep storage.xml and storage.json consistent, and the subsites should function in some sensible way with CMSSW releases using CMS_PATH, but I'm not sure if there could be a room for some inconsistency when SITECONFIG_PATH points to different place than CMS_PATH.

Maybe @stlammel would have a better view?

khurtado · 2023-02-09T19:06:09Z

The way I understand it is that SITECONFIG_PATH is actually defined in the CMS pilot (the same as it is for CMS_PATH). So I'd simply take it for granted as we used to do for CMS_PATH.

Regarding the local symlink, it should be just a special file name that is part of the absolute path. I fail to see any problems with that.

Just some extra content here:

From what I see, both variables get defined when sourcing /cvmfs/cms.cern.ch/cmsset_default.sh

(base) [khurtado@earth /etc/profile.d]$ unset CMS_PATH SITECONFIG_PATH
(base) [khurtado@earth /etc/profile.d]$ echo $CMS_PATH $SITECONFIG_PATH

(base) [khurtado@earth /etc/profile.d]$ source /cvmfs/cms.cern.ch/cmsset_default.sh
(base) [khurtado@earth /etc/profile.d]$ echo $CMS_PATH $SITECONFIG_PATH
/cvmfs/cms.cern.ch /cvmfs/cms.cern.ch/SITECONF/local

The environment variable SITECONFIG_PATH defaults to `$CMS_PATH/SITECONF/local" but it changes when they are already pre-defined for subsites:

https://github.com/cms-sw/cms-common/blob/9c8203bf637b153697b855dfa522d6741379bcc9/cmsset_default.sh#L40-L44

An example for a file structure of a subsite would be Wisconsin and its CHTC resource (subsite resource of Wisconsin)

(base) [khurtado@earth /cvmfs/cms.cern.ch/SITECONF]$ ls T2_US_Wisconsin/
GlideinConfig/ JobConfig/     PhEDEx/        storage.json   WISC-CHTC/
(base) [khurtado@earth /cvmfs/cms.cern.ch/SITECONF]$ ls T2_US_Wisconsin/WISC-CHTC/
JobConfig  storage.json

amaltaro · 2023-02-09T19:11:47Z

An example for a file structure of a subsite would be Wisconsin and its CHTC resource (subsite resource of Wisconsin)

@khurtado Kenyi, but in this case wouldn't the worker node/pilot use a different local symlink?

In other words, it's up to the site admin to define the correct symlink that each site and subsite has to use. While our jobs simply set the environment and consume the actual catalog files provided by SITECONFIG_PATH (meant to have different different content between subsites, but still accessed through the same path). Or am I misunderstanding something in here?

khurtado · 2023-02-09T19:26:37Z

An example for a file structure of a subsite would be Wisconsin and its CHTC resource (subsite resource of Wisconsin)

@khurtado Kenyi, but in this case wouldn't the worker node/pilot use a different local symlink?

In other words, it's up to the site admin to define the correct symlink that each site and subsite has to use. While our jobs simply set the environment and consume the actual catalog files provided by SITECONFIG_PATH (meant to have different different content between subsites, but still accessed through the same path). Or am I misunderstanding something in here?

I believe the local symlink doesn't change, but rather SITECONFIG_PATH would point to the subsite directory in the local symlinks (done by the site admins), e.g.:

SITECONFIG_PATH = /cvmfs/cms.cern.ch/SITECONF/local/WISC-CHTC

where local still points to the site name T2_US_Wisconsin in the example above.

But yes, I was just putting some extra information there for the record, regarding what happens with this new variable. But for us, we ultimately don't care how this is set, we just take it for granted. The validation of these paths are done elsewhere (during the setup of a site or sub-site, during the pilot validation tests, etc).

khurtado

@amaltaro Changes to the actual python scripts look good to me, I only have some comments/changes requested to the unit tests.

khurtado · 2023-02-09T19:13:58Z

test/python/WMCore_t/Misc_t/Runtime_t.py

@@ -155,7 +155,7 @@ def createTestWorkload(self, workloadName='Test', emulator=True):
            os.makedirs(siteConfigPath)
        shutil.copy('site-local-config.xml', siteConfigPath)
        environment = rereco.data.section_('environment')
-        environment.CMS_PATH = workloadDir
+        environment.SITECONFIG_PATH = workloadDir


Shouldn't SITECONFIG_PATH be:

environment.SITECONFIG_PATH = "%s/SITECONF/local" % workloadDir

To make it consistent with the default definition here:
https://github.com/cms-sw/cms-common/blob/9c8203bf637b153697b855dfa522d6741379bcc9/cmsset_default.sh#L40-L44

I am not sure from the L153 above. Maybe it even does not really matter, but let me make this change.

khurtado · 2023-02-09T19:15:05Z

test/python/WMCore_t/Storage_t/SiteLocalConfig_t.py

@@ -155,7 +155,7 @@ def testSlcPhedexNodesEqualPhedexApiNodes(self):
        For each site, verify that the stageout node specified in
        site-local-config.xml is the same as the one returned by the PhEDEx api.
        """
-        os.environ["CMS_PATH"] = "/cvmfs/cms.cern.ch"
+        os.environ["SITECONFIG_PATH"] = "/cvmfs/cms.cern.ch/SITECONFIG/local"


Replace SITECONFIG with SITECONF in the cvmfs path

test/python/WMCore_t/Storage_t/StageOutMgr_t.py

test/python/WMCore_t/WMSpec_t/Steps_t/Executors_t/LogArch_t.py

test/python/WMCore_t/WMSpec_t/Steps_t/Executors_t/StageOut_t.py

stlammel · 2023-02-09T19:43:56Z

So sites are keeping access via both CMS_PATH (for older CMSSW versions) and SITECONFIG_PATH (for newer CMSSW versions) working right now. How the local link in CVMFS is set is up to each site. Most sites set it to the sitename and site/subsite for subsites but in case of a local SITECONF copy this doesn't need to be the case. ${CMS_PATH}/SITECONF/local is guaranteed to point to a good SITECONF area and ${SITECONF_PATH}.
Hope this helps.

Stephan

cmsdmwmbot · 2023-02-09T23:42:50Z

Jenkins results:

Python3 Unit tests: failed
- 1 new failures
- 1 tests no longer failing
- 2 changes in unstable tests
Python3 Pylint check: failed
- 25 warnings and errors that must be fixed
- 12 warnings
- 177 comments to review
Pylint py3k check: succeeded
Pycodestyle check: succeeded
- 38 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14003/artifact/artifacts/PullRequestReport.html

cmsdmwmbot · 2023-02-10T00:18:26Z

Jenkins results:

Python3 Unit tests: succeeded
- 2 tests added
- 1 changes in unstable tests
Python3 Pylint check: failed
- 25 warnings and errors that must be fixed
- 12 warnings
- 177 comments to review
Pylint py3k check: succeeded
Pycodestyle check: succeeded
- 38 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14004/artifact/artifacts/PullRequestReport.html

amaltaro · 2023-02-10T00:52:05Z

Thanks for the review, @khurtado. Some unit tests were actually broken and I managed to get them fixed with this PR. For now, I kept unit tests defining both environment variables, as you suggested. Please have another look

khurtado · 2023-02-10T01:40:24Z

@amaltaro Looking good! There is just one unit test that I think is still missing the CMS_PATH, besides that it all looks good to me.

Keep CMS_PATH around in the Scram module

more fixes to unit tests trying to run integration unit test fix stageout unit test

cmsdmwmbot · 2023-02-10T01:59:49Z

Jenkins results:

Python3 Unit tests: failed
- 1 new failures
- 1 tests no longer failing
- 2 tests added
- 1 changes in unstable tests
Python3 Pylint check: failed
- 25 warnings and errors that must be fixed
- 12 warnings
- 177 comments to review
Pylint py3k check: succeeded
Pycodestyle check: succeeded
- 38 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14005/artifact/artifacts/PullRequestReport.html

germanfgv · 2023-02-10T09:02:30Z

@amaltaro I tested #11473 and this on top of T0 agent 3.0.8 (WMCore 2.1.6.1) and the replays were successful, both with CMSSW_12_6_2 (RucioCatalog) and CMSSW_12_6_3 (TrivialFileCatalog)

amaltaro · 2023-02-10T12:36:14Z

Thank you for the review and further tests!

stlammel · 2023-02-10T15:30:18Z

Thanks everybody for discovering and handling the dependency! - Stephan

makortel · 2023-02-10T16:42:53Z

@amaltaro I tested #11473 and this on top of T0 agent 3.0.8 (WMCore 2.1.6.1) and the replays were successful, both with CMSSW_12_6_2 (RucioCatalog) and CMSSW_12_6_3 (TrivialFileCatalog)

Thanks @germanfgv! Does this mean we could safely switch 12_6_X back to RucioCatalog?

germanfgv · 2023-02-13T15:22:54Z

@makortel I still would like to test how it will work with T2_CH_CERN_P5 worker nodes. I'll launch test this tonight with a replay.

makortel · 2023-02-13T15:29:21Z

Thanks @germanfgv!

makortel · 2023-02-13T18:36:36Z

@germanfgv In the mean time I took the liberty of opening a PR to change the file catalog to Rucio catalog in 12_6_X in cms-sw/cmssw#40755 . If your tests succeed and the PR could proceed, could mention it in that PR as well?

amaltaro added the PR: Do not merge yet label Feb 8, 2023

amaltaro added the PR: squashing needed label Feb 8, 2023

amaltaro changed the title ~~Move site config setup from CMS_PATH to SITECONFIG_PATH~~ Adopt CMS_PATH and SITECONFIG_PATH to locate the site catalog Feb 8, 2023

amaltaro added PR: Need wmagent branch and removed PR: squashing needed labels Feb 8, 2023

amaltaro force-pushed the fix-11449 branch from 37cd280 to e747857 Compare February 8, 2023 21:20

amaltaro mentioned this pull request Feb 9, 2023

Deprecate TFC override for multi-step jobs #11473

Merged

amaltaro requested review from khurtado and todor-ivanov February 9, 2023 13:00

amaltaro removed the PR: Do not merge yet label Feb 9, 2023

todor-ivanov reviewed Feb 9, 2023

View reviewed changes

khurtado requested a review from todor-ivanov February 9, 2023 19:21

khurtado requested changes Feb 9, 2023

View reviewed changes

amaltaro requested a review from khurtado February 10, 2023 00:06

amaltaro added the PR: squashing needed label Feb 10, 2023

amaltaro added 2 commits February 9, 2023 20:48

Move site config setup from CMS_PATH to SITECONFIG_PATH

178cd89

Keep CMS_PATH around in the Scram module

fix unit tests

c46cba5

more fixes to unit tests trying to run integration unit test fix stageout unit test

amaltaro force-pushed the fix-11449 branch from fecb5e9 to c46cba5 Compare February 10, 2023 01:48

amaltaro removed the PR: squashing needed label Feb 10, 2023

amaltaro merged commit 6695980 into dmwm:master Feb 10, 2023

amaltaro mentioned this pull request Feb 10, 2023

Refactor TFC override for multi-step jobs; adopt SITECONF_PATH during runtime - wmagent branch #11483

Merged

makortel mentioned this pull request Feb 13, 2023

[12_6_X] Switch file catalog back to Rucio catalog cms-sw/cmssw#40755

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adopt CMS_PATH and SITECONFIG_PATH to locate the site catalog #11481

Adopt CMS_PATH and SITECONFIG_PATH to locate the site catalog #11481

amaltaro commented Feb 8, 2023 •

edited

Loading

cmsdmwmbot commented Feb 8, 2023

makortel commented Feb 8, 2023

cmsdmwmbot commented Feb 8, 2023

amaltaro commented Feb 8, 2023

cmsdmwmbot commented Feb 8, 2023

todor-ivanov left a comment

todor-ivanov Feb 9, 2023

khurtado Feb 9, 2023

todor-ivanov Feb 10, 2023

amaltaro commented Feb 9, 2023

makortel commented Feb 9, 2023

khurtado commented Feb 9, 2023

amaltaro commented Feb 9, 2023

khurtado commented Feb 9, 2023 •

edited

Loading

khurtado left a comment

khurtado Feb 9, 2023

amaltaro Feb 9, 2023

khurtado Feb 9, 2023

stlammel commented Feb 9, 2023

cmsdmwmbot commented Feb 9, 2023

cmsdmwmbot commented Feb 10, 2023

amaltaro commented Feb 10, 2023

khurtado commented Feb 10, 2023

cmsdmwmbot commented Feb 10, 2023

germanfgv commented Feb 10, 2023

amaltaro commented Feb 10, 2023

stlammel commented Feb 10, 2023

makortel commented Feb 10, 2023

germanfgv commented Feb 13, 2023

makortel commented Feb 13, 2023

makortel commented Feb 13, 2023

Adopt CMS_PATH and SITECONFIG_PATH to locate the site catalog #11481

Adopt CMS_PATH and SITECONFIG_PATH to locate the site catalog #11481

Conversation

amaltaro commented Feb 8, 2023 • edited Loading

Status

Description

Is it backward compatible (if not, which system it affects?)

Related PRs

External dependencies / deployment changes

cmsdmwmbot commented Feb 8, 2023

makortel commented Feb 8, 2023

cmsdmwmbot commented Feb 8, 2023

amaltaro commented Feb 8, 2023

cmsdmwmbot commented Feb 8, 2023

todor-ivanov left a comment

Choose a reason for hiding this comment

todor-ivanov Feb 9, 2023

Choose a reason for hiding this comment

khurtado Feb 9, 2023

Choose a reason for hiding this comment

todor-ivanov Feb 10, 2023

Choose a reason for hiding this comment

amaltaro commented Feb 9, 2023

makortel commented Feb 9, 2023

khurtado commented Feb 9, 2023

amaltaro commented Feb 9, 2023

khurtado commented Feb 9, 2023 • edited Loading

khurtado left a comment

Choose a reason for hiding this comment

khurtado Feb 9, 2023

Choose a reason for hiding this comment

amaltaro Feb 9, 2023

Choose a reason for hiding this comment

khurtado Feb 9, 2023

Choose a reason for hiding this comment

stlammel commented Feb 9, 2023

cmsdmwmbot commented Feb 9, 2023

cmsdmwmbot commented Feb 10, 2023

amaltaro commented Feb 10, 2023

khurtado commented Feb 10, 2023

cmsdmwmbot commented Feb 10, 2023

germanfgv commented Feb 10, 2023

amaltaro commented Feb 10, 2023

stlammel commented Feb 10, 2023

makortel commented Feb 10, 2023

germanfgv commented Feb 13, 2023

makortel commented Feb 13, 2023

makortel commented Feb 13, 2023

amaltaro commented Feb 8, 2023 •

edited

Loading

khurtado commented Feb 9, 2023 •

edited

Loading