curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+458446@xxxxxxxxxxxxxxxxxx>]

Review: Approve

This looks fine, thanks! All the jenkins results seem to have been deleted so I'm not sure what CI is unhappy about?

Diff comments:

> diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
> index 4be2cb4..971499d 100644
> --- a/curtin/commands/curthooks.py
> +++ b/curtin/commands/curthooks.py
> @@ -1498,6 +1500,56 @@ def configure_mdadm(cfg, state_etcd, target, osfamily=DISTROS.debian):
>                  data=None, target=target)
>  
>  
> +def get_nvme_stas_controller_directives(cfg) -> Set[str]:
> +    """Parse the storage configuration and return a set of "controller ="
> +    directives to write in the [Controllers] section of a nvme-stas
> +    configuration file."""
> +    directives = set()
> +    if 'storage' not in cfg or not isinstance(cfg['storage'], dict):

I don't want to think about cases where the second case of this fails #justcurtinthings

> +        return directives
> +    storage = cfg['storage']
> +    if 'config' not in storage or storage['config'] == 'disabled':
> +        return directives
> +    config = storage['config']
> +    for item in config:
> +        if item['type'] != 'nvme_controller':
> +            continue
> +        if item['transport'] != 'tcp':
> +            continue
> +        controller_props = {
> +            'transport': 'tcp',
> +            'traddr': item["tcp_addr"],
> +            'trsvcid': item["tcp_port"],
> +        }
> +
> +        props_str = ';'.join([f'{k}={v}' for k, v in controller_props.items()])
> +        directives.add(f'controller = {props_str}')
> +
> +    return directives
> +
> +
> +def configure_nvme_stas(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."""
> +    controllers = get_nvme_stas_controller_directives(cfg)
> +
> +    if not controllers:
> +        return
> +
> +    LOG.info('NVMe-over-TCP configuration found, 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:
> +        print('[Controllers]', file=fh)
> +        for controller in controllers:
> +            print(controller, file=fh)
> +
> +    with contextlib.suppress(FileNotFoundError):
> +        (stas_dir / 'stafd.conf').replace(stas_dir / '.stafd.conf.bak')
> +    (stas_dir / 'stafd.conf').symlink_to('stafd-curtin.conf')
> +
>  def handle_cloudconfig(cfg, base_dir=None):
>      """write cloud-init configuration files into base_dir.
>  
> diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
> index 45b9bbe..f78aeb8 100644
> --- a/doc/topics/storage.rst
> +++ b/doc/topics/storage.rst
> @@ -331,6 +332,11 @@ configuration dictionary.  Currently the value is informational only.
>  Curtin already detects whether disks are part of a multipath and selects
>  one member path to operate upon.
>  
> +**nvme_controller**: *<NVMe controller id>*
> +
> +If the disk is a NVMe SSD, the ``nvme_controller`` key can be set to the
> +identifier of a ``nvme_controller`` object. This will help to determine the
> +type of transpot used (e.g., PCIe vs TCP).

typo here, transpot

>  
>  **Config Example**::
>  
> @@ -1193,6 +1199,42 @@ passed to the ZFS dataset creation command.
>       canmount: noauto
>       mountpoint: /
>  
> +NVMe Controller Command
> +~~~~~~~~~~~~~~~~~~~~~~~
> +NVMe Controller Commands (and NVMe over TCP support in general) are
> +**experimental**.
> +
> +The nvme_controller command describes how to communicate with a given NVMe
> +controller.
> +
> +**transport**: *pcie, tcp*

The "transport: pcie" case sort of feels a bit weird, like having one of these controller objects conveys absolutely no additional information. But well, consistency is a good thing I guess.

> +
> +The ``transport`` key specifies whether the communication with the NVMe
> +controller operates over PCIe or over TCP. Other transports like RDMA and FC
> +(aka. Fiber Channel) are not supported at the moment.
> +
> +**tcp_addr**: *<ip address>*
> +
> +The ``tcp_addr`` key specifies the IP where the NVMe controller can be reached.
> +This key is only meaningful in conjunction with ``transport: tcp``.
> +
> +**tcp_port**: *port*
> +
> +The ``tcp_port`` key specifies the TCP port where the NVMe controller can be
> +reached. This key is only meaningful in conjunction with ``transport: tcp``.
> +
> +**Config Example**::
> +
> + - type: nvme_controller
> +   id: nvme-controller-nvme0
> +   transport: pcie
> +
> + - type: nvme_controller
> +   id: nvme-controller-nvme1
> +   transport: tcp
> +   tcp_addr: 172.16.82.78
> +   tcp_port: 4420
> +
>  Device "Command"
>  ~~~~~~~~~~~~~~~~
>  


-- 
https://code.launchpad.net/~ogayot/curtin/+git/curtin/+merge/458446
Your team curtin developers is subscribed to branch curtin:master.