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

New Google Cloud SQL failover replica resource shouldn't try to delete root user #347

Closed
ryan-mf opened this issue Aug 22, 2017 · 7 comments · Fixed by #360
Closed

New Google Cloud SQL failover replica resource shouldn't try to delete root user #347

ryan-mf opened this issue Aug 22, 2017 · 7 comments · Fixed by #360
Assignees

Comments

@ryan-mf
Copy link

ryan-mf commented Aug 22, 2017

Terraform Version

0.10.2

Affected Resource(s)

Please list the resources as a list, for example:

  • google_sql_database_instance

Terraform Configuration Files

// GCP Cloud SQL Primary DB in Primary Region, Primary Zone
resource "google_sql_database_instance" "db-primary" {
  provider           = "google.gcp-${var.region_primary}"
  region             = "${var.sqlregion_primary}"
  database_version   = "MYSQL_5_7"

  settings {
    tier             = "db-n1-standard-1"

    database_flags {
        name         = "character_set_server"
        value        = "utf8mb4"
    }

    backup_configuration {
        binary_log_enabled = "true"
        enabled            = "true"
        start_time         = "18:00"
    }

    ip_configuration {
        require_ssl        = "true"
    }

    location_preference {
        zone               = "${var.region_primary_zone_primary}"
    }
  }
}

// GCP Cloud SQL Secondary DB in Primary Region, Secondary Zone
resource "google_sql_database_instance" "db-failover" {
  provider           = "google.gcp-${var.region_primary}"
  region             = "${var.sqlregion_primary}"
  database_version   = "MYSQL_5_7"
  master_instance_name = "${google_sql_database_instance.db-primary.name}"

  replica_configuration {
    connect_retry_interval = "30"
    failover_target        = "true"
  }

  settings {
    tier             = "db-n1-standard-1"

    database_flags {
        name         = "character_set_server"
        value        = "utf8mb4"
    }

    ip_configuration {
        require_ssl        = "true"
    }

    location_preference {
        zone               = "${var.region_primary_zone_secondary}"
    }
  }
}

Expected Behavior

The resource should be created without issue.

Actual Behavior

The resource is created, but with error output in red:

Error applying plan:

1 error(s) occurred:

* google_sql_database_instance.db-failover: 1 error(s) occurred:

* google_sql_database_instance.db-failover: Error, failed to delete default 'root'@'*' user, but the database was created successfully: googleapi: Error 400: The requested operation is not valid for a read-replica instance., errorReadReplicaInvalidOperation

Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.

Steps to Reproduce

  1. Create a database instance with resource type google_sql_database_instance
  2. Create a failover replica instance with almost all of the same settings, but remove the backup_configuration section and add the replica_configuration section.
  3. Watch error output.

Important Factoids

On page https://www.terraform.io/docs/providers/google/r/sql_database_instance.html, at the top in yellow it specifically says this resource type will delete the default 'root' user that Cloud SQL creates for you, and you need to define your own root user (which I do). However, this isn't a valid action for the failover replica.

References

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here? For example:

@selmanj
Copy link
Contributor

selmanj commented Aug 25, 2017

I'm unable to reproduce this. Previous history indicates that this may be fixed for newly-created instances, but existing ones may still have this issue. I don't suppose you could share a snippet of what's in your state file for this example (with anything sensitive removed of course)?

That said, it does seem reasonable to not attempt any sort of write modifications (of which deleting a root user is) on a replica as it will obviously fail.

@selmanj
Copy link
Contributor

selmanj commented Aug 25, 2017

OK, I was able to reproduce by creating only the master, adding a root@% user, and then creating the primary.

Working on a fix now.

@ryan-mf
Copy link
Author

ryan-mf commented Aug 29, 2017

Thanks @selmanj, I see now that your fix at #360 has been approved and passed tests. How long does it usually take to merge and get released?

@selmanj
Copy link
Contributor

selmanj commented Aug 29, 2017

#360 still needs at least one other committer to review and approve. Once that happens I can merge it in.

As for getting released, I'm not sure when the next release is but in general it seems to be around ~4 weeks. You can always build from master if you have an urgent need to see this working.

@selmanj
Copy link
Contributor

selmanj commented Sep 7, 2017

@ryan-mf The fix is now in master; hopefully should be released soon.

@ryan-mf
Copy link
Author

ryan-mf commented Sep 7, 2017

Awesome, thank you!

luis-silva pushed a commit to luis-silva/terraform-provider-google that referenced this issue May 21, 2019
…orp#347)

Reconcile Magician formatting changes due to Go tools upgrade
@ghost
Copy link

ghost commented Mar 31, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants