curtin

Merge ~mwhudson/curtin:lp-1893818 into curtin:master

  1. Git
  2. lp:~mwhudson/curtin
  3. lp-1893818
  4. Merge into master
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: 9348f089bf171d3d1df781dc0217a1508ad7aa81
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~mwhudson/curtin:lp-1893818
Merge into: curtin:master
Diff against target: 641 lines (+204/-243)
6 files modified
curtin/block/multipath.py (+4/-5)
curtin/storage_config.py (+61/-92)
examples/tests/multipath-reuse.yaml (+61/-0)
tests/unittests/test_block_multipath.py (+27/-10)
tests/unittests/test_storage_config.py (+35/-136)
tests/vmtests/test_multipath.py (+16/-0)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Paride Legovini Needs Fixing
Review via email: mp+392353@code.launchpad.net

Commit message

storage_config: return one type: disk action per multipathed disk

Currently extract_storage_config returns one type: disk action
for every member of a multipathed disk and type: partition
actions for each partition of each disk. This works by generating
a type: disk action for each disk and ignoring the block device
data for the /dev/dm-X device for the multipathed disk.

But in groovy+, the udev rule from multipath-tools that has
always attempted to remove the devices nodes for the partitions
of a disk that is a multipath member actually succeeds, and
trying to generate a type: partition action for a partition with
no underlying device node makes things blow up.

Instead, this branch generates type: disk and type: partitions
actions from the /dev/dm-X nodes for the mutipathed disk and its
partitions, and ignores and disks and partitions that are members
of a multipathed disk.

LP: #1893818

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I haven't tested this on real hw yet!

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:272eb009b9f8af1fe417db209e6a21a71fb4e90d
https://jenkins.ubuntu.com/server/job/curtin-ci/210/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/210/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/210/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/210/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/210/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/210//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Paride Legovini (paride) wrote :

Hi Michael,

Do you have access to the CI build logs above?

I can reproduce the failure locally by running `tox` on your branch.

review: Needs Fixing
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Does MAAS use this codepath? As far as I recall, MAAS has no concept of multipath and may get confused?!

~mwhudson/curtin:lp-1893818 updated
2268251... by Michael Hudson-Doyle

oops, had not noticed that I had made this test depend on jsonschema

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:482bfeee8519a3d5bb9db4b5e8bfcbf52ee6730e
https://jenkins.ubuntu.com/server/job/curtin-ci/213/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/213/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/213/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/213/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/213/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/213//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
8f86d08... by Michael Hudson-Doyle

ffs

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:8ca0440a9b8fa687cd7294c8b8a281374579be53
https://jenkins.ubuntu.com/server/job/curtin-ci/214/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/214/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/214/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/214/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/214/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/214//rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

> Does MAAS use this codepath? As far as I recall, MAAS has no concept of
> multipath and may get confused?!

Pretty sure MAAS doesn't use this code at all.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

In general this is good, and implements things as have been originally asked for in the "multipath google doc of doom".

See minor comments.

Also please rebase commits to have only descriptive commit messages without fixups =)

Revision history for this message
Paride Legovini (paride) wrote :

@xnox: the autolander squashes the MPs using the proposed commit message, so the fixups do not really hurt unless you merge manually.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Thanks for the comments. I think I'm not going to make any changes in response to them right now.

Revision history for this message
Ryan Harper (raharper) wrote :

> But in groovy+, the udev rule from multipath-tools that has
> always attempted to remove the devices nodes for the partitions
> of a disk that is a multipath member actually succeeds, and

Won't this break curtin on older multipaths that don't have this rule?

Or will this work with older multipath output since we're only referring to
the mpath dev (rather than a path of the mpath device)?

Some comments inline.

~mwhudson/curtin:lp-1893818 updated
06825b6... by Michael Hudson-Doyle

simplify is_mpath_partition, thanks ryan

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

> > But in groovy+, the udev rule from multipath-tools that has
> > always attempted to remove the devices nodes for the partitions
> > of a disk that is a multipath member actually succeeds, and
>
> Won't this break curtin on older multipaths that don't have this rule?

