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.

Sign up for GitHub

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

Jump to bottom

private_endpoints_service: wait for all regions #221

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+118 −39
Open

private_endpoints_service: wait for all regions #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 55 additions & 39 deletions internal/provider/private_endpoint_services_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ You must install/upgrade to [version 1.7.6](https://github.com/cockroachdb/terra
NestedObject: serviceObject,
},
"services": schema.ListNestedAttribute{
Computed: true,
Computed: true,
MarkdownDescription: "A list of regional private endpoint services for the cluster",
PlanModifiers: []planmodifier.List{
listplanmodifier.UseStateForUnknown(),
},
Expand Down Expand Up @@ -202,9 +203,13 @@ func (r *privateEndpointServicesResource) Create(
return
}
}
var services client.PrivateEndpointServices
err = retry.RetryContext(ctx, endpointServicesCreateTimeout,
waitForEndpointServicesCreatedFunc(ctx, cluster.Id, r.provider.service, &services))
var services *client.PrivateEndpointServices
var retryErr *retry.RetryError
err = retry.RetryContext(ctx, endpointServicesCreateTimeout, func() *retry.RetryError {
services, retryErr = arePrivateEndpointsServicesCreated(ctx, cluster, r.provider.service)
return retryErr
})

if err != nil {
resp.Diagnostics.AddError(
"Error enabling private endpoint services",
Expand All @@ -215,7 +220,7 @@ func (r *privateEndpointServicesResource) Create(
var state PrivateEndpointServices
state.ClusterID = plan.ClusterID
state.ID = plan.ClusterID
diags = loadEndpointServicesIntoTerraformState(ctx, &services, &state)
diags = loadEndpointServicesIntoTerraformState(ctx, services, &state)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
Expand Down Expand Up @@ -262,6 +267,7 @@ func loadEndpointServicesIntoTerraformState(
) diag.Diagnostics {
serviceList := apiServices.GetServices()
services := make([]PrivateEndpointService, len(serviceList))

for i, service := range serviceList {
services[i] = PrivateEndpointService{
RegionName: types.StringValue(service.GetRegionName()),
Expand All @@ -288,21 +294,20 @@ func loadEndpointServicesIntoTerraformState(
}
}
}
var diags diag.Diagnostics
state.Services, diags = types.ListValueFrom(

var diags, diags2 diag.Diagnostics
state.Services, diags2 = types.ListValueFrom(
ctx,
// Yes, it really does apparently need to be this complicated.
// https://github.com/hashicorp/terraform-plugin-framework/issues/713
endpointServicesSchema.Attributes["services"].(schema.ListNestedAttribute).NestedObject.Type(),
services,
)
diags.Append(diags2...)

servicesMap := map[string]PrivateEndpointService{}
for _, svc := range services {
servicesMap[svc.RegionName.ValueString()] = svc
}

var diags2 diag.Diagnostics
state.ServicesMap, diags2 = types.MapValueFrom(
ctx,
endpointServicesSchema.Attributes["services_map"].(schema.MapNestedAttribute).NestedObject.Type(),
Expand Down Expand Up @@ -339,44 +344,55 @@ func (r *privateEndpointServicesResource) ImportState(
resource.ImportStatePassthroughID(ctx, path.Root("cluster_id"), req, resp)
}

func waitForEndpointServicesCreatedFunc(
func arePrivateEndpointsServicesCreated(
ctx context.Context,
clusterID string,
cluster *client.Cluster,
cl client.Service,
services *client.PrivateEndpointServices,
) retry.RetryFunc {
return func() *retry.RetryError {
traceAPICall("ListPrivateEndpointServices")
apiServices, httpResp, err := cl.ListPrivateEndpointServices(ctx, clusterID)
if err != nil {
if httpResp != nil && httpResp.StatusCode < http.StatusInternalServerError {
return retry.NonRetryableError(fmt.Errorf("error getting endpoint services: %s", formatAPIErrorMessage(err)))
} else {
return retry.RetryableError(fmt.Errorf("encountered a server error while reading endpoint status - trying again: %v", err))
}
) (*client.PrivateEndpointServices, *retry.RetryError) {
// Set of all regions.
regions := func() map[string]bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think renaming this could help code readability later in this function

Suggested change
regions := func() map[string]bool {
pendingClusterRegions := func() map[string]bool {

m := map[string]bool{}
for _, r := range cluster.Regions {
m[r.Name] = true
}
return m
}()

if len(apiServices.Services) == 0 {
return retry.RetryableError(fmt.Errorf("private endpoint services not yet created"))
traceAPICall("ListPrivateEndpointServices")
services, httpResp, err := cl.ListPrivateEndpointServices(ctx, cluster.Id)
if err != nil {
if httpResp != nil && httpResp.StatusCode < http.StatusInternalServerError {
return nil, retry.NonRetryableError(fmt.Errorf("error getting endpoint services: %s", formatAPIErrorMessage(err)))
} else {
return nil, retry.RetryableError(fmt.Errorf("encountered a server error while reading endpoint status - trying again: %v", err))
}
}

*services = *apiServices
var creating bool
// If there's at least one still creating, keep checking.
// If any have failed or have any other non-available status, something went wrong.
// If all the services have been created, we're done.
for _, service := range services.GetServices() {
if service.GetStatus() == client.PRIVATEENDPOINTSERVICESTATUSTYPE_CREATING {
creating = true
} else if service.GetStatus() != client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE {
return retry.NonRetryableError(fmt.Errorf("endpoint service creation failed"))
}
if len(services.Services) == 0 {
return nil, retry.RetryableError(fmt.Errorf("private endpoint services not yet created"))
}

for _, service := range services.GetServices() {
// We don't care about services in other regions.
if _, ok := regions[service.GetRegionName()]; !ok {
continue
}
if creating {
return retry.RetryableError(fmt.Errorf("endpoint services are not ready yet"))

switch service.GetStatus() {
case client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE:
delete(regions, service.RegionName)
case client.PRIVATEENDPOINTSERVICESTATUSTYPE_CREATING:
return nil, retry.RetryableError(fmt.Errorf("endpoint services are not ready yet"))
default:
return nil, retry.NonRetryableError(fmt.Errorf("endpoint service creation failed"))
}
return nil
}

if len(regions) > 0 {
return nil, retry.RetryableError(fmt.Errorf("endpoint services are not ready yet"))
}

return services, nil
}

func NewPrivateEndpointServicesResource() resource.Resource {
Expand Down
63 changes: 63 additions & 0 deletions internal/provider/private_endpoint_services_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package provider

import (
"context"
"fmt"
"net/http"
"os"
Expand All @@ -27,7 +28,9 @@ import (
mock_client "github.com/cockroachdb/terraform-provider-cockroach/mock"
"github.com/golang/mock/gomock"
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/stretchr/testify/require"
)

// TestAccDedicatedPrivateEndpointServicesResource attempts to create, check,
Expand Down Expand Up @@ -268,6 +271,66 @@ func testPrivateEndpointServicesResource(
})
}

func TestArePrivateEndpointsServicesCreated(t *testing.T) {
ctx := context.Background()
ctrl := gomock.NewController(t)
s := mock_client.NewMockService(ctrl)

cluster := &client.Cluster{
Id: "1111",
Regions: []client.Region{
{Name: "us-east-1"},
{Name: "us-west-2"},
},
}

for _, tc := range []struct {
desc string
svcs []client.PrivateEndpointService
retryErr *retry.RetryError
}{
{
desc: "all available",
svcs: []client.PrivateEndpointService{
{RegionName: "us-east-1", Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE},
{RegionName: "us-west-2", Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE},
},
},
{
desc: "one service creating",
svcs: []client.PrivateEndpointService{
{RegionName: "us-east-1", Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE},
{RegionName: "us-west-2", Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_CREATING},
},
retryErr: retry.RetryableError(fmt.Errorf("endpoint services are not ready yet")),
},
{
desc: "extra regional service",
svcs: []client.PrivateEndpointService{
{RegionName: "us-east-1", Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE},
{RegionName: "us-west-2", Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE},
{RegionName: "us-central-2", Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_DELETING},
},
},
{
desc: "missing service",
svcs: []client.PrivateEndpointService{
{RegionName: "us-east-1", Status: client.PRIVATEENDPOINTSERVICESTATUSTYPE_AVAILABLE},
},
retryErr: retry.RetryableError(fmt.Errorf("endpoint services are not ready yet")),
},
} {
t.Run(tc.desc, func(t *testing.T) {
s.EXPECT().ListPrivateEndpointServices(gomock.Any(), "1111").Return(&client.PrivateEndpointServices{
Services: tc.svcs,
}, nil, nil)

_, retryErr := arePrivateEndpointsServicesCreated(ctx, cluster, s)
require.Equal(t, tc.retryErr, retryErr)
})
}
}

func getTestPrivateEndpointServicesResourceConfigForDedicatedGCP(clusterName string) string {
return fmt.Sprintf(`
resource "cockroach_cluster" "dedicated" {
Expand Down
Loading