Kami commented on code in PR #1983:
URL: https://github.com/apache/libcloud/pull/1983#discussion_r1644971865
##########
libcloud/compute/drivers/kubevirt.py:
##########
@@ -316,69 +360,333 @@ def create_node(
"resources": {"requests": {}, "limits": {}},
},
"networks": [],
- "terminationGracePeriodSeconds":
ex_termination_grace_period, # NOQA
+ "terminationGracePeriodSeconds": 0,
"volumes": [],
},
},
},
}
- memory = str(ex_memory) + "Mi"
-
vm["spec"]["template"]["spec"]["domain"]["resources"]["requests"]["memory"] =
memory
-
vm["spec"]["template"]["spec"]["domain"]["resources"]["limits"]["memory"] =
memory
- if ex_cpu < 10:
- cpu = int(ex_cpu)
-
vm["spec"]["template"]["spec"]["domain"]["resources"]["requests"]["cpu"] = cpu
+
+ @staticmethod
+ def _create_node_vm_from_ex_template(
+ name, ex_template, other_args
+ ): # type: (str, dict) -> dict
+ """
+ A part of create_node that deals with the VM template.
+ Returns the VM template with the name set.
+
+ :param name: A name to give the VM. The VM will be identified by
+ this name and atm it cannot be changed after it is set.
+ See also the name parameter in create_node.
+ :type name: ``str``
+
+ :param ex_template: A dictionary of kubernetes object that defines the
+ KubeVirt VM. See also the ex_template parameter in
create_node.
+ :type ex_template: ``dict`` with keys:
+ ``apiVersion: str``, ``kind: str``, ``metadata:
dict``
+ and ``spec: dict``
+
+ :param other_args: Other parameters passed to the create_node method.
+ This is used to warn the user about ignored
parameters.
+ See also the parameters of create_node.
+ :type other_args: ``dict``
+
+ :return: dict: The VM template with the name set.
+ """
+ assert isinstance(ex_template, dict), "ex_template must be a
dictionary"
+
+ other_params = {
+ "size": other_args.get("size"),
+ "image": other_args.get("image"),
+ "auth": other_args.get("auth"),
+ "ex_disks": other_args.get("ex_disks"),
+ "ex_network": other_args.get("ex_network"),
+ "ex_termination_grace_period":
other_args.get("ex_termination_grace_period"),
+ "ex_ports": other_args.get("ex_ports"),
+ }
+ ignored_non_none_param_keys = list(
+ filter(lambda x: other_params[x] is not None, other_params)
+ )
+ if ignored_non_none_param_keys:
+ warnings.warn(
+ "ex_template is provided, ignoring the following non-None "
+ "parameters: {}".format(ignored_non_none_param_keys)
+ )
+
+ vm = copy.deepcopy(ex_template)
+
+ if vm.get("metadata") is None:
+ vm["metadata"] = {}
+
+ if vm["metadata"].get("name") is None:
+ vm["metadata"]["name"] = name
+ elif vm["metadata"]["name"] != name:
+ warnings.warn(
+ "The name in the ex_template ({}) will be ignored. "
+ "The name provided in the arguments ({}) will be used.".format(
+ vm["metadata"]["name"], name
+ )
+ )
+ vm["metadata"]["name"] = name
+
+ return vm
+
+ @staticmethod
+ def _create_node_size(
+ vm, size=None, ex_cpu=None, ex_memory=None
+ ): # type: (dict, NodeSize, int, int) -> None
+ """
+ A part of create_node that deals with the size of the VM.
+ It will fill the vm with size information.
+
+ :param size: The size of the VM in terms of CPU and memory.
+ See also the size parameter in create_node.
+ :type size: ``NodeSize`` with
+
+ :param ex_cpu: The number of CPU cores to allocate to the VM.
+ See also the ex_cpu parameter in create_node.
+ :type ex_cpu: ``int`` or ``str``
+
+ :param ex_memory: The amount of memory to allocate to the VM in MiB.
+ See also the ex_memory parameter in create_node.
+ :type ex_memory: ``int``
+
+ :return: None
+ """
+ # size -> cpu and memory limits / requests
+
+ ex_memory_limit = ex_memory_request = ex_cpu_limit = ex_cpu_request =
None
+
+ if size is not None:
+ assert isinstance(size, NodeSize), "size must be a NodeSize"
+ ex_cpu_limit = size.extra["cpu"]
+ ex_memory_limit = size.ram
+ # optional resc requests: default = limit
+ ex_cpu_request = size.extra.get("cpu_request", None) or
ex_cpu_limit
+ ex_memory_request = size.extra.get("ram_request", None) or
ex_memory_limit
+
+ # memory
+
+ if ex_memory is not None: # legacy
+ ex_memory_limit = ex_memory
+ ex_memory_request = ex_memory
+
+ def _format_memory(memory_value): # type: (int) -> str
+ assert isinstance(memory_value, int), "memory must be an int in
MiB"
+ return str(memory_value) + "Mi"
+
+ if ex_memory_limit is not None:
+ memory = _format_memory(ex_memory_limit)
+
vm["spec"]["template"]["spec"]["domain"]["resources"]["limits"]["memory"] =
memory
+ if ex_memory_request is not None:
+ memory = _format_memory(ex_memory_request)
+
vm["spec"]["template"]["spec"]["domain"]["resources"]["requests"]["memory"] =
memory
+
+ # cpu
+
+ if ex_cpu is not None: # legacy
+ ex_cpu_limit = ex_cpu
+ ex_cpu_request = ex_cpu
+
+ def _format_cpu(cpu_value): # type: (Union[int, str]) -> Union[str,
float]
+ if isinstance(cpu_value, str) and cpu_value.endswith("m"):
+ return cpu_value
+ try:
+ return float(cpu_value)
+ except ValueError:
+ raise ValueError("cpu must be a number or a string ending with
'm'")
+
+ if ex_cpu_limit is not None:
+ cpu = _format_cpu(ex_cpu_limit)
vm["spec"]["template"]["spec"]["domain"]["resources"]["limits"]["cpu"] = cpu
- else:
- cpu = str(ex_cpu) + "m"
+ if ex_cpu_request is not None:
+ cpu = _format_cpu(ex_cpu_request)
vm["spec"]["template"]["spec"]["domain"]["resources"]["requests"]["cpu"] = cpu
-
vm["spec"]["template"]["spec"]["domain"]["resources"]["limits"]["cpu"] = cpu
- i = 0
- for disk in ex_disks:
+
+ @staticmethod
+ def _create_node_termination_grace_period(
+ vm, ex_termination_grace_period
+ ): # type: (dict, int) -> None
+ """
+ A part of create_node that deals with the termination grace period of
the VM.
+ It will fill the vm with the termination grace period information.
+
+ :param vm: The VM template to be filled.
+ :type vm: ``dict``
+ :param ex_termination_grace_period: The termination grace period of
the VM in seconds.
+ See also the
ex_termination_grace_period parameter in create_node.
+ :type ex_termination_grace_period: ``int``
+ :return: None
+ """
+ assert isinstance(
+ ex_termination_grace_period, int
+ ), "ex_termination_grace_period must be an int"
+
+ vm["spec"]["template"]["spec"][
+ "terminationGracePeriodSeconds"
+ ] = ex_termination_grace_period
+
+ @staticmethod
+ def _create_node_network(vm, ex_network, ex_ports): # type: (dict, dict,
dict) -> None
+ """
+ A part of create_node that deals with the network of the VM.
+ It will fill the vm with network information.
+
+ :param vm: The VM template to be filled.
+ :type vm: ``dict``
+ :param ex_network: The network configuration of the VM.
+ See also the ex_network parameter in create_node.
+ :type ex_network: ``dict``
+ :param ex_ports: The ports to expose in the VM.
+ See also the ex_ports parameter in create_node.
+ :type ex_ports: ``dict``
+ :return: None
+ """
+ # ex_network -> network and interface
+ if ex_network is not None:
+ try:
+ if isinstance(ex_network, dict):
+ interface = ex_network["interface"]
+ network_name = ex_network["name"]
+ network_type = ex_network["network_type"]
+ elif isinstance(ex_network, (tuple, list)): # legacy 3-tuple
+ network_type = ex_network[0]
+ interface = ex_network[1]
+ network_name = ex_network[2]
+ else:
+ raise KeyError("ex_network must be a dictionary or a
tuple/list")
+ except KeyError:
+ msg = (
+ "ex_network: You must provide a dictionary with keys: "
+ "'interface', 'name', 'network_type'."
+ )
+ raise KeyError(msg)
+ # add a default network
+ else:
+ interface = "masquerade"
+ network_name = "netw1"
+ network_type = "pod"
+
+ network_dict = {network_type: {}, "name": network_name}
+ interface_dict = {interface: {}, "name": network_name}
+
+ # ex_ports -> network.ports
+ ex_ports = ex_ports or {}
+ if ex_ports.get("ports_tcp"):
+ ports_to_expose = []
+ for port in ex_ports["ports_tcp"]:
+ ports_to_expose.append({"port": port, "protocol": "TCP"})
+ interface_dict[interface]["ports"] = ports_to_expose
+ if ex_ports.get("ports_udp"):
+ ports_to_expose = interface_dict[interface].get("ports", [])
+ for port in ex_ports.get("ports_udp"):
+ ports_to_expose.append({"port": port, "protocol": "UDP"})
+ interface_dict[interface]["ports"] = ports_to_expose
+
+ vm["spec"]["template"]["spec"]["networks"].append(network_dict)
+
vm["spec"]["template"]["spec"]["domain"]["devices"]["interfaces"].append(interface_dict)
+
+ @staticmethod
+ def _create_node_auth(vm, auth):
+ """
+ A part of create_node that deals with the authentication of the VM.
+ It will fill the vm with a cloud-init volume that injects the
authentication.
+
+ :param vm: The VM template to be filled.
+ :param auth: The authentication method for the VM.
+ See also the auth parameter in create_node.
+ :type auth: ``NodeAuthSSHKey`` or ``NodeAuthPassword``
+ :return: None
+ """
+ # auth requires cloud-init,
+ # and only one cloud-init volume is supported by kubevirt.
+ # So if both auth and cloud-init are provided, raise an error.
+ for volume in vm["spec"]["template"]["spec"]["volumes"]:
+ if "cloudInitNoCloud" in volume or "cloudInitConfigDrive" in
volume:
+ raise ValueError(
+ "Setting auth and cloudInit at the same time is not
supported."
+ "Use deploy_node() instead."
+ )
+
+ # cloud-init volume
+ cloud_init_volume = "auth-cloudinit-" + str(uuid.uuid4())
+ disk_dict = {"disk": {"bus": "virtio"}, "name": cloud_init_volume}
+ volume_dict = {
+ "name": cloud_init_volume,
+ "cloudInitNoCloud": {"userData": ""},
+ }
+
+ # cloud_init_config reference:
https://kubevirt.io/user-guide/virtual_machines/startup_scripts/#injecting-ssh-keys-with-cloud-inits-cloud-config
+ if isinstance(auth, NodeAuthSSHKey):
+ public_key = auth.pubkey
+ cloud_init_config = (
+ """#cloud-config\n""" """ssh_authorized_keys:\n""" """ -
{}\n"""
+ ).format(public_key)
+ elif isinstance(auth, NodeAuthPassword):
+ password = auth.password
+ cloud_init_config = (
+ """#cloud-config\n"""
+ """password: {}\n"""
Review Comment:
EDIT: I forgot we don't have a dependency on `pyyaml` yet so we would need
to add one which is not that great.
One option which would probably work is to just use `json.dumps()` on the
actual pubkey / password value to take care of the value escaping / quoting,
but we would need to verify it works correctly.
```python
In [4]: s = """#cloud-config\n""" """ssh_authorized_keys:\n""" """ - {}\n"""
In [5]: print(yaml.safe_load(s.format(json.dumps("key with \" quotes '
bar"))))
{'ssh_authorized_keys': ['key with " quotes \' bar']}
```
It looks like it should indeed do the trick, but more unit tests + testing
it with the actual cloud init would be better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
