-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add IPI Support for IBM Power Virtual Servers Offering #5224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -434,6 +434,58 @@ spec: | |
| - high_performance | ||
| type: string | ||
| type: object | ||
| powervs: | ||
| description: PowerVS is the configuration used when installing | ||
| on IBM Power VS. | ||
| properties: | ||
| imageName: | ||
| description: ImageName defines the boot image name for the | ||
| instance. | ||
| type: string | ||
| keypairname: | ||
| description: KeyPairName is the name of an SSH key pair | ||
| stored in the Power VS Service Instance | ||
| type: string | ||
| memory: | ||
| description: Memory defines the memory in GB for the instance. | ||
| type: integer | ||
| name: | ||
| description: Name is the name of the instance | ||
| type: string | ||
| networkIDs: | ||
| description: NetworkIDs defines the network IDs of the instance. | ||
| items: | ||
| type: string | ||
| type: array | ||
| procType: | ||
| description: ProcType defines the processor sharing model | ||
| for the instance. | ||
| type: string | ||
| processors: | ||
| description: 'Processors defines the processing units for | ||
| the instance. @TODO:' | ||
| serviceinstance: | ||
| description: ServiceInstance is Service Instance to install | ||
| into. | ||
| type: string | ||
| sysType: | ||
| description: SysType defines the system type for instance. | ||
| type: string | ||
| volumeIDs: | ||
| description: VolumeIDs is the list of volumes attached to | ||
| the instance. | ||
| items: | ||
| type: string | ||
| type: array | ||
| required: | ||
| - imageName | ||
| - keypairname | ||
| - memory | ||
| - name | ||
| - networkIDs | ||
| - processors | ||
| - serviceinstance | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't we have well defined defaults for most of these fields so that most of them are optional or come with some defaults?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment as above. we do have defaults though |
||
| type: object | ||
| vsphere: | ||
| description: VSphere is the configuration used when installing | ||
| on vSphere. | ||
|
|
@@ -859,6 +911,58 @@ spec: | |
| - high_performance | ||
| type: string | ||
| type: object | ||
| powervs: | ||
| description: PowerVS is the configuration used when installing | ||
| on IBM Power VS. | ||
| properties: | ||
| imageName: | ||
| description: ImageName defines the boot image name for the | ||
| instance. | ||
| type: string | ||
| keypairname: | ||
| description: KeyPairName is the name of an SSH key pair stored | ||
| in the Power VS Service Instance | ||
| type: string | ||
| memory: | ||
| description: Memory defines the memory in GB for the instance. | ||
| type: integer | ||
| name: | ||
| description: Name is the name of the instance | ||
| type: string | ||
| networkIDs: | ||
| description: NetworkIDs defines the network IDs of the instance. | ||
| items: | ||
| type: string | ||
| type: array | ||
| procType: | ||
| description: ProcType defines the processor sharing model | ||
| for the instance. | ||
| type: string | ||
| processors: | ||
| description: 'Processors defines the processing units for | ||
| the instance. @TODO:' | ||
| serviceinstance: | ||
| description: ServiceInstance is Service Instance to install | ||
| into. | ||
| type: string | ||
| sysType: | ||
| description: SysType defines the system type for instance. | ||
| type: string | ||
| volumeIDs: | ||
| description: VolumeIDs is the list of volumes attached to | ||
| the instance. | ||
| items: | ||
| type: string | ||
| type: array | ||
| required: | ||
| - imageName | ||
| - keypairname | ||
| - memory | ||
| - name | ||
| - networkIDs | ||
| - processors | ||
| - serviceinstance | ||
| type: object | ||
| vsphere: | ||
| description: VSphere is the configuration used when installing | ||
| on vSphere. | ||
|
|
@@ -2059,6 +2163,136 @@ spec: | |
| - ovirt_cluster_id | ||
| - ovirt_storage_domain_id | ||
| type: object | ||
| powervs: | ||
| description: PowerVS is the configuration used when installing on | ||
| Power VS. | ||
| properties: | ||
| APIKey: | ||
| description: APIKey is the API key for the user's IBM Cloud account. | ||
| type: string | ||
| SSHKeyName: | ||
| description: SSHKeyName is the name of an SSH key stored in the | ||
| Service Instance. | ||
| type: string | ||
| bootstrapOSImage: | ||
| description: BootstrapOSImage is a pre-created boot image to override | ||
| the default OS image for the bootstrap node. | ||
| type: string | ||
| clusterOSImage: | ||
| description: "ClusterOSImage is a pre-created Power VS boot image | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. once again these are referring to the bootimages - which is not what is supplied through the rhcos json. these would be internally created through the ova images?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol, once again -- bootimages. and yes :) |
||
| that overrides the default image for cluster nodes. \n @TODO: | ||
| make this +optional, add omitempty when we have TF support" | ||
| type: string | ||
| defaultMachinePlatform: | ||
| description: DefaultMachinePlatform is the default configuration | ||
| used when installing on Power VS for machine pools which do | ||
| not define their own platform configuration. | ||
| properties: | ||
| imageName: | ||
| description: ImageName defines the boot image name for the | ||
| instance. | ||
| type: string | ||
| keypairname: | ||
| description: KeyPairName is the name of an SSH key pair stored | ||
| in the Power VS Service Instance | ||
| type: string | ||
| memory: | ||
| description: Memory defines the memory in GB for the instance. | ||
| type: integer | ||
| name: | ||
| description: Name is the name of the instance | ||
| type: string | ||
| networkIDs: | ||
| description: NetworkIDs defines the network IDs of the instance. | ||
| items: | ||
| type: string | ||
| type: array | ||
| procType: | ||
| description: ProcType defines the processor sharing model | ||
| for the instance. | ||
| type: string | ||
| processors: | ||
| description: 'Processors defines the processing units for | ||
| the instance. @TODO:' | ||
| serviceinstance: | ||
| description: ServiceInstance is Service Instance to install | ||
| into. | ||
| type: string | ||
| sysType: | ||
| description: SysType defines the system type for instance. | ||
| type: string | ||
| volumeIDs: | ||
| description: VolumeIDs is the list of volumes attached to | ||
| the instance. | ||
| items: | ||
| type: string | ||
| type: array | ||
| required: | ||
| - imageName | ||
| - keypairname | ||
| - memory | ||
| - name | ||
| - networkIDs | ||
| - processors | ||
| - serviceinstance | ||
| type: object | ||
| powervsResourceGroup: | ||
| description: PowerVSResourceGroup is the resource group for creating | ||
| Power VS resources. | ||
| type: string | ||
| pvsNetworkName: | ||
| description: 'PVSNetworkName specifies an existing network within | ||
| the Power VS Service Instance. @TODO: make this +optional when | ||
| we have TF support' | ||
| type: string | ||
| region: | ||
| description: Region specifies the IBM Cloud region where the cluster | ||
| will be created. | ||
| type: string | ||
| serviceInstance: | ||
| description: ServiceInstanceID is the ID of the Power IAAS instance | ||
| created from the IBM Cloud Catalog | ||
| type: string | ||
| subnets: | ||
| description: "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 \n @TODO: make this +optional | ||
| when we have TF support" | ||
| items: | ||
| type: string | ||
| type: array | ||
| userID: | ||
| description: UserID is the login for the user's IBM Cloud account. | ||
| type: string | ||
| userTags: | ||
| additionalProperties: | ||
| type: string | ||
| description: UserTags additional keys and values that the installer | ||
| will add as tags to all resources that it creates. Resources | ||
| created by the cluster itself may not include these tags. | ||
| type: object | ||
| vpc: | ||
| description: "VPC is a VPC inside IBM Cloud. Needed in order to | ||
| create VPC Load Balancers. \n @TODO: make this +optional when | ||
| we have TF support" | ||
| type: string | ||
| vpcRegion: | ||
| description: Zone in the region used to create VPC resources. | ||
| Leave unset to allow installer to randomly select a zone. | ||
| type: string | ||
| zone: | ||
| description: Zone specifies the IBM Cloud colo region where the | ||
| cluster will be created. Required for multi-zone regions. | ||
| type: string | ||
| required: | ||
| - clusterOSImage | ||
| - powervsResourceGroup | ||
| - region | ||
| - serviceInstance | ||
| - userID | ||
| - zone | ||
| type: object | ||
| vsphere: | ||
| description: VSphere is the configuration used when installing on | ||
| vSphere. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| # TODO(mjturek): network and image data blocks can be in main module | ||
| # as master and bootstrap will be using the same | ||
| # network and image. Once we add in master module, make | ||
| # the move. | ||
| data "ibm_pi_network" "network" { | ||
| pi_network_name = var.network_name | ||
| pi_cloud_instance_id = var.cloud_instance_id | ||
| } | ||
|
|
||
| data "ibm_pi_image" "bootstrap_image" { | ||
| pi_image_name = var.image_name | ||
| pi_cloud_instance_id = var.cloud_instance_id | ||
| } | ||
|
|
||
| data "ignition_config" "bootstrap" { | ||
| merge { | ||
| source = ibms3presign.bootstrap_ignition.presigned_url | ||
| } | ||
| } | ||
|
|
||
| data "ibm_resource_group" "cos_group" { | ||
| name = var.resource_group | ||
| } | ||
|
|
||
| resource "ibm_resource_instance" "cos_instance" { | ||
| name = "${var.cluster_id}-cos" | ||
| resource_group_id = data.ibm_resource_group.cos_group.id | ||
| service = "cloud-object-storage" | ||
| plan = "standard" | ||
| location = var.cos_instance_location | ||
| tags = [var.cluster_id] | ||
| } | ||
|
|
||
| # Create an IBM COS Bucket to store ignition | ||
| resource "ibm_cos_bucket" "ignition" { | ||
| bucket_name = "${var.cluster_id}-bootstrap-ign" | ||
| resource_instance_id = ibm_resource_instance.cos_instance.id | ||
| region_location = var.cos_bucket_location | ||
| storage_class = var.cos_storage_class | ||
| } | ||
|
|
||
| resource "ibm_resource_key" "cos_service_cred" { | ||
| name = "${var.cluster_id}-cred" | ||
| role = "Reader" | ||
| resource_instance_id = ibm_resource_instance.cos_instance.id | ||
| parameters = { HMAC = true } | ||
| } | ||
|
|
||
| resource "ibms3presign" "bootstrap_ignition" { | ||
| access_key_id = ibm_resource_key.cos_service_cred.credentials["cos_hmac_keys.access_key_id"] | ||
| secret_access_key = ibm_resource_key.cos_service_cred.credentials["cos_hmac_keys.secret_access_key"] | ||
| bucket_name = "${var.cluster_id}-bootstrap-ign" | ||
| key = "bootstrap.ign" | ||
| region_location = ibm_cos_bucket.ignition.region_location | ||
| storage_class = ibm_cos_bucket.ignition.storage_class | ||
| } | ||
|
|
||
| # Place the bootstrap ignition file in the ignition COS bucket | ||
| resource "ibm_cos_bucket_object" "ignition" { | ||
| bucket_crn = ibm_cos_bucket.ignition.crn | ||
| bucket_location = ibm_cos_bucket.ignition.region_location | ||
| content = var.ignition | ||
| key = "bootstrap.ign" | ||
| } | ||
|
|
||
| # Create the bootstrap instance | ||
| resource "ibm_pi_instance" "bootstrap" { | ||
| pi_memory = var.memory | ||
| pi_processors = var.processors | ||
| pi_instance_name = "${var.cluster_id}-bootstrap" | ||
| pi_proc_type = var.proc_type | ||
| pi_image_id = data.ibm_pi_image.bootstrap_image.id | ||
| pi_sys_type = var.sys_type | ||
| pi_cloud_instance_id = var.cloud_instance_id | ||
| pi_network_ids = [data.ibm_pi_network.network.id] | ||
|
|
||
| pi_user_data = base64encode(data.ignition_config.bootstrap.rendered) | ||
| pi_key_pair_name = var.key_id | ||
| pi_health_status = "WARNING" | ||
| } | ||
|
|
||
| data "ibm_pi_instance_ip" "bootstrap_ip" { | ||
| depends_on = [ibm_pi_instance.bootstrap] | ||
|
|
||
| pi_instance_name = ibm_pi_instance.bootstrap.pi_instance_name | ||
| pi_network_name = data.ibm_pi_network.network.pi_network_name | ||
| pi_cloud_instance_id = var.cloud_instance_id | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| output "bootstrap_ip" { | ||
| value = data.ibm_pi_instance_ip.bootstrap_ip.ip | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| variable "memory" {} | ||
| variable "processors" {} | ||
| variable "ignition" {} | ||
|
|
||
| variable "cloud_instance_id" {} | ||
| variable "resource_group" {} | ||
| variable "image_name" {} | ||
| variable "network_name" {} | ||
| variable "proc_type" {} | ||
| variable "sys_type" {} | ||
| variable "cluster_id" {} | ||
| variable "key_id" {} | ||
|
|
||
| variable "cos_instance_location" {} | ||
| variable "cos_bucket_location" {} | ||
| variable "cos_storage_class" {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imageName shouldn't be required. this should come from the rhcos stream json once we have the pipeline setup and generating images
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm..are these the boot images and not the ovas? i think long term you want the bootimage creation handled by TF or by a separate component in the installer itself by consuming the ova image i would think. in any case because ovas are what are coming from the pipeline, this needs to be optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not clear on how to make this file show what we'd like to be required in the install-config.yaml. These seem to be pulled from the Config (type MachinePool) in the install config. Those items are required (see https://github.com/openshift/terraform-provider-ibm/blob/master/website/docs/r/pi_instance.html.markdown). But they're not required in the install-config.yaml itself.
To answer your questions though, -- yes, they should come from the rhcos stream json by default; and yes, they are the boot images; and yes, long-term the bootimage creation will be handled by TF.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore that. I'll look into this :D
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah. i just wasn't looking at the file as a whole. there's a compute section, a machinepools section, and a platform section all for powervs. and i didn't understand how the +optional directives would be used for each of those things w/in the code. after looking more closely, iiuc, every time i listed something as +optional, it was referring to it being defined as optional in the install config. so i'll go back and update those.
this leads to the question though -- for everything that shows up in this yaml (install.openshift.io_installconfigs.yaml) -- we don't have to make it configurable via the install-config.yaml do we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this is my view - things that can be configured optionally (for example through providerspec in the manifests) should not even be included in the install config. the user can modify them after doing a
openshift-install create manifestsand modifying the provider spec which will then be pulled in by the tfvars for master and for workers, it will be handled by the cluster api provider. so IMO those options shouldn't even be exposed here. Only things which are absolutely essential to change and makes sense to do so should be mentioned here like image name, region and even for that we need to have defaults. basically we are trying to make the users life easier and not complicate it by providing too many options even though they are optional.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so tl;dr is don't even define the ones that are optional in
pkg/types/powervs/platform.go. they can be changed through the provider spec defined in cluster-api-provider-powervs. only add things to the platform.go that absolutely must be exposed to the user through install-config