From f9eac00881b56cb350c0aa445d07f1c0d3dbee07 Mon Sep 17 00:00:00 2001 From: Roy Golan Date: Fri, 3 Apr 2020 12:04:18 +0300 Subject: [PATCH 1/2] ovirt: bump terraform and cluster-api providers Signed-off-by: Roy Golan --- go.mod | 4 +- go.sum | 4 +- .../pkg/apis/ovirtprovider/v1beta1/types.go | 99 ++++++++++++++----- .../v1beta1/zz_generated.deepcopy.go | 40 ++++++++ vendor/modules.txt | 4 +- 5 files changed, 118 insertions(+), 33 deletions(-) diff --git a/go.mod b/go.mod index 2ab923fd9d0..d1c21fb3ad6 100644 --- a/go.mod +++ b/go.mod @@ -69,11 +69,11 @@ require ( github.com/openshift/cluster-api v0.0.0-20191129101638-b09907ac6668 github.com/openshift/cluster-api-provider-gcp v0.0.1-0.20200120152131-1b09fd9e7156 github.com/openshift/cluster-api-provider-libvirt v0.2.1-0.20191219173431-2336783d4603 - github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200128081049-840376ca5c09 + github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200504092944-27473ea1ae43 github.com/openshift/library-go v0.0.0-20200324092245-db2a8546af81 github.com/openshift/machine-api-operator v0.2.1-0.20200429102619-d36974451290 github.com/openshift/machine-config-operator v4.2.0-alpha.0.0.20190917115525-033375cbe820+incompatible - github.com/ovirt/go-ovirt v4.3.4+incompatible + github.com/ovirt/go-ovirt v0.0.0-20200428093010-9bcc4fd4e6c0 github.com/ovirt/terraform-provider-ovirt v0.4.3-0.20200406133650-74a154c1d861 github.com/packer-community/winrmcp v0.0.0-20180921211025-c76d91c1e7db // indirect github.com/pborman/uuid v1.2.0 diff --git a/go.sum b/go.sum index 3d80c6f0645..6fff522edae 100644 --- a/go.sum +++ b/go.sum @@ -1797,8 +1797,8 @@ github.com/openshift/cluster-api-provider-libvirt v0.2.1-0.20191219173431-233678 github.com/openshift/cluster-api-provider-libvirt v0.2.1-0.20191219173431-2336783d4603/go.mod h1:7pQ9Bzha+ug/5zd+0ufbDEcnn2OnNlPwRwYrzhXk4NM= github.com/openshift/cluster-api-provider-openstack v0.0.0-20200323110431-3311de91e078 h1:Irj9ROcWhbeH6t2DEUDIpdIJgSLBaXww6AP/FMCmGmw= github.com/openshift/cluster-api-provider-openstack v0.0.0-20200323110431-3311de91e078/go.mod h1:ntMRKZlv++TExGO4g2jgsVIaHKJt8kKe72BAvMPV5vA= -github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200128081049-840376ca5c09 h1:QJxGgIB7f5BqNPEZOCgV29NsDf1P439Bs3q0B5O3fP8= -github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200128081049-840376ca5c09/go.mod h1:NcvJT99IauPosghKCineBG8yswe9JjuddiWuvsqge64= +github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200504092944-27473ea1ae43 h1:JO7t5tJcLiE0gk7VrdzKrJAcOv73GirpUxH/OvrOVms= +github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200504092944-27473ea1ae43/go.mod h1:Vl/bvZulLw6PdUADIFWGfoTWH1O4L1B80eN7BtLYEuo= github.com/openshift/cluster-autoscaler-operator v0.0.0-20190521201101-62768a6ba480/go.mod h1:/XmV44Fh28Vo3Ye93qFrxAbcFJ/Uy+7LPD+jGjmfJYc= github.com/openshift/cluster-etcd-operator v0.0.0-alpha.0.0.20191025163650-5854b5c48ce4/go.mod h1:vcBAUefK8pQmTPQ3jlCemORXhnRnq3aTsfOSVeaWliY= github.com/openshift/cluster-version-operator v3.11.1-0.20190629164025-08cac1c02538+incompatible/go.mod h1:0BbpR1mrN0F2ZRae5N1XHcytmkvVPaeKgSQwRRBWugc= diff --git a/vendor/github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1/types.go b/vendor/github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1/types.go index 3fc67590d8b..df1c1f2d8e6 100644 --- a/vendor/github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1/types.go +++ b/vendor/github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1/types.go @@ -27,7 +27,7 @@ import ( // OvirtMachineProviderSpec is the type that will be embedded in a Machine.Spec.ProviderSpec field // for an Ovirt VM. It is used by the Ovirt machine actuator to create a single machine instance. type OvirtMachineProviderSpec struct { - metav1.TypeMeta `json:",inline"` + metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` // UserDataSecret contains a local reference to a secret that contains the @@ -43,13 +43,61 @@ type OvirtMachineProviderSpec struct { // Name is the VM name Name string `json:"name"` - // The VM template this instance will be created from + // The VM template this instance will be created from. TemplateName string `json:"template_name"` - // the oVirt cluster this VM instance belongs too + // the oVirt cluster this VM instance belongs too. ClusterId string `json:"cluster_id"` + // InstanceTypeId defines the VM instance type and overrides + // the hardware parameters of the created VM, including cpu and memory. + // If InstanceTypeId is passed, all memory and cpu variables will be ignored. + InstanceTypeId string `json:"instance_type_id,omitempty"` + + // CPU defines the VM CPU. + CPU *CPU `json:"cpu,omitempty"` + + // MemoryMB is the size of a VM's memory in MiBs. + MemoryMB int32 `json:"memory_mb,omitempty"` + + // OSDisk is the the root disk of the node. + OSDisk *Disk `json:"os_disk,omitempty"` + + // VMType defines the workload type the instance will + // be used for and this effects the instance parameters. + // One of "desktop, server, high_performance" + VMType string `json:"type,omitempty"` + + // NetworkInterfaces defines the list of the network interfaces of the VM. + // All network interfaces from the template are discarded and new ones will + // be created, unless the list is empty or nil + NetworkInterfaces []*NetworkInterface `json:"network_interfaces,omitempty"` +} + +// CPU defines the VM cpu, made of (Sockets * Cores * Threads) +type CPU struct { + // Sockets is the number of sockets for a VM. + // Total CPUs is (Sockets * Cores * Threads) + Sockets int32 `json:"sockets"` + + // Cores is the number of cores per socket. + // Total CPUs is (Sockets * Cores * Threads) + Cores int32 `json:"cores"` + // Thread is the number of thread per core. + // Total CPUs is (Sockets * Cores * Threads) + Threads int32 `json:"threads"` +} + +type Disk struct { + // SizeGB size of the bootable disk in GiB. + SizeGB int64 `json:"size_gb"` +} + +// NetworkInterface defines a VM network interface +type NetworkInterface struct { + // VNICProfileID the id of the vNic profile + VNICProfileID string `json:"vnic_profile_id"` } // +genclient @@ -76,9 +124,6 @@ type OvirtClusterProviderStatus struct { // CAPrivateKey is a PEM encoded PKCS1 CA PrivateKey for the control plane nodes. CAPrivateKey []byte - - // Network contains all information about the created OpenStack Network. - // It includes Subnets and Router. } // +genclient @@ -89,11 +134,11 @@ type OvirtMachineProviderStatus struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // InstanceID is the ID of the instance in GCP + // InstanceID is the ID of the instance in oVirt // +optional InstanceID *string `json:"instanceId,omitempty"` - // InstanceState is the provisioning state of the GCP Instance. + // InstanceState is the provisioning state of the oVirt Instance. // +optional InstanceState *string `json:"instanceState,omitempty"` @@ -107,29 +152,29 @@ type OvirtMachineProviderConditionType string // Valid conditions for an oVirt machine instance const ( - // MachineCreated indicates whether the machine has been created or not. If not, - // it should include a reason and message for the failure. - MachineCreated OvirtMachineProviderConditionType = "MachineCreated" + // MachineCreated indicates whether the machine has been created or not. If not, + // it should include a reason and message for the failure. + MachineCreated OvirtMachineProviderConditionType = "MachineCreated" ) // OvirtMachineProviderCondition is a condition in a OvirtMachineProviderStatus type OvirtMachineProviderCondition struct { - // Type is the type of the condition. - Type OvirtMachineProviderConditionType `json:"type"` - // Status is the status of the condition. - Status corev1.ConditionStatus `json:"status"` - // LastProbeTime is the last time we probed the condition. - // +optional - LastProbeTime metav1.Time `json:"lastProbeTime,omitempty"` - // LastTransitionTime is the last time the condition transitioned from one status to another. - // +optional - LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` - // Reason is a unique, one-word, CamelCase reason for the condition's last transition. - // +optional - Reason string `json:"reason,omitempty"` - // Message is a human-readable message indicating details about last transition. - // +optional - Message string `json:"message,omitempty"` + // Type is the type of the condition. + Type OvirtMachineProviderConditionType `json:"type"` + // Status is the status of the condition. + Status corev1.ConditionStatus `json:"status"` + // LastProbeTime is the last time we probed the condition. + // +optional + LastProbeTime metav1.Time `json:"lastProbeTime,omitempty"` + // LastTransitionTime is the last time the condition transitioned from one status to another. + // +optional + LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` + // Reason is a unique, one-word, CamelCase reason for the condition's last transition. + // +optional + Reason string `json:"reason,omitempty"` + // Message is a human-readable message indicating details about last transition. + // +optional + Message string `json:"message,omitempty"` } func init() { diff --git a/vendor/github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1/zz_generated.deepcopy.go b/vendor/github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1/zz_generated.deepcopy.go index 4d473624f72..6b3ad9ac351 100644 --- a/vendor/github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1/zz_generated.deepcopy.go +++ b/vendor/github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1/zz_generated.deepcopy.go @@ -14,6 +14,36 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CPU) DeepCopyInto(out *CPU) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CPU. +func (in *CPU) DeepCopy() *CPU { + if in == nil { + return nil + } + out := new(CPU) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Disk) DeepCopyInto(out *Disk) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Disk. +func (in *Disk) DeepCopy() *Disk { + if in == nil { + return nil + } + out := new(Disk) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OvirtClusterProviderSpec) DeepCopyInto(out *OvirtClusterProviderSpec) { *out = *in @@ -106,6 +136,16 @@ func (in *OvirtMachineProviderSpec) DeepCopyInto(out *OvirtMachineProviderSpec) *out = new(v1.LocalObjectReference) **out = **in } + if in.CPU != nil { + in, out := &in.CPU, &out.CPU + *out = new(CPU) + **out = **in + } + if in.OSDisk != nil { + in, out := &in.OSDisk, &out.OSDisk + *out = new(Disk) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OvirtMachineProviderSpec. diff --git a/vendor/modules.txt b/vendor/modules.txt index 39da8d17cd4..9ddab962155 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1032,7 +1032,7 @@ github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1 # github.com/openshift/cluster-api-provider-libvirt v0.2.1-0.20191219173431-2336783d4603 github.com/openshift/cluster-api-provider-libvirt/pkg/apis github.com/openshift/cluster-api-provider-libvirt/pkg/apis/libvirtproviderconfig/v1beta1 -# github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200128081049-840376ca5c09 +# github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200504092944-27473ea1ae43 github.com/openshift/cluster-api-provider-ovirt/pkg/apis github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1 # github.com/openshift/library-go v0.0.0-20200324092245-db2a8546af81 @@ -1042,7 +1042,7 @@ github.com/openshift/machine-api-operator/pkg/apis/vsphereprovider github.com/openshift/machine-api-operator/pkg/apis/vsphereprovider/v1beta1 # github.com/openshift/machine-config-operator v4.2.0-alpha.0.0.20190917115525-033375cbe820+incompatible => github.com/openshift/machine-config-operator v0.0.1-0.20200130220348-e5685c0cf530 github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1 -# github.com/ovirt/go-ovirt v4.3.4+incompatible => github.com/ovirt/go-ovirt v0.0.0-20200320082526-4e97a11ff083 +# github.com/ovirt/go-ovirt v0.0.0-20200428093010-9bcc4fd4e6c0 => github.com/ovirt/go-ovirt v0.0.0-20200320082526-4e97a11ff083 github.com/ovirt/go-ovirt # github.com/ovirt/terraform-provider-ovirt v0.4.3-0.20200406133650-74a154c1d861 github.com/ovirt/terraform-provider-ovirt/ovirt From 5d0b06afb924d7c69c44006c3d299cde8e93a6ad Mon Sep 17 00:00:00 2001 From: Roy Golan Date: Wed, 25 Mar 2020 17:59:58 +0200 Subject: [PATCH 2/2] ovirt: extend ovirt's MachinePool Those items are now part of MachinePool - instance type - cpu (cores, sockets) - memory - os disk size - vm type With those we have controll on the way create worker/nodes, and we have proper defaults set in the machine pool definitions, one for the pkg/asset/machines/ master.go and worker.go. Default master pool: cpus: 4 mem: 16 GiB os disk: 120 GB VM type: high_performance Default worker pool: cpus: 4 mem: 16 GiB os disk: 120 GB VM type: server A compute pool snippet from the install-config.yaml may look like that: ```yaml compute: - architecture: amd64 hyperthreading: Enabled name: worker platform: ovirt: cpu: cores: 8 sockets: 1 instanceTypeID: memoryMB: 65536 osDisk: sizeGB: 120 vmType: high_performance replicas: 3 ``` Terraform now uses those values, and all the defaults in tf files are now deleted in favour of the machine-config ones. To support `osDisk` size which is differnt than the template master/main.tf has this block: block_device { interface = "virtio_scsi" size = var.ovirt_master_os_disk_size_gb } If needed TF will extend the disk size of the VM. Cluster-api-provider-ovirt also extends the disk of a VM if the definition in the machine spec is differnt than the template. Signed-off-by: Roy Golan --- .../install.openshift.io_installconfigs.yaml | 138 ++++++++++++++++++ data/data/ovirt/main.tf | 25 ++-- data/data/ovirt/masters/main.tf | 23 ++- data/data/ovirt/masters/variables.tf | 32 +++- data/data/ovirt/template/main.tf | 3 - data/data/ovirt/template/variables.tf | 13 -- data/data/ovirt/variables-ovirt.tf | 35 +++-- docs/user/ovirt/customization.md | 107 ++++++++++++++ pkg/asset/cluster/tfvars.go | 17 ++- pkg/asset/installconfig/ovirt/validaton.go | 30 ++++ pkg/asset/machines/master.go | 4 + pkg/asset/machines/ovirt/machines.go | 26 +++- pkg/asset/machines/ovirt/machinesets.go | 2 +- pkg/asset/machines/worker.go | 19 ++- pkg/tfvars/ovirt/ovirt.go | 52 ++++--- pkg/types/ovirt/machinepool.go | 86 ++++++++++- pkg/types/ovirt/validation/machinepool.go | 57 +++++++- .../ovirt/validation/machinepool_test.go | 54 +++++++ pkg/types/validation/installconfig.go | 7 + pkg/types/validation/installconfig_test.go | 3 + pkg/types/validation/machinepools.go | 5 + 21 files changed, 650 insertions(+), 88 deletions(-) create mode 100644 docs/user/ovirt/customization.md diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 8cf046a6bec..2edebe1d632 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -208,6 +208,52 @@ spec: ovirt: description: Ovirt is the configuration used when installing on oVirt. + properties: + cpu: + description: CPU defines the VM CPU. + properties: + cores: + description: Cores is the number of cores per socket. + Total CPUs is (Sockets * Cores) + format: int32 + type: integer + sockets: + description: Sockets is the number of sockets for a + VM. Total CPUs is (Sockets * Cores) + format: int32 + type: integer + required: + - cores + - sockets + type: object + instanceTypeID: + description: InstanceTypeID defines the VM instance type + and overrides the hardware parameters of the created VM, + including cpu and memory. If InstanceTypeID is passed, + all memory and cpu variables will be ignored. + type: string + memoryMB: + description: MemoryMB is the size of a VM's memory in MiBs. + format: int32 + type: integer + osDisk: + description: OSDisk is the the root disk of the node. + properties: + sizeGB: + description: SizeGB size of the bootable disk in GiB. + format: int64 + type: integer + required: + - sizeGB + type: object + vmType: + description: VMType defines the workload type of the VM. + enum: + - "" + - desktop + - server + - high_performance + type: string type: object vsphere: description: VSphere is the configuration used when installing @@ -419,6 +465,52 @@ spec: ovirt: description: Ovirt is the configuration used when installing on oVirt. + properties: + cpu: + description: CPU defines the VM CPU. + properties: + cores: + description: Cores is the number of cores per socket. + Total CPUs is (Sockets * Cores) + format: int32 + type: integer + sockets: + description: Sockets is the number of sockets for a VM. + Total CPUs is (Sockets * Cores) + format: int32 + type: integer + required: + - cores + - sockets + type: object + instanceTypeID: + description: InstanceTypeID defines the VM instance type and + overrides the hardware parameters of the created VM, including + cpu and memory. If InstanceTypeID is passed, all memory + and cpu variables will be ignored. + type: string + memoryMB: + description: MemoryMB is the size of a VM's memory in MiBs. + format: int32 + type: integer + osDisk: + description: OSDisk is the the root disk of the node. + properties: + sizeGB: + description: SizeGB size of the bootable disk in GiB. + format: int64 + type: integer + required: + - sizeGB + type: object + vmType: + description: VMType defines the workload type of the VM. + enum: + - "" + - desktop + - server + - high_performance + type: string type: object vsphere: description: VSphere is the configuration used when installing @@ -1073,6 +1165,52 @@ spec: used when installing on ovirt for machine pools which do not define their own platform configuration. Default will set the image field to the latest RHCOS image. + properties: + cpu: + description: CPU defines the VM CPU. + properties: + cores: + description: Cores is the number of cores per socket. + Total CPUs is (Sockets * Cores) + format: int32 + type: integer + sockets: + description: Sockets is the number of sockets for a VM. + Total CPUs is (Sockets * Cores) + format: int32 + type: integer + required: + - cores + - sockets + type: object + instanceTypeID: + description: InstanceTypeID defines the VM instance type and + overrides the hardware parameters of the created VM, including + cpu and memory. If InstanceTypeID is passed, all memory + and cpu variables will be ignored. + type: string + memoryMB: + description: MemoryMB is the size of a VM's memory in MiBs. + format: int32 + type: integer + osDisk: + description: OSDisk is the the root disk of the node. + properties: + sizeGB: + description: SizeGB size of the bootable disk in GiB. + format: int64 + type: integer + required: + - sizeGB + type: object + vmType: + description: VMType defines the workload type of the VM. + enum: + - "" + - desktop + - server + - high_performance + type: string type: object dns_vip: description: DNSVIP is the IP of the internal DNS which will be diff --git a/data/data/ovirt/main.tf b/data/data/ovirt/main.tf index 1e024e0d219..1355f9812c2 100644 --- a/data/data/ovirt/main.tf +++ b/data/data/ovirt/main.tf @@ -12,9 +12,6 @@ module "template" { cluster_id = var.cluster_id openstack_base_image_name = var.openstack_base_image_name openstack_base_image_local_file_path = var.openstack_base_image_local_file_path - ovirt_template_cpu = var.ovirt_template_cpu - ovirt_template_mem = var.ovirt_template_mem - disk_size_gib = var.ovirt_template_disk_size_gib ovirt_network_name = var.ovirt_network_name ovirt_vnic_profile_id = var.ovirt_vnic_profile_id } @@ -30,13 +27,17 @@ module "bootstrap" { } module "masters" { - source = "./masters" - master_count = var.master_count - ovirt_cluster_id = var.ovirt_cluster_id - ovirt_template_id = module.template.releaseimage_template_id - ignition_master = var.ignition_master - cluster_domain = var.cluster_domain - cluster_id = var.cluster_id - ovirt_master_cpu = var.ovirt_master_cpu - ovirt_master_mem = var.ovirt_master_mem + source = "./masters" + master_count = var.master_count + ovirt_cluster_id = var.ovirt_cluster_id + ovirt_template_id = module.template.releaseimage_template_id + ignition_master = var.ignition_master + cluster_domain = var.cluster_domain + cluster_id = var.cluster_id + ovirt_master_instance_type_id = var.ovirt_master_instance_type_id + ovirt_master_cores = var.ovirt_master_cores + ovirt_master_sockets = var.ovirt_master_sockets + ovirt_master_memory = var.ovirt_master_memory + ovirt_master_vm_type = var.ovirt_master_vm_type + ovirt_master_os_disk_size_gb = var.ovirt_master_os_disk_gb } diff --git a/data/data/ovirt/masters/main.tf b/data/data/ovirt/masters/main.tf index 066533538f1..20bd8e18c17 100644 --- a/data/data/ovirt/masters/main.tf +++ b/data/data/ovirt/masters/main.tf @@ -1,15 +1,26 @@ resource "ovirt_vm" "master" { - count = var.master_count - name = "${var.cluster_id}-master-${count.index}" - cluster_id = var.ovirt_cluster_id - template_id = var.ovirt_template_id - cores = var.ovirt_master_cpu - memory = var.ovirt_master_mem + count = var.master_count + name = "${var.cluster_id}-master-${count.index}" + cluster_id = var.ovirt_cluster_id + template_id = var.ovirt_template_id + instance_type_id = var.ovirt_master_instance_type_id + type = var.ovirt_master_vm_type + cores = var.ovirt_master_cores + sockets = var.ovirt_master_sockets + // if instance type is declared then memory is redundant. Since terraform + // doesn't allow to condionally omit it, it must be passed. + // The number passed is multiplied by 4 and becomes the maximum memory the VM can have. + memory = var.ovirt_master_instance_type_id != "" ? 16348 : var.ovirt_master_memory initialization { host_name = "${var.cluster_id}-master-${count.index}" custom_script = var.ignition_master } + + block_device { + interface = "virtio_scsi" + size = var.ovirt_master_os_disk_size_gb + } } resource "ovirt_tag" "cluster_tag" { diff --git a/data/data/ovirt/masters/variables.tf b/data/data/ovirt/masters/variables.tf index f093c22c673..e942a76bb20 100644 --- a/data/data/ovirt/masters/variables.tf +++ b/data/data/ovirt/masters/variables.tf @@ -27,10 +27,32 @@ variable "ignition_master" { description = "master ignition config" } -variable "ovirt_master_mem" { - type = string +variable "ovirt_master_memory" { + type = string + description = "master VM memory in MiB" +} + +variable "ovirt_master_cores" { + type = string + description = "master VM number of cores" } -variable "ovirt_master_cpu" { - type = string -} \ No newline at end of file +variable "ovirt_master_sockets" { + type = string + description = "master VM number of sockets" +} + +variable "ovirt_master_os_disk_size_gb" { + type = string + description = "master VM disk size in GiB" +} + +variable "ovirt_master_vm_type" { + type = string + description = "master VM type" +} + +variable "ovirt_master_instance_type_id" { + type = string + description = "master VM instance type ID" +} diff --git a/data/data/ovirt/template/main.tf b/data/data/ovirt/template/main.tf index 266d3ea87ac..9ffec5cf564 100644 --- a/data/data/ovirt/template/main.tf +++ b/data/data/ovirt/template/main.tf @@ -45,8 +45,6 @@ resource "ovirt_vm" "tmp_import_vm" { count = length(local.existing_id) == 0 ? 1 : 0 name = "tmpvm-for-${ovirt_image_transfer.releaseimage.0.alias}" cluster_id = var.ovirt_cluster_id - cores = var.ovirt_template_cpu - memory = var.ovirt_template_mem block_device { disk_id = ovirt_image_transfer.releaseimage.0.disk_id interface = "virtio_scsi" @@ -70,7 +68,6 @@ resource "ovirt_template" "releaseimage_template" { // create from vm vm_id = ovirt_vm.tmp_import_vm.0.id depends_on = [ovirt_vm.tmp_import_vm] - cores = var.ovirt_template_cpu } // finally get the template by name(should be unique), fail if it doesn't exist diff --git a/data/data/ovirt/template/variables.tf b/data/data/ovirt/template/variables.tf index 3e3c50e200d..39e6beb4d0d 100644 --- a/data/data/ovirt/template/variables.tf +++ b/data/data/ovirt/template/variables.tf @@ -37,16 +37,3 @@ variable "ovirt_vnic_profile_id" { type = string description = "The ID of the vnic profile of ovirt's logical network." } - -variable "ovirt_template_mem" { - type = string -} - -variable "ovirt_template_cpu" { - type = string -} - -variable "disk_size_gib" { - type = number - description = "The size of the template disk for worker/nodes in GiB." -} \ No newline at end of file diff --git a/data/data/ovirt/variables-ovirt.tf b/data/data/ovirt/variables-ovirt.tf index c0c49144d13..1be951d7ef7 100644 --- a/data/data/ovirt/variables-ovirt.tf +++ b/data/data/ovirt/variables-ovirt.tf @@ -51,27 +51,32 @@ variable "ovirt_vnic_profile_id" { description = "The ID of the vnic profile of ovirt's logical network." } -variable "ovirt_master_mem" { - type = string - default = "8192" +variable "ovirt_master_memory" { + type = string + description = "master VM memory in MiB" +} + +variable "ovirt_master_cores" { + type = string + description = "master VM number of cores" } -variable "ovirt_master_cpu" { - type = number - default = 4 +variable "ovirt_master_sockets" { + type = string + description = "master VM number of sockets" } -variable "ovirt_template_mem" { - type = string - default = "16384" +variable "ovirt_master_os_disk_gb" { + type = string + description = "master VM disk size in GiB" } -variable "ovirt_template_cpu" { - type = number - default = 4 +variable "ovirt_master_vm_type" { + type = string + description = "master VM type" } -variable "ovirt_template_disk_size_gib" { - type = number - default = 25 +variable "ovirt_master_instance_type_id" { + type = string + description = "master VM instance type ID" } diff --git a/docs/user/ovirt/customization.md b/docs/user/ovirt/customization.md new file mode 100644 index 00000000000..904433e599d --- /dev/null +++ b/docs/user/ovirt/customization.md @@ -0,0 +1,107 @@ +# oVirt Platform Customization + +Beyond the [platform-agnostic `install-config.yaml` properties](../customization.md#platform-customization), the installer supports additional, ovirt-specific properties. + +## Cluster-scoped properties + +* `ovirt_cluster_id` (required string): The oVirt cluster where the VMs will be created. +* `ovirt_storage_domain_id` (required string): The storage domain ID where the VM disks will be created. +* `ovirt_network_name` (required string): The network name where the VM nics will be created. +* `vnicProfileID` (required string): The ID of the [vNic profile][vnic-profile] used for the VM network interfaces. + This can be inferred if the cluster network has a single profile. +* `api_vip` (required string): An IP address on the machineNetwork that will be assigned to the API VIP. +* `dns_vip` (required string): An IP address on the machineNetwork that will be assigned to the DNS VIP. +* `ingress_vip` (required string): An IP address on the machineNetwork that will be assigned to the Ingress VIP. + +## Machine pools + +* `cpu` (optional object): Defines the CPU of the VM. + * `cores` (required integer): The number of cores. Total vCPUS is cores * sockets. + * `sockets` (required integer): The number of sockets. Total vCPUS is cores * sockets. +* `memoryMB` (optional integer): Memory of the VM in MiB. +* `instanceTypeID` (optional string): The VM [instance-type][instance-type]. +* `osDisk` (optional string): Defines the first and bootable disk of the VM. + * `sizeGB` (required number): Size of the disk in GiB. +* `vmType` (optional string): The VM workload type. One of [high-performance][high-perf], server or desktop. + + +## Installing to Existing VPC & Subnetworks + +The installer can use an existing VPC and subnets when provisioning an OpenShift cluster. A VPC will be inferred from the provided subnets. For a standard installation, a private and public subnet should be specified. ([see example below](#pre-existing-vpc--subnets)). Both of the subnets must be within the IP range specified in `networking.machineNetwork`. + +## Examples + +Some example `install-config.yaml` are shown below. +For examples of platform-agnostic configuration fragments, see [here](../customization.md#examples). + +### Minimal + +An example minimal oVirt install config is: + +```yaml +apiVersion: v1 +baseDomain: example.com +metadata: + name: test-cluster +platform: + ovirt: + api_vip: 10.46.8.230 + dns_vip: 10.46.8.231 + ingress_vip: 10.46.8.232 + ovirt_cluster_id: 68833f9f-e89c-4891-b768-e2ba0815b76b + ovirt_storage_domain_id: ed7b0f4e-0e96-492a-8fff-279213ee1468 + ovirt_network_name: ovirtmgmt + vnicProfileID: 3fa86930-0be5-4052-b667-b79f0a729692 +pullSecret: '{"auths": ...}' +sshKey: ssh-ed25519 AAAA... +``` + +### Custom machine pools + +An example oVirt install config with custom machine pools: + +```yaml +apiVersion: v1 +baseDomain: example.com +controlPlane: + name: master + platform: + ovirt: + cpu: + cores: 4 + sockets: 2 + memoryMB: 65536 + osDisk: + sizeGB: 100 + vmType: high_performance + replicas: 3 +compute: +- name: worker + platform: + ovirt: + cpu: + cores: 4 + sockets: 4 + memoryMB: 65536 + osDisk: + sizeGB: 200 + vmType: high_performance + replicas: 5 +metadata: + name: test-cluster +platform: + ovirt: + api_vip: 10.46.8.230 + dns_vip: 10.46.8.231 + ingress_vip: 10.46.8.232 + ovirt_cluster_id: 68833f9f-e89c-4891-b768-e2ba0815b76b + ovirt_storage_domain_id: ed7b0f4e-0e96-492a-8fff-279213ee1468 + ovirt_network_name: ovirtmgmt + vnicProfileID: 3fa86930-0be5-4052-b667-b79f0a729692 +pullSecret: '{"auths": ...}' +sshKey: ssh-ed25519 AAAA... +``` + +[instance-type]: https://www.ovirt.org/develop/release-management/features/virt/instance-types.html +[vnic-profile]: https://www.ovirt.org/develop/release-management/features/sla/vnic-profiles.html +[high-perf]: https://www.ovirt.org/develop/release-management/features/virt/high-performance-vm.html diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 31a0a435f34..21da9eba7f6 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -14,6 +14,7 @@ import ( igntypes "github.com/coreos/ignition/config/v2_2/types" gcpprovider "github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1" libvirtprovider "github.com/openshift/cluster-api-provider-libvirt/pkg/apis/libvirtproviderconfig/v1beta1" + ovirtprovider "github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1" vsphereprovider "github.com/openshift/machine-api-operator/pkg/apis/vsphereprovider/v1beta1" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -453,17 +454,25 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { installConfig.Config.Platform.Ovirt.VNICProfileID = profiles[0].MustId() } + masters, err := mastersAsset.Machines() + if err != nil { + return err + } + data, err := ovirttfvars.TFVars( - config.URL, - config.Username, - config.Password, - config.CAFile, + ovirttfvars.Auth{ + URL: config.URL, + Username: config.Username, + Password: config.Password, + Cafile: config.CAFile, + }, installConfig.Config.Platform.Ovirt.ClusterID, installConfig.Config.Platform.Ovirt.StorageDomainID, installConfig.Config.Platform.Ovirt.NetworkName, installConfig.Config.Platform.Ovirt.VNICProfileID, string(*rhcosImage), clusterID.InfraID, + masters[0].Spec.ProviderSpec.Value.Object.(*ovirtprovider.OvirtMachineProviderSpec), ) if err != nil { return errors.Wrapf(err, "failed to get %s Terraform variables", platform) diff --git a/pkg/asset/installconfig/ovirt/validaton.go b/pkg/asset/installconfig/ovirt/validaton.go index 1dcd192410d..969078b7b29 100644 --- a/pkg/asset/installconfig/ovirt/validaton.go +++ b/pkg/asset/installconfig/ovirt/validaton.go @@ -40,9 +40,39 @@ func Validate(ic *types.InstallConfig) error { field.Invalid(ovirtPlatformPath.Child("vnicProfileID"), ic.Ovirt.VNICProfileID, err.Error())) } + if ic.ControlPlane != nil && ic.ControlPlane.Platform.Ovirt != nil { + allErrs = append( + allErrs, + validateMachinePool(con, field.NewPath("controlPlane", "platform", "ovirt"), ic.ControlPlane.Platform.Ovirt)...) + } + for idx, compute := range ic.Compute { + fldPath := field.NewPath("compute").Index(idx) + if compute.Platform.Ovirt != nil { + allErrs = append( + allErrs, + validateMachinePool(con, fldPath.Child("platform", "ovirt"), compute.Platform.Ovirt)...) + } + } + return allErrs.ToAggregate() } +func validateMachinePool(con *ovirtsdk.Connection, child *field.Path, pool *ovirt.MachinePool) field.ErrorList { + allErrs := field.ErrorList{} + allErrs = append(allErrs, validateInstanceTypeID(con, child, pool)) + return allErrs +} + +func validateInstanceTypeID(con *ovirtsdk.Connection, child *field.Path, machinePool *ovirt.MachinePool) *field.Error { + if machinePool.InstanceTypeID != "" { + _, err := con.SystemService().InstanceTypesService().InstanceTypeService(machinePool.InstanceTypeID).Get().Send() + if err != nil { + return field.NotFound(child.Child("instanceTypeID"), machinePool.InstanceTypeID) + } + } + return nil +} + // authenticated takes an ovirt platform and validates // its connection to the API by establishing // the connection and authenticating successfully. diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index b5c9de2f96f..a35e6299fd5 100644 --- a/pkg/asset/machines/master.go +++ b/pkg/asset/machines/master.go @@ -319,7 +319,11 @@ func (m *Master) Generate(dependencies asset.Parents) error { } case ovirttypes.Name: mpool := defaultOvirtMachinePoolPlatform() + mpool.VMType = ovirttypes.VMTypeHighPerformance + mpool.Set(ic.Platform.Ovirt.DefaultMachinePlatform) + mpool.Set(pool.Platform.Ovirt) pool.Platform.Ovirt = &mpool + imageName, _ := rhcosutils.GenerateOpenStackImageName(string(*rhcosImage), clusterID.InfraID) machines, err = ovirt.Machines(clusterID.InfraID, ic, pool, imageName, "master", "master-user-data") diff --git a/pkg/asset/machines/ovirt/machines.go b/pkg/asset/machines/ovirt/machines.go index bb67d61e741..b768f32f02d 100644 --- a/pkg/asset/machines/ovirt/machines.go +++ b/pkg/asset/machines/ovirt/machines.go @@ -29,7 +29,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine if pool.Replicas != nil { total = *pool.Replicas } - provider := provider(platform, userDataSecret, osImage) + provider := provider(platform, pool, userDataSecret, osImage) var machines []machineapi.Machine for idx := int64(0); idx < total; idx++ { machine := machineapi.Machine{ @@ -59,15 +59,31 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine return machines, nil } -func provider(platform *ovirt.Platform, userDataSecret string, osImage string) *ovirtprovider.OvirtMachineProviderSpec { - return &ovirtprovider.OvirtMachineProviderSpec{ +func provider(platform *ovirt.Platform, pool *types.MachinePool, userDataSecret string, osImage string) *ovirtprovider.OvirtMachineProviderSpec { + spec := ovirtprovider.OvirtMachineProviderSpec{ TypeMeta: metav1.TypeMeta{ - APIVersion: "ovirtproviderconfig.openshift.io/v1beta1", + APIVersion: "ovirtproviderconfig.machine.openshift.io/v1beta1", Kind: "OvirtMachineProviderSpec", }, UserDataSecret: &corev1.LocalObjectReference{Name: userDataSecret}, CredentialsSecret: &corev1.LocalObjectReference{Name: "ovirt-credentials"}, - ClusterId: platform.ClusterID, TemplateName: osImage, + ClusterId: platform.ClusterID, + InstanceTypeId: pool.Platform.Ovirt.InstanceTypeID, + MemoryMB: pool.Platform.Ovirt.MemoryMB, + VMType: string(pool.Platform.Ovirt.VMType), + } + if pool.Platform.Ovirt.CPU != nil { + spec.CPU = &ovirtprovider.CPU{ + Cores: pool.Platform.Ovirt.CPU.Cores, + Sockets: pool.Platform.Ovirt.CPU.Sockets, + Threads: 1, + } + } + if pool.Platform.Ovirt.OSDisk != nil { + spec.OSDisk = &ovirtprovider.Disk{ + SizeGB: pool.Platform.Ovirt.OSDisk.SizeGB, + } } + return &spec } diff --git a/pkg/asset/machines/ovirt/machinesets.go b/pkg/asset/machines/ovirt/machinesets.go index ac70a37ebaa..df5f6513afd 100644 --- a/pkg/asset/machines/ovirt/machinesets.go +++ b/pkg/asset/machines/ovirt/machinesets.go @@ -30,7 +30,7 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach total = *pool.Replicas } - provider := provider(platform, userDataSecret, osImage) + provider := provider(platform, pool, userDataSecret, osImage) name := fmt.Sprintf("%s-%s-%d", clusterID, pool.Name, 0) mset := &machineapi.MachineSet{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index b4a4f9d4a6b..e21469369b8 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -109,7 +109,17 @@ func defaultBareMetalMachinePoolPlatform() baremetaltypes.MachinePool { } func defaultOvirtMachinePoolPlatform() ovirttypes.MachinePool { - return ovirttypes.MachinePool{} + return ovirttypes.MachinePool{ + CPU: &ovirttypes.CPU{ + Cores: 4, + Sockets: 1, + }, + MemoryMB: 16348, + OSDisk: &ovirttypes.Disk{ + SizeGB: 120, + }, + VMType: ovirttypes.VMTypeServer, + } } func defaultVSphereMachinePoolPlatform() vspheretypes.MachinePool { @@ -340,8 +350,13 @@ func (w *Worker) Generate(dependencies asset.Parents) error { machineSets = append(machineSets, set) } case ovirttypes.Name: - pool.Platform.Ovirt = &ovirttypes.MachinePool{} + mpool := defaultOvirtMachinePoolPlatform() + mpool.Set(ic.Platform.Ovirt.DefaultMachinePlatform) + mpool.Set(pool.Platform.Ovirt) + pool.Platform.Ovirt = &mpool + imageName, _ := rhcosutils.GenerateOpenStackImageName(string(*rhcosImage), clusterID.InfraID) + sets, err := ovirt.MachineSets(clusterID.InfraID, ic, &pool, imageName, "worker", "worker-user-data") if err != nil { return errors.Wrap(err, "failed to create worker machine objects for ovirt provider") diff --git a/pkg/tfvars/ovirt/ovirt.go b/pkg/tfvars/ovirt/ovirt.go index 00ff791e7be..efc5cebc666 100644 --- a/pkg/tfvars/ovirt/ovirt.go +++ b/pkg/tfvars/ovirt/ovirt.go @@ -4,46 +4,62 @@ package ovirt import ( "encoding/json" + "github.com/openshift/cluster-api-provider-ovirt/pkg/apis/ovirtprovider/v1beta1" + "github.com/openshift/installer/pkg/rhcos" "github.com/openshift/installer/pkg/tfvars/internal/cache" ) +// Auth is the collection of credentials that will be used by terrform. +type Auth struct { + URL string `json:"ovirt_url"` + Username string `json:"ovirt_username"` + Password string `json:"ovirt_password"` + Cafile string `json:"ovirt_cafile,omitempty"` +} + type config struct { - URL string `json:"ovirt_url"` - Username string `json:"ovirt_username"` - Password string `json:"ovirt_password"` - Cafile string `json:"ovirt_cafile,omitempty"` + Auth `json:",inline"` ClusterID string `json:"ovirt_cluster_id"` StorageDomainID string `json:"ovirt_storage_domain_id"` NetworkName string `json:"ovirt_network_name,omitempty"` VNICProfileID string `json:"ovirt_vnic_profile_id,omitempty"` BaseImageName string `json:"openstack_base_image_name,omitempty"` BaseImageLocalFilePath string `json:"openstack_base_image_local_file_path,omitempty"` + MasterInstanceTypeID string `json:"ovirt_master_instance_type_id"` + MasterVMType string `json:"ovirt_master_vm_type,omitempty"` + MasterMemory int32 `json:"ovirt_master_memory"` + MasterCores int32 `json:"ovirt_master_cores"` + MasterSockets int32 `json:"ovirt_master_sockets"` + MasterOsDiskGB int64 `json:"ovirt_master_os_disk_gb"` } // TFVars generates ovirt-specific Terraform variables. func TFVars( - engineURL string, - engineUser string, - enginePass string, - engineCafile string, + auth Auth, clusterID string, stoarageDomainID string, networkName string, vnicProfileID string, baseImage string, - infraID string) ([]byte, error) { + infraID string, + masterSpec *v1beta1.OvirtMachineProviderSpec) ([]byte, error) { cfg := config{ - URL: engineURL, - Username: engineUser, - Password: enginePass, - Cafile: engineCafile, - ClusterID: clusterID, - StorageDomainID: stoarageDomainID, - NetworkName: networkName, - VNICProfileID: vnicProfileID, - BaseImageName: baseImage, + Auth: auth, + ClusterID: clusterID, + StorageDomainID: stoarageDomainID, + NetworkName: networkName, + VNICProfileID: vnicProfileID, + BaseImageName: baseImage, + MasterInstanceTypeID: masterSpec.InstanceTypeId, + MasterVMType: masterSpec.VMType, + MasterOsDiskGB: masterSpec.OSDisk.SizeGB, + MasterMemory: masterSpec.MemoryMB, + } + if masterSpec.CPU != nil { + cfg.MasterCores = masterSpec.CPU.Cores + cfg.MasterSockets = masterSpec.CPU.Sockets } imageName, isURL := rhcos.GenerateOpenStackImageName(baseImage, infraID) diff --git a/pkg/types/ovirt/machinepool.go b/pkg/types/ovirt/machinepool.go index 2775094e00f..16a87aef450 100644 --- a/pkg/types/ovirt/machinepool.go +++ b/pkg/types/ovirt/machinepool.go @@ -3,11 +3,91 @@ package ovirt // MachinePool stores the configuration for a machine pool installed // on ovirt. type MachinePool struct { + // InstanceTypeID defines the VM instance type and overrides + // the hardware parameters of the created VM, including cpu and memory. + // If InstanceTypeID is passed, all memory and cpu variables will be ignored. + InstanceTypeID string `json:"instanceTypeID,omitempty"` + + // CPU defines the VM CPU. + // +optional + CPU *CPU `json:"cpu,omitempty"` + + // MemoryMB is the size of a VM's memory in MiBs. + // +optional + MemoryMB int32 `json:"memoryMB,omitempty"` + + // OSDisk is the the root disk of the node. + // +optional + OSDisk *Disk `json:"osDisk,omitempty"` + + // VMType defines the workload type of the VM. + // +kubebuilder:validation:Enum="";desktop;server;high_performance + // +optional + VMType VMType `json:"vmType,omitempty"` +} + +// Disk defines a VM disk +type Disk struct { + // SizeGB size of the bootable disk in GiB. + SizeGB int64 `json:"sizeGB"` } -// Set sets the values from `required` to `a`. -func (l *MachinePool) Set(required *MachinePool) { - if required == nil || l == nil { +// CPU defines the VM cpu, made of (Sockets * Cores). +type CPU struct { + // Sockets is the number of sockets for a VM. + // Total CPUs is (Sockets * Cores) + Sockets int32 `json:"sockets"` + + // Cores is the number of cores per socket. + // Total CPUs is (Sockets * Cores) + Cores int32 `json:"cores"` +} + +// VMType defines the type of the VM, which will change the VM configuration, +// like including or excluding devices (like excluding sound-card), +// device configuration (like using multi-queues for vNic), and several other +// configuration tweaks. This doesn't effect properties like CPU count and amount of memory. +type VMType string + +const ( + // VMTypeDesktop set the VM type to desktop. Virtual machines optimized to act + // as desktop machines do have a sound card, use an image (thin allocation), + // and are stateless. + VMTypeDesktop VMType = "desktop" + // VMTypeServer sets the VM type to server. Virtual machines optimized to act + // as servers have no sound card, use a cloned disk image, and are not stateless. + VMTypeServer VMType = "server" + // VMTypeHighPerformance sets a VM type to high_performance which sets various + // properties of a VM to optimize for performance, like enabling headless mode, + // disabling usb, smart-card, and sound devices, enabling host cpu pass-through, + // multi-queues for vNics and several more items. + // See https://www.ovirt.org/develop/release-management/features/virt/high-performance-vm.html. + VMTypeHighPerformance VMType = "high_performance" +) + +// Set sets the values from `required` to `p`. +func (p *MachinePool) Set(required *MachinePool) { + if required == nil || p == nil { return } + + if required.InstanceTypeID != "" { + p.InstanceTypeID = required.InstanceTypeID + } + + if required.VMType != "" { + p.VMType = required.VMType + } + + if required.CPU != nil { + p.CPU = required.CPU + } + + if required.MemoryMB != 0 { + p.MemoryMB = required.MemoryMB + } + + if required.OSDisk != nil { + p.OSDisk = required.OSDisk + } } diff --git a/pkg/types/ovirt/validation/machinepool.go b/pkg/types/ovirt/validation/machinepool.go index 2516f2192d6..5d8ce58de6b 100644 --- a/pkg/types/ovirt/validation/machinepool.go +++ b/pkg/types/ovirt/validation/machinepool.go @@ -1,12 +1,67 @@ package validation import ( + "fmt" + "k8s.io/apimachinery/pkg/util/validation/field" "github.com/openshift/installer/pkg/types/ovirt" + "github.com/openshift/installer/pkg/validate" ) // ValidateMachinePool checks that the specified machine pool is valid. func ValidateMachinePool(p *ovirt.MachinePool, fldPath *field.Path) field.ErrorList { - return field.ErrorList{} + allErrs := field.ErrorList{} + + if p.CPU != nil { + if p.CPU.Cores <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("cores"), p.CPU.Cores, "CPU cores must be positive")) + } + if p.CPU.Sockets <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("sockets"), p.CPU.Sockets, "CPU sockets must be positive")) + } + } + + if p.VMType != "" && !ValidVMType(p.VMType) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("vmType"), p.VMType, fmt.Sprintf("VM type must be one of %s", supportedVMTypes()))) + } + + if p.InstanceTypeID != "" { + if p.CPU != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("instanceTypeID"), p.InstanceTypeID, "mixing instanceTypeID and CPU is not supported")) + } + if p.MemoryMB > 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("instanceTypeID"), p.InstanceTypeID, "mixing instanceTypeID and Memory is not supported")) + } + if err := validate.UUID(p.InstanceTypeID); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("instanceTypeID"), p.InstanceTypeID, err.Error())) + } + } + + if p.OSDisk != nil { + if p.OSDisk.SizeGB <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("sizeGB"), p.OSDisk.SizeGB, "disk size must be positive")) + } + } + + return allErrs +} + +// ValidVMType returns true if the vmType is supported. +func ValidVMType(vmType ovirt.VMType) bool { + for _, v := range supportedVMTypes() { + if vmType == v { + return true + } + } + return false +} + +// supportedVMTypes returns a slice of all supported VMTypes. +func supportedVMTypes() []ovirt.VMType { + return []ovirt.VMType{ + ovirt.VMTypeDesktop, + ovirt.VMTypeServer, + ovirt.VMTypeHighPerformance, + } } diff --git a/pkg/types/ovirt/validation/machinepool_test.go b/pkg/types/ovirt/validation/machinepool_test.go index 6c68ffce383..9d86f81e12e 100644 --- a/pkg/types/ovirt/validation/machinepool_test.go +++ b/pkg/types/ovirt/validation/machinepool_test.go @@ -20,6 +20,60 @@ func TestValidateMachinePool(t *testing.T) { pool: &ovirt.MachinePool{}, valid: true, }, + { + name: "invalid CPU cores", + pool: &ovirt.MachinePool{ + CPU: &ovirt.CPU{ + Cores: 0, + Sockets: 1, + }, + }, + valid: false, + }, + { + name: "invalid CPU sockets", + pool: &ovirt.MachinePool{ + CPU: &ovirt.CPU{ + Cores: 1, + Sockets: 0, + }, + }, + valid: false, + }, + { + name: "invalid OSDisk", + pool: &ovirt.MachinePool{ + OSDisk: &ovirt.Disk{ + SizeGB: 0, + }, + }, + valid: false, + }, + { + name: "invalid vmType", + pool: &ovirt.MachinePool{ + VMType: "strong", + }, + valid: false, + }, + { + name: "invalid instance type id", + pool: &ovirt.MachinePool{ + InstanceTypeID: "aaaaa-123", + }, + valid: false, + }, + { + name: "invalid mix of cpu and instance type id", + pool: &ovirt.MachinePool{ + CPU: &ovirt.CPU{ + Sockets: 1, + Cores: 4, + }, + InstanceTypeID: "85c65199-2df1-43bf-94f6-7e1567e6b238", + }, + valid: false, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index e592b4f3134..3ee95fd1d32 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -25,6 +25,8 @@ import ( libvirtvalidation "github.com/openshift/installer/pkg/types/libvirt/validation" "github.com/openshift/installer/pkg/types/openstack" openstackvalidation "github.com/openshift/installer/pkg/types/openstack/validation" + "github.com/openshift/installer/pkg/types/ovirt" + ovirtvalidation "github.com/openshift/installer/pkg/types/ovirt/validation" "github.com/openshift/installer/pkg/types/vsphere" vspherevalidation "github.com/openshift/installer/pkg/types/vsphere/validation" "github.com/openshift/installer/pkg/validate" @@ -375,6 +377,11 @@ func validatePlatform(platform *types.Platform, fldPath *field.Path, openStackVa return baremetalvalidation.ValidatePlatform(platform.BareMetal, network, f, c) }) } + if platform.Ovirt != nil { + validate(ovirt.Name, platform.Ovirt, func(f *field.Path) field.ErrorList { + return ovirtvalidation.ValidatePlatform(platform.Ovirt, f) + }) + } return allErrs } diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index 073819460f4..2dea35bd16c 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -206,6 +206,9 @@ func validOvirtPlatform() *ovirt.Platform { return &ovirt.Platform{ ClusterID: uuid.NewRandom().String(), StorageDomainID: uuid.NewRandom().String(), + APIVIP: "1.1.1.1", + DNSVIP: "1.1.1.2", + IngressVIP: "1.1.1.3", } } diff --git a/pkg/types/validation/machinepools.go b/pkg/types/validation/machinepools.go index 207236c956d..768f76c4011 100644 --- a/pkg/types/validation/machinepools.go +++ b/pkg/types/validation/machinepools.go @@ -16,6 +16,8 @@ import ( libvirtvalidation "github.com/openshift/installer/pkg/types/libvirt/validation" "github.com/openshift/installer/pkg/types/openstack" openstackvalidation "github.com/openshift/installer/pkg/types/openstack/validation" + "github.com/openshift/installer/pkg/types/ovirt" + ovirtvalidation "github.com/openshift/installer/pkg/types/ovirt/validation" "github.com/openshift/installer/pkg/types/vsphere" vspherevalidation "github.com/openshift/installer/pkg/types/vsphere/validation" ) @@ -98,5 +100,8 @@ func validateMachinePoolPlatform(platform *types.Platform, p *types.MachinePoolP if p.VSphere != nil { validate(vsphere.Name, p.VSphere, func(f *field.Path) field.ErrorList { return vspherevalidation.ValidateMachinePool(p.VSphere, f) }) } + if p.Ovirt != nil { + validate(ovirt.Name, p.Ovirt, func(f *field.Path) field.ErrorList { return ovirtvalidation.ValidateMachinePool(p.Ovirt, f) }) + } return allErrs }