From: Ondrej Lichtner olichtne@redhat.com

This patch removes the ConfigParser and instead adds our own parser. The main difference is that you can now specify multiple lines for one option with += operator in the same file for additive options.

To create a comment you now have to begin a line with the '#' character.

Closes issue #84.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Common/Config.py | 86 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 27 deletions(-)

diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py index 547dd99..23f77e6 100644 --- a/lnst/Common/Config.py +++ b/lnst/Common/Config.py @@ -13,7 +13,6 @@ olichtne@redhat.com (Ondrej Lichtner) import os import sys import re -from ConfigParser import ConfigParser from lnst.Common.Utils import bool_it from lnst.Common.NetUtils import verify_mac_address from lnst.Common.Colours import get_preset_conf @@ -165,15 +164,55 @@ class Config(): sect = self.get_section(section) sect[option]["value"] = value

+ def _preprocess_lines(self, lines): + comment_re = re.compile(r'^#.*$') + empty_line_re = re.compile(r'^\s*$') + result = [] + for line in lines: + if comment_re.match(line): + continue + if empty_line_re.match(line): + continue + result.append(line.strip()) + return result + + def _parse_file(self, path): + result = {} + current_section = None + + section_re = re.compile(r'^[(\w+)]$') + option_re = re.compile(r'^(\w+)\s*(+?=)\s*(.*)$') + with open(path, "r") as f: + lines = f.readlines() + + lines = self._preprocess_lines(lines) + for line in lines: + section = section_re.match(line) + option = option_re.match(line) + if section: + current_section = section.group(1) + if current_section in result: + raise ConfigError("Section '[%s]' already defined." %\ + current_section) + result[current_section] = [] + elif option: + if current_section is None: + raise ConfigError("No section defined yet.") + opt = {"name": option.group(1), + "operator": option.group(2), + "value": option.group(3)} + result[current_section].append(opt) + else: + msg = "Invalid format of config line:\n%s" % line + raise ConfigError(msg) + return result + def load_config(self, path): '''Parse and load the config file''' exp_path = os.path.expanduser(path) abs_path = os.path.abspath(exp_path) - parser = ConfigParser(dict_type=dict) print >> sys.stderr, "Loading config file '%s'" % abs_path - parser.read(abs_path) - - sections = parser._sections + sections = self._parse_file(abs_path)

self.handleSections(sections, abs_path)

@@ -188,37 +227,30 @@ class Config(): def handleOptions(self, section_name, config, cfg_path): section = self._options[section_name]

- config.pop('__name__', None) for opt in config: - if not config[opt]: + opt_name = opt["name"] + opt_operator = opt["operator"] + opt_value = opt["value"] + if not opt_value: continue - option = self._find_option_by_name(section, opt) + option = self._find_option_by_name(section, opt_name) if option != None: - if option[1]: #additive? - option[0]["value"] +=\ - option[0]["action"](config[opt], cfg_path) - else: - option[0]["value"] =\ - option[0]["action"](config[opt], cfg_path) + if opt_operator == "=": + option["value"] = option["action"](opt_value, cfg_path) + elif opt_operator == "+=" and option["additive"]: + option["value"] += option["action"](opt_value, cfg_path) + elif opt_operator == "+=": + msg = "Operator += not allowed for option %s" % opt_name + raise ConfigError(msg) else: - msg = "Unknown option: %s in section %s" % (opt, section_name) + msg = "Unknown option: %s in section %s" % (opt_name, + section_name) raise ConfigError(msg)

def _find_option_by_name(self, section, opt_name): - match = re.match(r'^(\w*)(\s++)$', opt_name) - if match != None: - additive = True - opt_name = match.groups()[0] - else: - additive = False - for option in section.itervalues(): if option["name"] == opt_name: - if (not option["additive"]) and additive: - msg = "Operator += cannot be used in option %s" % opt_name - raise ConfigError(msg) - return (option, additive) - + return option return None

def optionPort(self, option, cfg_path):

From: Ondrej Lichtner olichtne@redhat.com

This commit removes the machine_pool_dirs option for the LNST Controller, and replaces it with a new section - [pools]. In this section you can specify your machine pools as options like this: my_pool = ./path/to/my_pool/directory/

