LU-16502 lutf: cleanup lutf_start.py, fix bugs

Remove code duplication by adding __check_env_var
method. Use os.environ.get to remove needlessly
verbose try-except blocks. Use __cfg_yaml more,
rather than passing this value explicitly
between methods.

Add check for environment variables, so LUTF
can fail gracefully if the environment is not
set correctly.

Update .gitignore to ignore __pycache__

Fix syntax error in python/infra/lutf.py

Test-Parameters: @lnet
Signed-off-by: Timothy Day <timday@amazon.com>
Change-Id: I16cf114ac4253d22a42399e2f2cb2fad49dd96cb
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/49767
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Arshad Hussain <arshad.hussain@aeoncomputing.com>
Reviewed-by: Cyril Bordage <cbordage@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/tests/lutf/.gitignore b/lustre/tests/lutf/.gitignore
index 70845e0..d10b898 100644 (file)
--- a/lustre/tests/lutf/.gitignore
+++ b/lustre/tests/lutf/.gitignore
@@ -1 +1,2 @@
 Makefile.in
+__pycache__

diff --git a/lustre/tests/lutf/python/config/lutf_start.py b/lustre/tests/lutf/python/config/lutf_start.py
index d91d259..d92cd1b 100644 (file)
--- a/lustre/tests/lutf/python/config/lutf_start.py
+++ b/lustre/tests/lutf/python/config/lutf_start.py
@@ -3,10 +3,10 @@ lutf_start.py is a script intended to be run from the lutf.sh
 It relies on a set of environment variables to be set. If they
 are not set the script will exit:
 
-*_HOST: nodes to run the LUTF on. They must be unique
-ONLY: A script to run
-SUITE: A suite to run
-LUTF_SHELL: If specified it'll run the python interpreter
+*_HOST: nodes to run the LUTF on. They must be unique (optional)
+ONLY: A script to run (optional)
+SUITE: A suite to run (optional)
+LUTF_SHELL: If specified it'll run the python interpreter (optional)
 MASTER_PORT: The port on which the master will listen
 TELNET_PORT: The port on which a telnet session can be established to the agent
 LUSTRE: The path to the lustre tests directory
@@ -61,7 +61,7 @@ class LUTF:
                out = subprocess.Popen(args, stderr=subprocess.STDOUT,
                        stdout=subprocess.PIPE)
                t = out.communicate()[0],out.returncode
-               print(cmd+"\n"+"rc = "+str(t[1])+" "+t[0].decode("utf-8"))
+               print(cmd+"\n"+"rc = "+str(t[1])+"\n"+t[0].decode("utf-8"))
                return int(t[1])
 
        def __stop_lutf(self, key, host):
@@ -70,14 +70,9 @@ class LUTF:
                        lutf_exec_remote_cmd("pkill -9 lutf", host)
 
        def __install_deps_on_hosts(self, host):
-               try:
-                       installbin = os.environ['INSTALLBIN']
-               except:
-                       installbin = 'yum'
-               try:
-                       pip = os.environ['PIPBIN']
-               except:
-                       pip = 'pip3'
+               installbin = os.environ.get('INSTALLBIN', 'yum')
+               pip = os.environ.get('PIPBIN', 'pip3')
+
                print("%s: %s install -y python3" % (host, installbin))
                lutf_exec_remote_cmd(installbin+" install -y python3", host, ignore_err=True)
                print("%s: %s install paramiko" % (host, pip))
@@ -90,97 +85,63 @@ class LUTF:
                lutf_exec_remote_cmd(pip+" install psutil", host, ignore_err=True)
 
        def __start_lutf(self, key, host, mname, master, agent_list=[], agent=True):
-               cfg = copy.deepcopy(cfg_yaml)
-               shell = 'batch'
-               try:
-                       shell = os.environ['LUTF_SHELL']
-               except:
-                       pass
+               self.__cfg_yaml = copy.deepcopy(cfg_yaml)
+
+               self.__cfg_yaml['lutf']['shell'] = os.environ.get('LUTF_SHELL', 'batch')
+
                # agent will always run in daemon mode
                if (agent):
