feat: Add MySQL support #138

akashgangil · 2024-07-17T10:56:01Z

Introduces a new variable database_type to configure
the three tier web app to use either MySQL or PostgreSQL.

Resuses gcr.io/sic-container-repo/todo-api, due to this MySQL
instance authentication is configured via a databases user/password
instead of IAM service account like PG.

MySQL

module "three_tier_app" {
  source                 = "GoogleCloudPlatform/three-tier-web-app/google"
  version                = "~> 0.1"
  project_id             = <dummy>
  database_type          = "mysql"
  region                 = "us-central1"
  zone                   = "us-central1-a"
}

PostgreSQL

module "three_tier_app" {
  source                 = "GoogleCloudPlatform/three-tier-web-app/google"
  version                = "~> 0.1"
  project_id             = <dummy>
  database_type          =  "postgres"
  region                 = "us-central1"
  zone                   = "us-central1-a"
}

Introduces a new variable database_type

NimJay · 2024-07-23T18:20:50Z

/gcbrun

NimJay

Thanks so much for working on these updates, @akashgangil!
Overall, it's looks good. I've left some comments.

We'll also need to add an integration test for the MySQL option.
We'll need to:

add an /examples/mysql/ folder (see this suffix_example/ folder from another repo) (this folder will be auto-discovered by our integration tests) — it'll be mostly identical to the existing /examples/simple_example folder expect we'll need to pass in database_type = 'mysql'
generate the /examples/mysql/README.md by running make docker_generate_docs
update the /build/int.cloudbuild.yaml file (see example from another repo)
add a /test/integration/mysql/mysql_test.go file (see example from another repo here)

variables.tf

NimJay · 2024-07-24T22:00:28Z

main.tf

  instance        = google_sql_database_instance.main.name
  deletion_policy = "ABANDON"
+  name            = var.database_type == "postgres" ? "${google_service_account.runsa.account_id}@${var.project_id}.iam" : "foo"
+  type            = var.database_type == "postgres" ? "CLOUD_IAM_SERVICE_ACCOUNT" : null
+  password        = var.database_type == "mysql" ? "bar" : null


Question: Does MySQL allow us to use type = CLOUD_IAM_SERVICE_ACCOUNT? I want to see if we can use the same google_sql_user definition for both PostgreSQL and MySQL.

I tried it but didn't work for me just right out of the box. Maybe we can take this in a follow up PR if this is not blocking?

