curtin-dev team mailing list archive

[Ryan Harper <rharper@xxxxxxxxxxx>]

I like your initial commit message, can you update the commit box with that one as that's what the autolander will take when it squash merges.

Once suggestion on the md_check refactor below.

Diff comments:

> diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
> index 3923321..d846505 100644
> --- a/curtin/block/schemas.py
> +++ b/curtin/block/schemas.py
> @@ -308,9 +308,13 @@ RAID = {
>      'title': 'curtin storage configuration for a RAID.',
>      'description': ('Declarative syntax for specifying RAID.'),
>      'definitions': definitions,
> -    'required': ['id', 'type', 'name', 'raidlevel', 'devices'],
> +    'required': ['id', 'type', 'name', 'raidlevel'],
>      'type': 'object',
>      'additionalProperties': False,
> +    'oneOf': [
> +        {'required': ['devices']},
> +        {'required': ['container']},
> +    ],

At some point we should starting putting some sort of version in this schema...

>      'properties': {
>          'id': {'$ref': '#/definitions/id'},
>          'devices': {'$ref': '#/definitions/devices'},
> diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
> index 5a087a3..bb5e236 100644
> --- a/curtin/commands/block_meta.py
> +++ b/curtin/commands/block_meta.py
> @@ -1496,24 +1496,22 @@ def dm_crypt_handler(info, storage_config):
>              so not writing crypttab")
>  
>  
> -def verify_md_components(md_devname, raidlevel, device_paths, spare_paths):
> +def verify_md_components(md_devname, raidlevel, device_paths, spare_paths,
> +                         container):
>      # check if the array is already up, if not try to assemble
> -    check_ok = mdadm.md_check(md_devname, raidlevel, device_paths,
> -                              spare_paths)
> -    if not check_ok:
> +    try:
> +        mdadm.md_check(md_devname, raidlevel, device_paths,
> +                       spare_paths, container)
> +    except ValueError:
>          LOG.info("assembling preserved raid for %s", md_devname)
>          mdadm.mdadm_assemble(md_devname, device_paths, spare_paths)
> -        check_ok = mdadm.md_check(md_devname, raidlevel, device_paths,
> -                                  spare_paths)
> -    msg = ('Verifying %s raid composition, found raid is %s'
> -           % (md_devname, 'OK' if check_ok else 'not OK'))
> -    LOG.debug(msg)
> -    if not check_ok:
> -        raise RuntimeError(msg)
> +        mdadm.md_check(md_devname, raidlevel, device_paths,
> +                       spare_paths, container)

If we fail md_check a second time, it's done inside an exception handler, so we see that odd python An Exception occurred while handling an Exception.  Also, nice to keep the status message and dump the exceptions into the log if we end up not-ok after second check.

errors = []
check_ok = False
try:
   md_check()
   check_ok = True
except ValueError as err1:
   errors.append(err1)
   md_assemble()
   try:
      md_check()
      check_ok = True
   except ValueError as err2:
      errors.append(err2)

msg = ('Verified %s raid composition, raid is %s'
        % (md_devname, 'OK' if check_ok else 'not OK'))
LOG.debug(msg)
if not check_ok:
   for err in errors:
       log.error("Error checking raid %s: %s", md_devname, err)
   raise RuntimeError(msg)

>  
>  
> -def raid_verify(md_devname, raidlevel, device_paths, spare_paths):
> -    verify_md_components(md_devname, raidlevel, device_paths, spare_paths)
> +def raid_verify(md_devname, raidlevel, device_paths, spare_paths, container):
> +    verify_md_components(
> +        md_devname, raidlevel, device_paths, spare_paths, container)
>  
>  
>  def raid_handler(info, storage_config):


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/402384
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:mdadm-check-container into curtin:master.