The available_for_partitions function wrongly assumed that the
underlying disk had a GPT partition table. Updated the implementation to
use the alignment data as expected.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
In the new tests recently added to test_filesystem.py, the initial disk
object was marked having a msdos partition table.
This is fine for computing offsets manually. However, what really needs
to be done is to mark the partition table as msdos in the YAML itself.
Otherwise, the disk is reformatted as GPT.
This makes gap functions use the right alignment data when computing the
list of gaps available.
Sadly it breaks the tests because of other issues, so I added FIXME tags
in the broken tests.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
To avoid having nested contexts for one step. It's a bit unsatisfying
because conceptually the code in CurtinPartitioningStep.run should be
considered part of the same context but I don't see a good way to do
that and it's not of practical significance.
When defining a partition with size: -1, the partition should expand to
the size of the largest gap. To determine which gap to use, we call the
function largest_gap(partition.disk).
That being said, the gap returned is sometimes bigger than expected when
using a msdos partition table. This happens because we return the
largest gap without checking if it is inside or outside the extended
partition.
Fixed by making sure that:
* for a logical partition, we pick the largest gap within the extended
partition
* for a primary partition, we pick the largest gap outside the extended
partition (if any).
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Our documentation of autoinstall mentions that one can specify the size
of a partition as -1 to use the remaining space on the disk.
Because it only makes sense in practice to support this option for the
right-most partition of a given disk, specifying size: -1 is only
accepted when defining the last partition of the disk.
Having said that, when making an autoinstall configuration that includes
an extended partition, the user must define the logical partitions after
the extended partition.
For the following configuration:
| disk |
+--------------+--------------+--------------+-------------------------|
| p1 (primary) | p2 (primary) | p3 (primary) | p4 (extended) |
| | | | p5 (logic) | p6 (logic) |
, the representation in Subiquity will be analogous to:
[
Partition(p1, "primary"),
Partition(p2, "primary"),
Partition(p3, "primary"),
Partition(p4, "extended"),
Partition(p5, "logical"),
Partition(p6, "logical"),
]
This means that specifying size: -1 on p4 gets rejected because it is
not the last partition defined.p6 is the last partition defined.
This is however a valid use-case because p5 and p6 are contained within
p4. So p4 should be able to use the remaining space and there is no
reason to reject this configuration
Fixed by ignoring logical partitions when checking if a size of -1 is
allowed on an extended partition.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
The second `trap ... EXIT` registration meant that the first one, the
one repsonsible for providing a clear PASS / FAIL at the end, was not
being shown.
The SingleInstanceTask does not allow us to control cancellation the way
we want. We now use a standard asyncio.Task to execute the coroutine
that fetches the list of snaps.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
The list snaps task used to return successfully even in case of error.
To make the distinction between and error and a successful fetch, we
used to rely on the task to set an attribute at the loader level:
* failed = True; or
* failed = False
The task was responsible for setting this attribute and there were
scenarios where it would not set the attribute properly. Usually, this
was when an exception was raised in the task.
We now raise an exception when an error occurs in the task ; instead of
returning. This is better because we can know if the task:
* got cancelled - by calling "task.cancelled()"
* finished successfully - by calling "task.done() and not
task.exception()"
* failed - by calling "task.done() and task.exception()"
* has not finished - by calling "not task.done()"
This also allows us to detect errors and retry
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Once we have access to the list of snaps, we can start prefetching
information. We need to wait for the list snaps task to be completed
first.
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
The --cloud-config option now replaces the old --autoinstall-file
option.
The use of --autoinstall-file always made kvm-test pass the autoinstall
argument to the kernel command line. With the --cloud-config option, we
now determine if the autoinstall argument is needed by reading the cloud
config and checking if an autoinstall section is present.
The two new options --force-autoinstall and --force-no-autoinstall can
be used to override this behaviour.
The --autoinstall option is also dropped. If we want to use the default,
hardcoded cloud-config, one can use --cloud-config-default.
This can be useful to ease debugging of the VM using a config that looks
like:
#cloud-config
users:
- default
- name: root
ssh_import_id: lp:ogayot
Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>