Add a spec draft for the new distro namespace.#1
Conversation
This initial draft targets a proposed API for the new distro submodule, which in turn is composed by other cohesive subnamespaces, that targets one specialized are. A basic API is laid out, which describes how namespaces interacts with objects from other namespaces and so on.
There was a problem hiding this comment.
One concern I would have with structuring the modules this way (i.e. cloudinit.distros.<function>.<distro>) is that it makes it more difficult to install new distro packages cleanly. Let's say that Foo GNU/Linux want to use cloud-init by installing a separate package (which just puts their own files in place, rather than them forking cloud-init downstream and modifying it there).
They will have to maintain an odd looking tree (.../network/foo.py, .../users/foo.py, etc.), and install a separate file in to each of these directories on install.
If, instead, it were to be cloudinit.distros.., then they could just have .../foo/network.py, .../foo/users.py and install that entire directory in to .../cloudinit/distros.
Ideally (in my mind), the distros would sit in a Python namespace package, so that installing different distros would be relatively painless.
What do you think?
There was a problem hiding this comment.
cloudinit/distros/**init**.pybase.pyfactory.py
network/One concern I would have with structuring the modules this way (i.e. cloudinit.distros..) is that it makes it more difficult to install new distro packages cleanly. Let's say that Foo GNU/Linux want to use cloud-init by installing a separate package (which just puts their own files in place, rather than them forking cloud-init downstream and modifying it there).
They will have to maintain an odd looking tree (
.../network/foo.py,.../users/foo.py, etc.), and install a separate file in to each of these directories on install.If, instead, it were to be cloudinit.distros.., then they could just have
.../foo/network.py,.../foo/users.pyand install that entire directory in to.../cloudinit/distros.Ideally (in my mind), the distros would sit in a Python namespace package, so that installing different distros would be relatively painless.
What do you think?
I'm apt to agree with Daniel here.
Claudiu, was there a reason for your layout ?
There was a problem hiding this comment.
I'm actually agreeing with Daniel, too. My first intention was to separate the concerns from inside the distro into more useful and cohesive subnamespaces, but the side effect is an increased maintenance for adding custom distros, which didn't crossed my mind initially. I'll update the spec then.
There was a problem hiding this comment.
I'd also think we should use namespaces (and likely https://pypi.python.org/pypi/stevedore ? ) for finding distros; although I'd also rather have distros actually contribute to cloud-init and not have everything they do be a plugin; seems slightly premature I think to do that at this early of a stage; but something we should keep in mind...
|
Thanks guys for the review! |
|
Claudiu, can you address those comments above and update ? looks good other than that. |
This patch drops the use of separate namespaces for different concerns, such as networking, users and filesystem, instead, it proposes using modules in a distro, as in `distro_name.network`, `distro_name.filesystem` etc. which is better when trying to add new additional distros. Also, it drops the concept of containers-as-creators, instead the objects returned by methods as `routes` or `users` could be implemented in the term of a sequence (for instance a namedtuple). The creation of an object is moved in an object class itself, e.g. Route.create, instead of RouteContainer.create.
Add a spec draft for the new distro namespace.
This reverts commit 3ef22ed. Comments are not supported in debconf templates and cause errors like: Template parse error near `# Oracle DS absent to retain orig behavior: Oracle(Bionic) == OpenStack detected', in stanza canonical#1 of /var/lib/dpkg/info/cloud-init.templates
Fixes the creation of single partitions on MBR devices. Currently this
fails with the following debug output:
cc_disk_setup.py[DEBUG]: Calculating partition layout
cc_disk_setup.py[DEBUG]: Layout is: 0,
cc_disk_setup.py[DEBUG]: Creating partition table on /dev/sdb
subp.py[DEBUG]: Running command ['/sbin/sfdisk', '--Linux', '--unit=S',
'--force', '/dev/sdb'] with allowed return codes [0] (shell=False,
capture=True)
util.py[DEBUG]: Creating partition on /dev/sdb took 0.237 seconds
util.py[WARNING]: Failed partitioning operation
Failed to partition device /dev/sdb
Unexpected error while running command.
Command: ['/sbin/sfdisk/', '--Linux', '--unit=S', '--force', '/dev/sdb']
Exit code: 1
Reason: -
Stdout: Checking that no-one is using this disk right now ... OK
Disk /dev/sdb: 16 MiB, 16777216 bytes, 32768 sectors
Disk model: HARDDISK
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
>>> Created a new DOS disklabel with disk identifier 0xb3604c9a.
/dev/sdb1: Leaving.
Stderr: sfdisk: --Linux option is unnecessary and deprecated
Start sector 0 out of range.
Failed to add canonical#1 partition: Result not representable
util.py[DEBUG]: Failed partitioning operation
On a BIOS/MBR partitioned device the 1st partition cannot start at sector
0 as this is reserved for the MBR.
Fixes LP: #1851438.
Documentation clarifications/corrections and additional examples added.
Also remove "--Linux" and "--unit=S" options from sfdisk calls, these
options have been deprecated since October 2014.
Fixes the creation of single partitions on MBR devices. Currently this
fails with the following debug output:
cc_disk_setup.py[DEBUG]: Calculating partition layout
cc_disk_setup.py[DEBUG]: Layout is: 0,
cc_disk_setup.py[DEBUG]: Creating partition table on /dev/sdb
subp.py[DEBUG]: Running command ['/sbin/sfdisk', '--Linux', '--unit=S',
'--force', '/dev/sdb'] with allowed return codes [0] (shell=False,
capture=True)
util.py[DEBUG]: Creating partition on /dev/sdb took 0.237 seconds
util.py[WARNING]: Failed partitioning operation
Failed to partition device /dev/sdb
Unexpected error while running command.
Command: ['/sbin/sfdisk/', '--Linux', '--unit=S', '--force', '/dev/sdb']
Exit code: 1
Reason: -
Stdout: Checking that no-one is using this disk right now ... OK
Disk /dev/sdb: 16 MiB, 16777216 bytes, 32768 sectors
Disk model: HARDDISK
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
>>> Created a new DOS disklabel with disk identifier 0xb3604c9a.
/dev/sdb1: Leaving.
Stderr: sfdisk: --Linux option is unnecessary and deprecated
Start sector 0 out of range.
Failed to add canonical#1 partition: Result not representable
util.py[DEBUG]: Failed partitioning operation
On a BIOS/MBR partitioned device the 1st partition cannot start at sector
0 as this is reserved for the MBR.
Fixes LP: #1851438.
Documentation clarifications/corrections and additional examples added.
Also remove "--Linux" and "--unit=S" options from sfdisk calls, these
options have been deprecated since October 2014.
Fixes the creation of single partitions on MBR devices. Currently this
fails with the following debug output:
cc_disk_setup.py[DEBUG]: Calculating partition layout
cc_disk_setup.py[DEBUG]: Layout is: 0,
cc_disk_setup.py[DEBUG]: Creating partition table on /dev/sdb
subp.py[DEBUG]: Running command ['/sbin/sfdisk', '--Linux', '--unit=S',
'--force', '/dev/sdb'] with allowed return codes [0] (shell=False,
capture=True)
util.py[DEBUG]: Creating partition on /dev/sdb took 0.237 seconds
util.py[WARNING]: Failed partitioning operation
Failed to partition device /dev/sdb
Unexpected error while running command.
Command: ['/sbin/sfdisk/', '--Linux', '--unit=S', '--force', '/dev/sdb']
Exit code: 1
Reason: -
Stdout: Checking that no-one is using this disk right now ... OK
Disk /dev/sdb: 16 MiB, 16777216 bytes, 32768 sectors
Disk model: HARDDISK
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
>>> Created a new DOS disklabel with disk identifier 0xb3604c9a.
/dev/sdb1: Leaving.
Stderr: sfdisk: --Linux option is unnecessary and deprecated
Start sector 0 out of range.
Failed to add #1 partition: Result not representable
util.py[DEBUG]: Failed partitioning operation
On a BIOS/MBR partitioned device the 1st partition cannot start at sector
0 as this is reserved for the MBR.
Documentation clarifications/corrections and additional examples added.
Also remove "--Linux" and "--unit=S" options from sfdisk calls, these
options have been deprecated since October 2014.
Note: This is not a change of behavior because the change provoking
the error was introduced in util-linux 2.26 in Xenial. Thus, every
supported cloud-init version fails.
LP: #1851438
5 participants
This initial draft targets a proposed API for the new distro submodule,
which in turn is composed by other cohesive subnamespaces, that targets
one specialized are. A basic API is laid out, which describes how namespaces
interacts with objects from other namespaces and so on.
I propose to have a spec folder in the repo, where we could track our design decissions and all that, a la Python's PEPs. A location such as this helps when we are in doubt why something was added as is.
Anyway, this draft mostly targets a shallow description of the API. A more thorough document will follow it, if the proposed changes are accepted. I'll also have a pull request with the base classes and all the goodies, so that we could start putting actual code in the distros in the meanwhile.