Skip to content

Commit

Permalink
Changes for #558 - Integrate LB with backwards compatibility
Browse files Browse the repository at this point in the history
Signed-off-by: Chandan Abhyankar <chandan.abhyankar@gmail.com>
  • Loading branch information
Chandan-Abhyankar committed Dec 5, 2023
1 parent f750f1d commit 594da60
Show file tree
Hide file tree
Showing 11 changed files with 176 additions and 86 deletions.
11 changes: 7 additions & 4 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ resource "random_id" "label" {
locals {
cluster_id = var.cluster_id == "" ? random_id.label[0].hex : (var.cluster_id_prefix == "" ? var.cluster_id : "${var.cluster_id_prefix}-${var.cluster_id}")
# Generates vm_id as combination of vm_id_prefix + (random_id or user-defined vm_id)
name_prefix = var.name_prefix == "" ? "mac-${random_id.label[0].hex}" : "${var.name_prefix}"
node_prefix = var.use_zone_info_for_names ? "${var.powervs_zone}-" : ""
vpc_name = var.vpc_create ? "${local.name_prefix}-vpc" : var.vpc_name
name_prefix = var.name_prefix == "" ? "mac-${random_id.label[0].hex}" : "${var.name_prefix}"
node_prefix = var.use_zone_info_for_names ? "${var.powervs_zone}-" : ""
vpc_name = var.vpc_create ? "${local.name_prefix}-vpc" : var.vpc_name
skip_transit_gateway_create = var.ibm_cloud_cis == true ? true : false

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Change to skip_transit_gateway_create = var.ibm_cloud_cis

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

not sure - maybe we shouldn't even have that var?

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 5, 2023

Author Collaborator

We want to skip transit gateway creation if ibm_cloud_cis is false. So I added new variable in locals and used it at line 123.

skip_transit_gateway_create = var.ibm_cloud_cis [make sense, so will use this]

}

### Prepares the VPC Support Machine
Expand Down Expand Up @@ -106,6 +107,7 @@ module "vpc_gateway" {

### Prepares the VPC Support Machine
module "pvs_link" {
count = var.ibm_cloud_cis ? 0 : 1
providers = {
ibm = ibm.powervs
}
Expand All @@ -118,7 +120,7 @@ module "pvs_link" {
}

module "transit_gateway" {
count = var.skip_transit_gateway_create ? 0 : 1
count = local.skip_transit_gateway_create ? 0 : 1
providers = {
ibm = ibm.vpc
}
Expand Down Expand Up @@ -208,3 +210,4 @@ module "post" {
worker_3 = var.worker_3
cicd_image_pruner_cleanup = var.cicd_image_pruner_cleanup
}

21 changes: 21 additions & 0 deletions modules/7_post/haproxy_lb/.terraform.lock.hcl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,4 @@ ansible-playbook lb.yaml --extra-vars=@vars.yaml -i inventory
echo "Restart haproxy"
sleep 10
systemctl restart haproxy
echo "Done with the haproxy"
echo "Done with the haproxy"
92 changes: 92 additions & 0 deletions modules/7_post/haproxy_lb/haproxy_lb.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
################################################################
# Copyright 2023 - IBM Corporation. All rights reserved
# SPDX-License-Identifier: Apache-2.0
################################################################

locals {
ansible_post_path = "/root/ocp4-upi-compute-powervs-ibmcloud/post"
}

# Dev Note: only on destroy - restore the load balancers
resource "null_resource" "remove_lbs" {
# depends_on = [null_resource.patch_nfs_arch_ppc64le]

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Remove depends_on line

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 5, 2023

Author Collaborator

Sure


triggers = {
count_1 = var.worker_1["count"]
count_2 = var.worker_2["count"]
count_3 = var.worker_3["count"]
user = var.rhel_username
timeout = "${var.connection_timeout}m"
name_prefix = "${var.name_prefix}"
private_key = file(var.private_key_file)
host = var.bastion_public_ip
agent = var.ssh_agent
ansible_post_path = local.ansible_post_path
}

connection {
type = "ssh"
user = self.triggers.user
private_key = self.triggers.private_key
host = self.triggers.host
agent = self.triggers.agent
timeout = self.triggers.timeout
}

provisioner "remote-exec" {
inline = [<<EOF
mkdir -p /root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/
EOF
]
}

provisioner "file" {
source = "${path.module}/files/remove_lbs.sh"
destination = "/root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/remove_lbs.sh"
}

provisioner "remote-exec" {
when = destroy
on_failure = continue
inline = [<<EOF
cd /root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/
bash remove_lbs.sh
EOF
]
}
}

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Remove extra line

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 5, 2023

Author Collaborator

Sure


resource "null_resource" "updating_load_balancers" {
# depends_on = [null_resource.patch_nfs_arch_ppc64le, null_resource.remove_lbs]

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Must have depends_on

 depends_on = [null_resource.remove_lbs]

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 5, 2023

Author Collaborator

Sure

connection {
type = "ssh"
user = var.rhel_username
private_key = file(var.private_key_file)
host = var.bastion_public_ip
agent = var.ssh_agent
timeout = "${var.connection_timeout}m"
}

provisioner "remote-exec" {
inline = [<<EOF
mkdir -p /root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/
EOF
]
}

provisioner "file" {
source = "${path.module}/files/update_lbs.sh"
destination = "/root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/update_lbs.sh"
}

# Dev Note: Updates the load balancers
provisioner "remote-exec" {
inline = [<<EOF
cd /root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/
bash update_lbs.sh
EOF
]
}
}

5 changes: 5 additions & 0 deletions modules/7_post/haproxy_lb/outputs.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
################################################################
# Copyright 2023 - IBM Corporation. All rights reserved
# SPDX-License-Identifier: Apache-2.0
################################################################

16 changes: 16 additions & 0 deletions modules/7_post/haproxy_lb/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
################################################################
# Copyright 2023 - IBM Corporation. All rights reserved
# SPDX-License-Identifier: Apache-2.0
################################################################

variable "ssh_agent" {}
variable "connection_timeout" {}
variable "rhel_username" {}
variable "bastion_public_ip" {}
variable "private_key_file" {}
variable "vpc_region" {}
variable "vpc_zone" {}
variable "name_prefix" {}
variable "worker_1" {}
variable "worker_2" {}
variable "worker_3" {}
8 changes: 8 additions & 0 deletions modules/7_post/haproxy_lb/versions.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
################################################################
# Copyright 2023 - IBM Corporation. All rights reserved
# SPDX-License-Identifier: Apache-2.0
################################################################

terraform {
required_version = ">= 1.5.0"

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Shouldn't we be adding the null provider here?

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 5, 2023

Author Collaborator

It is defined in parent module 7_post/versions.tf, so I thought not needed in sub module haproxy_lb. Do you still think we need to add it?

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

If it's not complaining, that's great - it's no problem then

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 6, 2023

Author Collaborator

All right. So won't change code.

}
94 changes: 14 additions & 80 deletions modules/7_post/post.tf
Original file line number Diff line number Diff line change
Expand Up @@ -187,85 +187,19 @@ EOF
}
}

# Dev Note: only on destroy - restore the load balancers
resource "null_resource" "remove_lbs" {
depends_on = [null_resource.patch_nfs_arch_ppc64le]

triggers = {
count_1 = var.worker_1["count"]
count_2 = var.worker_2["count"]
count_3 = var.worker_3["count"]
user = var.rhel_username
timeout = "${var.connection_timeout}m"
name_prefix = "${var.name_prefix}"
private_key = file(var.private_key_file)
host = var.bastion_public_ip
agent = var.ssh_agent
ansible_post_path = local.ansible_post_path
}

connection {
type = "ssh"
user = self.triggers.user
private_key = self.triggers.private_key
host = self.triggers.host
agent = self.triggers.agent
timeout = self.triggers.timeout
}

provisioner "remote-exec" {
inline = [<<EOF
mkdir -p /root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/
EOF
]
}

provisioner "file" {
source = "${path.module}/files/remove_lbs.sh"
destination = "/root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/remove_lbs.sh"
}

provisioner "remote-exec" {
when = destroy
on_failure = continue
inline = [<<EOF
cd /root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/
bash remove_lbs.sh
EOF
]
}
module "haproxy_lb_support" {
source = "./haproxy_lb"

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Add depends_on = [null_resource.patch_nfs_arch_ppc64le]

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Should have a count =var.skip_transit_gateway_create ? 0 : 1

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 5, 2023

Author Collaborator

Sure, will add depends_on = [null_resource.patch_nfs_arch_ppc64le].
However do we want to skip haproxy_lb_support module based on var.skip_transit_gateway_create ? I thought it is always needed to modify haproxy.cfg whenever we add worker node.

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Right... you need to add the count to skip it.

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 6, 2023

Author Collaborator

[Can you please share more details?, my impression is 'haproxy_lb' module should always run, as it is taking care of modifying /etc/haproxy/haproxy.cfg. So as per my understanding, we don't need conditional count statement.]

ssh_agent = var.ssh_agent
rhel_username = var.rhel_username
connection_timeout = var.connection_timeout
bastion_public_ip = var.bastion_public_ip
private_key_file = var.private_key_file
vpc_region = var.vpc_region
vpc_zone = var.vpc_zone
name_prefix = var.name_prefix
worker_1 = var.worker_1
worker_2 = var.worker_2
worker_3 = var.worker_3
}


resource "null_resource" "updating_load_balancers" {
depends_on = [null_resource.patch_nfs_arch_ppc64le, null_resource.remove_lbs]
connection {
type = "ssh"
user = var.rhel_username
private_key = file(var.private_key_file)
host = var.bastion_public_ip
agent = var.ssh_agent
timeout = "${var.connection_timeout}m"
}

provisioner "remote-exec" {
inline = [<<EOF
mkdir -p /root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/
EOF
]
}

provisioner "file" {
source = "${path.module}/files/update_lbs.sh"
destination = "/root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/update_lbs.sh"
}

# Dev Note: Updates the load balancers
provisioner "remote-exec" {
inline = [<<EOF
cd /root/ocp4-upi-compute-powervs-ibmcloud/intel/lbs/
bash update_lbs.sh
EOF
]
}
}
6 changes: 5 additions & 1 deletion var.tfvars
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,8 @@ worker_3 = { count = "0", profile = "cx2-8x16", "zone" = "au-syd-3" }

# Public and Private Key for Bastion Nodes
public_key_file = "data/id_rsa.pub"
private_key_file = "data/id_rsa"
private_key_file = "data/id_rsa"

# Required for Ignition and Automation to Run
powervs_bastion_private_ip = "<Private IP Address of Bastion> e.g. 192.168.200.x"

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

and the contents to a comment above

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 5, 2023

Author Collaborator

Sure

powervs_bastion_ip = "<Public IP Address of Bastion>"
7 changes: 7 additions & 0 deletions variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ variable "powervs_network_name" {
}
}


################################################################
# Configure the Intel workers to be added to the compute plane
################################################################
Expand Down Expand Up @@ -351,3 +352,9 @@ variable "skip_route_creation" {
default = false
}

variable "ibm_cloud_cis" {
type = bool
description = "ibm_cloud_cis flag which indicates LoadBalancer and Security Groups are created by UPI automation"

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

remove ibm_cloud_cis from description

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Also change and Security Groups to and Transit Gateway

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 5, 2023

Author Collaborator

Ok, About description.
However I don't see CC and Transit Gateway is created by UPI automation. It is manual pre-requisite.

This comment has been minimized.

Copy link
@prb112

prb112 Dec 5, 2023

Member

Right. - no change.

This comment has been minimized.

Copy link
@Chandan-Abhyankar

Chandan-Abhyankar Dec 6, 2023

Author Collaborator

ok

default = false
}

1 comment on commit 594da60

@Chandan-Abhyankar
Copy link
Collaborator Author

@Chandan-Abhyankar Chandan-Abhyankar commented on 594da60 Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Paul, I have added my comments. Please re-check and provide your thoughts.

Please sign in to comment.