No, if the device node for a partition of a disk that is a multipath member is still present, this branch ignores it (or at least that's the intent...)

> Or will this work with older multipath output since we're only referring to
> the mpath dev (rather than a path of the mpath device)?

Er, I think so. Not completely sure about the distinction you're making here.

> Some comments inline.

Replies inline.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:fe8906661e3b6dee1bea7dd1dc7603dc447e4dff
https://jenkins.ubuntu.com/server/job/curtin-ci/13/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/13/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/13/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/13/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/13/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/13//rebuild

review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

> > > But in groovy+, the udev rule from multipath-tools that has
> > > always attempted to remove the devices nodes for the partitions
> > > of a disk that is a multipath member actually succeeds, and
> >
> > Won't this break curtin on older multipaths that don't have this rule?
>
> No, if the device node for a partition of a disk that is a multipath member is still present, this branch ignores it (or at least that's the intent...)
>
> > Or will this work with older multipath output since we're only referring to
> > the mpath dev (rather than a path of the mpath device)?
>
> Er, I think so. Not completely sure about the distinction you're making here.

In the description, in groovy+ we will no longer have the partitions on the
path members IIUC. For example (say a 2 path disk with 1 partition) we'd see:

/dev/sda
/dev/sda1
/dev/sdb
/dev/sdb1
/dev/dm-0 (mpatha)
/dev/dm-1 (mpatha-part1)

And on Groovy+ we see:

/dev/sda
/dev/sdb
/dev/dm-0 (mpatha)
/dev/dm-1 (mpatha-part1)

Is that correct?

My concern was if the single-path partitions are present (as they are on
Focal and older) does the new code still handle things OK?

I *think* it's yes due to the fact that curtin knows how to create partitions
on MP devices since 20.04.

I've added some replies in-line, awkwardly, you have to select your previous
commit to see those.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

> > > > But in groovy+, the udev rule from multipath-tools that has
> > > > always attempted to remove the devices nodes for the partitions
> > > > of a disk that is a multipath member actually succeeds, and
> > >
> > > Won't this break curtin on older multipaths that don't have this rule?
> >
> > No, if the device node for a partition of a disk that is a multipath member
> is still present, this branch ignores it (or at least that's the intent...)
> >
> > > Or will this work with older multipath output since we're only referring
> to
> > > the mpath dev (rather than a path of the mpath device)?
> >
> > Er, I think so. Not completely sure about the distinction you're making
> here.
>
> In the description, in groovy+ we will no longer have the partitions on the
> path members IIUC. For example (say a 2 path disk with 1 partition) we'd see:
>
> /dev/sda
> /dev/sda1
> /dev/sdb
> /dev/sdb1
> /dev/dm-0 (mpatha)
> /dev/dm-1 (mpatha-part1)
>
> And on Groovy+ we see:
>
> /dev/sda
> /dev/sdb
> /dev/dm-0 (mpatha)
> /dev/dm-1 (mpatha-part1)
>
> Is that correct?

Yes.

> My concern was if the single-path partitions are present (as they are on
> Focal and older) does the new code still handle things OK?

Oh I see, you're worried about the downstream impact of this: whether the storage configs produced by curtin after this change can actually be installed. That's a good question!

> I *think* it's yes due to the fact that curtin knows how to create partitions
> on MP devices since 20.04.

I think you're probably right but I should also do some testing of this situation.

> I've added some replies in-line, awkwardly, you have to select your previous
> commit to see those.

Replied.

Revision history for this message
Michael Hudson-Doyle (mwhudson) :
~mwhudson/curtin:lp-1893818 updated
d012706... by Michael Hudson-Doyle

make is_mpath_partition more correct

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:009fe6c04772b318baf27f5537573b262d666d04
https://jenkins.ubuntu.com/server/job/curtin-ci/25/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/25/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/25/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/25/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/25/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/25//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
f3734d2... by Michael Hudson-Doyle

a qemu test had DM_WWN=0xQEMU_QEMU_HARDDISK_001 somehow

6d8ae26... by Michael Hudson-Doyle

disable pretty path for multipath disks

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:51aef65c3d176ac1a8ef51c5878036af1f8eb240
https://jenkins.ubuntu.com/server/job/curtin-ci/40/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/40/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/40/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/40/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/40/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/40//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:5741e21a79800a4ef8d65cac63cefe23a200e9be
https://jenkins.ubuntu.com/server/job/curtin-ci/41/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/41/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/41/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/41/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/41/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/41//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:a0dcb78d3bb1ae3827dbbc18aa54d90b728632b1
https://jenkins.ubuntu.com/server/job/curtin-ci/42/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/42/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/42/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/42/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/42/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/42//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:f06e8a241ce584febfbda02c72c4190448ab61dc
https://jenkins.ubuntu.com/server/job/curtin-ci/43/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/43/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/43/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/43/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/43/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/43//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:f8c8828797e3e3fe09d99d3b57a1bfc43cd59188
https://jenkins.ubuntu.com/server/job/curtin-ci/44/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/44/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/44/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/44/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/44/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/44//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
fee2947... by Michael Hudson-Doyle

remove commented out code

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:f79b88fd740937f2bf100f8c79859ff002201d98
https://jenkins.ubuntu.com/server/job/curtin-ci/46/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/46/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/46/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/46/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/46/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/46//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:57adf408d201a2194477bba96345728f1759700c
https://jenkins.ubuntu.com/server/job/curtin-ci/48/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/48/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/48/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/48/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/48/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/48//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
3f56616... by Michael Hudson-Doyle

another

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:7e68ad56850b2c9497119289226317da912fa40a
https://jenkins.ubuntu.com/server/job/curtin-ci/50/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/50/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/50/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/50/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/50/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/50//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
844a1c1... by Michael Hudson-Doyle

more tests, surely there is a more systematic way to do this

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:88d4b7e628615fa2b29fc517cbeb897b12de4b1d
https://jenkins.ubuntu.com/server/job/curtin-ci/52/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/52/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/52/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/52/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/52/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/52//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
fe9f59f... by Michael Hudson-Doyle

flake8

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:c07309ba946f967665442f007f745b00de372da4
https://jenkins.ubuntu.com/server/job/curtin-ci/54/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/54/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/54/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/54/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/54/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/54//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
710522f... by Michael Hudson-Doyle

lint

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:a7f3f467ad1b1cdde3f686c6ac7b5f6ab153b390
https://jenkins.ubuntu.com/server/job/curtin-ci/70/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/70/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/70/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/70/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/70/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/70//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
cccce7b... by Michael Hudson-Doyle

light self review

98bacc6... by Michael Hudson-Doyle

don't need these changes any more

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:29281d57d55c6d492642f896269674e4feeea9c9
https://jenkins.ubuntu.com/server/job/curtin-ci/71/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/71/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/71/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/71/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/71/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/71//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:29281d57d55c6d492642f896269674e4feeea9c9
https://jenkins.ubuntu.com/server/job/curtin-ci/73/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/73/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/73/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/73/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/73/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/73//rebuild

review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

Some more inline questions.

I definitely want to discuss the devtype mpath-partition, mpath-disk values. We cannot write out a curtin storage config with type:mpath-disk type:mpath-partition values; they aren't in the block schema.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Thanks for the comments, some replies to the inline questions.

Can you look at https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/396462 first?

I changed the ProbertParser.is_mpath_* methods to just forward to the multipath.is_mpath_* functions, which meant I had to change one of the latter to work via udev. We don't need any of this knowledge even more spread around that it was already...

~mwhudson/curtin:lp-1893818 updated
d50afda... by Michael Hudson-Doyle

address review comments

b2f7156... by Michael Hudson-Doyle

clean up finding parent partitiontable from partition

Revision history for this message
Michael Hudson-Doyle (mwhudson) :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:c3e8bc9a6906b541586d6a04f67c72ba64acecce
https://jenkins.ubuntu.com/server/job/curtin-ci/74/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/74/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/74/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/74/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/74/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/74//rebuild

review: Needs Fixing (continuous-integration)
~mwhudson/curtin:lp-1893818 updated
e8da9bb... by Michael Hudson-Doyle

flakes

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:3ce75132f5e9543fc342f63a00fc3d5c8c362814
https://jenkins.ubuntu.com/server/job/curtin-ci/75/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/75/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/75/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/75/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/75/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/75//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:364f9cb2868e7d08bab17f81c79cd425ba9ffff8
https://jenkins.ubuntu.com/server/job/curtin-ci/76/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/76/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/76/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/76/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/76/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/76//rebuild

review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:ffcc7a2a0080f33a8f5efa0031c23f108cf8760f
https://jenkins.ubuntu.com/server/job/curtin-ci/82/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/82/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/82/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/82/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/82/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/82//rebuild

review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:e1f271c5c289cd70b9842af85ef9c7e3540905cb
https://jenkins.ubuntu.com/server/job/curtin-ci/93/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/93/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/93/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/93/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/93/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/93//rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

So I think this is ready to go. I've tested it in KVM to install to both new and existing partitions on a multipath disk and it worked fine. Anything else I should add, do we think?

Revision history for this message
Ryan Harper (raharper) wrote :

Do we have a reuse existing mpath with partitions in vmtest? If not, that'd be a good thing to add.

~mwhudson/curtin:lp-1893818 updated
0790cfc... by Michael Hudson-Doyle

add a vmtest for reusing partitions of multipath disk

have not managed to actually run this yet

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:a35c764477eb20a2739e294fff683e60cc6bd080
https://jenkins.ubuntu.com/server/job/curtin-ci/95/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/95/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/95/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/95/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/95/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/95//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Paride Legovini (paride) wrote :

I ran the new vmtest but it failed across all the releases:

https://jenkins.ubuntu.com/server/job/curtin-vmtest-devel-debug/222/console

but I don't know if that's really a useful failure, as running the test_multipath.py tests from master also failed:

cmd: nosetests3 --process-timeout=86400 --processes=4 -vv --nologcapture tests/vmtests/test_multipath.py

console log: https://jenkins.ubuntu.com/server/job/curtin-vmtest-devel-debug/221/console
copypaste here: https://paste.ubuntu.com/p/HTp4xh2qYH/

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Hm thanks, that definitely looks like some kind of problem in the newly added yaml file :/ make sync-images finally completed here so I can run some tests myself now :)

~mwhudson/curtin:lp-1893818 updated
ca8b1c0... by Michael Hudson-Doyle

fix things

9348f08... by Michael Hudson-Doyle

fix things some more

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I fixed some simple and some obscure things and at least some of the tests pass for me now :) Can you have another whack at in jenkins?

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:5250114df722d1088e737335af8513bec7894971
https://jenkins.ubuntu.com/server/job/curtin-ci/96/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/96/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/96/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/96/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/96/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/96//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Paride Legovini (paride) wrote :

pylint is flaky with the usual:

00:12:11.509 ************* Module curtin.commands.install_grub
00:12:11.509 curtin/commands/install_grub.py:93:19: E1101: Instance of 'Distros' has no 'debian' member (no-member)
00:12:11.509 curtin/commands/install_grub.py:155:28: E1101: Instance of 'Distros' has no 'redhat' member (no-member)
00:12:11.509 curtin/commands/install_grub.py:246:28: E1101: Instance of 'Distros' has no 'debian' member (no-member)

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:5250114df722d1088e737335af8513bec7894971
https://jenkins.ubuntu.com/server/job/curtin-ci/97/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/97/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/97/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/97/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/97/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/97//rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

So the new vmtests passed. Of the old multipath tests, Centos70TestMultipathBasic fails but it also fails the same way on master (like this:

[ 214.243025] cloud-init[1257]: start: cmd-install/stage-late/00_grub1_boot: running 'curtin in-target -- sed -i.curtin -e s|(hd1,0)|(hd0,0)|g /boot/grub/grub.conf'
[ 214.710207] cloud-init[1257]: start: cmd-install/stage-late/00_grub1_boot/cmd-in-target: curtin command in-target
[ 214.718102] cloud-init[1257]: Running command ['mount', '--bind', '/dev', '/tmp/tmp69un0w5l/target/dev'] with allowed return codes [0] (capture=False)
[ 214.726514] cloud-init[1257]: Running command ['mount', '--bind', '/proc', '/tmp/tmp69un0w5l/target/proc'] with allowed return codes [0] (capture=False)
[ 214.734124] cloud-init[1257]: Running command ['mount', '--bind', '/run', '/tmp/tmp69un0w5l/target/run'] with allowed return codes [0] (capture=False)
[ 214.741353] cloud-init[1257]: Running command ['mount', '--bind', '/sys', '/tmp/tmp69un0w5l/target/sys'] with allowed return codes [0] (capture=False)
[ 214.749061] cloud-init[1257]: Running command ['unshare', '--help'] with allowed return codes [0] (capture=True)
[ 214.755668] cloud-init[1257]: Running command ['unshare', '--fork', '--pid', '--', 'chroot', '/tmp/tmp69un0w5l/target', 'sed', '-i.curtin', '-e', 's|(hd1,0)|(hd0,0)|g', '/boot/grub/grub.conf'] with allowed return codes [0] (capture=False)
[ 214.760850] cloud-init[1257]: sed: can't read /boot/grub/grub.conf: No such file or directory
[ 214.764817] cloud-init[1257]: Running command ['udevadm', 'settle'] with allowed return codes [0] (capture=False)
[ 214.784140] cloud-init[1257]: TIMED subp(['udevadm', 'settle']): 0.018
[ 214.785396] cloud-init[1257]: Running command ['umount', '/tmp/tmp69un0w5l/target/sys'] with allowed return codes [0] (capture=False)
[ 214.792526] cloud-init[1257]: Running command ['umount', '/tmp/tmp69un0w5l/target/run'] with allowed return codes [0] (capture=False)
[ 214.799577] cloud-init[1257]: Running command ['umount', '/tmp/tmp69un0w5l/target/proc'] with allowed return codes [0] (capture=False)
[ 214.806709] cloud-init[1257]: Running command ['umount', '/tmp/tmp69un0w5l/target/dev'] with allowed return codes [0] (capture=False)
[ 214.813171] cloud-init[1257]: finish: cmd-install/stage-late/00_grub1_boot/cmd-in-target: FAIL: curtin command in-target
[ 214.858039] cloud-init[1257]: 00_grub1_boot command failed

)

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Anyway, given that this is not a regression caused by this branch, I think this is OK to merge. It's been so long though that I'm going to leave it 24 hours for any more objections :-)

