[DO NOT MERGE] Adds Throughput to CosmosDB Resources

[k-k]

As a follow up to #4467, this would add the ability to specify throughput on the remaining, supported CosmosDB resources. This would resolve #3623.

[k-k]

@katbyte As mentioned in my previous PR, followed up to add throughput to the other ComosDB resources that supported it.

cc @ragoragino

[k-k]

@katbyte I had to fix the tests in Travis and doing so, I ran the Acceptance tests as well. I actually get an 404 error from each of the Throughput methods exposed in the Go SDK. At first, I thought it was from the initial create not blocking before I was attempting to update throughput.

However, I opened up my REST client and tried the underlying rest endpoints directly and receive a 404 from the route on an existing resource - I don't believe these endpoints work as expected.

For example, I created a CosmosDB Account with a SQL Database and used the API to verify it existed, which it did. I then hit the endpoint to fetch it's throughput and I get a 404 - its very reproducible.

This would actually block this PR and we should probably revert the change from #4467 until the API is fixed.

[katbyte]

@kmfk,

The tests for #4467 don't fail with a 404, in fact i can see the throughput is actually being read & updated correctly. (it fails due to indexs not matching)

Maybe this is a transient issue or due to the region you are using? (we run our tests in west europe)

[ragoragino]

@katbyte, @kmfk

We have been also getting 404 with an update of the MongoDB database after its creation.

It seems that the database needs to be created with the throughput property in order to be applicable to all collections. If it is only updated on throughput property (without the creation including that property), then the REST request fails with 404.

Here is the gist that reproduces the issue, where the first MongoDB database is created with throughput property (and succeeds), while the second one is only updated after its creation (and only creation succeeds, the update returns 404). If I modify the second case and create the database with the throughput property, the update also succeeds):

https://gist.github.com/ragoragino/7f6488a34f0b2d7fda235afcb40fd74b

The header "x-ms-offer-throughput" needs to be sent in the request as documented in https://docs.microsoft.com/en-us/rest/api/cosmos-db/create-a-collection for the creation of the collection with the throughput property to work correctly (however, the feature seems to be documented only for collections! It seems SDK is not explicit about the possibility of using it for databases, but it works).

[k-k]

@ragoragino I had done some more testing last night and mostly agree with your summary. It does seem like Mongo Collections and SQL Containers can actually update throughput even if they were first created without throughput explicitly set at creation - regardless of if the Database was created with throughput explicitly set.

While the PUT endpoint is essentially an upsert (create/update) - my testing found that if you create a database without explicitly setting throughput initially, subsequent PUT calls trying to add throughput wouldn't error - but would still not update the throughput. The throughput endpoints will still 404.

So, this aligns with what @katbyte said - the MongoDB Collection succeeded when I changed only that resource. The acceptance test now fails due to the changes above on the MongoDB Database resource - since the collection has a dependency on it and the Database creation fails.

While I could try to catch the 404 errors, it seems like if throughput was not set initially, setting it on an existing Database / Keyspace resource would require a forceNew. I can also change this to always set the header to add throughput - but that wouldn't help existing resources created with an earlier version of the provider.

Finally, since (to my knowledge) there is no way to do a conditional forceNew, I think it would mean we'd have to set forceNew on throughput for the Database and Keyspace resources to maintain backwards compatibility - even though they should technically be able to be modified in place if they were created with throughput explicitly set.

Sorry for the wall of text - thoughts or anything I'm missing?

[pmccloghrylaing]

I'm keen to see this feature get in. Would failing the update and providing an appropriate error be a solution to this, instead of forceNew? Something that lets the user know that they'll need to recreate the database in order to set throughput. If it only happens when encountering the 404 error it also means that if/when the Azure API allows these updates they'll start working and the error won't be displayed.

[katbyte]

@kmfk, @pmcatominey,

I've been thinking about it over the weekend and I agree. I think what we could do is:

• open an issue on the azure-for-go SDK detailing that the update fails in this situation
• if we get a error wrap it in our own error "Error updating throughput, was this collection/db/etc created with a custom throughput? you may need to recreate it. for more details http://url_to_sdk_issue"
• update the docs

WDYT @kmfk?

[katbyte]

@kmfk,

I've opened #5116 to address issues with the existing throughput properties. That technique can now be used for the remaining cosmos resources.

[aristosvo]

Thanks for your work on de cosmosdb resources!

edit: didn't see the other pull request, sorry

[katbyte]

Hi @kmfk,

I hope you don't mind but i have opened #5203 with the required changes, as such i'm going to close this PR. Thanks again for all the work you put in!

[k-k]

@katbyte Nope, sorry I didn't get back to this - thanks for pushing it forward!

[ghost]

This has been released in version 1.40.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.40.0"
}
# ... other configuration ...

[ghost]

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 👉 hashibot-feedback@hashicorp.com. Thanks!