curtin-dev team mailing list archive

[Olivier Gayot <mp+461452@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
> index 695ba30..132ccab 100644
> --- a/curtin/commands/curthooks.py
> +++ b/curtin/commands/curthooks.py
> @@ -1528,21 +1531,130 @@ def get_nvme_stas_controller_directives(cfg) -> Set[str]:
>      return directives
>  
>  
> -def configure_nvme_stas(cfg, target):
> +def nvmeotcp_get_nvme_commands(cfg) -> Set[Tuple[str]]:
> +    """Parse the storage configuration and return a set of commands
> +    to run to bring up the NVMe over TCP block devices."""
> +    commands: Set[Tuple[str]] = set()
> +    if 'storage' not in cfg or not isinstance(cfg['storage'], dict):
> +        return commands
> +    storage = cfg['storage']
> +    if 'config' not in storage or storage['config'] == 'disabled':
> +        return commands
> +    config = storage['config']
> +    for item in config:
> +        if item['type'] != 'nvme_controller':
> +            continue
> +        if item['transport'] != 'tcp':
> +            continue
> +
> +        commands.add((
> +            'nvme', 'connect-all',
> +            '--transport', 'tcp',
> +            '--traddr', item['tcp_addr'],
> +            '--trsvcid', str(item['tcp_port']),
> +        ))
> +
> +    return commands
> +
> +
> +def nvmeotcp_need_network_in_initramfs(cfg) -> bool:
> +    """Parse the storage configuration and check if any of the mountpoints
> +    essential for booting requires network."""
> +    if 'storage' not in cfg or not isinstance(cfg['storage'], dict):
> +        return False
> +    storage = cfg['storage']
> +    if 'config' not in storage or storage['config'] == 'disabled':
> +        return False
> +    config = storage['config']
> +    for item in config:
> +        if item['type'] != 'mount':
> +            continue
> +        if not '_netdev' in item.get('options', '').split(','):
> +            continue
> +
> +        # We found a mountpoint that requires network. Let's check if it is
> +        # essential for booting.
> +        path = item['path']
> +        if path == '/' or path.startswith('/usr') or path.startswith('/var'):
> +            return True
> +
> +    return False
> +
> +
> +def nvmeotcp_get_ip_commands(cfg) -> List[Tuple[str]]:
> +    """Look for the netplan configuration (supplied by subiquity using
> +    write_files directives) and attempt to extrapolate a set of 'ip' + 'dhcpcd'
> +    commands that would produce more or less the expected network
> +    configuration. At the moment, only trivial network configurations are
> +    supported, which are ethernet interfaces with or without DHCP and optional
> +    static routes."""
> +    commands: List[Tuple[str]] = []
> +
> +    try:
> +        content = cfg['write_files']['etc_netplan_installer']['content']
> +    except KeyError:
> +        return []
> +
> +    config = yaml.safe_load(content)
> +
> +    try:
> +        ethernets = config['network']['ethernets']
> +    except KeyError:
> +        return []
> +
> +    for ifname, ifconfig in ethernets.items():
> +        # Handle static IP addresses
> +        for address in ifconfig.get('addresses', []):
> +            commands.append(('ip', 'address', 'add', address, 'dev', ifname))

Right, I don't think we should expect subiquity to write a configuration that has matches directives but I will double check. Thanks for pointing out!

> +
> +        # Handle DHCPv4 and DHCPv6
> +        dhcp4 = ifconfig.get('dhcp4', False)
> +        dhcp6 = ifconfig.get('dhcp6', False)
> +        if dhcp4 and dhcp6:
> +            commands.append(('dhcpcd', ifname))
> +        elif dhcp4:
> +            commands.append(('dhcpcd', '-4', ifname))
> +        elif dhcp6:
> +            commands.append(('dhcpcd', '-6', ifname))
> +        else:
> +            commands.append(('ip', 'link', 'set', ifname, 'up'))
> +
> +        # Handle static routes
> +        for route in ifconfig.get('routes', []):
> +            cmd = ['ip', 'route', 'add', route['to']]
> +            with contextlib.suppress(KeyError):
> +                cmd += ['via', route['via']]
> +            if route.get('on-link', False):
> +                cmd += ['dev', ifconfig]

Indeed, thanks for noticing! Fixed.

> +            commands.append(tuple(cmd))
> +
> +    return commands
> +
> +def configure_nvme_over_tcp(cfg, target):
>      """If any NVMe controller using the TCP transport is present in the storage
> -    configuration, create a nvme-stas configuration so that the remote drives
> -    can be made available at boot."""
> +    configuration, create a nvme-stas configuration and configure the initramfs
> +    so that the remote drives can be made available at boot.
> +    Please note that the NVMe over TCP support in curtin is experimental and in
> +    active development. Currently, it only works with trivial network
> +    configurations ; supplied by Subiquity."""
>      controllers = get_nvme_stas_controller_directives(cfg)
>  
>      if not controllers:
>          return
>  
> -    LOG.info('NVMe-over-TCP configuration found'
> -             ' , writing nvme-stas configuration')
> +    LOG.info('NVMe-over-TCP configuration found')
> +    LOG.info('writing nvme-stas configuration')
>      target = pathlib.Path(target)
>      stas_dir = target / 'etc' / 'stas'
>      stas_dir.mkdir(parents=True, exist_ok=True)
>      with (stas_dir / 'stafd-curtin.conf').open('w', encoding='utf-8') as fh:
> +        header = '''\
> +# This file was created by curtin.
> +# If you make modifications to it, please remember to also update
> +# scripts in etc/curtin-nvme-over-tcp and then regenerate the initramfs using
> +# the command `update-initramfs -u`.
> +'''
> +        print(header, file=fh)
>          print('[Controllers]', file=fh)
>          for controller in controllers:
>              print(controller, file=fh)


-- 
https://code.launchpad.net/~curtin-dev/curtin/+git/curtin/+merge/461452
Your team curtin developers is requested to review the proposed merge of curtin:nvme-initramfs into curtin:master.