cloud-init team mailing list archive

[Igor Galić <igalic@xxxxxxxxxxxxx>]

Hi folks,

i'm writing this so long as it's fresh to my memory, as we discussed it 
on Friday evening in IRC … despite the subject, I think it should be 
persisted somewhere ;)

I've been working on "porting" coudinit/net to freebsd:

- 
https://code.launchpad.net/~i.galic/cloud-init/+git/cloud-init/+merge/358228
- https://lists.launchpad.net/cloud-init/msg00185.html

so my problem is with class EphemeralIPv4Network
https://git.launchpad.net/cloud-init/tree/cloudinit/net/__init__.py#n650

"""Context manager which sets up temporary static network configuration.

No operations are performed if the provided interface is already 
connected.
If unconnected, bring up the interface with valid ip, prefix and 
broadcast.
If router is provided setup a default route for that interface. Upon
context exit, clean up the interface leaving no configuration behind.
"""

Let me emphasize, "No operations are performed if the provided 
interface is already connected."

However, this is currently not done, or at least not directly:

https://git.launchpad.net/cloud-init/tree/cloudinit/net/__init__.py#n684

   def __enter__(self):
       """Perform ephemeral network setup if interface is not 
connected."""
       self._bringup_device()
       if self.router:
           self._bringup_router()

it's only that somewhere in _bringup_device()'s exception path, that 
"nothing" is done.

Given this convoluted code, it's perhaps no wonder that 
EphemeralIPv4Network is currently suffering from this bug:

- https://bugs.launchpad.net/cloud-init/+bug/1802598


I think from an API point of view, it's important that "No operation is 
performed". Especially since this class implements __enter__() and 
__exit__()[1] — and is used by six different sources in this exact 
way.

Scott brought up the issue of performance, of wasted cycles thru 
repeated checking if the link is_up(). But i'm convinced that the 
benefit from a consistent API is far greater than a few wasted calls 
that are probably easily cached by a modern Kernel.

To summarize:

- EphemeralIPv4Network is currently not implemented to its own spec
- EphemeralIPv4Network has basic functionality bugs[2]

Fixing these issues would make porting cloudinit.net to non-linux 
platforms easier as well.

---

[1]: see https://docs.python.org/3/reference/compound_stmts.html#with 
for that
[2]: you could say it's not idempotent, but i would claim it's not 
functional