Enhance `KubeVirtNodeDriver` Compute Driver

[cdfmlr]

Enhance KubeVirtNodeDriver Compute Driver

Description

This pull request brings several improvements to the create_node method and related functions within the KubeVirtNodeDriver (libcloud/compute/drivers/kubevirt.py).

Features added to KubeVirtNodeDriver.create_node:

• Improved compatibility with the base NodeDriver class:
  • Added support for the size: NodeSize parameter, while retaining legacy compatibility with ex_cpu and ex_memory.
  • Added support for the auth: NodeAuthSSHKey|NodeAuthPassword parameter.
• Support for deployment (deploy_node):
  • KubeVirtNodeDriver now supports node deployment automatically, benefiting from the above compatibility changes.
• Support for general volume (disks) types that KubeVirt supports:
  • (==Breaking change==) Modified the content of the ex_disks parameter to align with the related KubeVirt API, making it compatible with any volume types rather than hardcoded support for limited volume or disk types (previously only persistentVolumeClaim was supported).
• Added support for the ex_template parameter, allowing users to customize the entire Kubernetes object declaring the virtual machine. This is particularly useful for:
  • Supporting advanced configurations not covered by other parameters.
  • Facilitating the reuse of existing virtual machine configurations.

Fixes:

• _to_node: Improved the logic for parsing memory, eliminating crashes on virtual machines with more than 1 GiB RAM.
• create_node: (==Breaking change==) Renamed the parameter from ports to ex_ports.
• Addressed various bugs in the create_node method, which were eliminated during the refactor and implementation of new features.

Other Changes:

• libcloud/compute/drivers/kubevirt.py:
  • Added a KubeVirtNodeSize function to assist in constructing NodeSize instances for KubeVirtNodeDriver.
  • Introduced a KubeVirtNodeImage function to help construct NodeImage instances for KubeVirtNodeDriver.
  • Code reorganization: Moved DISK_TYPES out of the create_node and exported it, enabling users to access a list of supported disk types.
  • Updated docstrings for new features, adhering to sphinx grammar standards.
• libcloud/test/compute/test_kubevirt.py:
  • Introduced tests for the create_node method: This method was not tested previously.

Status

• done, ready for review

Checklist (tick everything that applies)

• [x] Code linting (required, can be done after the PR checks)
• [x] Documentation
• [x] Tests
• [ ] ICLA (required for bigger changes)

[Kami]

@cdfmlr Thanks for the contribution and good PR description.

I will have a look as soon as I get a chance. In the mean time, would you mind documenting breaking changes (disks, ports) in docs/upgrades_notes.rst?

[cdfmlr]

would you mind documenting breaking changes (disks, ports) in docs/upgrades_notes.rst?

Sure, I will add it recently.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 36.79245% with 134 lines in your changes are missing coverage. Please review.

Project coverage is 83.25%. Comparing base (6f1f83d) to head (0b07480).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## trunk #1983 +/- ## ========================================== - Coverage 83.26% 83.25% -0.00% ========================================== Files 353 353 Lines 81305 81445 +140 Branches 8565 8606 +41 ========================================== + Hits 67692 67807 +115 + Misses 10823 10814 -9 - Partials 2790 2824 +34 ``` | [Files](https://app.codecov.io/gh/apache/libcloud/pull/1983?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [libcloud/test/compute/test\_kubevirt.py](https://app.codecov.io/gh/apache/libcloud/pull/1983?src=pr&el=tree&filepath=libcloud%2Ftest%2Fcompute%2Ftest_kubevirt.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvdGVzdC9jb21wdXRlL3Rlc3Rfa3ViZXZpcnQucHk=) | `71.43% <80.00%> (+4.76%)` | :arrow_up: | | [libcloud/compute/drivers/kubevirt.py](https://app.codecov.io/gh/apache/libcloud/pull/1983?src=pr&el=tree&filepath=libcloud%2Fcompute%2Fdrivers%2Fkubevirt.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL2t1YmV2aXJ0LnB5) | `32.81% <32.29%> (+9.85%)` | :arrow_up: |

[Kami]

@cdfmlr Thanks.

I had a look again. It mostly looks good, but there are a couple of improvements which can be made:

• _create_node() functionality could be refactored into multiple smaller methods. Right now the method is very large and complex.
• Test case situation should be improved. There is a lot of code and conditional branches which are not not exercised / covered (https://app.codecov.io/gh/apache/libcloud/pull/1983?src=pr&el=tree&filepath=libcloud%2Fcompute%2Fdrivers%2Fkubevirt.py#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL2t1YmV2aXJ0LnB5, https://github.com/apache/libcloud/pull/1983#issuecomment-2080694604). Existing test cases exercise only a small amount of "happy" code paths. I know you didn't add all of that code, but since you updated / touched a lot of it, it would be great to also improve the test coverage.

[cdfmlr]

@Kami Thank you for the review. Indeed, the create_node() looks clumsy. I've actually started refactoring it, but recent higher priority tasks have delayed its completion.

Also, at the moment, I'm unable to add more test cases due to these priority. However, there are some existing test cases for _deep_merge_dict and _memory_in_MB that I believe were copied from our other projects. I plan to incorporate these tests soon.

Despite the problems with code readability and test coverage, we've been using this code in a production environment for several months. It has been performing well and handling a considerable number of situations that aren't covered by the test cases.

[Kami]

@cdfmlr Thanks.

Do you happen to have any ETA when you will be to add some more tests + refactor the code a bit? Since I'm planning to do a v3.9.0 release this week.

[cdfmlr]

@Kami sorry for my delay.

I have added more tests covering the helper functions (_deep_merge_dict and _memory_in_MB) and some typical unhappy code paths. The _create_node() method has been refactored into smaller functions as well. Meanwhile, a useful new feature to set cpu/mem requests and limits separately is introduced and tested.

[Kami]

@cdfmlr Thanks for adding those changes.

I added a couple of more comments. It's mostly a couple of small things, plus the potential security issue with possible YAML injection in case the pub key or password is supplied by the end user and not sanitized before being passed to the Libcloud code.

[Kami]

Merged into trunk. Thanks.