Agreed (let's move this type = CLOUD_IAM_SERVICE_ACCOUNT issue out of scope from this pull-request).
Issue created: #151

main.tf

variables.tf

NimJay · 2024-07-24T22:16:32Z

main.tf

@@ -19,7 +19,7 @@ data "google_project" "project" {
 }

 locals {
-  api_image = "gcr.io/sic-container-repo/todo-api-postgres:latest"
+  api_image = (var.database_type == "mysql" ? "gcr.io/sic-container-repo/todo-api" : "gcr.io/sic-container-repo/todo-api-postgres:latest")


Praise: Thanks for digging into the differences between these two images! :) I didn't realize to original 3-tier web app used MySQL.

NimJay · 2024-07-24T22:39:05Z

/gcbrun

akashgangil · 2024-09-11T10:50:46Z

Thanks so much for working on these updates, @akashgangil! Overall, it's looks good. I've left some comments.

We'll also need to add an integration test for the MySQL option. We'll need to:

add an /examples/mysql/ folder (see this suffix_example/ folder from another repo) (this folder will be auto-discovered by our integration tests) — it'll be mostly identical to the existing /examples/simple_example folder expect we'll need to pass in database_type = 'mysql'

generate the /examples/mysql/README.md by running make docker_generate_docs

update the /build/int.cloudbuild.yaml file (see example from another repo)

add a /test/integration/mysql/mysql_test.go file (see example from another repo here)

Done.

Thanks Nim for the review. Addressed all your comments

NimJay · 2024-09-12T15:26:04Z

/gcbrun (to trigger the integration tests)

This fixes lint errors.

Before this change, we would: 1. Test (deploy, verify, and destroy) simple_example 2. Wait for simple_example to destroy 3. Test two new (parallel) deployments of simple_example With this change, we now: 1. Test two deployments in parallel: the simple_example (which uses PostgreSQL); and the MySQL version.

NimJay · 2024-09-13T20:49:21Z

/gcbrun (to trigger the integration tests)

NimJay · 2024-09-13T21:05:27Z

/gcbrun

NimJay · 2024-09-13T21:16:11Z

/gcbrun

NimJay · 2024-09-13T22:30:48Z

/gcbrun

NimJay · 2024-09-14T00:06:47Z

/gcbrun

examples/mysql/main.tf

NimJay · 2024-09-14T15:15:37Z

/gcbrun

NimJay

I've updated the integration tests such that:

we now deploy the MySQL variation — in parallel to the PostgreSQL version
we no longer deploy the pre-existing "multiple instance" test since it's now redundant

I've also made some minor lint-related tweaks.

Thank you so much, @akashgangil, for doing this (and addressing all my feedback)! 👏 Everything looks good. Integration tests are passing. Approved. Merging.

Add mysql support to three tier web app

2686d8d

Introduces a new variable database_type

akashgangil requested review from tpryan and a team as code owners July 17, 2024 10:56

NimJay changed the title ~~Add mysql support to three tier web app~~ feat: Add MySQL support Jul 24, 2024

Merge branch 'main' into main

d9461bb

NimJay requested changes Jul 24, 2024

View reviewed changes

Akash Gangil added 3 commits September 11, 2024 10:34

address feedback

1fca721

update copyright year and readme nit

7405b79

nit

49d9d7a

akashgangil requested a review from NimJay September 11, 2024 11:01

NimJay added 6 commits September 12, 2024 11:36

Run terraform fmt (to fix lint erros)

2ea1d30

Run terraform fmt (to fix lint erros)

fb48428

Run terraform fmt (to fix lint erros)

a72590f

Remove unused database_type from variables.tf

6a1a119

This fixes lint errors.

Remove database_type from README.md of examples/mysql

3e64a37

Fix syntax error in mysql_test.go

faa35cf

NimJay added 2 commits September 13, 2024 17:15

Remove fmt (unused import)

0502943

Merge branch 'main' into main

93bf9af

NimJay self-assigned this Sep 13, 2024

Use different deployment_name for MySQL version

1b0dc07

NimJay added 2 commits September 13, 2024 20:02

Invoke Test() in mysql_test.go

b896950

Update mysql/README.md

8a1bb29

NimJay reviewed Sep 14, 2024

View reviewed changes

examples/mysql/main.tf Outdated Show resolved Hide resolved

Shorten deployment name in examples/mysql/main.tf

f9caaf9

NimJay mentioned this pull request Sep 16, 2024

Use CLOUD_IAM_SERVICE_ACCOUNT for MySQL variation #151

Open

NimJay approved these changes Sep 16, 2024

View reviewed changes

NimJay merged commit ab8330c into GoogleCloudPlatform:main Sep 16, 2024
4 checks passed

release-please bot mentioned this pull request Sep 16, 2024

chore(main): release 0.2.0 #127

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add MySQL support #138

feat: Add MySQL support #138

akashgangil commented Jul 17, 2024 •

edited

Loading

NimJay commented Jul 23, 2024

NimJay left a comment •

edited

Loading

NimJay Jul 24, 2024

akashgangil Sep 11, 2024

NimJay Sep 16, 2024 •

edited

Loading

NimJay Jul 24, 2024

NimJay commented Jul 24, 2024

akashgangil commented Sep 11, 2024

NimJay commented Sep 12, 2024

NimJay commented Sep 13, 2024

NimJay commented Sep 13, 2024

NimJay commented Sep 13, 2024

NimJay commented Sep 13, 2024

NimJay commented Sep 14, 2024

NimJay commented Sep 14, 2024

NimJay left a comment •

edited

Loading

feat: Add MySQL support #138

feat: Add MySQL support #138

Conversation

akashgangil commented Jul 17, 2024 • edited Loading

NimJay commented Jul 23, 2024

NimJay left a comment • edited Loading

Choose a reason for hiding this comment

NimJay Jul 24, 2024

Choose a reason for hiding this comment

akashgangil Sep 11, 2024

Choose a reason for hiding this comment

NimJay Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

NimJay Jul 24, 2024

Choose a reason for hiding this comment

NimJay commented Jul 24, 2024

akashgangil commented Sep 11, 2024

NimJay commented Sep 12, 2024

NimJay commented Sep 13, 2024

NimJay commented Sep 13, 2024

NimJay commented Sep 13, 2024

NimJay commented Sep 13, 2024

NimJay commented Sep 14, 2024

NimJay commented Sep 14, 2024

NimJay left a comment • edited Loading

Choose a reason for hiding this comment

akashgangil commented Jul 17, 2024 •

edited

Loading

NimJay left a comment •

edited

Loading

NimJay Sep 16, 2024 •

edited

Loading

NimJay left a comment •

edited

Loading