Conversation
slyon
added
the
community
slyon
left a comment
slyon
left a comment
There was a problem hiding this comment.
Thank you very much for your contribution to Netplan!
This looks mostly good to me! It is related to #89 and the discussion in LP#1743200.
Do you have experience with using netplan migrate? I haven't seen it widely used out in the wild. Furthermore, this command is hidden behind the ENABLE_TEST_COMMANDS env variable. I was actually pondering about dropping support for it altogether..
@Kristof0127 What do you think about my in-line comment? Do you want to improve upon this, or shall we merge it as-is? Both is fine for me!
| 'auto en1:1\niface en1:1 inet static\naddress 1.2.3.5/8', dry_run=True)[0] | ||
| self.assertEqual(_load_yaml(out), {'network': { | ||
| 'version': 2, | ||
| 'ethernets': {'en1': {'addresses': ["1.2.3.4/8", "1.2.3.5/8"]}}}}, out.decode()) |
There was a problem hiding this comment.
thought (non-blocking): I'm pondering if this should be translated to something like that instead:
version: 2
ethernets:
en1:
addresses:
- "1.2.3.4/8"
- "1.2.3.5/8":
label: "en1:1"
But this could also be done as a separate PR, as currently this is a targeted fix for a specific problem. The solution could be enhanced to retain more information via the label.
|
@slyon thank you for your review. I don't have much experience with netplan migrate. However, in my current project, using migrate was the easiest way to ensure backward compatibility with old interface files, and I think it's quite a powerful tool. In response to your in-line comment, I agree that including the label would be the comprehensive solution, but if I saw correctly using labels is only supported by the networkd backend. Unfortunately, I use NetworkManager as renderer, so using labels is not possible in my case. I think adding the label as well can be done in a later separate PR, if both backends can support this feature. |
2 participants
Description
Netplan migrate haven't supported old syntax network interface aliases yet (eg. eth1:1 alias for eth1). An if statement is added to migrate.py that checks if interface name contains ':', this case the iface will be renamed and handled as not the alias one. The expected output is a simple interface that has multiple addresses. A test case is also provided in cli_legacy.py with the following name: test_static_ipv4_alias.
Checklist
make checksuccessfully.make check-coverage). (for migrate.py)