This change required some adjustments to the SlavePool module which are contained in this commit. With that I did some variable renaming so that it matches the current way of storing pools and I also made small changes to the logs reported from the SlavePool module so that they make more sense.

Related to issue #157.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- install/lnst-ctl.conf.in | 16 ++++---- lnst-ctl | 6 +-- lnst-ctl.conf | 3 +- lnst/Common/Config.py | 38 +++++++++++++++--- lnst/Controller/SlavePool.py | 95 +++++++++++++++++++++++--------------------- 5 files changed, 94 insertions(+), 64 deletions(-)

diff --git a/install/lnst-ctl.conf.in b/install/lnst-ctl.conf.in index ad0efc1..295f9ba 100644 --- a/install/lnst-ctl.conf.in +++ b/install/lnst-ctl.conf.in @@ -18,14 +18,6 @@ mac_pool_range = 52:54:01:00:00:01 52:54:01:FF:FF:FF # value. rpcport = 9999

-# This option specifies where the controller should look for specifications -# of machines that can be used for matching templates inside recipes. It -# accepts a variable number of directory paths separated by white spaces. -# It is also important to note that you can use the operator += to add to -# the list of directories. If the operator = is used, the previous list is -# replaced. -machine_pool_dirs = - # 'test_tool_dirs' specifies where the controller looks for custom tools that # are used in tests. Every tool has it's own subdirectory in one of the # directories in this list. The option behaves the same way as the option @@ -56,6 +48,14 @@ log_dir = @ctl_logs_dir@ # developers when testing new features. allow_virtual = True

+ +# This section specifies where the controller should look for specifications +# of machines that can be used for matching templates inside recipes. You can +# specify pools as you would any other option, the name of the option becomes +# the name of the pool and the value of the option should point to a directory +# which contains the slave machine descriptions. +[pools] + # Optional section for configuring access to a PerfRepo instance #[perfrepo] #url = diff --git a/lnst-ctl b/lnst-ctl index 66c617e..eaa2558 100755 --- a/lnst-ctl +++ b/lnst-ctl @@ -220,9 +220,9 @@ def main(): usr_cfg_dir = os.path.dirname(usr_cfg) pool_dir = usr_cfg_dir + "/pool" mkdir_p(pool_dir) - global_pool = lnst_config.get_option("environment", "pool_dirs") - if (len(global_pool) == 0): - lnst_config.set_option("environment", "pool_dirs", [pool_dir]) + global_pools = lnst_config.get_section("pools") + if (len(global_pools) == 0): + lnst_config.add_pool("default", pool_dir, "./") with open(usr_cfg, 'w') as f: f.write(lnst_config.dump_config())

diff --git a/lnst-ctl.conf b/lnst-ctl.conf index cc2e6b3..8f4b95c 100644 --- a/lnst-ctl.conf +++ b/lnst-ctl.conf @@ -6,7 +6,6 @@ [environment] mac_pool_range = 52:54:01:00:00:01 52:54:01:FF:FF:FF rpcport = 9999 -machine_pool_dirs = test_tool_dirs = ./test_tools test_module_dirs = ./test_modules log_dir = ./Logs @@ -17,3 +16,5 @@ allow_virtual = True url = username = password = + +[pools] diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py index 23f77e6..ddaa508 100644 --- a/lnst/Common/Config.py +++ b/lnst/Common/Config.py @@ -41,11 +41,6 @@ class Config(): "additive" : False, "action" : self.optionPort, "name" : "rpcport"} - self._options['environment']['pool_dirs'] = {\ - "value" : [], - "additive" : True, - "action" : self.optionDirList, - "name" : "machine_pool_dirs"} self._options['environment']['tool_dirs'] = {\ "value" : [], "additive" : True, @@ -100,6 +95,8 @@ class Config(): "name" : "password" }

+ self._options['pools'] = dict() + self.colours_scheme()

def slave_init(self): @@ -219,7 +216,10 @@ class Config(): def handleSections(self, sections, path): for section in sections: if section in self._options: - self.handleOptions(section, sections[section], path) + if section == "pools": + self.handlePools(sections[section], path) + else: + self.handleOptions(section, sections[section], path) else: msg = "Unknown section: %s" % section raise ConfigError(msg) @@ -247,6 +247,32 @@ class Config(): section_name) raise ConfigError(msg)