-                       cfg['lutf']['shell'] = 'daemon'
-               else:
-                       cfg['lutf']['shell'] = shell
-               cfg['lutf']['agent'] = agent
-               cfg['lutf']['telnet-port'] = int(os.environ['TELNET_PORT'])
-               cfg['lutf']['master-address'] = master
-               cfg['lutf']['master-port'] = int(os.environ['MASTER_PORT'])
-               cfg['lutf']['node-name'] = key
-               cfg['lutf']['master-name'] = mname
-               cfg['lutf']['lutf-path'] = os.environ['LUTFPATH']
-               cfg['lutf']['py-path'] = os.environ['PYTHONPATH']
+                       self.__cfg_yaml['lutf']['shell'] = 'daemon'
+
+               self.__cfg_yaml['lutf']['agent'] = agent
+               self.__cfg_yaml['lutf']['telnet-port'] = int(os.environ['TELNET_PORT'])
+               self.__cfg_yaml['lutf']['master-address'] = master
+               self.__cfg_yaml['lutf']['master-port'] = int(os.environ['MASTER_PORT'])
+               self.__cfg_yaml['lutf']['node-name'] = key
+               self.__cfg_yaml['lutf']['master-name'] = mname
+               self.__cfg_yaml['lutf']['lutf-path'] = os.environ['LUTFPATH']
+               self.__cfg_yaml['lutf']['py-path'] = os.environ['PYTHONPATH']
+
                try:
                        sl = re.split(',| ', os.environ['SUITE'])
                        if len(sl) == 0:
                                raise ValueError
                        if len(sl) == 1:
-                               cfg['lutf']['suite'] = os.environ['SUITE']
-                               del(cfg['lutf']['suite-list'])
+                               self.__cfg_yaml['lutf']['suite'] = os.environ['SUITE']
+                               del(self.__cfg_yaml['lutf']['suite-list'])
                        else:
-                               cfg['lutf']['suite-list'] = os.environ['SUITE']
-                               del(cfg['lutf']['suite'])
-               except:
-                       if 'suite' in cfg['lutf']:
-                               del(cfg['lutf']['suite'])
-               try:
-                       cfg['lutf']['lustre-path']  = os.environ['LUSTRE']
-               except:
-                       if 'lustre-path' in cfg['lutf']:
-                               del(cfg['lutf']['lustre-path'])
-               try:
-                       cfg['lutf']['script']  = os.environ['ONLY']
-               except:
-                       if 'script' in cfg['lutf']:
-                               del(cfg['lutf']['script'])
-               try:
-                       cfg['lutf']['pattern']  = os.environ['PATTERN']
-               except:
-                       if 'pattern' in cfg['lutf']:
-                               del(cfg['lutf']['pattern'])
-               try:
-                       cfg['lutf']['results']  = os.environ['RESULTS']
-               except:
-                       if 'results' in cfg['lutf']:
-                               del(cfg['lutf']['results'])
-               try:
-                       cfg['lutf']['always_except'] = os.environ['ALWAYS_EXCEPT']
-               except:
-                       if 'always_except' in cfg['lutf']:
-                               del(cfg['lutf']['always_except'])
-               try:
-                       cfg['lutf']['num_intfs'] = os.environ['NUM_INTFS']
+                               self.__cfg_yaml['lutf']['suite-list'] = os.environ['SUITE']
+                               del(self.__cfg_yaml['lutf']['suite'])
                except:
-                       if 'num_intfs' in cfg['lutf']:
-                               del(cfg['lutf']['num_intfs'])
-               try:
-                       cfg['lutf']['lutf-env-vars'] = os.environ['LUTF_ENV_VARS']
-               except:
-                       if 'lutf-env-vars' in cfg['lutf']:
-                               del(cfg['lutf']['lutf-env-vars'])
-               try:
-                       cfg['lutf']['test-progress'] = os.environ['LUTF_TEST_PROGRESS']
-               except:
-                       if 'test-progress' in cfg['lutf']:
-                               del(cfg['lutf']['test-progress'])
-               try:
-                       cfg['lutf']['tmp-dir'] = os.environ['LUTF_TMP_DIR']
-               except:
-                       pass
+                       if 'suite' in self.__cfg_yaml['lutf']:
+                               del(self.__cfg_yaml['lutf']['suite'])
+
+               self.__check_env_var('lustre-path', 'LUSTRE')
+               self.__check_env_var('script', 'ONLY')
+               self.__check_env_var('pattern')
+               self.__check_env_var('results')
+               self.__check_env_var('always_except')
+               self.__check_env_var('num_intfs')
+               self.__check_env_var('lutf-env-vars')
+               self.__check_env_var('test-progress', 'LUTF_TEST_PROGRESS')
+
+               if os.environ.get('LUTF_TMP_DIR') is not None:
+                       cfg['lutf']['tmp-dir'] = os.environ.get('LUTF_TMP_DIR')
 
                if len(agent_list) > 0:
-                       cfg['lutf']['agent-list'] = agent_list
+                       self.__cfg_yaml['lutf']['agent-list'] = agent_list
                else:
