kubevirt / kubevirt

Kubernetes Virtualization API and runtime in order to define and manage virtual machines.
https://kubevirt.io
Apache License 2.0
5.45k stars 1.31k forks source link

spec.running in body is required when create VM from template #945

Closed gouyang closed 5 years ago

gouyang commented 6 years ago
# oc process -f vm-template-fedora.yaml -p NAME=Fedora27 | oc create -f -
The OfflineVirtualMachine "Fedora27" is invalid: []: Invalid value: map[string]interface {}{"apiVersion":"kubevirt.io/v1alpha1", "kind":"OfflineVirtualMachine", "metadata":map[string]interface {}{"creationTimestamp":"2018-04-24T06:51:24Z", "uid":"e74fe886-478b-11e8-bff4-44a842130b3e", "selfLink":"", "clusterName":"", "labels":map[string]interface {}{"kubevirt-ovm":"ovm-Fedora27"}, "name":"Fedora27", "namespace":"golden-images"}, "spec":map[string]interface {}{"template":map[string]interface {}{"spec":map[string]interface {}{"domain":map[string]interface {}{"resources":map[string]interface {}{"requests":map[string]interface {}{"memory":"4096Mi"}}, "cpu":map[string]interface {}{"cores":4}, "devices":map[string]interface {}{"disks":[]interface {}{map[string]interface {}{"volumeName":"registryvolume", "disk":map[string]interface {}{"bus":"virtio"}, "name":"disk0"}, map[string]interface {}{"disk":map[string]interface {}{"bus":"virtio"}, "name":"disk1", "volumeName":"cloudinitvolume"}}}}, "volumes":[]interface {}{map[string]interface {}{"name":"registryvolume", "registryDisk":map[string]interface {}{"image":"kubevirt/fedora-cloud-registry-disk-demo:devel"}}, map[string]interface {}{"name":"cloudinitvolume", "cloudInitNoCloud":map[string]interface {}{"userDataBase64":"I2Nsb3VkLWNvbmZpZwpwYXNzd29yZDogYXRvbWljCnNzaF9wd2F1dGg6IFRydWUKY2hwYXNzd2Q6IHsgZXhwaXJlOiBGYWxzZSB9Cg=="}}}}, "metadata":map[string]interface {}{"labels":map[string]interface {}{"kubevirt-ovm":"ovm-Fedora27"}}}}}: validation failure list:
spec.running in body is required

If I add to running: false to the spec section, it works then.

# git diff
diff --git a/cluster/vm-template-fedora.yaml b/cluster/vm-template-fedora.yaml
index 617089c..e53dd1f 100644
--- a/cluster/vm-template-fedora.yaml
+++ b/cluster/vm-template-fedora.yaml
@@ -17,6 +17,7 @@ objects:
     labels:
       kubevirt-ovm: ovm-${NAME}
   spec:
+    running: false
     template:
       metadata:
         labels:
davidvossel commented 6 years ago

Running has always been marked as a required field in our API. The only difference now is required fields are being enforced at creation time.

All our example ovms reflect this requirement.

petrkotas commented 6 years ago

The running is required field and have to be provided. If you do not add running:true|false than the ovm does not have any information how to proceed. We can discuss whether the ovm should do some defaulting to set default value to be running: false. @rmohr @fabiand what do you think?

fabiand commented 6 years ago

I'd say we can sanely default to running: false - it would just be a default like with any other field.

davidvossel commented 6 years ago

I'd say we can sanely default to running: false - it would just be a default like with any other field.

Defaulting running to "false" makes sense when the object is called a OfflineVirtualMachine. What about when we change the name to something like a StatefulVirtualMachine? I'd be confused if I posted a SVM and it didn't run.

The name OVM will change soon. I'd suggest leaving running as a required field until then. After we settle on a name, we can decide what the default should be for running.

petrkotas commented 6 years ago

I tend to agree with David. running: false for statefull VM is confusing. Having it required means, user have to decide what he wants to do with it.

itamarh commented 6 years ago

I agree the different names may be confusing. I don't agree with putting a burden of the user to have to decide as a simple way out for us (as a general rule of thinking, sometimes we have to)

davidvossel commented 6 years ago

I agree the different names may be confusing. I don't agree with putting a burden of the user to have to decide as a simple way out for us (as a general rule of thinking, sometimes we have to)

yep, I agree. We need a default. I suggest we hold off on making a decision about the default until after we are 100% committed to a name (ovm vs svm discussion) because the name impacts the default we use.