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 <rgolan@redhat.com>

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
 }

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

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
-}
+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"
+}

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

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."
-}

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

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)

pkg/asset/installconfig/ovirt/validaton.go

@@ -40,9 +40,46 @@ 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.

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

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
 }

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{

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