-                       if 'agent-list' in cfg['lutf']:
-                               del(cfg['lutf']['agent-list'])
+                       if 'agent-list' in self.__cfg_yaml['lutf']:
+                               del(self.__cfg_yaml['lutf']['agent-list'])
 
                lutf_bin = 'lutf'
                cfg_name = host+'.yaml'
-               lutf_cfg_path = os.path.join(cfg['lutf']['tmp-dir'], 'config')
+               lutf_cfg_path = os.path.join(self.__cfg_yaml['lutf']['tmp-dir'], 'config')
                Path(lutf_cfg_path).mkdir(parents=True, exist_ok=True)
                lutf_cfg = os.path.join(lutf_cfg_path, cfg_name)
                # write the config file
                with open(lutf_cfg, 'w') as f:
-                       f.write(yaml.dump(cfg, Dumper=LutfDumper, indent=4))
+                       f.write(yaml.dump(self.__cfg_yaml, Dumper=LutfDumper, indent=4))
                # copy it over to the remote
                if host != os.environ['HOSTNAME']:
                        lutf_exec_remote_cmd("mkdir -p " + lutf_cfg_path, host)
@@ -223,8 +184,17 @@ class LUTF:
                                print(e)
                                rc = -22
 
-               self.__cfg_yaml = cfg
-               return cfg, rc
+               return rc
+
+       def __check_env_var(self, var_name, env_var_name=None):
+               if env_var_name is None:
+                       env_var_name = var_name.upper().replace('-', '_')
+
+               if os.environ.get(env_var_name) is not None:
+                       self.__cfg_yaml['lutf'][var_name] = os.environ.get(env_var_name)
+               else:
+                       if var_name in self.__cfg_yaml['lutf']:
+                               del(self.__cfg_yaml['lutf'][var_name])
 
        def __collect_lutf_logs(self, host):
                if host != os.environ['HOSTNAME']:
@@ -241,6 +211,19 @@ class LUTF:
                        lutf_exec_remote_cmd(cmd, host, ignore_err=True);
                        lutf_get_file(host, rfpath, os.path.join(tmp_dir, rfname))
 
+       def check_environment(self):
+               needed_vars = ['LUTFPATH','PYTHONPATH','TELNET_PORT', 'MASTER_PORT',
+                               'HOSTNAME', 'LD_LIBRARY_PATH', 'PATH', 'LUSTRE']
+
+               for var in needed_vars:
+                       try:
+                               os.environ[var]
+                       except KeyError:
+                               print("Missing ", var, " environment variable\n")
+                               return -22
+
+               return 0
+
        def run(self):
                master = ''
                mname = ''
@@ -263,13 +246,13 @@ class LUTF:
                rc = 0
                for k, v in self.nodes.items():
                        if v != master:
-                               cfg, rc = self.__start_lutf(k, v, mname, master)
+                               rc = self.__start_lutf(k, v, mname, master)
 
                if rc == 0:
                        # run master locally
                        agent_list.remove(mname)
-                       master_cfg, rc = self.__start_lutf(mname, master, mname, master, agent_list=agent_list, agent=False)
-                       if master_cfg['lutf']['shell'] == 'batch':
+                       rc = self.__start_lutf(mname, master, mname, master, agent_list=agent_list, agent=False)
+                       if self.__cfg_yaml['lutf']['shell'] == 'batch':
                                for k, v in self.nodes.items():
                                        self.__stop_lutf(k, v)
 
@@ -282,5 +265,10 @@ class LUTF:
 
 if __name__ == '__main__':
        lutf = LUTF()
+
+       rc = lutf.check_environment()
+       if (rc != 0):
+               exit(rc)
+
        rc = lutf.run()
        exit(rc)

diff --git a/lustre/tests/lutf/python/infra/lutf.py b/lustre/tests/lutf/python/infra/lutf.py
index bec5e6c..2d4e5cf 100644 (file)
--- a/lustre/tests/lutf/python/infra/lutf.py
+++ b/lustre/tests/lutf/python/infra/lutf.py
@@ -652,7 +652,7 @@ class TestSuites:
                        raise LUTFError('No LUTF path provided')
                self.__lutf_tests = self.__lutf_path + '/python/tests/'
                if not os.path.isdir(self.__lutf_tests):
-                       raise LUTFError('No tests suites director: ' + sef.lutf_tests)
+                       raise LUTFError('No tests suites directory: ' + self.__lutf_tests)
                self.__generate_test_db(self.__test_db)
 
        def __getitem__(self, key):