Merge ~sw37th/cloud-init:fix_for_opennebula into cloud-init:master
| Status: | Needs review |
|---|---|
| Proposed branch: | ~sw37th/cloud-init:fix_for_opennebula |
| Merge into: | cloud-init:master |
| Diff against target: |
501 lines (+241/-104) 4 files modified
cloudinit/net/__init__.py (+2/-2) cloudinit/sources/DataSourceOpenNebula.py (+59/-53) tests/unittests/test_datasource/test_opennebula.py (+177/-46) tests/unittests/test_net.py (+3/-3) |
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Scott Moser | 2017-12-14 | Needs Fixing on 2017-12-21 | |
| Server Team CI bot | continuous-integration | Needs Fixing on 2017-12-21 | |
|
Review via email:
|
|||
Commit Message
OpenNebula: Improve network configuration support.
Network configuration in OpenNebula would only work if the host correctly
guessed the names of the devices in the guest. OpenNebula provided data
in its context.sh like 'ETH0_NETWORK', but if the guest named devices
differently then results were not predictable. This would occur with
Predictable Network Interface Names. To address this,
newer versions (of OpenNebula provide the mac address ETH0_MAC.
This function is present in 4.14 and documented officially in 5.0 docs.
This provides support for reading the mac addresses from the context.sh.
It also fixes cases where context.sh provided a field (ETH0_NETWORK
or ETH0_MASK) with a empty string. Previously the empty string would
be used rather than falling back to the default.
| Scott Moser (smoser) wrote : | # |
http://
^ is an updated paste to support get_mask and get_network having empty strings.
- 72e6213... by Akihiko Ota on 2017-12-16
- 0abbfc5... by Akihiko Ota on 2017-12-18
| Akihiko Ota (sw37th) wrote : | # |
Scott,
> Some requests:
> a.) can you add some tests? there are tests at
> tests/unittests
I have added some tests. Please reviwe it.
> b.) here is some general cleanups i saw when reviewing. Just reduces some
> copy-and-paste. http://
I applied this patch. Thanks.
> c.) can you find and add information here on what versions of OpenNebula add
> the mac address fields?
The discription of ETHx_MAC field can be found on OpenNebula 5.0 document.
(http://
But it was already implemented at least in OpenNebula 4.14.
> d.) Do you happen to know if trunk was actually just completely broken for
> network config generation?
> e.) ultimately i'd like for OpenNebulaNetwo
> network config rather than returning ENI format and then converting.
>
> I wont push on 'd' now, but i'd like to have that in the future. I think its
> probably pretty easy to do, you'll a 'v1' or 'v2' as described in
> http://
> format-v1.html
> http://
> format-v2.html
I agree. It's better to modify gen_conf() to use 'v1' or 'v2' configuration.
> Also, I'll update your "commit message" here to give squashed description (we
> squash when we merge).
Thanks!
FAILED: Continuous integration, rev:
https:/
Executed test runs:
SUCCESS: Checkout
SUCCESS: Unit & Style Tests
SUCCESS: Ubuntu LTS: Build
SUCCESS: Ubuntu LTS: Integration
FAILED: MAAS Compatability Testing
Click here to trigger a rebuild:
https:/
| Scott Moser (smoser) wrote : | # |
Akihiko,
I looked to pull this in, but thought I'd add one more test for multiple nics.
Can you review and then integrate this patch?
https:/
thanks for your patience.
Scott
| Scott Moser (smoser) wrote : | # |
Also, this one *has* to be done, it is why the c-i bot complained.
http://
- 849b139... by Akihiko Ota on 2017-12-23
- 3cca981... by Akihiko Ota on 2017-12-23
| Akihiko Ota (sw37th) wrote : | # |
Scott,
That's fine! I have merged your patches. Thanks!
Unmerged commits
- 3cca981... by Akihiko Ota on 2017-12-23
- 849b139... by Akihiko Ota on 2017-12-23
- 0abbfc5... by Akihiko Ota on 2017-12-18
- 72e6213... by Akihiko Ota on 2017-12-16
- 73268b9... by Akihiko Ota on 2017-12-14
- be9b67e... by Akihiko Ota on 2017-12-13
- b022c4c... by Akihiko Ota on 2017-12-13
- eae1424... by Akihiko Ota on 2017-12-13


Akihko,
Hi, Thanks for your merge proposal and for your patience in sticking with it. I'm sorry you went largely ignored in those bugs.
Over all this looks good and you're clearly fixing things that were broken or unimplemented.
Please feel free to ping me in IRC if you need more immediate feedback.
Some requests: /test_datasourc e/test_ opennebula. py paste.ubuntu. com/26183949/
a.) can you add some tests? there are tests at tests/unittests
b.) here is some general cleanups i saw when reviewing. Just reduces some copy-and-paste. http://
c.) can you find and add information here on what versions of OpenNebula add the mac address fields?
d.) Do you happen to know if trunk was actually just completely broken for network config generation? rk.gen_ conf() to return the newer network config rather than returning ENI format and then converting.
e.) ultimately i'd like for OpenNebulaNetwo
I wont push on 'd' now, but i'd like to have that in the future. I think its probably pretty easy to do, you'll a 'v1' or 'v2' as described in cloudinit. readthedocs. io/en/latest/ topics/ network- config- format- v1.html cloudinit. readthedocs. io/en/latest/ topics/ network- config- format- v2.html
http://
http://
Also, I'll update your "commit message" here to give squashed description (we squash when we merge).
Thanks again!