+ def handlePools(self, config, cfg_path): + for pool in config: + if pool["operator"] != "=": + msg = "Only opetator '=' is allowed for section pools." + raise ConfigError(msg) + self.add_pool(pool["name"], pool["value"], cfg_path) + + def add_pool(self, pool_name, pool_dir, cfg_path): + pool = {"value" : self.optionPath(pool_dir, cfg_path), + "additive" : False, + "action" : self.optionPath, + "name" : pool_name} + self._options["pools"][pool_name] = pool + + def get_pools(self): + pools = {} + for pool_name, pool in self._options["pools"].items(): + pools[pool_name] = pool["value"] + return pools + + def get_pool(self, pool_name): + try: + return self._options["pools"][pool_name] + except KeyError: + return None + def _find_option_by_name(self, section, opt_name): for option in section.itervalues(): if option["name"] == opt_name: diff --git a/lnst/Controller/SlavePool.py b/lnst/Controller/SlavePool.py index c6b7883..1a1114d 100644 --- a/lnst/Controller/SlavePool.py +++ b/lnst/Controller/SlavePool.py @@ -31,9 +31,9 @@ class SlavePool: This class is responsible for managing test machines that are available at the controler and can be used for testing. """ - def __init__(self, pool_dirs, pool_checks=True): + def __init__(self, pools, pool_checks=True): self._map = {} - self._pool_dirs = {} + self._pools = {} self._pool = {}

self._machine_matches = [] @@ -48,36 +48,37 @@ class SlavePool: self._mreqs = None

logging.info("Checking machine pool availability.") - for pool_dir in pool_dirs: - self._pool_dirs[pool_dir] = {} - self.add_dir(pool_dir) - if len(self._pool_dirs[pool_dir]) == 0: - del self._pool_dirs[pool_dir] + for pool_name, pool_dir in pools.items(): + self._pools[pool_name] = {} + self.add_dir(pool_name, pool_dir) + if len(self._pools[pool_name]) == 0: + del self._pools[pool_name]

- self._mapper.set_pool_dirs(self._pool_dirs) + self._mapper.set_pools(self._pools) + logging.info("Finished loading pools.")

- def add_dir(self, dir_path): - logging.info("Processing pool dir '%s'" % dir_path) - - pool_dir = self._pool_dirs[dir_path] + def add_dir(self, pool_name, dir_path): + logging.info("Processing pool '%s', directory '%s'" % (pool_name, + dir_path)) + pool = self._pools[pool_name]

dentries = os.listdir(dir_path) for dirent in dentries: - m_id, m = self.add_file(dir_path, dirent) + m_id, m = self.add_file(pool_name, dir_path, dirent) if m_id != None and m != None: - pool_dir[m_id] = m + pool[m_id] = m

- if len(pool_dir) == 0: - logging.warn("No machines found in this directory") + if len(pool) == 0: + logging.warn("No machines found in this pool")

max_len = 0 - for m_id in pool_dir.keys(): + for m_id in pool.keys(): if len(m_id) > max_len: max_len = len(m_id)

if self._pool_checks: check_sockets = {} - for m_id, m in pool_dir.iteritems(): + for m_id, m in pool.iteritems(): hostname = m["params"]["hostname"] if "rpc_port" in m["params"]: port = m["params"]["rpc_port"] @@ -101,17 +102,17 @@ class SlavePool: err = s.getsockopt(socket.SOL_SOCKET, socket.SO_ERROR) m_id = check_sockets[s] if err == 0: - pool_dir[m_id]["available"] = True + pool[m_id]["available"] = True del check_sockets[s] else: - pool_dir[m_id]["available"] = False + pool[m_id]["available"] = False del check_sockets[s] else: - for m_id in pool_dir.keys(): - pool_dir[m_id]["available"] = True + for m_id in pool.keys(): + pool[m_id]["available"] = True

- for m_id in list(pool_dir.keys()): - m = pool_dir[m_id] + for m_id in list(pool.keys()): + m = pool[m_id] if m["available"]: if 'libvirt_domain' in m['params']: libvirt_msg = " libvirt_domain: %s" %\ @@ -124,13 +125,13 @@ class SlavePool: else: msg = "%s%s [%s]" % (m_id, (max_len - len(m_id)) * " ", decorate_with_preset("DOWN", "fail")) - del pool_dir[m_id] + del pool[m_id]

logging.info(msg)

- def add_file(self, dir_path, dirent): + def add_file(self, pool_name, dir_path, dirent): filepath = dir_path + "/" + dirent - pool_dir = self._pool_dirs[dir_path] + pool = self._pools[pool_name] if os.path.isfile(filepath) and re.search(".xml$", filepath, re.I): dirname, basename = os.path.split(filepath) m_id = re.sub(".[xX][mM][lL]$", "", basename) @@ -148,7 +149,7 @@ class SlavePool:

# Check if there isn't any machine with the same # hostname or libvirt_domain already in the pool - for pm_id, m in pool_dir.iteritems(): + for pm_id, m in pool.iteritems(): pm = m["params"] rm = machine_spec["params"] if pm["hostname"] == rm["hostname"]: @@ -246,9 +247,8 @@ class SlavePool: :return: XML machineconfigs of requested machines :rtype: dict """ - - mapper = self._mapper + logging.info("Matching machines, without virtuals.") res = mapper.match()

