From 6cdb32973434768ab1ce4cad2755387d31ad6f71 Mon Sep 17 00:00:00 2001 From: cnorman Date: Thu, 16 Sep 2021 16:17:00 -0500 Subject: [PATCH] fixing the sshkey, nw, and image name vs id issues (#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 --- data/data/powervs/main.tf | 33 +++++++++++----------- data/data/powervs/variables-powervs.tf | 10 +++++-- pkg/asset/cluster/tfvars.go | 6 +++- pkg/asset/machines/master.go | 2 +- pkg/asset/machines/powervs/machines.go | 11 ++++++-- pkg/asset/machines/powervs/machinesets.go | 5 ++++ pkg/tfvars/powervs/powervs.go | 21 +++++++++----- pkg/types/powervs/machinepools.go | 9 +++++- pkg/types/powervs/platform.go | 34 ++++++++++++++++++----- 9 files changed, 92 insertions(+), 39 deletions(-) diff --git a/data/data/powervs/main.tf b/data/data/powervs/main.tf index 71a226f418b..7322ce3eb54 100644 --- a/data/data/powervs/main.tf +++ b/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 diff --git a/data/data/powervs/variables-powervs.tf b/data/data/powervs/variables-powervs.tf index ffd82622a49..8e144f23fa4 100644 --- a/data/data/powervs/variables-powervs.tf +++ b/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}" } ################################################################ diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 3198f67f097..b84ea24edfc 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/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 { diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 3683bab8037..2b7e54b9075 100644 --- a/pkg/asset/machines/master.go +++ b/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") } diff --git a/pkg/asset/machines/powervs/machines.go b/pkg/asset/machines/powervs/machines.go index 8edb9fdb5a5..2802b01abb0 100644 --- a/pkg/asset/machines/powervs/machines.go +++ b/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 } diff --git a/pkg/asset/machines/powervs/machinesets.go b/pkg/asset/machines/powervs/machinesets.go index ca8da23dea0..93fb3d0cc86 100644 --- a/pkg/asset/machines/powervs/machinesets.go +++ b/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}) } diff --git a/pkg/tfvars/powervs/powervs.go b/pkg/tfvars/powervs/powervs.go index d70b6ac23f0..82302adb5dd 100644 --- a/pkg/tfvars/powervs/powervs.go +++ b/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, diff --git a/pkg/types/powervs/machinepools.go b/pkg/types/powervs/machinepools.go index 7f326715ad6..3854d573ce2 100644 --- a/pkg/types/powervs/machinepools.go +++ b/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 + } } diff --git a/pkg/types/powervs/platform.go b/pkg/types/powervs/platform.go index 48fb58ea956..efe9185408f 100644 --- a/pkg/types/powervs/platform.go +++ b/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,9 +28,12 @@ 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 @@ -36,13 +41,19 @@ type Platform struct { // 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