WIP: Initial IPv6 support for esxi vmkernel execution and state modules #266
Conversation
There was a problem hiding this comment.
Loving these PRs/improvements!
Just got a chance to review this and it looks pretty good - obviously we want at least some unit tests to cover this, though integration tests wouldn't go amiss. As in my last PR I'm still working on being able to setup the better fixtures for being able to handle the integration tests. I'll let you know when I get those all sorted 👍
| existing_ipv6_addresses = vnic.spec.ip.ipV6Config.ipV6Address | ||
| # Use a shadow list of dicts so we can compare only ipAddress and prefixLength | ||
| final_ipv6_addresses_keys = [] | ||
| final_ipv6_addresses = [] | ||
| # Loop through already-configured addresses | ||
| for index, x in enumerate(existing_ipv6_addresses): | ||
| # We only operate on addresses that are manually configured | ||
| if x.origin == "manual": | ||
| y = { | ||
| "ipAddress": x.ipAddress, | ||
| "prefixLength": x.prefixLength, | ||
| } | ||
| z = existing_ipv6_addresses[index] | ||
| # By default we set all existing addresses to be removed. | ||
| # We'll delete them from the list (noop) later if we need to keep them | ||
| z.operation = "remove" | ||
| final_ipv6_addresses_keys.append(y) | ||
| final_ipv6_addresses.append(z) | ||
| # Loop through desired-state addresses | ||
| for x in desired_ipv6_addresses: | ||
| y = { | ||
| "ipAddress": x.ipAddress, | ||
| "prefixLength": x.prefixLength, | ||
| } | ||
| if y not in final_ipv6_addresses_keys: | ||
| # Add any addresses that are not already configured | ||
| final_ipv6_addresses_keys.append(y) | ||
| final_ipv6_addresses.append(x) | ||
| else: | ||
| index = final_ipv6_addresses_keys.index(y) | ||
| # Remove any addresses that are already configured (noop) | ||
| del final_ipv6_addresses[index] | ||
| vnic_config.ip.ipV6Config.ipV6Address = final_ipv6_addresses |
There was a problem hiding this comment.
suggestion we definitely want a test to cover this. It could probably just be a unit test (though an integration test would also be welcome).
It might also be clearer to re-write this using sets. Something to this effect:
manual_ipv6_keys = set({'ipAddress':addr.ipAddress, 'prefixLength': addr.prefixLength} for addr in vnic.spec.ip.ipV6Config.ipv6_address if addr.orgin = "manual")
desired_ipv6_keys = set({'ipAddress': addr.ipAddress, 'prefixLength', addr.prefixLength} for addr in desired_ipv6_addresses)
ipv6_to_remove = manual_ipv6_keys - desired_ipv6_keys
ipv6_to_add = desired_ipv6_keys - manual_ipv6_keys
final_ipv6_addresses = []
for addr in itertools.chain(existing_ipv6_addresses, desired_ipv6_addresses):
by_key = {'ipAddress': addr.ipAddress, 'prefixLength': addr.prefixLength}
if by_key in ipv6_to_remove:
addr.operation = "remove"
elif by_key not in ipv6_to_add:
continue
final_ipv6_addresses.append(addr)
A unit/functional test covering the existing proposed behavior would give confidence that this refactor would be correct (if you agree that it reads a bit cleaner, anyway)
|
@ggiesen, VMware has approved your signed contributor license agreement. |
Resolves #265