Merge pull request #55719 from phlogistonjohn/jjm-teuth-tasks-cephadm-jt

qa/tasks/cephadm: add generic templating where subst_vip was used

Reviewed-by: Adam King <adking@redhat.com>

qa/tasks/cephadm.py

@@ -4,13 +4,16 @@
 import argparse
 import configobj
 import contextlib
+import functools
+import json
 import logging
 import os
-import json
 import re
 import uuid
 import yaml
 
+import jinja2
+
 from copy import deepcopy
 from io import BytesIO, StringIO
 from tarfile import ReadError
@@ -27,13 +30,68 @@
 
 # these items we use from ceph.py should probably eventually move elsewhere
 from tasks.ceph import get_mons, healthy
-from tasks.vip import subst_vip
 
 CEPH_ROLE_TYPES = ['mon', 'mgr', 'osd', 'mds', 'rgw', 'prometheus']
 
 log = logging.getLogger(__name__)
 
 
+def _convert_strs_in(o, conv):
+    """A function to walk the contents of a dict/list and recurisvely apply
+    a conversion function (`conv`) to the strings within.
+    """
+    if isinstance(o, str):
+        return conv(o)
+    if isinstance(o, dict):
+        for k in o:
+            o[k] = _convert_strs_in(o[k], conv)
+    if isinstance(o, list):
+        o[:] = [_convert_strs_in(v, conv) for v in o]
+    return o
+
+
+def _apply_template(jinja_env, rctx, template):
+    """Apply jinja2 templating to the template string `template` via the jinja
+    environment `jinja_env`, passing a dictionary containing top-level context
+    to render into the template.
+    """
+    if '{{' in template or '{%' in template:
+        return jinja_env.from_string(template).render(**rctx)
+    return template
+
+
+def _template_transform(ctx, config, target):
+    """Apply jinja2 based templates to strings within the target object,
+    returning a transformed target. Target objects may be a list or dict or
+    str.
+
+    Note that only string values in the list or dict objects are modified.
+    Therefore one can read & parse yaml or json that contain templates in
+    string values without the risk of changing the structure of the yaml/json.
+    """
+    jenv = getattr(ctx, '_jinja_env', None)
+    if jenv is None:
+        loader = jinja2.BaseLoader()
+        jenv = jinja2.Environment(loader=loader)
+        setattr(ctx, '_jinja_env', jenv)
+    rctx = dict(ctx=ctx, config=config, cluster_name=config.get('cluster', ''))
+    _vip_vars(rctx)
+    conv = functools.partial(_apply_template, jenv, rctx)
+    return _convert_strs_in(target, conv)
+
+
+def _vip_vars(rctx):
+    """For backwards compat with the previous subst_vip function."""
+    ctx = rctx['ctx']
+    if 'vnet' in getattr(ctx, 'vip', {}):
+        rctx['VIPPREFIXLEN'] = str(ctx.vip["vnet"].prefixlen)
+        rctx['VIPSUBNET'] = str(ctx.vip["vnet"].network_address)
+    if 'vips' in getattr(ctx, 'vip', {}):
+        vips = ctx.vip['vips']
+        for idx, vip in enumerate(vips):
+            rctx[f'VIP{idx}'] = str(vip)
+
+
 def _shell(ctx, cluster_name, remote, args, extra_cephadm_args=[], **kwargs):
     teuthology.get_testdir(ctx)
     return remote.run(
@@ -1356,18 +1414,19 @@ def shell(ctx, config):
         roles = teuthology.all_roles(ctx.cluster)
         config = dict((id_, a) for id_ in roles if id_.startswith('host.'))
 
+    config = _template_transform(ctx, config, config)
     for role, cmd in config.items():
         (remote,) = ctx.cluster.only(role).remotes.keys()
         log.info('Running commands on role %s host %s', role, remote.name)
         if isinstance(cmd, list):
             for c in cmd:
                 _shell(ctx, cluster_name, remote,
-                       ['bash', '-c', subst_vip(ctx, c)],
+                       ['bash', '-c', c],
                        extra_cephadm_args=args)
         else:
             assert isinstance(cmd, str)
             _shell(ctx, cluster_name, remote,
-                   ['bash', '-ex', '-c', subst_vip(ctx, cmd)],
+                   ['bash', '-ex', '-c', cmd],
                    extra_cephadm_args=args)
 
 
@@ -1393,7 +1452,8 @@ def apply(ctx, config):
     cluster_name = config.get('cluster', 'ceph')
 
     specs = config.get('specs', [])
-    y = subst_vip(ctx, yaml.dump_all(specs))
+    specs = _template_transform(ctx, config, specs)
+    y = yaml.dump_all(specs)
 
     log.info(f'Applying spec(s):\n{y}')
     _shell(

qa/tasks/vip.py

@@ -5,6 +5,7 @@
 
 from teuthology import misc as teuthology
 from teuthology.config import config as teuth_config
+from teuthology.exceptions import ConfigError
 
 log = logging.getLogger(__name__)
 
@@ -68,24 +69,33 @@ def exec(ctx, config):
                 )
 
 
-def map_vips(mip, count):
-    for mapping in teuth_config.get('vip', []):
+def _map_vips(mip, count):
+    vip_entries = teuth_config.get('vip', [])
+    if not vip_entries:
+        raise ConfigError(
+            'at least one item must be configured for "vip" config key'
+            ' to use the vip task'
+        )
+    for mapping in vip_entries:
         mnet = ipaddress.ip_network(mapping['machine_subnet'])
         vnet = ipaddress.ip_network(mapping['virtual_subnet'])
         if vnet.prefixlen >= mnet.prefixlen:
             log.error(f"virtual_subnet {vnet} prefix >= machine_subnet {mnet} prefix")
-            return None
-        if mip in mnet:
-            pos = list(mnet.hosts()).index(mip)
-            log.info(f"{mip} in {mnet}, pos {pos}")
-            r = []
-            for sub in vnet.subnets(new_prefix=mnet.prefixlen):
-                r += [list(sub.hosts())[pos]]
-                count -= 1
-                if count == 0:
-                    break
-            return vnet, r
-    return None
+            raise ConfigError('virtual subnet too small')
+        if mip not in mnet:
+            # not our machine subnet
+            log.info(f"machine ip {mip} not in machine subnet {mnet}")
+            continue
+        pos = list(mnet.hosts()).index(mip)
+        log.info(f"{mip} in {mnet}, pos {pos}")
+        r = []
+        for sub in vnet.subnets(new_prefix=mnet.prefixlen):
+            r += [list(sub.hosts())[pos]]
+            count -= 1
+            if count == 0:
+                break
+        return vnet, r
+    raise ConfigError(f"no matching machine subnet found for {mip}")
 
 
 @contextlib.contextmanager
@@ -136,14 +146,14 @@ def task(ctx, config):
         ip = remote.ssh.get_transport().getpeername()[0]
         log.info(f'peername {ip}')
         mip = ipaddress.ip_address(ip)
-        vnet, vips = map_vips(mip, count + 1)
+        vnet, vips = _map_vips(mip, count + 1)
         static = vips.pop(0)
         log.info(f"{remote.hostname} static {static}, vnet {vnet}")
 
         if not ctx.vip:
             # do this only once (use the first remote we see), since we only need 1
             # set of virtual IPs, regardless of how many remotes we have.
-            log.info("VIPs are {map(str, vips)}")
+            log.info(f"VIPs are {vips!r}")
             ctx.vip = {
                 'vnet': vnet,
                 'vips': vips,