Merge pull request #137 from matyasselmeci/pr/v4/SOFTWARE-5115.gratia-with-htcondor

Check for right PER_JOB_HISTORY_DIR with htcondor-ce probe and a condor batch system

Author: brianhlin

osg_configure/configure_modules/gratia.py

@@ -22,8 +22,6 @@
 
 CE_PROBE_RPMS = ['gratia-probe-htcondor-ce']
 
-CONDOR_CE_CONFIG_VAL = "/usr/bin/condor_ce_config_val"
-
 
 def requirements_are_installed():
     return (utilities.gateway_installed() and
@@ -69,6 +67,7 @@ def __init__(self, *args, **kwargs):
         self._old_job_managers = ['pbs', 'sge', 'lsf', 'condor', 'slurm']
         self._probe_config = {}
         self.grid_group = 'OSG'
+        self.condor_enabled = False
 
         self.log("GratiaConfiguration.__init__ completed")
 
@@ -126,6 +125,10 @@ def parse_configuration(self, configuration):
             return
 
         self._set_enabled_probe_host(self.options['probes'].value)
+
+        if utilities.config_safe_getboolean(configuration, "Condor", "enabled"):
+            self.condor_enabled = True
+
         self.log('GratiaConfiguration.parse_configuration completed')
 
     def configure(self, attributes):
@@ -241,7 +244,7 @@ def check_attributes(self, attributes):
             self.log("GratiaConfiguration.check_attributes completed")
             return True
         status = self._check_servers()
-        if 'htcondor-ce' in self._probe_config:
+        if 'htcondor-ce' in self._probe_config and requirements_are_installed():
             status &= self._verify_gratia_dirs_for_htcondor_ce_probe()
         self.log("GratiaConfiguration.check_attributes completed")
         return status
@@ -429,13 +432,19 @@ def _configure_htcondor_ce_probe(self):
     def _verify_gratia_dirs_for_htcondor_ce_probe(self) -> bool:
         """
         Verify that the HTCondor-CE PER_JOB_HISTORY_DIR and the DataFolder
-        directory are the same and warn the admin if the two don't match
-        """
+        directory are the same and warn the admin if the two don't match.
 
-        if not os.path.exists(CONDOR_CE_CONFIG_VAL):
-            raise exceptions.ConfigureError(f"{CONDOR_CE_CONFIG_VAL} missing")
+        If the batch system is condor, look at its PER_JOB_HISTORY_DIR instead;
+        we want the routed jobs, but those are moved to the batch system schedd.
 
-        history_dir = self._get_condor_ce_history_dir()
+        """
+
+        if self.condor_enabled:
+            history_dir = self._get_condor_history_dir()
+            condor_name = "Condor"
+        else:
+            history_dir = self._get_condor_ce_history_dir()
+            condor_name = "HTCondor-CE"
 
         config_location = GRATIA_CONFIG_FILES['htcondor-ce']
         contents = utilities.read_file(config_location, default="")
@@ -445,7 +454,7 @@ def _verify_gratia_dirs_for_htcondor_ce_probe(self) -> bool:
 
         advice_on_error = (
             f"Make sure DataFolder in {config_location} ({data_folder or 'missing'})"
-            f" and PER_JOB_HISTORY_DIR in the HTCondor-CE config ({history_dir or 'missing'})"
+            f" and PER_JOB_HISTORY_DIR in the {condor_name} config ({history_dir or 'missing'})"
             f" exist and are the same, accessible directory."
         )
 
@@ -480,24 +489,15 @@ def _verify_gratia_dirs_for_htcondor_ce_probe(self) -> bool:
             self.logger.error(advice_on_error)
             return False
 
-    def _get_condor_ce_history_dir(self):
-        cmd = [CONDOR_CE_CONFIG_VAL, '-subsystem', 'SCHEDD', 'PER_JOB_HISTORY_DIR']
-        try:
-            process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="latin-1")
-            history_dir, errtext = process.communicate()
-            if process.returncode != 0:
-                self.logger.info("While checking gratia parameters: %s failed. Output follows:\n%s",
-                                 CONDOR_CE_CONFIG_VAL, errtext)
-                return None
-        except OSError as err:
-            self.logger.info("While checking gratia parameters: Error running %s: %s",
-                             CONDOR_CE_CONFIG_VAL, err)
-            return None
-        history_dir = history_dir.strip()
-        if history_dir.startswith('Not defined'):
-            return None
+    @staticmethod
+    def _get_condor_history_dir():
+        history_dir = utilities.get_condor_config_val("PER_JOB_HISTORY_DIR", subsystem="SCHEDD", quiet_undefined=True)
         return history_dir
 
+    @staticmethod
+    def _get_condor_ce_history_dir():
+        return utilities.get_condor_ce_config_val("PER_JOB_HISTORY_DIR", subsystem="SCHEDD", quiet_undefined=True)
+
     @staticmethod
     def replace_setting(buf, setting, value, xml_file=True):
         """

osg_configure/modules/utilities.py

@@ -296,7 +296,7 @@ def get_condor_config(default_config='/etc/condor/condor_config'):
         return os.path.join(get_condor_location(), 'etc/condor_config')
 
 
-def get_condor_config_val(variable, executable='condor_config_val', quiet_undefined=False):
+def get_condor_config_val(variable, executable=None, quiet_undefined=False, subsystem=None):
     """
     Use condor_config_val to return the expanded value of a variable.
 
@@ -306,12 +306,24 @@ def get_condor_config_val(variable, executable='condor_config_val', quiet_undefi
                  poll condor_ce_config_val or condor_cron_config_val)
     quiet_undefined - set to True if messages from condor_config_val
                  claiming the variable is undefined should be silenced
+    subsystem - if passed, query a specific subsystem (SCHEDD, COLLECTOR, etc.)
     Returns:
     The stripped output of condor_config_val, or None if
     condor_config_val reports an error.
     """
+    if not executable:
+        condor_location = get_condor_location()
+        if condor_location:
+            executable = os.path.join(condor_location, "bin/condor_config_val")
+        else:
+            executable = "condor_config_val"
+
     try:
-        process = subprocess.Popen([executable, variable],
+        cmd = [executable]
+        if subsystem:
+            cmd.extend(["-subsystem", subsystem])
+        cmd.append(variable)
+        process = subprocess.Popen(cmd,
                                    stdout=subprocess.PIPE, stderr=subprocess.PIPE,
                                    encoding="latin-1")
         output, error = process.communicate()
@@ -324,6 +336,11 @@ def get_condor_config_val(variable, executable='condor_config_val', quiet_undefi
         return None
 
 
+def get_condor_ce_config_val(variable, *args, **kwargs):
+    kwargs["executable"] = "/usr/bin/condor_ce_config_val"
+    return get_condor_config_val(variable, *args, **kwargs)
+
+
 def read_file(filename, default=None):
     """
     Read the contents of a file, returning default if the file cannot be read.