Revision history for this message
Ryan Harper (raharper) wrote :

That script is from tests/exampltes/centos_defaults.yaml

It was inplace for dealing with an older grub in centos66... that said, I wonder what's changed in the Centos7X image such that the grub.cfg file isn't present any more...

It master passed on this centos image:

 centos/centos70/amd64/20201222_01/root-tgz

And there's now a new centos7 image

 centos/centos70/amd64/20210210_01/root-tgz

where it fails. I'll look a little deeper into the image.

Revision history for this message
Ryan Harper (raharper) wrote :

The new maas image moved grub config location. =(

https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/399028

This fixed the multipath test case for sure. I also plan to see if
I can only run that fix up on centos66 to confirm

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:9348f089bf171d3d1df781dc0217a1508ad7aa81
https://jenkins.ubuntu.com/server/job/curtin-ci/99/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/99/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/99/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/99/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/99/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/99//rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

All the multipath vmtests now pass \o/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/block/multipath.py b/curtin/block/multipath.py
2index e317184..0f9170e 100644
3--- a/curtin/block/multipath.py
4+++ b/curtin/block/multipath.py
5@@ -65,11 +65,10 @@ def is_mpath_device(devpath, info=None):
6 def is_mpath_member(devpath, info=None):
7 """ Check if a device is a multipath member (a path), returns boolean. """
8 result = False
9- try:
10- util.subp(['multipath', '-c', devpath], capture=True)
11+ if not info:
12+ info = udev.udevadm_info(devpath)
13+ if info.get("DM_MULTIPATH_DEVICE_PATH") == "1":
14 result = True
15- except util.ProcessExecutionError:
16- pass
17
18 LOG.debug('%s is multipath device member? %s', devpath, result)
19 return result
20@@ -81,7 +80,7 @@ def is_mpath_partition(devpath, info=None):
21 if devpath.startswith('/dev/dm-'):
22 if not info:
23 info = udev.udevadm_info(devpath)
24- if 'DM_PART' in udev.udevadm_info(devpath):
25+ if 'DM_PART' in info and 'DM_MPATH' in info:
26 result = True
27
28 LOG.debug("%s is multipath device partition? %s", devpath, result)
29diff --git a/curtin/storage_config.py b/curtin/storage_config.py
30index e6c33cc..81b7385 100644
31--- a/curtin/storage_config.py
32+++ b/curtin/storage_config.py
33@@ -7,7 +7,7 @@ import re
34 import yaml
35
36 from curtin.log import LOG
37-from curtin.block import schemas
38+from curtin.block import multipath, schemas
39 from curtin import config as curtin_config
40 from curtin import util
41
42@@ -453,72 +453,15 @@ class ProbertParser(object):
43
44 return None
45
46- def is_mpath(self, blockdev):
47- if blockdev.get('DM_MULTIPATH_DEVICE_PATH') == "1":
48- return True
49+ def is_mpath_member(self, blockdev):
50+ return multipath.is_mpath_member(blockdev.get('DEVNAME', ''), blockdev)
51
52- return bool('mpath-' in blockdev.get('DM_UUID', ''))
53+ def is_mpath_device(self, blockdev):
54+ return multipath.is_mpath_device(blockdev.get('DEVNAME', ''), blockdev)
55
56- def get_mpath_name(self, blockdev):
57- mpath_data = self.probe_data.get('multipath')
58- if not mpath_data:
59- return None
60-
61- bd_name = blockdev['DEVNAME']
62- if blockdev['DEVTYPE'] == 'partition':
63- bd_name = self.partition_parent_devname(blockdev)
64- bd_name = os.path.basename(bd_name)
65- for path in mpath_data.get('paths', []):
66- if bd_name == path.get('device'):
67- rv = path.get('multipath')
68- return rv
69-
70- def find_mpath_member(self, blockdev):
71- if blockdev.get('DM_MULTIPATH_DEVICE_PATH') == "1":
72- # find all other DM_MULTIPATH_DEVICE_PATH devs with same serial
73- serial = blockdev.get('ID_SERIAL')
74- members = sorted([os.path.basename(dev['DEVNAME'])
75- for dev in self.blockdev_data.values()
76- if dev.get("ID_SERIAL", "") == serial and
77- dev['DEVTYPE'] == blockdev['DEVTYPE']])
78- # [/dev/sda, /dev/sdb]
79- # [/dev/sda1, /dev/sda2, /dev/sdb1, /dev/sdb2]
80-
81- else:
82- dm_mpath = blockdev.get('DM_MPATH')
83- dm_uuid = blockdev.get('DM_UUID')
84- dm_part = blockdev.get('DM_PART')
85- dm_name = blockdev.get('DM_NAME')
86-
87- if dm_mpath:
88- multipath = dm_mpath
89- elif dm_name:
90- multipath = dm_name
91- else:
92- # part1-mpath-30000000000000064
93- # mpath-30000000000000064
94- # mpath-36005076306ffd6b60000000000002406
95- match = re.search(r'mpath\-([a-zA-Z]*|\d*)+$', dm_uuid)
96- if not match:
97- LOG.debug('Failed to extract multipath ID pattern from '
98- 'DM_UUID value: "%s"', dm_uuid)
99- return None
100- # remove leading 'mpath-'
101- multipath = match.group(0)[6:]
102- mpath_data = self.probe_data.get('multipath')
103- if not mpath_data:
104- return None
105- members = sorted([path['device'] for path in mpath_data['paths']
106- if path['multipath'] == multipath])
107-
108- # append partition number if present
109- if dm_part:
110- members = [member + dm_part for member in members]
111-
112- if len(members):
113- return members[0]
114-
115- return None
116+ def is_mpath_partition(self, blockdev):
117+ return multipath.is_mpath_partition(
118+ blockdev.get('DEVNAME', ''), blockdev)
119
120 def blockdev_to_id(self, blockdev):
121 """ Examine a blockdev dictionary and return a tuple of curtin
122@@ -539,15 +482,13 @@ class ProbertParser(object):
123 if 'DM_LV_NAME' in blockdev:
124 devtype = 'lvm-partition'
125 name = blockdev['DM_LV_NAME']
126- elif self.is_mpath(blockdev):
127- # extract a multipath member device
128- member = self.find_mpath_member(blockdev)
129- if member:
130- name = member
131- else:
132- name = blockdev['DM_UUID']
133- if 'DM_PART' in blockdev:
134- devtype = 'partition'
135+ elif self.is_mpath_device(blockdev):
136+ devtype = 'mpath-disk'
137+ name = blockdev['DM_NAME']
138+ elif self.is_mpath_partition(blockdev):
139+ devtype = 'mpath-partition'
140+ name = '{}-part{}'.format(
141+ blockdev['DM_MPATH'], blockdev['DM_PART'])
142 elif is_dmcrypt(blockdev):
143 devtype = 'dmcrypt'
144 name = blockdev['DM_NAME']
145@@ -681,10 +622,15 @@ class BlockdevParser(ProbertParser):
146 errors = []
147
148 for devname, data in self.blockdev_data.items():
149- # skip composed devices here, except partitions
150+ # skip composed devices here, except partitions and multipath
151 if data.get('DEVPATH', '').startswith('/devices/virtual/block'):
152- if data.get('DEVTYPE', '') != "partition":
153- continue
154+ if not self.is_mpath_device(data):
155+ if not self.is_mpath_partition(data):
156+ if data.get('DEVTYPE', '') != "partition":
157+ continue
158+ # skip disks that are members of multipath devices
159+ if self.is_mpath_member(data):
160+ continue
161 entry = self.asdict(data)
162 if entry:
163 try:
164@@ -698,7 +644,10 @@ class BlockdevParser(ProbertParser):
165 def valid_id(self, id_value):
166 # reject wwn=0x0+
167 if id_value.lower().startswith('0x'):
168- return int(id_value, 16) > 0
169+ try:
170+ return int(id_value, 16) > 0
171+ except ValueError:
172+ return True
173 # accept non-empty (removing whitspace) strings
174 return len(''.join(id_value.split())) > 0
175
176@@ -710,10 +659,16 @@ class BlockdevParser(ProbertParser):
177 blockdev attribute.
178 """
179 uniq = {}
180- source_keys = {
181- 'wwn': ['ID_WWN_WITH_EXTENSION', 'ID_WWN'],
182- 'serial': ['ID_SERIAL', 'ID_SERIAL_SHORT'],
183- }
184+ if self.is_mpath_device(blockdev):
185+ source_keys = {
186+ 'wwn': ['DM_WWN'],
187+ 'serial': ['DM_SERIAL'], # only present with focal+
188+ }
189+ else:
190+ source_keys = {
191+ 'wwn': ['ID_WWN_WITH_EXTENSION', 'ID_WWN'],
192+ 'serial': ['ID_SERIAL', 'ID_SERIAL_SHORT'],
193+ }
194 for skey, id_keys in source_keys.items():
195 for id_key in id_keys:
196 if id_key in blockdev and skey not in uniq:
197@@ -740,6 +695,10 @@ class BlockdevParser(ProbertParser):
198 storage config dictionary. This method
199 will return curtin storage types: disk, partition.
200 """
201+ dev_type = blockdev_data['DEVTYPE']
202+ if self.is_mpath_partition(blockdev_data):
203+ dev_type = 'partition'
204+
205 # just disks and partitions
206 if blockdev_data['DEVTYPE'] not in ["disk", "partition"]:
207 return None
208@@ -752,13 +711,13 @@ class BlockdevParser(ProbertParser):
209
210 devname = blockdev_data.get('DEVNAME')
211 entry = {
212- 'type': blockdev_data['DEVTYPE'],
213+ 'type': dev_type,
214 'id': self.blockdev_to_id(blockdev_data),
215 }
216- if blockdev_data.get('DM_MULTIPATH_DEVICE_PATH') == "1":
217- mpath_name = self.get_mpath_name(blockdev_data)
218- if mpath_name:
219- entry['multipath'] = mpath_name
220+ if self.is_mpath_device(blockdev_data):
221+ entry['multipath'] = blockdev_data['DM_NAME']
222+ elif self.is_mpath_partition(blockdev_data):
223+ entry['multipath'] = blockdev_data['DM_MPATH']
224
225 # default disks to gpt
226 if entry['type'] == 'disk':
227@@ -796,8 +755,17 @@ class BlockdevParser(ProbertParser):
228
229 if entry['type'] == 'partition':
230 attrs = blockdev_data['attrs']
231- entry['number'] = int(attrs['partition'])
232- parent_devname = self.partition_parent_devname(blockdev_data)
233+ if self.is_mpath_partition(blockdev_data):
234+ entry['number'] = int(blockdev_data['DM_PART'])
235+ parent_devname = self.lookup_devname(
236+ '/dev/mapper/' + blockdev_data['DM_MPATH'])
237+ if parent_devname is None:
238+ raise ValueError(
239+ "Cannot find parent mpath device %s for %s" % (
240+ blockdev_data['DM_MPATH'], devname))
241+ else:
242+ entry['number'] = int(attrs['partition'])
243+ parent_devname = self.partition_parent_devname(blockdev_data)
244 parent_blockdev = self.blockdev_data[parent_devname]
245 if 'ID_PART_TABLE_TYPE' not in parent_blockdev:
246 # Exclude the fake partition that the kernel creates
247@@ -810,9 +778,7 @@ class BlockdevParser(ProbertParser):
248 if ptable:
249 part = None
250 for pentry in ptable['partitions']:
251- node = pentry['node']
252- node_p = node.replace(parent_devname, '')
253- if node_p.replace('p', '') == attrs['partition']:
254+ if self.lookup_devname(pentry['node']) == devname:
255 part = pentry
256 break
257
258@@ -880,6 +846,9 @@ class FilesystemParser(ProbertParser):
259 errors.append(err)
260 continue
261
262+ if self.is_mpath_member(blockdev_data):
263+ continue
264+
265 # no floppy, no cdrom
266 if blockdev_data['MAJOR'] in ["11", "2"]:
267 continue
268diff --git a/examples/tests/multipath-reuse.yaml b/examples/tests/multipath-reuse.yaml
269new file mode 100644
270index 0000000..24e193e
271--- /dev/null
272+++ b/examples/tests/multipath-reuse.yaml
273@@ -0,0 +1,61 @@
274+
275+# The point of this test is to test installing to a existing
276+# partitions of a multipathed disk
277+
278+bucket:
279+ - &setup |
280+ parted /dev/disk/by-id/dm-name-mpatha --script -- \
281+ mklabel msdos \
282+ mkpart primary ext4 1GiB 4GiB \
283+ mkpart primary ext4 4GiB 5GiB \
284+ set 1 boot on
285+ udevadm settle
286+
287+early_commands:
288+ 00-setup-msdos-ptable: [sh, -exuc, *setup]
289+
290+install:
291+ unmount: disabled
292+showtrace: true
293+storage:
294+ version: 1
295+ config:
296+ - id: sda
297+ type: disk
298+ ptable: msdos
299+ serial: 'IPR-0 1234567890'
300+ name: mpath_a
301+ grub_device: true
302+ multipath: mpatha
303+ path: /dev/disk/by-id/dm-name-mpatha
304+ preserve: true
305+ - id: sda1
306+ type: partition
307+ number: 1
308+ size: 3GB
309+ device: sda
310+ flag: boot
311+ preserve: true
312+ - id: sda2
313+ type: partition
314+ number: 2
315+ size: 1GB
316+ device: sda
317+ preserve: true
318+ - id: sda1_root
319+ type: format
320+ fstype: ext4
321+ volume: sda1
322+ - id: sda2_home
323+ type: format
324+ fstype: ext4
325+ volume: sda2
326+ - id: sda1_mount
327+ type: mount
328+ path: /
329+ device: sda1_root
330+ - id: sda2_mount
331+ type: mount
332+ path: /home
333+ device: sda2_home
334+ options: 'defaults,nofail'
335diff --git a/tests/unittests/test_block_multipath.py b/tests/unittests/test_block_multipath.py
336index 426be56..db767ab 100644
337--- a/tests/unittests/test_block_multipath.py
338+++ b/tests/unittests/test_block_multipath.py
339@@ -1,7 +1,7 @@
340 import mock
341
342 from curtin.block import multipath
343-from .helpers import CiTestCase, raise_pexec_error
344+from .helpers import CiTestCase
345
346
347 # dmsetup uses tabs as separators
348@@ -63,23 +63,40 @@ class TestMultipath(CiTestCase):
349 self.m_udev.udevadm_info.return_value = {'DM_UUID': 'lvm-vg-foo-lv1'}
350 self.assertFalse(multipath.is_mpath_device(self.random_string()))
351
352- def test_is_mpath_member_true(self):
353- """is_mpath_device returns false when DM_UUID doesnt start w/ mpath-"""
354- self.assertTrue(multipath.is_mpath_member(self.random_string()))
355-
356 def test_is_mpath_member_false(self):
357- """is_mpath_member returns false if 'multipath -c <dev>' exits err."""
358- self.m_subp.side_effect = raise_pexec_error
359+ """is_mpath_member returns false if DM_MULTIPATH_DEVICE_PATH is not
360+ present"""
361+ self.m_udev.udevadm_info.return_value = {}
362+ self.assertFalse(multipath.is_mpath_member(self.random_string()))
363+
364+ def test_is_mpath_member_false_2(self):
365+ """is_mpath_member returns false if DM_MULTIPATH_DEVICE_PATH is not
366+ '1'"""
367+ self.m_udev.udevadm_info.return_value = {
368+ "DM_MULTIPATH_DEVICE_PATH": "2",
369+ }
370 self.assertFalse(multipath.is_mpath_member(self.random_string()))
371
372+ def test_is_mpath_member_true(self):
373+ """is_mpath_member returns true if DM_MULTIPATH_DEVICE_PATH is
374+ '1'"""
375+ self.m_udev.udevadm_info.return_value = {
376+ "DM_MULTIPATH_DEVICE_PATH": "1",
377+ }
378+ self.assertTrue(multipath.is_mpath_member(self.random_string()))
379+
380 def test_is_mpath_partition_true(self):
381- """is_mpath_member returns true if 'multipath -c <dev>' exits 0."""
382+ """is_mpath_partition returns true if udev info contains right keys."""
383 dm_device = "/dev/dm-" + self.random_string()
384- self.m_udev.udevadm_info.return_value = {'DM_PART': '1'}
385+ self.m_udev.udevadm_info.return_value = {
386+ 'DM_PART': '1',
387+ 'DM_MPATH': 'a',
388+ }
389 self.assertTrue(multipath.is_mpath_partition(dm_device))
390
391 def test_is_mpath_partition_false(self):
392- """is_mpath_member returns false if DM_PART is not present for dev."""
393+ """is_mpath_partition returns false if DM_PART is not present for dev.
394+ """
395 self.assertFalse(multipath.is_mpath_partition(self.random_string()))
396
397 def test_mpath_partition_to_mpath_id_and_partnumber(self):
398diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
399index 2d27d4e..0ed7659 100644
400--- a/tests/unittests/test_storage_config.py
401+++ b/tests/unittests/test_storage_config.py
402@@ -467,149 +467,51 @@ class TestBlockdevParser(CiTestCase):
403 self.assertDictEqual(expected_dict,
404 self.bdevp.asdict(blockdev))
405
406- def test_blockdev_detects_multipath(self):
407+ def test_blockdev_multipath_disk(self):
408 self.probe_data = _get_data('probert_storage_multipath.json')
409 self.bdevp = BlockdevParser(self.probe_data)
410- blockdev = self.bdevp.blockdev_data['/dev/sda2']
411+ blockdev = self.bdevp.blockdev_data['/dev/dm-0']
412 expected_dict = {
413- 'flag': 'linux',
414- 'id': 'partition-sda2',
415- 'offset': 2097152,
416+ 'id': 'mpath-disk-mpatha',
417 'multipath': 'mpatha',
418- 'size': 10734272512,
419- 'type': 'partition',
420- 'device': 'disk-sda',
421- 'number': 2}
422+ 'path': '/dev/dm-0',
423+ 'ptable': 'gpt',
424+ 'type': 'disk',
425+ 'wwn': '0x0000000000000064',
426+ }
427 self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
428
429- def test_blockdev_skips_multipath_entry_if_no_multipath_data(self):
430+ def test_blockdev_multipath_partition(self):
431 self.probe_data = _get_data('probert_storage_multipath.json')
432- del self.probe_data['multipath']
433 self.bdevp = BlockdevParser(self.probe_data)
434- blockdev = self.bdevp.blockdev_data['/dev/sda2']
435+ blockdev = self.bdevp.blockdev_data['/dev/dm-2']
436 expected_dict = {
437+ 'device': 'mpath-disk-mpatha',
438 'flag': 'linux',
439- 'id': 'partition-sda2',
440+ 'id': 'mpath-partition-mpatha-part2',
441+ 'multipath': 'mpatha',
442+ 'number': 2,
443 'offset': 2097152,
444 'size': 10734272512,
445 'type': 'partition',
446- 'device': 'disk-sda',
447- 'number': 2}
448+ }
449 self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
450
451- def test_blockdev_skips_multipath_entry_if_bad_multipath_data(self):
452+ @skipUnlessJsonSchema()
453+ def test_blockdev_skips_underlying_disks_and_partitions(self):
454 self.probe_data = _get_data('probert_storage_multipath.json')
455- for path in self.probe_data['multipath']['paths']:
456- path['multipath'] = ''
457 self.bdevp = BlockdevParser(self.probe_data)
458- blockdev = self.bdevp.blockdev_data['/dev/sda2']
459- expected_dict = {
460- 'flag': 'linux',
461- 'id': 'partition-sda2',
462- 'offset': 2097152,
463- 'size': 10734272512,
464- 'type': 'partition',
465- 'device': 'disk-sda',
466- 'number': 2}
467- self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
468-
469- def test_blockdev_skips_multipath_entry_if_no_mp_paths(self):
470- self.probe_data = _get_data('probert_storage_multipath.json')
471- del self.probe_data['multipath']['paths']
472- self.bdevp = BlockdevParser(self.probe_data)
473- blockdev = self.bdevp.blockdev_data['/dev/sda2']
474- expected_dict = {
475- 'flag': 'linux',
476- 'id': 'partition-sda2',
477- 'offset': 2097152,
478- 'size': 10734272512,
479- 'type': 'partition',
480- 'device': 'disk-sda',
481- 'number': 2}
482- self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
483+ configs = self.bdevp.parse()[0]
484+ config_paths = {c.get('path') for c in configs}
485+ self.assertNotIn('/dev/sda', config_paths)
486
487 def test_blockdev_finds_multipath_id_from_dm_uuid(self):
488 self.probe_data = _get_data('probert_storage_zlp6.json')
489 self.bdevp = BlockdevParser(self.probe_data)
490 blockdev = self.bdevp.blockdev_data['/dev/dm-2']
491 result = self.bdevp.blockdev_to_id(blockdev)
492- self.assertEqual('disk-sda', result)
493-
494- def test_blockdev_find_mpath_members_checks_dm_name(self):
495- """ BlockdevParser find_mpath_members uses dm_name if present."""
496- dm14 = {
497- "DEVTYPE": "disk",
498- "DEVLINKS": "/dev/disk/by-id/dm-name-mpathb",
499- "DEVNAME": "/dev/dm-14",
500- "DEVTYPE": "disk",
501- "DM_NAME": "mpathb",
502- "DM_UUID": "mpath-360050768028211d8b000000000000062",
503- "DM_WWN": "0x60050768028211d8b000000000000062",
504- "MPATH_DEVICE_READY": "1",
505- "MPATH_SBIN_PATH": "/sbin",
506- }
507- multipath = {
508- "maps": [
509- {
510- "multipath": "360050768028211d8b000000000000061",
511- "sysfs": "dm-11",
512- "paths": "4"
513- },
514- {
515- "multipath": "360050768028211d8b000000000000062",
516- "sysfs": "dm-14",
517- "paths": "4"
518- },
519- {
520- "multipath": "360050768028211d8b000000000000063",
521- "sysfs": "dm-15",
522- "paths": "4"
523- }],
524- "paths": [
525- {
526- "device": "sdej",
527- "serial": "0200a084762cXX00",
528- "multipath": "mpatha",
529- "host_wwnn": "0x20000024ff9127de",
530- "target_wwnn": "0x5005076802065e38",
531- "host_wwpn": "0x21000024ff9127de",
532- "target_wwpn": "0x5005076802165e38",
533- "host_adapter": "[undef]"
534- },
535- {
536- "device": "sdel",
537- "serial": "0200a084762cXX00",
538- "multipath": "mpathb",
539- "host_wwnn": "0x20000024ff9127de",
540- "target_wwnn": "0x5005076802065e38",
541- "host_wwpn": "0x21000024ff9127de",
542- "target_wwpn": "0x5005076802165e38",
543- "host_adapter": "[undef]"
544- },
545- {
546- "device": "sdet",
547- "serial": "0200a084762cXX00",
548- "multipath": "mpatha",
549- "host_wwnn": "0x20000024ff9127de",
550- "target_wwnn": "0x5005076802065e37",
551- "host_wwpn": "0x21000024ff9127de",
552- "target_wwpn": "0x5005076802165e37",
553- "host_adapter": "[undef]"
554- },
555- {
556- "device": "sdev",
557- "serial": "0200a084762cXX00",
558- "multipath": "mpathb",
559- "host_wwnn": "0x20000024ff9127de",
560- "target_wwnn": "0x5005076802065e37",
561- "host_wwpn": "0x21000024ff9127de",
562- "target_wwpn": "0x5005076802165e37",
563- "host_adapter": "[undef]"
564- }],
565- }
566- self.bdevp.blockdev_data['/dev/dm-14'] = dm14
567- self.probe_data['multipath'] = multipath
568- self.assertEqual('disk-sdel', self.bdevp.blockdev_to_id(dm14))
569+ self.assertEqual(
570+ 'mpath-disk-36005076306ffd6b60000000000002406', result)
571
572 def test_blockdev_detects_dasd_device_id_and_vtoc_ptable(self):
573 self.probe_data = _get_data('probert_storage_dasd.json')
574@@ -1017,22 +919,6 @@ class TestExtractStorageConfig(CiTestCase):
575 extracted)
576
577 @skipUnlessJsonSchema()
578- def test_find_all_multipath(self):
579- """ verify probed multipath paths are included in config. """
580- self.probe_data = _get_data('probert_storage_multipath.json')
581- extracted = storage_config.extract_storage_config(self.probe_data)
582- config = extracted['storage']['config']
583- blockdev = self.probe_data['blockdev']
584-
585- for mpmap in self.probe_data['multipath']['maps']:
586- nr_disks = int(mpmap['paths'])
587- mp_name = blockdev['/dev/%s' % mpmap['sysfs']]['DM_NAME']
588- matched_disks = [cfg for cfg in config
589- if cfg['type'] == 'disk' and
590- cfg.get('multipath', '') == mp_name]
591- self.assertEqual(nr_disks, len(matched_disks))
592-
593- @skipUnlessJsonSchema()
594 def test_find_raid_partition(self):
595 """ verify probed raid partitions are found. """
596 self.probe_data = _get_data('probert_storage_raid1_partitions.json')
597@@ -1081,6 +967,19 @@ class TestExtractStorageConfig(CiTestCase):
598 self.assertEqual(expected_dict, disks[0])
599
600 @skipUnlessJsonSchema()
601+ def test_blockdev_multipath(self):
602+ self.probe_data = _get_data('probert_storage_zlp6.json')
603+ extracted = storage_config.extract_storage_config(self.probe_data)
604+ config = extracted['storage']['config']
605+ disks = [cfg for cfg in config if cfg['type'] == 'disk']
606+ expected_count = len([
607+ 1 for bd_name, bd_data in self.probe_data['blockdev'].items()
608+ if bd_data.get('DM_UUID', '').startswith('mpath-')
609+ or bd_name.startswith('/dev/dasd') and bd_data['DEVTYPE'] == 'disk'
610+ ])
611+ self.assertEqual(expected_count, len(disks))
612+
613+ @skipUnlessJsonSchema()
614 def test_blockdev_skips_invalid_wwn(self):
615 self.probe_data = _get_data('probert_storage_bogus_wwn.json')
616 extracted = storage_config.extract_storage_config(self.probe_data)
617diff --git a/tests/vmtests/test_multipath.py b/tests/vmtests/test_multipath.py
618index f924d25..163b73a 100644
619--- a/tests/vmtests/test_multipath.py
620+++ b/tests/vmtests/test_multipath.py
621@@ -170,4 +170,20 @@ class GroovyTestMultipathBasic(relbase.groovy, TestMultipathBasicAbs):
622 __test__ = True
623
624
625+class TestMultipathReuseAbs(TestMultipathBasicAbs):
626+ conf_file = "examples/tests/multipath-reuse.yaml"
627+
628+
629+class FocalTestMultipathReuse(relbase.focal, TestMultipathReuseAbs):
630+ __test__ = True
631+
632+
633+class HirsuteTestMultipathReuse(relbase.hirsute, TestMultipathReuseAbs):
634+ __test__ = True
635+
636+
637+class GroovyTestMultipathReuse(relbase.groovy, TestMultipathReuseAbs):
638+ __test__ = True
639+
640+
641 # vi: ts=4 expandtab syntax=python

Subscribers

People subscribed via source and target branches