On Wed, May 31, 2017 at 4:11 PM, Scott Moser <email address hidden> wrote:

> well, first few comments
> i didn't get very far. sorry.
>
>
> Diff comments:
>
> > === modified file 'curtin/__init__.py'
> > --- curtin/__init__.py        2017-03-21 16:41:33 +0000
> > +++ curtin/__init__.py        2017-05-31 00:24:53 +0000
> > @@ -37,6 +37,8 @@
> >      'SUBCOMMAND_SYSTEM_UPGRADE',
> >      # supports new format of apt configuration
> >      'APT_CONFIG_V1',
> > +    # supports passing network config to target
> > +    'NETWORK_PASSTHROUGH',
>
> Why would someone need to know this ?
> i think it is related to our plan on implementing images that have builtin
> hooks but dont want to use them.
> that is worth an explanation in the commit message.
>

It's worth comment and *documentation*; will add.  I think we might also
want to allow caller to
control whether we render eni or passthrough; ie, MaaS may want to *force*
an eni rendering
even if passthrough is possible.


>
> >      # has version module
> >      'HAS_VERSION_MODULE',
> >  ]
> >
> > === modified file 'curtin/block/__init__.py'
> > --- curtin/block/__init__.py  2017-05-19 18:52:21 +0000
> > +++ curtin/block/__init__.py  2017-05-31 00:24:53 +0000
> > @@ -978,4 +978,71 @@
> >      else:
> >          raise ValueError("wipe mode %s not supported" % mode)
> >
> > +
> > +def storage_config_required_packages(storage_config, mapping):
> > +    """Read storage configuration dictionary and determine
> > +       which packages are required for the supplied configuration
> > +       to function.  Return a list of packaged to install.
> > +    """
> > +
> > +    if not storage_config or not isinstance(storage_config, dict):
> > +        raise ValueError('Invalid storage configuration.  '
> > +                         'Must be a dict:\n %s' % storage_config)
> > +
> > +    if not mapping or not isinstance(mapping, dict):
> > +        raise ValueError('Invalid storage mapping.  Must be a dict')
> > +
> > +    if 'storage' in storage_config:
> > +        storage_config = storage_config.get('storage')
> > +
> > +    needed_packages = []
> > +
> > +    # get reqs by device operation type
> > +    dev_configs = set(operation['type']
> > +                      for operation in storage_config['config'])
> > +
> > +    for dev_type in dev_configs:
> > +        if dev_type in mapping:
> > +            needed_packages.extend(mapping[dev_type])
> > +
> > +    # for any format operations, check the fstype and
> > +    # determine if we need any mkfs tools as well.
> > +    format_configs = set([operation['fstype']
> > +                         for operation in storage_config['config']
> > +                         if operation['type'] == 'format'])
> > +    for format_type in format_configs:
> > +        if format_type in mapping:
> > +            needed_packages.extend(mapping[format_type])
> > +
> > +    return needed_packages
> > +
> > +
> > +def detect_required_packages_mapping():
> > +    """Return a dictionary providing a versioned configuration which
> maps
> > +       storage configuration elements to the packages which are required
> > +       for functionality.
> > +
> > +       The mapping key is either a config type value, or an fstype
> value.
>
> how does this relate to curtin/deps/__init__.py
> i think we'd like to have just one list.
>

I'll check there as well; there should be just one.


>
> > +
> > +    """
> > +    version = 1
> > +    mapping = {
> > +        version: {
> > +            'handler': storage_config_required_packages,
> > +            'mapping': {
> > +                'bcache': ['bcache-tools'],
> > +                'btrfs': ['btrfs-tools'],
> > +                'ext2': ['e2fsprogs'],
> > +                'ext3': ['e2fsprogs'],
> > +                'ext4': ['e2fsprogs'],
> > +                'lvm_partition': ['lvm2'],
> > +                'lvm_volgroup': ['lvm2'],
> > +                'raid': ['mdadm'],
> > +                'xfs': ['xfsprogs']
> > +            },
> > +        },
> > +    }
> > +    return mapping
> > +
> > +
> >  # vi: ts=4 expandtab syntax=python
>
>
> --
> https://code.launchpad.net/~raharper/curtin/trunk.
> passthrough-netconfig/+merge/324801
> You are the owner of lp:~raharper/curtin/trunk.passthrough-netconfig.
>