feat(cluster)!: use min_nodes instead of node_count#228
Merged
Conversation
100c97c to
2023d9e
Compare
ac9ffde to
39d52ff
Compare
3a0e203 to
4d880f6
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a breaking schema change for the scylladbcloud_cluster resource to model cluster sizing dynamically: node_count becomes computed, while min_nodes becomes the user-controlled input that triggers resize operations.
Changes:
- Replaces
node_count(static/ForceNew) withmin_nodes(updatable) and makesnode_countcomputed from actual active nodes. - Adds resize support in the Scylla client and implements update behavior to issue a resize request and wait for completion.
- Updates acceptance tests and state migration (SchemaVersion 2) to support upgrading existing state from
node_counttomin_nodes.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tfcontext/context.go | Removes provider endpoint logging helper; keeps HTTP request info fields. |
| internal/scylla/endpoints.go | Adds ResizeCluster and refactors ListClusterRequest to accept a params struct. |
| internal/provider/serverless/serverless_cluster.go | Updates cluster-request polling calls to new ListClusterRequest/wait helper naming. |
| internal/provider/provider_test.go | Updates/extends acceptance tests for min_nodes behavior and migration coverage. |
| internal/provider/cluster/cluster_v1.go | Adds schema v1 upgrader to migrate node_count → min_nodes. |
| internal/provider/cluster/cluster_v0.go | Moves v0 state upgrade logic into versioned file (adds disk size upgrade). |
| internal/provider/cluster/cluster.go | Implements SchemaVersion 2, adds min_nodes, adds resize-based Update, and introduces request-wait helpers. |
| go.mod / go.sum | Adds testify and updates indirect deps for new test helpers. |
| Makefile | Tweaks test timeouts/parallelism and adds a target to list acceptance tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a190e24 to
ddd2176
Compare
node_count is a static value which does not describe the reality. ScyllaDB clusters are dynamic and they scale out automatically when the disk is running out of scape. node_count also can't be used to resize the cluster manually as any change to it results in recreation of the resource. This change introduces min_nodes and makes node_count a computed property. min_nodes can be used for manual resizing as well. Note that there is no max_nodes as the platform does not support it at the moment.
… updates Now that min_nodes is updatable in-place, the original reason for commenting out ForceNew on enable_dns no longer applies (not all user-facing attrs are ForceNew anymore). Without ForceNew, a change to enable_dns is routed to Update which silently returns nil without making any API call, leaving the cluster DNS setting out of sync with state. Co-Authored-By: Claude <noreply@anthropic.com>
- Update docs/index.md to reflect that horizontal scaling is now supported via min_nodes attribute, removing outdated limitation - Add documentation for scale-in behavior (may fail if insufficient disk space) - Add 'Running Tests' section to CONTRIBUTING.md with instructions for unit tests and acceptance tests - Document required environment variables including SCYLLADB_CLOUD_GCP_BYOA_ID for GCP BYOA tests
ddd2176 to
70451a4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
node_countis a static value which does not describethe reality. ScyllaDB clusters are dynamic and
they scale out automatically when the disk is
running out of scape.
node_countalso can't be used to resize the clustermanually as any change to it results in recreation
of the resource.
This change introduces
min_nodesand makesnode_counta computed property.min_nodesissues a resize request and waits for the completion.
Note that there is no max_nodes as the platform
does not support it at the moment.
Summary
node_countbecomes a computed property.min_nodescontrols the minimum number of nodes. It works this way that any change tomin_nodes, after the resource is created, issues a resize request. The value is updated only if the resize request succeeds.Todo
node_countshould be updated but the plan should be empty. Next, updatemin_nodesto equalnode_count. Expectation:min_nodesshould be updated, but no resize should be triggered.min_nodes = 6, scale-in using API, refresh the TF state. Expectations:node_countshould be updated, the plan should result in resize.