if not res and not mapper.get_virtual() and self._allow_virt: @@ -267,7 +267,7 @@ class SlavePool: self._pool = {} return False else: - self._pool = self._pool_dirs[self._map["pool_dir"]] + self._pool = self._pools[self._map["pool_name"]]

if self._map["virtual"]: mreqs = self._mreqs @@ -364,10 +364,10 @@ class MapperError(Exception):

class SetupMapper(object): def __init__(self): - self._pool_dirs = {} - self._pool_dir_stack = [] + self._pools = {} + self._pool_stack = [] self._pool = {} - self._pool_dir = None + self._pool_name = None self._mreqs = {} self._unmatched_req_machines = [] self._matched_pool_machines = [] @@ -378,8 +378,8 @@ class SetupMapper(object): def set_requirements(self, mreqs): self._mreqs = mreqs

- def set_pool_dirs(self, pool_dirs): - self._pool_dirs = pool_dirs + def set_pools(self, pools): + self._pools = pools

def set_virtual(self, virt_value): self._virtual_matching = virt_value @@ -404,11 +404,10 @@ class SetupMapper(object): self._machine_stack = [] self._unmatched_req_machines = self._mreqs.keys()

- self._pool_dir_stack = list(self._pool_dirs.keys()) - if len(self._pool_dir_stack) > 0: - self._pool_dir = self._pool_dir_stack.pop() - self._pool = self._pool_dirs[self._pool_dir] - logging.info("Using pool dir: %s" % self._pool_dir) + self._pool_stack = list(self._pools.keys()) + if len(self._pool_stack) > 0: + self._pool_name = self._pool_stack.pop() + self._pool = self._pools[self._pool_name]

self._unmatched_pool_machines = [] for p_id, p_machine in self._pool.iteritems(): @@ -422,6 +421,7 @@ class SetupMapper(object): self._push_machine_stack()

def match(self): + logging.info("Trying match with pool: %s" % self._pool_name) while len(self._machine_stack)>0: stack_top = self._machine_stack[-1] if self._virtual_matching and stack_top["virt_matched"]: @@ -466,10 +466,13 @@ class SetupMapper(object): else: self._pop_machine_stack() if len(self._machine_stack) == 0 and\ - len(self._pool_dir_stack) > 0: - self._pool_dir = self._pool_dir_stack.pop() - self._pool = self._pool_dirs[self._pool_dir] - logging.info("Using pool dir: %s" % self._pool_dir) + len(self._pool_stack) > 0: + logging.info("Match with pool %s not found." % + self._pool_name) + self._pool_name = self._pool_stack.pop() + self._pool = self._pools[self._pool_name] + logging.info("Trying match with pool: %s" % + self._pool_name)

self._unmatched_pool_machines = [] for p_id, p_machine in self._pool.iteritems(): @@ -599,7 +602,7 @@ class SetupMapper(object):

def get_mapping(self): mapping = {"machines": {}, "networks": {}, "virtual": False, - "pool_dir": self._pool_dir} + "pool_name": self._pool_name}

for req_label, label_map in self._net_label_mapping.iteritems(): mapping["networks"][req_label] = label_map[0]