Merge pull request #12077 from todor-ivanov/feature_SeteWhitelist_Sup…

…portChangeInReqmgr2_fix-12307

Add SiteList actions to transient statuses in ReqMgr2

src/python/WMCore/ReqMgr/DataStructs/RequestStatus.py

@@ -128,10 +128,10 @@
  "NonCustodialSites", "Override",
  "SubscriptionPriority"],
  "assigned": ["RequestPriority"],
- "staging": ["RequestPriority"],
+ "staging": ["RequestPriority", "SiteWhitelist", "SiteBlacklist"],
  "staged": ["RequestPriority"],
- "acquired": ["RequestPriority"],
- "running-open": ["RequestPriority"],
+ "acquired": ["RequestPriority", "SiteWhitelist", "SiteBlacklist"],
+ "running-open": ["RequestPriority", "SiteWhitelist", "SiteBlacklist"],
  "running-closed": ["RequestPriority"],
  "failed": [],
  "force-complete": [],
@@ -146,6 +146,18 @@
  "rejected-archived": [],
 }
 
+# NOTE: We need to Explicitly add RequestStatus to reqArgsDiff, since it is
+# missing from the ALLOWED_ACTIONS_FOR_STATUS mapping. The alternative
+# would be to add it as allowed action to every state.
+# The same applies to few more, such as all the keys needed for the
+# workqueue_stat_validation() calls, but we do this only during
+# request parameters validation.
+ALLOWED_ACTIONS_ALL_STATUS = ["RequestStatus"]
+
+# NOTE: We need to explicitly add all stat keys during validation
+# They are needed for the workqueue_stat_validation() calls
+ALLOWED_STAT_KEYS = ['total_jobs', 'input_lumis', 'input_events', 'input_num_files']
+
 # Workflow state transition automatically controlled by ReqMgr2
 ### NOTE: order of this list matters and it's used for status transition
 AUTO_TRANSITION = {"staged": ["acquired", "running-open", "running-closed", "completed"],
@@ -184,7 +196,10 @@ def get_modifiable_properties(status=None):
  TODO: Currently gets the result from hardcoded list. change to get from configuration or db
  """
  if status:
- return ALLOWED_ACTIONS_FOR_STATUS.get(status, 'all_attributes')
+ allowedKeys = ALLOWED_ACTIONS_FOR_STATUS.get(status, 'all_attributes')
+ if not allowedKeys == 'all_attributes':
+ allowedKeys.extend(ALLOWED_ACTIONS_ALL_STATUS)
+ return allowedKeys
  else:
  return ALLOWED_ACTIONS_FOR_STATUS
 

src/python/WMCore/ReqMgr/Service/Request.py

@@ -10,6 +10,7 @@
 import traceback
 
 import cherrypy
+from copy import deepcopy
 
 import WMCore.ReqMgr.Service.RegExp as rx
 from WMCore.Database.CMSCouch import CouchError
@@ -404,25 +405,51 @@ def _handleNoStatusUpdate(self, workload, request_args, dn):
  """
  For no-status update, we only support the following parameters:
  1. RequestPriority
- 2. Global workqueue statistics, while acquiring a workflow
+ 2. SiteWhitelist
+ 3. SiteBlacklist
+ 4. Global workqueue statistics, while acquiring a workflow
+ As Global workqueue statistics updates are exclusive to the rest of the
+ parameters. Meaning if it is about to be updated all the rest of the
+ request_args will be ignored.
  """
- if 'RequestPriority' in request_args:
- # Yes, we completely ignore any other arguments posted by the user (web UI case)
- request_args = {'RequestPriority': request_args['RequestPriority']}
- validate_request_priority(request_args)
- # must update three places: GQ elements, workload_cache and workload spec
- self.gq_service.updatePriority(workload.name(), request_args['RequestPriority'])
- report = self.reqmgr_db_service.updateRequestProperty(workload.name(), request_args, dn)
- workload.setPriority(request_args['RequestPriority'])
- workload.saveCouchUrl(workload.specUrl())
- cherrypy.log('Updated priority of "{}" to: {}'.format(workload.name(), request_args['RequestPriority']))
- elif workqueue_stat_validation(request_args):
- report = self.reqmgr_db_service.updateRequestStats(workload.name(), request_args)
- cherrypy.log('Updated workqueue statistics of "{}", with: {}'.format(workload.name(), request_args))
- else:
- msg = "There are invalid arguments for no-status update: %s" % request_args
+ reqArgs = deepcopy(request_args)
+
+ if not reqArgs:
+ cherrypy.log("Nothing to be changed at this stage")
+ return 'OK'
+
+ if workqueue_stat_validation(reqArgs):
+ report = self.reqmgr_db_service.updateRequestStats(workload.name(), reqArgs)
+ cherrypy.log('Updated workqueue statistics of "{}", with: {}'.format(workload.name(), reqArgs))
+ return report
+
+ reqArgsNothandled = []
+ for reqArg in reqArgs:
+ if reqArg == 'RequestPriority':
+ validate_request_priority(reqArgs)
+ # must update three places: GQ elements, workload_cache and workload spec
+ self.gq_service.updatePriority(workload.name(), reqArgs['RequestPriority'])
+ workload.setPriority(reqArgs['RequestPriority'])
+ cherrypy.log('Updated priority of "{}" to: {}'.format(workload.name(), reqArgs['RequestPriority']))
+ elif reqArg == "SiteWhitelist":
+ workload.setSiteWhitelist(reqArgs["SiteWhitelist"])
+ cherrypy.log('Updated SiteWhitelist of "{}", with: {}'.format(workload.name(), reqArgs['SiteWhitelist']))
+ elif reqArg == "SiteBlacklist":
+ workload.setSiteBlacklist(reqArgs["SiteBlacklist"])
+ cherrypy.log('Updated SiteBlacklist of "{}", with: {}'.format(workload.name(), reqArgs['SiteBlacklist']))
+ else:
+ reqArgsNothandled.append(reqArg)
+ cherrypy.log("Unhandled argument for no-status update: %s" % reqArg)
+
+ if reqArgsNothandled:
+ msg = "There were unhandled arguments left for no-status update: %s" % reqArgsNothandled
  raise InvalidSpecParameterValue(msg)
 
+ # Commit the changes of the current workload object to the database:
+ workload.saveCouchUrl(workload.specUrl())
+
+ report = self.reqmgr_db_service.updateRequestProperty(workload.name(), reqArgs, dn)
+
  return report
 
  def _handleAssignmentApprovedTransition(self, workload, request_args, dn):

src/python/WMCore/ReqMgr/Utils/Validation.py

@@ -6,14 +6,15 @@
 from future.utils import viewitems, viewvalues
 
 from hashlib import md5
+from copy import deepcopy
 
 from Utils.PythonVersion import PY3
 from Utils.Utilities import encodeUnicodeToBytesConditional
 from WMCore.Lexicon import procdataset
 from WMCore.REST.Auth import authz_match
 from WMCore.ReqMgr.DataStructs.Request import initialize_request_args, initialize_clone
 from WMCore.ReqMgr.DataStructs.RequestError import InvalidStateTransition, InvalidSpecParameterValue
-from WMCore.ReqMgr.DataStructs.RequestStatus import check_allowed_transition, STATES_ALLOW_ONLY_STATE_TRANSITION
+from WMCore.ReqMgr.DataStructs.RequestStatus import check_allowed_transition, get_modifiable_properties, STATES_ALLOW_ONLY_STATE_TRANSITION, ALLOWED_STAT_KEYS
 from WMCore.ReqMgr.Tools.cms import releases, architectures, dashboardActivities
 from WMCore.Services.DBS.DBS3Reader import getDataTiers
 from WMCore.WMFactory import WMFactory
@@ -22,11 +23,48 @@
 from WMCore.WMSpec.WMWorkloadTools import loadSpecClassByType, setArgumentsWithDefault
 from WMCore.Cache.GenericDataCache import GenericDataCache, MemoryCacheStruct
 
+
 def workqueue_stat_validation(request_args):
  stat_keys = ['total_jobs', 'input_lumis', 'input_events', 'input_num_files']
  return set(request_args.keys()) == set(stat_keys)
 
 
+def _validate_request_allowed_args(reqArgs, newReqArgs):
+ """
+ Compares two request configuration dictionaries and returns the difference
+ between them, but only at the first level (no recursive operations are attempted).
+ Returns the key/value pairs taken from the newReqArgs only if they are allowed
+ for the given request status.
+ :param reqArgs: A dictionary with the current request definition
+ :param newReqArgs: A dictionary with user-provided request parameter changes
+ :return: A dictionary reflecting the difference between the above two
+ NOTE: This is asymmetrical operation/comparison, where newReqArs' values
+ are considered to take precedence but not the other way around. They
+ are in fact checked if they differ from the values at reqArgs and only those
+ items which differ are returned.
+ """
+ reqArgsDiff = {}
+ status = reqArgs["RequestStatus"]
+
+ # Checking all keys that differ:
+ reqArgsDiffKeys = []
+ for key in newReqArgs:
+ if key in reqArgs and newReqArgs[key] == reqArgs[key]:
+ continue
+ else:
+ reqArgsDiffKeys.append(key)
+
+ allowedKeys = deepcopy(get_modifiable_properties(status))
+ allowedKeys.extend(ALLOWED_STAT_KEYS)
+
+ # Filter out all fields which are not allowed for the given source status:
+ for key in reqArgsDiffKeys:
+ if key in allowedKeys:
+ reqArgsDiff[key] = newReqArgs[key]
+
+ return reqArgsDiff
+
+
 def validate_request_update_args(request_args, config, reqmgr_db_service, param):
  """
  param and safe structure is RESTArgs structure: named tuple
@@ -43,25 +81,30 @@ def validate_request_update_args(request_args, config, reqmgr_db_service, param)
  """
  # this needs to be deleted for validation
  request_name = request_args.pop("RequestName")
+
  couchurl = '%s/%s' % (config.couch_host, config.couch_reqmgr_db)
  workload = WMWorkloadHelper()
  workload.loadSpecFromCouch(couchurl, request_name)
 
+ request = reqmgr_db_service.getRequestByNames(request_name)
+ request = request[request_name]
+ reqArgsDiff = _validate_request_allowed_args(request, request_args)
+
  # validate the status
- if "RequestStatus" in request_args:
- validate_state_transition(reqmgr_db_service, request_name, request_args["RequestStatus"])
- if request_args["RequestStatus"] in STATES_ALLOW_ONLY_STATE_TRANSITION:
+ if "RequestStatus" in reqArgsDiff:
+ validate_state_transition(reqmgr_db_service, request_name, reqArgsDiff["RequestStatus"])
+ if reqArgsDiff["RequestStatus"] in STATES_ALLOW_ONLY_STATE_TRANSITION:
  # if state change doesn't allow other transition nothing else to validate
  args_only_status = {}
- args_only_status["RequestStatus"] = request_args["RequestStatus"]
- args_only_status["cascade"] = request_args.get("cascade", False)
+ args_only_status["RequestStatus"] = reqArgsDiff["RequestStatus"]
+ args_only_status["cascade"] = reqArgsDiff.get("cascade", False)
  return workload, args_only_status
- elif request_args["RequestStatus"] == 'assigned':
- workload.validateArgumentForAssignment(request_args)
+ elif reqArgsDiff["RequestStatus"] == 'assigned':
+ workload.validateArgumentForAssignment(reqArgsDiff)
 
- validate_request_priority(request_args)
+ validate_request_priority(reqArgsDiff)
 
- return workload, request_args
+ return workload, reqArgsDiff
 
 
 def validate_request_priority(reqArgs):

test/python/WMCore_t/ReqMgr_t/Utils_t/Validation_t.py

@@ -8,8 +8,11 @@
 import unittest
 
 from WMCore.ReqMgr.DataStructs.RequestError import InvalidSpecParameterValue
+from WMCore.ReqMgr.DataStructs.RequestStatus import ACTIVE_STATUS, get_modifiable_properties
 from WMCore.ReqMgr.Utils.Validation import (validateOutputDatasets,
- validate_request_priority)
+ validate_request_priority,
+ _validate_request_allowed_args)
+from WMCore.WMSpec.StdSpecs.StdBase import StdBase
 
 
 class ValidationTests(unittest.TestCase):
@@ -74,5 +77,27 @@ def testRequestPriorityValidation(self):
  validate_request_priority(reqArgs)
 
 
+ def testValidateRequestAllowedArgs(self):
+ """
+ Tests the `_validate_request_allowed_args` functions, which validates two pairs
+ of request arguments and returns the difference between them and on top of that
+ applies a filter of allowed parameters changes per status
+ :return: nothing, raises an exception if there are problems
+ """
+ defReqArgs = StdBase.getWorkloadAssignArgs()
+ newReqArgs = {key: None for key in defReqArgs.keys()}
+
+ for status in ACTIVE_STATUS:
+ # NOTE: We need to add the RequestStatus artificially and assign it
+ # to the currently tested active status
+ defReqArgs["RequestStatus"] = status
+ expectedReqArgs = {key: None for key in get_modifiable_properties(status)}
+ reqArgsDiff = _validate_request_allowed_args(defReqArgs, newReqArgs)
+ print(f"reqArgsDiff: {reqArgsDiff}")
+ print(f"expectedReqArgs: {expectedReqArgs}")
+ self.assertDictEqual(reqArgsDiff, expectedReqArgs)
+ print("===============================")
+
+
 if __name__ == '__main__':
  unittest.main()