-
Notifications
You must be signed in to change notification settings - Fork 4.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
[New Resource] - azurerm_postgresql_flexible_server_virtual_endpoint
#26708
Conversation
azurerm_postgresql_flexible_server_virtual_endpoint
…endpoint_resource_test.go use standard error message Co-authored-by: stephybun <[email protected]>
…endpoint_resource.go use standard pointer conversion Co-authored-by: stephybun <[email protected]>
…endpoint_resource_test.go use more explicit pointer conversion on return type Co-authored-by: stephybun <[email protected]>
…endpoint_resource.go Use consistent styling Co-authored-by: stephybun <[email protected]>
…endpoint_resource_test.go remove unnecessary test conditions Co-authored-by: stephybun <[email protected]>
|
||
data.ResourceTest(t, r, []acceptance.TestStep{ | ||
{ | ||
Config: r.update(data, "azurerm_postgresql_flexible_server.test_replica_0.id"), |
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.
We try to be as explicit as possible in the test configurations and don't pass updated ID values in as arguments to the config. This also fails at the moment:
------- Stdout: -------
=== RUN TestAccPostgresqlFlexibleServerVirtualEndpoint_update
=== PAUSE TestAccPostgresqlFlexibleServerVirtualEndpoint_update
=== CONT TestAccPostgresqlFlexibleServerVirtualEndpoint_update
testcase.go:121: Step 1/4 error: Error running pre-apply plan: exit status 1
Error: length should be equal to or less than 63, got "acctest-psql-virtualendpoint-primary-240807125523377850-replica-0"
with azurerm_postgresql_flexible_server.test_replica_0,
on terraform_plugin_test.tf line 51, in resource "azurerm_postgresql_flexible_server" "test_replica_0":
51: name = "${azurerm_postgresql_flexible_server.test.name}-replica-0"
--- FAIL: TestAccPostgresqlFlexibleServerVirtualEndpoint_update (15.02s)
FAIL
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.
That's interesting, it was passing locally. Easy enough to shorten the name though.
Would you recommend passing in the full virtual endpoint resources rather than just the ID? Can you point me towards something more in line with what you're looking for?
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.
I'm not sure I'll be able to find an example quickly for this specific case, but typically update tests will take advantage of an already existing basic
or complete
config, so the acceptance test would like:
func TestAccPostgresqlFlexibleServerVirtualEndpoint_update(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_postgresql_flexible_server_virtual_endpoint", "test")
r := PostgresqlFlexibleServerVirtualEndpointResource{}
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.basic(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
{
Config: r.update(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}
But on second though it's probably sufficient to just shorten the name to prevent the error.
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.
I decreased the length of resource names, and the tests all passed on the TC server
internal/services/postgres/postgresql_flexible_server_virtual_endpoint_resource.go
Outdated
Show resolved
Hide resolved
…endpoint_resource.go Co-authored-by: stephybun <[email protected]>
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.
Thanks @bruceharrison1984 LGTM 👍
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
This adds support to Flexible Postgres Servers for creating Virtual Endpoints.
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_postgresql_flexible_server_virtual_endpoint
- New Resource added [Support for virtual endpoints for Postgres Flexible Servers #25803]This is a (please select all that apply):
Related Issue(s)
Fixes #25803