Kami commented on code in PR #1983:
URL: https://github.com/apache/libcloud/pull/1983#discussion_r1644953046


##########
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

Review Comment:
   It would be safer to do:
   
   ```python
   ex_memory_limit, ex_memory_request, ex_cpu_limit, ex_cpu_request = None, 
None, None, None
   ```
   
   Right now the code above works fine since we are defaulting to `None`, but 
if this ever changed to default to dictionary or a list this could have 
unintended side affects.



-- 
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

Reply via email to