Merge lp://staging/~rvb/maas-test/power-type-support into lp://staging/maas-test
Proposed by
Raphaël Badin
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Raphaël Badin | ||||
Approved revision: | 166 | ||||
Merged at revision: | 157 | ||||
Proposed branch: | lp://staging/~rvb/maas-test/power-type-support | ||||
Merge into: | lp://staging/maas-test | ||||
Diff against target: |
596 lines (+332/-70) 8 files modified
docs/man/maas-test.8.rst (+43/-9) maastest/cases.py (+57/-36) maastest/kvmfixture.py (+10/-5) maastest/main.py (+10/-0) maastest/parser.py (+79/-9) maastest/tests/test_utils.py (+54/-1) maastest/utils.py (+36/-1) man/maas-test.8 (+43/-9) |
||||
To merge this branch: | bzr merge lp://staging/~rvb/maas-test/power-type-support | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeroen T. Vermeulen (community) | Approve | ||
Review via email:
|
Commit message
Add two parameters, --power-type and --power-parameters, to pass the power details about a node. This allows maas-test to support any power mechanism MAAS itself support instead of supporting only IPMI.
Description of the change
Notes:
- The legacy --bmc-* parameters (used when the node is IPMI-based) are still supported: the value of the --bmc-* parameters is converted into power-type and power-parameters.
- the trick use to power up the node to get it enlistment can appear a bit fishy but the nice thing about it is that is relies exclusively on API methods that won't change any time soon; this, I think, is much better than call MAAS' code directly.
To post a comment you must log in.
Thanks for working on this. The timeout is no longer needed, I suppose, now that the API client is fixed.
Is there a race condition between the node booting and maas-test deleting it? I suppose it won't matter until we get magical ultra-fast speed-booting nodes.
I have a few syntactical questions about the documentation:
$ sudo maas-test --maas-series trusty --architecture amd64 parameters= \"mac_address= aa:bb:cc: dd:ee:ff
--power-type=amt --power-
power_pass='my password'"
What does it mean for the opening double-quote after --power-parameters to be escaped, but not the closing quote? And, do the single quotes inside the quoted dict really work the way you'd expect? Who parses them?
I would like to recommend a few style rules for technical writing, e.g:
Try to keep elements of a sentence in an easy-to-understand order, so readers don't have to remember or re-read text that may only make sense later. Limit the nesting of phrases. That second sentence is structured like “A if B or C if D (E)” — that's quite hard to read if you're confused, annoyed, under pressure, or not very good at English.
Only use "[statement] if [condition]" if [statement] is very brief and very simple. Even then, keep an eye out for ambiguity around ‘and’/‘or’: is it part of the condition? Or does it start a whole new statement? These are parsing ambiguities. If the reader picks the wrong interpretation at first, they will hit a “parse error” later and have to re-read, which is annoying.
So: don't try to cram all relevant information into your sentence. See how _little_ information you can put in a sentence while still making a clear chain of steps towards full understanding. Good technical writing follows Antoine de Saint Exupéry's rule: “perfection is achieved, not when there is nothing more to add, but when there is nothing more to take away.”
The same things go for these messages:
)
...
Can you find a way to make these easier to read? Changing from passive voice to active voice is usually a good step.
But back to code. In convert_ bmc_details_ into_parameters you use an OrderedDict to produce consistent ordering — but the ordering isn't documented. Wouldn't it be just as easy to make the tests run parameters.split() and do an assertItemsEqual on the result? Or use sorted() inside convert_ bmc_details_ into_parameters ?