ovirt: extend ovirt's MachinePool

Those items are now part of MachinePool
- instance type
- cpu (cores, sockets)
- memory
- os disk size
- vm type

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: 100
      type: 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.

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
 }
 
@@ -29,13 +26,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

@@ -22,15 +22,33 @@ variable "ovirt_template_id" {
   description = "The ID of oVirt's VM template"
 }
 
+variable "ovirt_master_instance_type_id" {
+  type        = string
+  description = "The ID of oVirt's instance type"
+}
+
+variable "ovirt_master_vm_type" {
+  type        = string
+  description = "The master's VM type"
+}
+
 variable "ignition_master" {
   type        = string
   description = "master ignition config"
 }
 
-variable "ovirt_master_mem" {
+variable "ovirt_master_memory" {
+  type = string
+}
+
+variable "ovirt_master_cores" {
+  type = string
+}
+
+variable "ovirt_master_sockets" {
   type = string
 }
 
-variable "ovirt_master_cpu" {
+variable "ovirt_master_os_disk_size_gb" {
   type = string
 }

data/data/ovirt/template/main.tf

@@ -51,8 +51,9 @@ 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
+  os {
+    type = "rhcos_x64"
+  }
   block_device {
     disk_id   = ovirt_image_transfer.releaseimage.0.disk_id
     interface = "virtio_scsi"
@@ -73,7 +74,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

@@ -33,16 +33,3 @@ variable "ovirt_network_name" {
   default     = "ovirtmgmt"
   description = "The name of ovirt's logical network for the selected ovirt cluster."
 }
-
-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

@@ -46,27 +46,26 @@ variable "ovirt_network_name" {
   description = "The name of ovirt's logical network for the selected ovirt cluster."
 }
 
-variable "ovirt_master_mem" {
-  type    = string
-  default = "8192"
+variable "ovirt_master_memory" {
+  type = string
 }
 
-variable "ovirt_master_cpu" {
-  type    = number
-  default = 4
+variable "ovirt_master_cores" {
+  type = string
 }
 
-variable "ovirt_template_mem" {
-  type    = string
-  default = "16384"
+variable "ovirt_master_sockets" {
+  type = string
 }
 
-variable "ovirt_template_cpu" {
-  type    = number
-  default = 4
+variable "ovirt_master_os_disk_gb" {
+  type = string
 }
 
-variable "ovirt_template_disk_size_gib" {
-  type    = number
-  default = 25
+variable "ovirt_master_vm_type" {
+  type = string
 }
+
+variable "ovirt_master_instance_type_id" {
+  type = string
+}

go.mod

@@ -69,12 +69,12 @@ 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.20200402173228-4981fa21d0d7
 	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/terraform-provider-ovirt v0.4.3-0.20200122105935-65b89ad00553
+	github.com/ovirt/go-ovirt v0.0.0-20200320082526-4e97a11ff083
+	github.com/ovirt/terraform-provider-ovirt v0.4.3-0.20200402170624-73f2734a19ba
 	github.com/packer-community/winrmcp v0.0.0-20180921211025-c76d91c1e7db // indirect
 	github.com/pborman/uuid v1.2.0
 	github.com/pierrec/lz4 v2.3.0+incompatible // indirect

go.sum

@@ -1799,8 +1799,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.20200402173228-4981fa21d0d7 h1:FsDhFEe4gPnfF3VOB+HM3qiGxLe9baaTKrykqyK+bCM=
+github.com/openshift/cluster-api-provider-ovirt v0.1.1-0.20200402173228-4981fa21d0d7/go.mod h1:vZ6HC/n4m4Pbbc8Ta1rXK2J4cA6oYOU55wVd39FZ8Ao=
 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=
@@ -1838,10 +1838,11 @@ github.com/operator-framework/operator-sdk v0.5.1-0.20190301204940-c2efe6f74e7b/
 github.com/oracle/oci-go-sdk v7.0.0+incompatible/go.mod h1:VQb79nF8Z2cwLkLS35ukwStZIg5F66tcBccjip/j888=
 github.com/ory/dockertest v3.3.4+incompatible/go.mod h1:1vX4m9wsvi00u5bseYwXaSnhNrne+V0E6LAcBILJdPs=
 github.com/ostreedev/ostree-go v0.0.0-20190702140239-759a8c1ac913/go.mod h1:J6OG6YJVEWopen4avK3VNQSnALmmjvniMmni/YFYAwc=
-github.com/ovirt/go-ovirt v4.3.4+incompatible h1:jXcJpcXyNZ3mXJ1IVU3l3tMpE4JEUSNjqRiEJnVpG40=
-github.com/ovirt/go-ovirt v4.3.4+incompatible/go.mod h1:r33ZGjVKCPMiI6hw791/Zx8tNKk0Gn+4VFWbOfyIvZQ=
-github.com/ovirt/terraform-provider-ovirt v0.4.3-0.20200122105935-65b89ad00553 h1:vmWD1P79PygdRMQluGsJ1Xl7PIIcJiiL8ZnawfUTv24=
-github.com/ovirt/terraform-provider-ovirt v0.4.3-0.20200122105935-65b89ad00553/go.mod h1:IJf1C8MY1f4djN506HESURNfvcjrMVjBWEHN13dw8R0=
+github.com/ovirt/go-ovirt v0.0.0-20200313072907-d30f754823a6/go.mod h1:fLDxPk1Sf64DBYtwIYxrnx3gPZ1q0xPdWdI1y9vxUaw=
+github.com/ovirt/go-ovirt v0.0.0-20200320082526-4e97a11ff083 h1:nW3qTnZ+ytoEZ1JL0EW3+mwjVcfjZ9WB3tSfOHQREgM=
+github.com/ovirt/go-ovirt v0.0.0-20200320082526-4e97a11ff083/go.mod h1:fLDxPk1Sf64DBYtwIYxrnx3gPZ1q0xPdWdI1y9vxUaw=
+github.com/ovirt/terraform-provider-ovirt v0.4.3-0.20200402170624-73f2734a19ba h1:/77PRDYCNDOlW8RVfGIA9oXTeBWrdPX/cUHO7hR5l8I=
+github.com/ovirt/terraform-provider-ovirt v0.4.3-0.20200402170624-73f2734a19ba/go.mod h1:XFDLN/srNA1s2Dq+gp4zBvql6nRnfNJzDGzI5vtK85g=
 github.com/oxtoacart/bpool v0.0.0-20150712133111-4e1c5567d7c2/go.mod h1:L3UMQOThbttwfYRNFOWLLVXMhk5Lkio4GGOtw5UrxS0=
 github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c/go.mod h1:X07ZCGwUbLaax7L0S3Tw4hpejzu63ZrrQiUe6W0hcy0=
 github.com/packer-community/winrmcp v0.0.0-20180102160824-81144009af58/go.mod h1:f6Izs6JvFTdnRbziASagjZ2vmf55NSIkC/weStxCHqk=

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"
@@ -434,16 +435,24 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 			return err
 		}
 
+		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,
 			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/validators_test.go

@@ -26,6 +26,10 @@ func setup() {
 			writer.Write([]byte("{}"))
 			return
 		}
+		if request.Method == http.MethodOptions {
+			writer.WriteHeader(http.StatusOK)
+		}
+		return
 	})
 }
 
@@ -48,7 +52,7 @@ func Test_validateAuth(t *testing.T) {
 		expectSuccess: true,
 	},
 		{
-			url:           "https://nonexisting",
+			url:           "https://nonexisting.com",
 			username:      "foo",
 			password:      "bar",
 			insecure:      false,

pkg/asset/machines/master.go

@@ -319,7 +319,11 @@ func (m *Master) Generate(dependencies asset.Parents) error {
 		}
 	case ovirttypes.Name:
 		mpool := defaultOvirtMachinePoolPlatform()
+		mpool.OSDisk.SizeGB = 30
+		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,30 @@ 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:            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,
+		}
+	}
+	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: "server",
+	}
 }
 
 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")