Skip to content
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

test: refactor basic test #567

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/basic/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ An end-to-end example that uses the module's default variable values. This examp

- Create a new resource group if one is not passed in.
- Create a new ICD Postgresql database instance.
- Create a read-only replica of the leader Postgresql database instance. For more info on Read-only Replicas, see [Configuring Read-only Replicas](https://cloud.ibm.com/docs/databases-for-postgresql?topic=databases-for-postgresql-read-only-replicas)
- Create a read-only replica of the leader database instance. For more info on Read-only Replicas, see [Configuring Read-only Replicas](https://cloud.ibm.com/docs/databases-for-postgresql?topic=databases-for-postgresql-read-only-replicas)
Comment on lines 6 to +7
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A few references to the ICD type (postgres) still remain in the readme for clarity

42 changes: 4 additions & 38 deletions examples/basic/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -11,50 +11,16 @@ module "resource_group" {
}

##############################################################################
# ICD postgresql database
# ICD database
##############################################################################

module "postgresql_db" {
module "database" {
source = "../.."
resource_group_id = module.resource_group.resource_group_id
name = "${var.prefix}-postgres"
pg_version = var.pg_version
name = "${var.prefix}-data-store"
pg_version = var.db_version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This becomes the only line in this file that needs to change from one ICD type to another.

region = var.region
resource_tags = var.resource_tags
access_tags = var.access_tags
member_host_flavor = var.member_host_flavor
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To support EDB, which does not support multitenant. Default is multitenant.

}

# On destroy, we are seeing that even though the replica has been returned as
# destroyed by terraform, the leader instance destroy can fail with: "You
# must delete all replicas before disabling the leader. Try again with valid
# values or contact support if the issue persists."
# The ICD team have recommended to wait for a period of time after the replica
# destroy completes before attempting to destroy the leader instance, so hence
# adding a time sleep here.

resource "time_sleep" "wait_time" {
depends_on = [module.postgresql_db]

destroy_duration = "5m"
}

##############################################################################
# ICD postgresql read-only-replica
##############################################################################

module "read_only_replica_postgresql_db" {
count = var.read_only_replicas_count
source = "../.."
resource_group_id = module.resource_group.resource_group_id
name = "${var.prefix}-read-only-replica-${count.index}"
region = var.region
resource_tags = var.resource_tags
access_tags = var.access_tags
pg_version = var.pg_version
remote_leader_crn = module.postgresql_db.crn
member_host_flavor = "multitenant"
member_memory_mb = 4096 # Must be an increment of 384 megabytes. The minimum size of a read-only replica is 2 GB RAM, new hosting model minimum is 4 GB RAM.
member_disk_mb = 5120 # Must be an increment of 512 megabytes. The minimum size of a read-only replica is 5 GB of disk
depends_on = [time_sleep.wait_time]
}
16 changes: 8 additions & 8 deletions examples/basic/outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,32 @@
# Outputs
##############################################################################
output "id" {
description = "Postgresql instance id"
value = module.postgresql_db.id
description = "Database instance id"
value = module.database.id
Comment on lines +5 to +6
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All outputs are generic. This file can be used unchanged in every ICD type

}

output "version" {
description = "Postgresql instance version"
value = module.postgresql_db.version
description = "Database instance version"
value = module.database.version
}

output "adminuser" {
description = "Database admin user name"
value = module.postgresql_db.adminuser
value = module.database.adminuser
}

output "hostname" {
description = "Database connection hostname"
value = module.postgresql_db.hostname
value = module.database.hostname
}

output "port" {
description = "Database connection port"
value = module.postgresql_db.port
value = module.database.port
}

output "certificate_base64" {
description = "Database connection certificate"
value = module.postgresql_db.certificate_base64
value = module.database.certificate_base64
sensitive = true
}
34 changes: 34 additions & 0 deletions examples/basic/replica.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
##############################################################################
# ICD read-only-replica extension for edb, mysql and postgresql
##############################################################################

# On destroy, after the replica has been returned as destroyed by terraform,
# the leader instance destroy can fail with: "You must delete all replicas
# before disabling the leader. Try again with valid values or contact support
# if the issue persists."
# The ICD team recommend to wait for a period of time after the replica
# destroy completes before attempting to destroy the leader instance, so
# hence adding a time sleep here.

resource "time_sleep" "wait_time" {
depends_on = [module.database]

destroy_duration = "5m"
}

##############################################################################
# ICD read-only-replica
##############################################################################

module "replica" {
source = "../.."
resource_group_id = module.resource_group.resource_group_id
name = "${var.prefix}-replica"
pg_version = var.db_version
region = var.region
resource_tags = var.resource_tags
access_tags = var.access_tags
member_host_flavor = var.member_host_flavor
remote_leader_crn = module.database.crn
depends_on = [time_sleep.wait_time]
}
Comment on lines +23 to +34
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to a new file. This file can be include in the few ICD types that support replicas and excluded from the others.

21 changes: 4 additions & 17 deletions examples/basic/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ variable "prefix" {
description = "Prefix to append to all resources created by this example"
}

variable "pg_version" {
description = "Version of the postgresql instance. If no value passed, the current ICD preferred version is used."
variable "db_version" {
description = "Version of the ICD instance. If no value passed, the current ICD preferred version is used."
type = string
default = null
}
Expand All @@ -34,25 +34,12 @@ variable "resource_tags" {

variable "access_tags" {
type = list(string)
description = "A list of access tags to apply to the PostgreSQL instance created by the module, see https://cloud.ibm.com/docs/account?topic=account-access-tags-tutorial for more details"
description = "A list of access tags to apply to the ICD instance created by the module, see https://cloud.ibm.com/docs/account?topic=account-access-tags-tutorial for more details"
default = []
}

variable "read_only_replicas_count" {
type = number
description = "Number of read-only replicas per leader"
default = 1
validation {
condition = alltrue([
var.read_only_replicas_count >= 1,
var.read_only_replicas_count <= 5
])
error_message = "There is a limit of five read-only replicas per leader"
}
}
Comment on lines -41 to -52
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unnecessary complication for an example. Hard code it to 1.


variable "member_host_flavor" {
type = string
description = "Allocated host flavor per member. For more information, see https://registry.terraform.io/providers/IBM-Cloud/ibm/latest/docs/resources/database#host_flavor"
default = null
default = "multitenant"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use multitenant for default, will need to override for EDB where it is not supported.

This file can be copied to all ICD types with no changes, but EDB should change the default.

}
40 changes: 0 additions & 40 deletions tests/other_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,26 +52,6 @@ func TestRunPointInTimeRecoveryDBExample(t *testing.T) {
assert.NotNil(t, output, "Expected some output")
}

func TestRunBasicExample(t *testing.T) {
t.Parallel()

options := testhelper.TestOptionsDefaultWithVars(&testhelper.TestOptions{
Testing: t,
TerraformDir: "examples/basic",
Prefix: "postgres",
BestRegionYAMLPath: regionSelectionPath,
ResourceGroup: resourceGroup,
TerraformVars: map[string]interface{}{
"pg_version": "13",
},
CloudInfoService: sharedInfoSvc,
})

output, err := options.RunTestConsistency()
assert.Nil(t, err, "This should not have errored")
assert.NotNil(t, output, "Expected some output")
}

func testPlanICDVersions(t *testing.T, version string) {
t.Parallel()

Expand Down Expand Up @@ -117,23 +97,3 @@ func TestRunCompleteExample(t *testing.T) {
assert.Nil(t, err, "This should not have errored")
assert.NotNil(t, output, "Expected some output")
}

func TestRunBasicExampleWithFlavorMultitenant(t *testing.T) {
t.Parallel()

options := testhelper.TestOptionsDefaultWithVars(&testhelper.TestOptions{
Testing: t,
TerraformDir: "examples/basic",
Prefix: "pg-flvr-multitenant",
BestRegionYAMLPath: regionSelectionPath,
ResourceGroup: resourceGroup,
TerraformVars: map[string]interface{}{
"member_host_flavor": "multitenant",
},
CloudInfoService: sharedInfoSvc,
})

output, err := options.RunTestConsistency()
assert.Nil(t, err, "This should not have errored")
assert.NotNil(t, output, "Expected some output")
}
41 changes: 21 additions & 20 deletions tests/pr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,27 @@ func TestMain(m *testing.M) {
os.Exit(m.Run())
}

func TestRunBasicExample(t *testing.T) {
t.Parallel()

options := testhelper.TestOptionsDefaultWithVars(&testhelper.TestOptions{
Testing: t,
TerraformDir: "examples/basic",
Prefix: "pg",
BestRegionYAMLPath: regionSelectionPath,
ResourceGroup: resourceGroup,
TerraformVars: map[string]interface{}{
"db_version": "13",
"member_host_flavor": "multitenant",
Comment on lines +54 to +55
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only difference to this test between ICD types is the vars.
Should pinned to minimum supported ICD version.
host flavor could probably be removed here... since it will default to a supported value.

},
CloudInfoService: sharedInfoSvc,
})

output, err := options.RunTestConsistency()
assert.Nil(t, err, "This should not have errored")
assert.NotNil(t, output, "Expected some output")
}

func TestRunFSCloudExample(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -76,26 +97,6 @@ func TestRunFSCloudExample(t *testing.T) {
options.TestTearDown()
}

func TestRunBasicExampleWithFlavor(t *testing.T) {
t.Parallel()

options := testhelper.TestOptionsDefaultWithVars(&testhelper.TestOptions{
Testing: t,
TerraformDir: "examples/basic",
Prefix: "postgres-flvr",
BestRegionYAMLPath: regionSelectionPath,
ResourceGroup: resourceGroup,
TerraformVars: map[string]interface{}{
"member_host_flavor": "b3c.4x16.encrypted",
},
CloudInfoService: sharedInfoSvc,
})

output, err := options.RunTestConsistency()
assert.Nil(t, err, "This should not have errored")
assert.NotNil(t, output, "Expected some output")
}

func TestRunStandardSolution(t *testing.T) {
t.Parallel()

Expand Down