Scaling issue (#152)

ClickHouse · Sep 13, 2024 · a8b7490 · a8b7490
1 parent c29524a
commit a8b7490
Show file tree

Hide file tree

Showing 15 changed files with 676 additions and 531 deletions.
diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml
@@ -112,7 +112,7 @@ jobs:
         uses: ravsamhq/notify-slack-action@v2
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
-          status: ${{ job.status }}
+          status: 'failure'
           notification_title: "E2E tests failed for {branch}"
           footer: "{run_url}"
         env:

diff --git a/README.md b/README.md
@@ -12,7 +12,38 @@ You can find examples in the [examples/full](https://github.com/ClickHouse/terra
 
 Please refer to the [official docs](https://registry.terraform.io/providers/ClickHouse/clickhouse/latest/docs) for more details.
 
-## Breaking changes
+## Breaking changes and deprecations
+
+### Upgrading to version >= 1.1.0
+
+In version 1.1.0 we deprecated the `min_total_memory_gb` and `max_total_memory_gb` fields. You can keep using them, but they will eventually be removed.
+
+The intended replacement for those fields are:
+
+- `min_replica_memory_gb`: Minimum memory used by *each replica* during auto-scaling 
+- `max_replica_memory_gb`: Maximum memory used by *each replica* during auto-scaling
+
+The key difference between the old and new fields is that the old ones indicated a *total amount of memory* for the whole service (the sum of all replicas) while the new ones act on a *single replica*.
+
+For example, if you had a 3 replica cluster with the following settings:
+
+```terraform
+resource "clickhouse_service" "svc" {
+  ...
+  min_total_memory_gb = 24
+  max_total_memory_gb = 36
+}
+```
+
+you should convert it to
+
+```terraform
+resource "clickhouse_service" "svc" {
+  ...
+  min_replica_memory_gb = 8
+  max_replica_memory_gb = 12
+}
+```
 
 ### Upgrading to version >= 1.0.0 of the Clickhouse Terraform Provider
 
@@ -86,108 +117,6 @@ If everyting is fine, there should be no changes in existing infrastructure but
 
 If you have trouble, please open an issue and we'll try to help!
 
-## Development
-
-Create a new file called .terraformrc in your home directory (~), then add the dev_overrides block below. Change the `<PATH>` to the full path of the `tmp` directory in this repo. For example:
-
-```t
-provider_installation {
-
-  dev_overrides {
-      "ClickHouse/clickhouse" = "<PATH example /home/user/workdir/terraform-provider-clickhouse/tmp>"
-  }
-
-  # For all other providers, install them directly from their origin provider
-  # registries as normal. If you omit this, Terraform will _only_ use
-  # the dev_overrides block, and so no other providers will be available.
-  direct {}
-}
-```
-
-Ensure you have [`air`](https://github.com/air-verse/air) or install it with:
-
-```bash
-go install github.com/air-verse/air@latest
-```
-
-Run `air` to automatically build the plugin binary every time you make changes to the code:
-
-```bash
-$ air
-```
-
-You can now run `terraform` and you'll be using the locally built binary. Please note that the `dev_overrides` make it so that you have to skip `terraform init`).
-For example, go to the `examples/basic` directory and :
-
-```bash
-terraform apply -var-file="variables.tfvars"
-╷
-│ Warning: Provider development overrides are in effect
-│
-│ The following provider development overrides are set in the CLI configuration:
-│  - ClickHouse/clickhouse in /home/user/workdir/terraform-provider-clickhouse/tmp
-│
-│ The behavior may therefore not match any released version of the provider and applying changes may
-│ cause the state to become incompatible with published releases.
-╵
-
-Terraform used the selected providers to generate the following execution plan. Resource actions are
-indicated with the following symbols:
-  + create
-
-Terraform will perform the following actions:
-
-  # clickhouse_service.service will be created
-  + resource "clickhouse_service" "service" {
-      + cloud_provider       = "aws"
-      + id                   = (known after apply)
-      + idle_scaling         = true
-      + idle_timeout_minutes = 5
-      + ip_access            = [
-          + {
-              + description = "Test IP"
-              + source      = "192.168.2.63"
-            },
-        ]
-      + last_updated         = (known after apply)
-      + max_total_memory_gb  = 360
-      + min_total_memory_gb  = 24
-      + name                 = "My Service"
-      + region               = "us-east-1"
-      + tier                 = "production"
-    }
-
-Plan: 1 to add, 0 to change, 0 to destroy.
-
-Do you want to perform these actions?
-  Terraform will perform the actions described above.
-  Only 'yes' will be accepted to approve.
-
-  Enter a value:
-```
-
-
-Make sure to change the organization id, token key, and token secret to valid values.
-
-## Git hooks
-
-We suggest to add git hooks to your local repo, by running:
-
-```bash
-make enable_git_hooks
-```
-
-Code will be formatted and docs generated before each commit.
-
-## Docs
-
-If you made any changes to the provider's interface, please run `make docs` to update documentation as well.
-
-NOTE: this is done automatically by git hooks.
-
-## Release
+## Development and contributing
 
-To make a new public release:
-- ensure the `main` branch contains all the changes you want to release
-- Run the [`Release`](https://github.com/ClickHouse/terraform-provider-clickhouse/actions/workflows/release.yaml) workflow against the main branch (enter the desired release version in semver format without leading `v`, example: "1.2.3")
-- Release will be automatically created if end to end tests will be successful.
+Please read the [Development readme](https://github.com/ClickHouse/terraform-provider-clickhouse/blob/main/development/README.md)
diff --git a/development/README.md b/development/README.md
@@ -0,0 +1,97 @@
+# Development
+
+Quick start guide for contributors.
+
+## Preparation
+
+Create a new file called .terraformrc in your home directory (~), then add the dev_overrides block below. Change the `<PATH>` to the full path of the `tmp` directory in this repo. For example:
+
+```t
+provider_installation {
+
+  dev_overrides {
+      "ClickHouse/clickhouse" = "<PATH example /home/user/workdir/terraform-provider-clickhouse/tmp>"
+  }
+
+  # For all other providers, install them directly from their origin provider
+  # registries as normal. If you omit this, Terraform will _only_ use
+  # the dev_overrides block, and so no other providers will be available.
+  direct {}
+}
+```
+
+Ensure you have [`air`](https://github.com/air-verse/air) or install it with:
+
+```bash
+go install github.com/air-verse/air@latest
+```
+
+Run `air` to automatically build the plugin binary every time you make changes to the code:
+
+```bash
+$ air
+```
+
+You can now run `terraform` and you'll be using the locally built binary. Please note that the `dev_overrides` make it so that you have to skip `terraform init`).
+For example, go to the `examples/full/basic/aws` directory and :
+
+```bash
+terraform apply -var-file="variables.tfvars"
+╷
+│ Warning: Provider development overrides are in effect
+│
+│ The following provider development overrides are set in the CLI configuration:
+│  - ClickHouse/clickhouse in /home/user/workdir/terraform-provider-clickhouse/tmp
+│
+│ The behavior may therefore not match any released version of the provider and applying changes may
+│ cause the state to become incompatible with published releases.
+╵
+
+Terraform used the selected providers to generate the following execution plan. Resource actions are
+indicated with the following symbols:
+  + create
+
+Terraform will perform the following actions:
+
+  # clickhouse_service.service will be created
+  + resource "clickhouse_service" "service" {
+      ...
+    }
+
+Plan: 1 to add, 0 to change, 0 to destroy.
+
+Do you want to perform these actions?
+  Terraform will perform the actions described above.
+  Only 'yes' will be accepted to approve.
+
+  Enter a value:
+```
+
+
+Make sure to change the organization id, token key, and token secret to valid values.
+
+## Git hooks
+
+We suggest to add git hooks to your local repo, by running:
+
+```bash
+make enable_git_hooks
+```
+
+Code will be formatted and docs generated before each commit.
+
+## Docs
+
+If you made any changes to the provider's interface, please run `make docs` to update documentation as well.
+
+NOTE: this is done automatically by git hooks.
+
+## Release
+
+NOTE: Release process is only possible for ClickHouse employees.
+
+To make a new public release:
+
+- ensure the `main` branch contains all the changes you want to release
+- Run the [`Release`](https://github.com/ClickHouse/terraform-provider-clickhouse/actions/workflows/release.yaml) workflow against the main branch (enter the desired release version in semver format without leading `v`, example: "1.2.3")
+- Release will be automatically created if end to end tests will be successful.
diff --git a/docs/resources/service.md b/docs/resources/service.md
@@ -53,8 +53,10 @@ resource "clickhouse_service" "service" {
 - `encryption_key` (String) Custom encryption key arn
 - `idle_scaling` (Boolean) When set to true the service is allowed to scale down to zero when idle.
 - `idle_timeout_minutes` (Number) Set minimum idling timeout (in minutes). Must be greater than or equal to 5 minutes. Must be set if idle_scaling is enabled
-- `max_total_memory_gb` (Number) Maximum total memory of all workers during auto-scaling in Gb. Available only for 'production' services. Must be a multiple of 12 and lower than 360 for non paid services or 720 for paid services.
-- `min_total_memory_gb` (Number) Minimum total memory of all workers during auto-scaling in Gb. Available only for 'production' services. Must be a multiple of 12 and greater than 24.
+- `max_replica_memory_gb` (Number) Maximum memory of a single replica during auto-scaling in Gb. Available only for 'production' services. Must be a multiple of 8. `max_replica_memory_gb` x `num_replicas` (default 3) must be lower than 360 for non paid services or 720 for paid services.
+- `max_total_memory_gb` (Number, Deprecated) Maximum total memory of all workers during auto-scaling in Gb. Available only for 'production' services. Must be a multiple of 12 and lower than 360 for non paid services or 720 for paid services.
+- `min_replica_memory_gb` (Number) Minimum memory of a singe replica during auto-scaling in Gb. Available only for 'production' services. Must be a multiple of 8. `min_replica_memory_gb` x `num_replicas` (default 3) must be lower than 360 for non paid services or 720 for paid services.
+- `min_total_memory_gb` (Number, Deprecated) Minimum total memory of all workers during auto-scaling in Gb. Available only for 'production' services. Must be a multiple of 12 and greater than 24.
 - `num_replicas` (Number) Number of replicas for the service. Available only for 'production' services. Must be between 3 and 20. Contact support to enable this feature.
 - `password` (String, Sensitive) Password for the default user. One of either `password` or `password_hash` must be specified.
 - `password_hash` (String, Sensitive) SHA256 hash of password for the default user. One of either `password` or `password_hash` must be specified.

diff --git a/examples/full/basic/aws/main.tf b/examples/full/basic/aws/main.tf
@@ -21,6 +21,7 @@ resource "clickhouse_service" "service" {
   region                    = "us-east-1"
   tier                      = "production"
   idle_scaling              = true
+  idle_timeout_minutes      = 5
   password_hash             = "n4bQgYhMfWWaL+qgxVrQFaO/TxsrC4Is0V1sFbDwCgg=" # base64 encoded sha256 hash of "test"
 
   ip_access = [
@@ -30,9 +31,8 @@ resource "clickhouse_service" "service" {
     }
   ]
 
-  min_total_memory_gb  = 24
-  max_total_memory_gb  = 360
-  idle_timeout_minutes = 5
+  min_replica_memory_gb = 8
+  max_replica_memory_gb = 120
 }
 
 output "service_endpoints" {

diff --git a/examples/full/basic/azure/main.tf b/examples/full/basic/azure/main.tf
@@ -21,6 +21,7 @@ resource "clickhouse_service" "service" {
   region                    = "westus3"
   tier                      = "production"
   idle_scaling              = true
+  idle_timeout_minutes      = 5
   password_hash             = "n4bQgYhMfWWaL+qgxVrQFaO/TxsrC4Is0V1sFbDwCgg=" # base64 encoded sha256 hash of "test"
 
   ip_access = [
@@ -30,9 +31,8 @@ resource "clickhouse_service" "service" {
     }
   ]
 
-  min_total_memory_gb  = 24
-  max_total_memory_gb  = 360
-  idle_timeout_minutes = 5
+  min_replica_memory_gb  = 8
+  max_replica_memory_gb  = 120
 }
 
 output "service_endpoints" {

diff --git a/examples/full/basic/gcp/main.tf b/examples/full/basic/gcp/main.tf
@@ -21,6 +21,7 @@ resource "clickhouse_service" "service" {
   region                    = "europe-west4"
   tier                      = "production"
   idle_scaling              = true
+  idle_timeout_minutes      = 5
   password_hash             = "n4bQgYhMfWWaL+qgxVrQFaO/TxsrC4Is0V1sFbDwCgg=" # base64 encoded sha256 hash of "test"
 
   ip_access = [
@@ -30,9 +31,8 @@ resource "clickhouse_service" "service" {
     }
   ]
 
-  min_total_memory_gb  = 24
-  max_total_memory_gb  = 360
-  idle_timeout_minutes = 5
+  min_replica_memory_gb  = 8
+  max_replica_memory_gb  = 120
 }
 
 output "service_endpoints" {

diff --git a/examples/full/private_endpoint/aws/main.tf b/examples/full/private_endpoint/aws/main.tf
@@ -16,19 +16,19 @@ variable "service_name" {
 }
 
 resource "clickhouse_service" "aws_red" {
-  name           = var.service_name
-  cloud_provider = "aws"
-  region         = var.aws_region
-  tier           = "production"
-  idle_scaling   = true
-  password_hash  = "n4bQgYhMfWWaL+qgxVrQFaO/TxsrC4Is0V1sFbDwCgg=" # base64 encoded sha256 hash of "test"
+  name                 = var.service_name
+  cloud_provider       = "aws"
+  region               = var.aws_region
+  tier                 = "production"
+  idle_scaling         = true
+  idle_timeout_minutes = 5
+  password_hash        = "n4bQgYhMfWWaL+qgxVrQFaO/TxsrC4Is0V1sFbDwCgg=" # base64 encoded sha256 hash of "test"
 
   // keep it empty to block access from internet
   ip_access = []
 
-  min_total_memory_gb  = 24
-  max_total_memory_gb  = 360
-  idle_timeout_minutes = 5
+  min_replica_memory_gb = 8
+  max_replica_memory_gb = 120
 }
 
 // add AWS PrivateLink from VPC foo to organization

diff --git a/pkg/internal/api/client.go b/pkg/internal/api/client.go
@@ -258,6 +258,8 @@ func (c *ClientImpl) GetOrgPrivateEndpointConfig(ctx context.Context, cloudProvi
 }
 
 func (c *ClientImpl) CreateService(ctx context.Context, s Service) (*Service, string, error) {
+	// Needed until we have alignment between service creation and replicaScaling calls.
+	s.FixMemoryBounds()
 	rb, err := json.Marshal(s)
 	if err != nil {
 		return nil, "", err
@@ -338,13 +340,13 @@ func (c *ClientImpl) UpdateService(ctx context.Context, serviceId string, s Serv
 	return &serviceResponse.Result, nil
 }
 
-func (c *ClientImpl) UpdateServiceScaling(ctx context.Context, serviceId string, s ServiceScalingUpdate) (*Service, error) {
+func (c *ClientImpl) UpdateReplicaScaling(ctx context.Context, serviceId string, s ReplicaScalingUpdate) (*Service, error) {
 	rb, err := json.Marshal(s)
 	if err != nil {
 		return nil, err
 	}
 
-	req, err := http.NewRequest("PATCH", c.getServicePath(serviceId, "/scaling"), strings.NewReader(string(rb)))
+	req, err := http.NewRequest("PATCH", c.getServicePath(serviceId, "/replicaScaling"), strings.NewReader(string(rb)))
 	if err != nil {
 		return nil, err
 	}
@@ -415,7 +417,7 @@ func (c *ClientImpl) DeleteService(ctx context.Context, serviceId string) (*Serv
 		}
 	}
 
-	err = c.WaitForServiceState(ctx, serviceId, func(state string) bool { return state == StateStopped }, 300)
+	err = c.WaitForServiceState(ctx, serviceId, func(state string) bool { return state == StateStopped }, 10*60)
 	if err != nil {
 		return nil, err
 	}