Add KubeVirt driver & tests #1394
Conversation
|
Thanks for the contribution. Since this is a larger contribution, can you please also work on signing an ICLA (https://libcloud.readthedocs.io/en/latest/development.html#contributing-bigger-changes)? |
| proxy_url = kwargs.pop('proxy_url', http_proxy_url_env) | ||
|
|
||
| self._setup_verify() | ||
| self._setup_ca_cert() | ||
|
|
||
| LibcloudBaseConnection.__init__(self) | ||
|
|
||
| self.session.timeout = kwargs.pop('timeout', 60) |
There was a problem hiding this comment.
Does this change the default requests timeout behavior?
If so, we should probably only set that attribute if timeout is explicitly specified (for backward compatibility reasons and to make sure it doesn't negatively affect long running and streaming connections)?
There was a problem hiding this comment.
EDIT: Never mind, I see you just moved this statement a couple of lines above, please ignore my comment.
| @@ -0,0 +1,996 @@ | |||
| """ | |||
There was a problem hiding this comment.
Please add a missing Apache 2.0 license header to the new Python files.
libcloud/compute/drivers/kubevirt.py
Outdated
|
|
||
| class KubeVirtNode(Node): | ||
|
|
||
| def start_node(self): |
There was a problem hiding this comment.
This was changed recently (#1375) so this custom node class shouldn't be needed anymore.
|
|
||
| if ca_cert: | ||
| self.connection.connection.ca_cert = ca_cert | ||
| else: |
There was a problem hiding this comment.
Since ca_cert is set to None by default it might not be a bad idea to emit a warning if ca cert is not used that server SSL certificate won't be validated against the CA cert.
And when the documentation is added, it would also be good to point that out there.
libcloud/compute/drivers/kubevirt.py
Outdated
| self.connection.secret = secret | ||
| self.connection.key = key | ||
|
|
||
| def list_nodes(self, namespace=None): |
There was a problem hiding this comment.
namespace argument is unique to this driver and not part of the standard API so please prefix it with ex_.
|
|
||
| return vms | ||
|
|
||
| def get_node(self, id=None, name=None): |
There was a problem hiding this comment.
This method is not part of the standard API so please prefix the method name with ex_.
libcloud/compute/drivers/kubevirt.py
Outdated
| except StopIteration: | ||
| raise ValueError("Node does not exist") | ||
|
|
||
| def ex_start_node(self, node): |
There was a problem hiding this comment.
This method name can now be changed to start_node() (see #1375 for details).
libcloud/compute/drivers/kubevirt.py
Outdated
| except Exception as exc: | ||
| raise | ||
|
|
||
| def ex_stop_node(self, node): |
There was a problem hiding this comment.
This method name can now be changed to stop_node() (see #1375 for details).
libcloud/compute/drivers/kubevirt.py
Outdated
| raise | ||
|
|
||
| # only has container disk support atm with no persistency | ||
| def create_node(self, **kwargs): |
There was a problem hiding this comment.
Please explicitly declare all the supported arguments (we just did **kwargs misuse cleanup in #1389).
libcloud/compute/drivers/kubevirt.py
Outdated
| this name and atm it cannot be changed after it is set. | ||
| :type name: ``str`` | ||
|
|
||
| :param namespace: The namespace where the VM will live. |
There was a problem hiding this comment.
All the arguments which are not part of the standard API should be prefixed with ex_ (namespace, memory, cpu, disks, network).
| :param size: An int of the ammount of gigabytes desired | ||
| :type size: `int` | ||
|
|
||
| :param namespace: The namespace where the claim will live |
There was a problem hiding this comment.
Please prefix non standard arguments with ex_ (namespace, storageClassName, accessMode, matchlabels).
libcloud/compute/drivers/kubevirt.py
Outdated
| except Exception as exc: | ||
| raise | ||
|
|
||
| def attach_volume(self, volume, ex_node, **kwargs): |
There was a problem hiding this comment.
Please make sure the method signature matches the base API one and that the non standard arguments are prefixed with ex_.
Base signature looks like this:
def attach_volume(self, node, volume, device=None):So node argument needs to be first, etc.
| except Exception as exc: | ||
| raise | ||
|
|
||
| def detach_volume(self, volume, ex_node): |
There was a problem hiding this comment.
Same here - base API method only takes volume argument. If node argument is also needed, it should be left as as is (prefixed with ex_ since it's indeed not a standard argument).
libcloud/compute/drivers/kubevirt.py
Outdated
| except Exception as exc: | ||
| raise | ||
|
|
||
| def list_persistent_volume_claims(self, namespace="default"): |
There was a problem hiding this comment.
Please prefix this method with ex_ since is not part of the standard API.
libcloud/compute/drivers/kubevirt.py
Outdated
| pvcs = [item['metadata']['name'] for item in result['items']] | ||
| return pvcs | ||
|
|
||
| def list_storage_classes(self): |
There was a problem hiding this comment.
Please prefix this method with ex_ since is not part of the standard API.
|
Thanks for the contribution and nice work 👍 I've added some comments, most of them are related to making sure the driver complies with the "standard" Libcloud compute API (aka making sure non standard arguments and method names are prefixed with |
|
@Eis-D-Z LXD driver has been merged, can you please also sync this branch with latest trunk and address the PR comments when you get a chance so this PR can be merged as well. Thanks. |
|
Hello, Kami. |
Eis-D-Z
force-pushed
the
kubevirt-driver
branch
from
d99a9ab
to
bb9f478
Compare
|
Fixed all the commented segments and did a wrong push which I undid subsequently. |
Codecov Report
@@ Coverage Diff @@
## trunk #1394 +/- ##
==========================================
- Coverage 86.46% 86.02% -0.45%
==========================================
Files 366 368 +2
Lines 76799 77449 +650
Branches 7529 7658 +129
==========================================
+ Hits 66404 66622 +218
- Misses 7527 7922 +395
- Partials 2868 2905 +37
Continue to review full report at Codecov.
|
KubeVirt driver
Description
Kubevirt driver with initial support for the k8s/KubeVirt add-on.
Can list, start, stop, destroy kubevirt type vm's from a kubernetes cluster.
Can create a Persistent Volume Claim with the assumption that the Persistent Volume will be created dynamically by Kubernetes, so a relevant storage class that allows for such provisioning must be declared. (You can list storage classes to view them)
Can create a vm with a docker image with embedded disk as image, and as many persistent volume claims as desired.
Status
Checklist (tick everything that applies)