Merge lp://qastaging/~blake-rouse/maas/commissioning-get-disk-info into lp://qastaging/~maas-committers/maas/trunk
Proposed by
Blake Rouse
Status: | Merged | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Approved by: | Blake Rouse | ||||||||||||
Approved revision: | no longer in the source branch. | ||||||||||||
Merged at revision: | 3443 | ||||||||||||
Proposed branch: | lp://qastaging/~blake-rouse/maas/commissioning-get-disk-info | ||||||||||||
Merge into: | lp://qastaging/~maas-committers/maas/trunk | ||||||||||||
Prerequisite: | lp://qastaging/~blake-rouse/maas/physical-block-device-model | ||||||||||||
Diff against target: |
532 lines (+467/-0) 2 files modified
src/metadataserver/models/commissioningscript.py (+121/-0) src/metadataserver/models/tests/test_noderesults.py (+346/-0) |
||||||||||||
To merge this branch: | bzr merge lp://qastaging/~blake-rouse/maas/commissioning-get-disk-info | ||||||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Raphaël Badin (community) | Approve | ||
Review via email:
|
Commit message
Populates the PhysicalBlockDevice model for a node when commissioning. Uses a combination of lsblk, udevadm, and blockdev to gather the information from the node during commissioning.
To post a comment you must log in.
This looks generally good. I've got a bunch of comments but more importantly, I'm wondering about the failure modes for this. See below.
This uses a lot of commands and a fair share of parsing. All of which could go wrong at some point and break the commissioning. I think it's worth taking a step back and thinking about the failure modes here:
- which part of the code are likely to fail (command failure, parsing failure, etc)? Can we make this more robust against non-fatal error?
- what's the failure mode? In other words, what is going to happen when this fails, what will the consequences be, how easy will it be to the user to get information about what's wrong?
What do you think?