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

[CC-30768] don't allow node_count for serverless cluster #255

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [1.11.0] - 2024-12-05
### Fixed

- Validate that `node_count` is not passed in with serverless cluster regions.
Also, clear up in the documentation that this is not allowed.

### Added

Expand Down
2 changes: 1 addition & 1 deletion docs/resources/cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ Required:

Optional:

- `node_count` (Number) Number of nodes in the region. Will always be 0 for serverless clusters.
- `node_count` (Number) Number of nodes in the region. Valid for Advanced clusters only.
- `primary` (Boolean) Set to true to mark this region as the primary for a serverless cluster. Exactly one region must be primary. Dedicated clusters expect to have no primary region.

Read-Only:
Expand Down
2 changes: 1 addition & 1 deletion docs/resources/cmek.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Required:

Optional:

- `node_count` (Number) Number of nodes in the region. Will always be 0 for serverless clusters.
- `node_count` (Number) Number of nodes in the region. Valid for Advanced clusters only.
- `primary` (Boolean) Set to true to mark this region as the primary for a serverless cluster. Exactly one region must be primary. Dedicated clusters expect to have no primary region.

Read-Only:
Expand Down
34 changes: 33 additions & 1 deletion internal/provider/cluster_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ var regionSchema = schema.NestedAttributeObject{
PlanModifiers: []planmodifier.Int64{
int64planmodifier.UseStateForUnknown(),
},
Description: "Number of nodes in the region. Will always be 0 for serverless clusters.",
Description: "Number of nodes in the region. Valid for Advanced clusters only.",
},
"primary": schema.BoolAttribute{
Optional: true,
Expand Down Expand Up @@ -376,6 +376,38 @@ func (r *clusterResource) ConfigValidators(_ context.Context) []resource.ConfigV
}
}

func (r *clusterResource) ValidateConfig(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) {

// ValidateConfig seems to be called multiple times, once before any values
// are known. When all values are unknown we can't convert to our cluster
// model because we use some custom fields such as Regions. Wait until
// things are defined before converting the config to our model.
var regions types.List
resp.Diagnostics.Append(req.Config.GetAttribute(ctx, path.Root("regions"), &regions)...)
if regions.IsUnknown() {
return
}

var cluster CockroachCluster
resp.Diagnostics.Append(req.Config.Get(ctx, &cluster)...)
if resp.Diagnostics.HasError() {
return
}

plan, err := derivePlanType(&cluster)
if err != nil {
resp.Diagnostics.AddError("Invalid Configuration", err.Error())
return
}
if plan != client.PLANTYPE_ADVANCED {
for i, region := range cluster.Regions {
if IsKnown(region.NodeCount) {
resp.Diagnostics.AddAttributeError(path.Root("regions").AtListIndex(i), "Invalid Attribute", "node_count is supported for ADVANCED clusters only.")
}
}
}
}

func derivePlanType(cluster *CockroachCluster) (client.PlanType, error) {
var planType client.PlanType
if IsKnown(cluster.Plan) {
Expand Down
24 changes: 24 additions & 0 deletions internal/provider/cluster_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,30 @@ func TestIntegrationServerlessClusterResource(t *testing.T) {
finalCluster client.Cluster
ignoreImportPaths []string
}{
{
name: "STANDARD clusters must not have a node_count",
createStep: func() resource.TestStep {
return resource.TestStep{
Config: fmt.Sprintf(`
resource "cockroach_cluster" "test" {
name = "%s"
cloud_provider = "GCP"
serverless = {}
regions = [
{
name = "us-central1"
},
{
name = "us-central1"
node_count = 1
},
]
}
`, clusterName),
ExpectError: regexp.MustCompile("node_count is supported for ADVANCED clusters only"),
}
},
},
{
name: "single-region serverless BASIC cluster converted to unlimited resources",
createStep: func() resource.TestStep {
Expand Down
Loading