fixing the sshkey, nw, and image name vs id issues (openshift#62)

until the machine provider config can resolve ids, we need the name for the TF, and the IDs for the provider config. not pretty but hopefully very temporary

Signed-off-by: Christy Norman <christy@linux.vnet.ibm.com>

data/data/powervs/main.tf

@@ -30,14 +30,14 @@ module "bootstrap" {
   cos_bucket_location   = var.powervs_cos_bucket_location
   cos_storage_class     = var.powervs_cos_storage_class
 
-  memory      = var.powervs_bootstrap_memory
-  processors  = var.powervs_bootstrap_processors
-  ignition    = var.ignition_bootstrap
-  sys_type    = var.powervs_sys_type
-  proc_type   = var.powervs_proc_type
-  key_id      = ibm_pi_key.cluster_key.key_id
-  image_id    = data.ibm_pi_image.boot_image.id
-  network_id  = data.ibm_pi_network.pvs_net.id
+  memory       = var.powervs_bootstrap_memory
+  processors   = var.powervs_bootstrap_processors
+  ignition     = var.ignition_bootstrap
+  sys_type     = var.powervs_sys_type
+  proc_type    = var.powervs_proc_type
+  key_id       = ibm_pi_key.cluster_key.key_id
+  image_name   = var.powervs_image_name
+  network_name = var.powervs_network_name
 }
 
 module "master" {
@@ -50,17 +50,16 @@ module "master" {
   resource_group    = var.powervs_resource_group
   instance_count    = var.master_count
 
-  memory      = var.powervs_master_memory
-  processors  = var.powervs_master_processors
-  ignition    = var.ignition_master
-  sys_type    = var.powervs_sys_type
-  proc_type   = var.powervs_proc_type
-  key_id      = ibm_pi_key.cluster_key.key_id
-  image_id    = data.ibm_pi_image.boot_image.id
-  network_id  = data.ibm_pi_network.pvs_net.id
+  memory       = var.powervs_master_memory
+  processors   = var.powervs_master_processors
+  ignition     = var.ignition_master
+  sys_type     = var.powervs_sys_type
+  proc_type    = var.powervs_proc_type
+  key_id       = ibm_pi_key.cluster_key.key_id
+  image_name   = var.powervs_image_name
+  network_name = var.powervs_network_name
 }
 
-
 data "ibm_is_subnet" "vpc_subnet" {
   provider = ibm.vpc
   name = var.powervs_vpc_subnet_name

data/data/powervs/variables-powervs.tf

@@ -95,9 +95,16 @@ variable "powervs_sys_type" {
   default     = "s922"
 }
 
+variable "powervs_key_name" {
+  type        = string
+  description = "The name for the SSH key created in the Service Instance"
+  default     = ""
+}
+
 variable "powervs_ssh_key" {
   type        = string
   description = "Public key for keypair used to access cluster. Required when creating 'ibm_pi_instance' resources."
+  default     = ""
 }
 
 ################################################################
@@ -106,19 +113,16 @@ variable "powervs_ssh_key" {
 variable "powervs_network_name" {
   type        = string
   description = "Name of the network within the Power VS instance."
-  default     = "pvs-net-${var.cluster_id}"
 }
 
 variable "powervs_vpc_name" {
   type        = string
   description = "Name of the IBM Cloud Virtual Private Cloud (VPC) to setup the load balancer."
-  default     = "vpc-${var.cluster_id}"
 }
 
 variable "powervs_vpc_subnet_name" {
   type        = string
   description = "Name of the VPC subnet connected via DirectLink to the Power VS private network."
-  default     = "vpc-subnet-${var.cluster_id}"
 }
 
 ################################################################

pkg/asset/cluster/tfvars.go

@@ -638,6 +638,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 			Data:     data,
 		})
 	case powervs.Name:
+		// @TODO: Can we just use the install config for all these values?
 		session, err := powervsconfig.GetSession()
 		if err != nil {
 			return err
@@ -664,10 +665,13 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 				MasterConfigs:        masterConfigs,
 				PowerVSZone:          session.Session.Zone,
 				APIKey:               session.Session.IAMToken,
+				SSHKey:               installConfig.Config.SSHKey,
 				PowerVSResourceGroup: installConfig.Config.PowerVS.PowerVSResourceGroup,
+				NetworkName:          installConfig.Config.PowerVS.PVSNetworkName,
+				ImageName:            installConfig.Config.PowerVS.ImageName,
 				CISInstanceCRN:       crn,
 				VPCSubnetName:        installConfig.Config.PowerVS.Subnets[0],
-				VPCID:                installConfig.Config.PowerVS.VPC,
+				VPCName:              installConfig.Config.PowerVS.VPC,
 			},
 		)
 		if err != nil {

pkg/asset/machines/master.go

@@ -414,7 +414,7 @@ func (m *Master) Generate(dependencies asset.Parents) error {
 		// The other two, we should standardize a name including the cluster id. At this point, all
 		// we have are names.
 		pool.Platform.PowerVS = &mpool
-		machines, err = powervs.Machines(clusterID.InfraID, ic, &pool, "master", "master-user-data", installConfig.Config.Platform.PowerVS.UserTags)
+		machines, err = powervs.Machines(clusterID.InfraID, ic, &pool, "master", "master-user-data")
 		if err != nil {
 			return errors.Wrap(err, "failed to create master machine objects")
 		}

pkg/asset/machines/powervs/machines.go

@@ -16,7 +16,7 @@ import (
 )
 
 // Machines returns a list of machines for a machinepool.
-func Machines(clusterID string, config *types.InstallConfig, pool *types.MachinePool, role, userDataSecret string, userTags map[string]string) ([]machineapi.Machine, error) {
+func Machines(clusterID string, config *types.InstallConfig, pool *types.MachinePool, role, userDataSecret string) ([]machineapi.Machine, error) {
 	if poolPlatform := pool.Platform.Name(); poolPlatform != powervs.Name {
 		return nil, fmt.Errorf("non-PowerVS machine-pool: %q", poolPlatform)
 	}
@@ -26,6 +26,11 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine
 	// Only the service instance is guaranteed to exist and be passed via the install config
 	// The other two, we should standardize a name including the cluster id.
 
+	if platform.SSHKeyName != "" {
+		mpool.KeyPairName = platform.SSHKeyName
+	} else {
+		mpool.KeyPairName = fmt.Sprintf("%s-key", clusterID)
+	}
 	if platform.PVSNetworkID != "" {
 		mpool.NetworkIDs = append([]string{platform.PVSNetworkID})
 	}
@@ -42,7 +47,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine
 	}
 	var machines []machineapi.Machine
 	for idx := int64(0); idx < total; idx++ {
-		provider, err := provider(clusterID, platform, mpool, userDataSecret, userTags)
+		provider, err := provider(clusterID, platform, mpool, userDataSecret, platform.UserTags)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create provider")
 		}
@@ -80,6 +85,7 @@ func provider(clusterID string, platform *powervs.Platform, mpool *powervs.Machi
 			APIVersion: powervsprovider.GroupVersion.String(),
 		},
 		ObjectMeta:        metav1.ObjectMeta{},
+		Region:            platform.Region,
 		ServiceInstanceID: platform.ServiceInstanceID,
 		ImageID:           mpool.ImageID,
 		UserDataSecret:    &corev1.LocalObjectReference{Name: userDataSecret},
@@ -89,6 +95,7 @@ func provider(clusterID string, platform *powervs.Platform, mpool *powervs.Machi
 		Processors:        fmt.Sprintf("%f", mpool.Processors),
 		Memory:            fmt.Sprintf("%d", mpool.Memory),
 		NetworkIDs:        mpool.NetworkIDs,
+		KeyPairName:       &mpool.KeyPairName,
 	}
 	return config, nil
 }

pkg/asset/machines/powervs/machinesets.go

@@ -26,6 +26,11 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach
 	platform := config.Platform.PowerVS
 	mpool := pool.Platform.PowerVS
 
+	if platform.SSHKeyName != "" {
+		mpool.KeyPairName = platform.SSHKeyName
+	} else {
+		mpool.KeyPairName = fmt.Sprintf("%s-key", clusterID)
+	}
 	if platform.PVSNetworkID != "" {
 		mpool.NetworkIDs = append([]string{platform.PVSNetworkID})
 	}

pkg/tfvars/powervs/powervs.go

@@ -27,15 +27,17 @@ var powervsRegionToIBMRegion = map[string]string{
 type config struct {
 	ServiceInstanceID    string `json:"powervs_cloud_instance_id"`
 	APIKey               string `json:"powervs_api_key"`
+	SSHKey               string `json:"powervs_ssh_key"`
 	PowerVSRegion        string `json:"powervs_region"`
 	PowerVSZone          string `json:"powervs_zone"`
 	VPCRegion            string `json:"powervs_vpc_region"`
 	PowerVSResourceGroup string `json:"powervs_resource_group"`
 	CISInstanceCRN       string `json:"powervs_cis_crn"`
-	SSHKey               string `json:"powervs_ssh_key"`
-	ImageID              string `json:"powervs_image_name"`
-	NetworkIDs           string `json:"powervs_network_name"`
-	VPCID                string `json:"powervs_vpc_id"`
+	ImageName            string `json:"powervs_image_name"`
+	ImageID              string `json:"powervs_image_id"`
+	NetworkName          string `json:"powervs_network_name"`
+	NetworkIDs           string `json:"powervs_network_id"`
+	VPCName              string `json:"powervs_vpc_name"`
 	VPCSubnetName        string `json:"powervs_vpc_subnet_name"`
 	BootstrapMemory      string `json:"powervs_bootstrap_memory"`
 	BootstrapProcessors  string `json:"powervs_bootstrap_processors"`
@@ -49,10 +51,13 @@ type config struct {
 type TFVarsSources struct {
 	MasterConfigs        []*v1alpha1.PowerVSMachineProviderConfig
 	APIKey               string
+	SSHKey               string
 	PowerVSZone          string
+	NetworkName          string
+	ImageName            string
 	PowerVSResourceGroup string
 	CISInstanceCRN       string
-	VPCID                string
+	VPCName              string
 	VPCSubnetName        string
 }
 
@@ -64,15 +69,17 @@ func TFVars(sources TFVarsSources) ([]byte, error) {
 	cfg := &config{
 		ServiceInstanceID:    masterConfig.ServiceInstanceID,
 		APIKey:               sources.APIKey,
+		SSHKey:               sources.SSHKey,
 		PowerVSRegion:        masterConfig.Region,
 		PowerVSZone:          sources.PowerVSZone,
 		VPCRegion:            powervsRegionToIBMRegion[masterConfig.Region],
 		PowerVSResourceGroup: sources.PowerVSResourceGroup,
 		CISInstanceCRN:       sources.CISInstanceCRN,
-		SSHKey:               *masterConfig.KeyPairName,
+		ImageName:            sources.ImageName,
 		ImageID:              masterConfig.ImageID,
+		NetworkName:          sources.NetworkName,
 		NetworkIDs:           masterConfig.NetworkIDs[0],
-		VPCID:                sources.VPCID,
+		VPCName:              sources.VPCName,
 		VPCSubnetName:        sources.VPCSubnetName,
 		BootstrapMemory:      masterConfig.Memory,
 		BootstrapProcessors:  masterConfig.Processors,

pkg/types/powervs/machinepools.go

@@ -1,6 +1,6 @@
 package powervs
 
-// MachinePool stores the configuration for a machine pool installed on Ibm Power VS.
+// MachinePool stores the configuration for a machine pool installed on IBM Power VS.
 type MachinePool struct {
 	// ServiceInstance is Service Instance to install into.
 	//
@@ -10,6 +10,10 @@ type MachinePool struct {
 	//
 	Name string `json:"name"`
 
+	// KeyPairName is the name of an SSH key pair stored in the Power VS
+	// Service Instance
+	KeyPairName string `json:"keypairname"`
+
 	// VolumeIDs is the list of volumes attached to the instance.
 	//
 	VolumeIDs []string `json:"volumeIDs"`
@@ -60,4 +64,7 @@ func (a *MachinePool) Set(required *MachinePool) {
 	if required.ServiceInstance != "" {
 		a.ServiceInstance = required.ServiceInstance
 	}
+	if required.KeyPairName != "" {
+		a.KeyPairName = required.KeyPairName
+	}
 }

pkg/types/powervs/platform.go

@@ -2,7 +2,9 @@ package powervs
 
 // Platform stores all the global configuration that all machinesets
 // use.
-/// used by the installconfig, and filled in by the installconfig/platform/powervs::Platform() func
+// Note: The subsequent mentions of future-TF support refer to work that is
+// undergoing and should be available to test well in time for 4.10 feature-
+// freeze. We do not plan to GA with these as required inputs.
 type Platform struct {
 
 	// ServiceInstanceID is the ID of the Power IAAS instance created from the IBM Cloud Catalog
@@ -26,23 +28,32 @@ type Platform struct {
 	// +optional
 	APIKey string `json:"APIKey,omitempty"`
 
+	// SSHKeyName is the name of an SSH key stored in the Service Instance.
+	SSHKeyName string `json:"SSHKeyName,omitempty"`
+
 	// VPC is a VPC inside IBM Cloud. Needed in order to create VPC Load Balancers.
 	//
-	// +optional
+	// @TODO: make this +optional when we have TF support
 	VPC string `json:"vpc,omitempty"`
 
 	// Subnets specifies existing subnets (by ID) where cluster
 	// resources will be created.  Leave unset to have the installer
 	// create subnets in a new VPC on your behalf.
 	// @TODO: Rename VPCSubnetID & make into string
 	//
-	// +optional
+	// @TODO: make this +optional when we have TF support
 	Subnets []string `json:"subnets,omitempty"`
 
-	// PVSNetworkID specifies an existing network withing the Power VS Service Instance.
+	// PVSNetworkName specifies an existing network within the Power VS Service Instance.
+	// @TODO: make this +optional when we have TF support
+	PVSNetworkName string `json:pvsNetworkName,omitempty"`
+
+	// PVSNetworkID is the associated ID for the PVSNetworkName. This is currently required
+	// For the machine config.
+	// @TODO: Remove when machine config resolves the ID from name.
 	// Leave unset to have the installer create the network.
 	//
-	// +optional
+	// @TODO: make this +optional when we have TF support
 	PVSNetworkID string `json:"pvsNetworkID,omitempty"`
 
 	// UserTags additional keys and values that the installer will add
@@ -51,18 +62,27 @@ type Platform struct {
 	// +optional
 	UserTags map[string]string `json:"userTags,omitempty"`
 
+	// ImageName is equivalent to BootStrap/ClusterOSImage.
+	// Until the machine provider config in cluster-api-provider-powervs
+	// takes an ID instead of a name, we're using this for TF Creation,
+	// and the other two for machine-config.
+	//
+	// @TODO: Remove when provider resolves ID from name
+	// @TODO: make this +optional when we have TF support
+	ImageName string `json:"imageName,omitempty"`
+
 	// BootstrapOSImage is a URL to override the default OS image
 	// for the bootstrap node. The URL must contain a sha256 hash of the image
 	// e.g https://mirror.example.com/images/image.ova.gz?sha256=a07bd...
 	//
-	// +optional
+	// @TODO: make this +optional when we have TF support
 	BootstrapOSImage string `json:"bootstrapOSImage,omitempty"`
 
 	// ClusterOSImage is a URL to override the default OS image
 	// for cluster nodes. The URL must contain a sha256 hash of the image
 	// e.g https://mirror.example.com/images/powervs.ova.gz?sha256=3b5a8...
 	//
-	// +optional
+	// @TODO: make this +optional when we have TF support
 	ClusterOSImage string `json:"clusterOSImage,omitempty"`
 
 	// DefaultMachinePlatform is the default configuration used when