-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
provider/aws: Allow port
on aws_db_instance
to be updated
#7441
Conversation
log.Printf("[DEBUG] Send DB Instance Modification request: %t", requestUpdate) | ||
if requestUpdate { | ||
log.Printf("[DEBUG] DB Instance Modification request: %s", req) | ||
_, err := conn.ModifyDBInstance(req) | ||
if err != nil { | ||
return fmt.Errorf("Error modifying DB Instance %s: %s", d.Id(), err) | ||
} | ||
|
||
log.Println("[INFO] Waiting for DB Instance to be available") |
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.
Probably change this to be "updated" instead of available. Does this refresh change anything else? Meaning, if you have apply immediately, does it wait until the change is applied?
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.
The state of the database goes back to available
after it has finished updating
This will wait until the changes have applied - the update didn't before
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.
The state of the database goes back to available after it has finished updating
I think this is true for some attributes, like port
, which happen to change quickly. I'm not sure what others there that behave like this, but things like allocated_storage
do not. We still see a plan/diff on follow up if we're not using the apply_immediately
flag.
That said, it prevents unnecessary plan/diffs for things like port
, so we'll go with it 👍
Fixes #2439 Port used to ForceNew, it has now been changed to allow it to be updated. 2 changes were needed: 1. Setting the port back to state 2. Adding the wait for state function to the Update func ``` make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSDBInstance_portUpdate' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /vendor/) TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSDBInstance_portUpdate -timeout 120m === RUN TestAccAWSDBInstance_portUpdate --- PASS: TestAccAWSDBInstance_portUpdate (699.84s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 699.861s ```
081e211
to
06d6a5e
Compare
|
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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Fixes #2439
Port used to ForceNew, it has now been changed to allow it to be
updated. 2 changes were needed: