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

Lumosviridi v20 kubernetes updates #6356

Merged

Conversation

LumosViridi
Copy link
Contributor

Updates for v20+ and misc terraform bug fixes. Also refactored to use terraform variables instead of locals which helps with readability and ease of use for new users.

Terraform validation is currently passing:
Screenshot 2024-07-21 at 13 18 37

Additionally added terraform-docs to generate a more helpful README for terraform specific configuration.

Raw K8s manifests were updated with changes for v20+ as well.

Refactored to use vars instead of locals
Added variable descriptions
Misc bug fixed for terraform
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This pull request introduces updates for v20+ and various Terraform bug fixes, focusing on improving readability and ease of use by replacing local variables with Terraform input variables.

  • New Kubernetes Deployment: Added packages/twenty-docker/k8s/manifests/deployment-worker.yaml for the twentycrm-worker component.
  • Environment Variables: Updated packages/twenty-docker/k8s/manifests/deployment-server.yaml to include new environment variables (FRONT_BASE_URL, BACKEND_SERVER_URL, MESSAGE_QUEUE_TYPE).
  • Terraform Refactor: Replaced local variables with input variables across multiple Terraform files (deployment-db.tf, deployment-server.tf, variables.tf).
  • Documentation: Added packages/twenty-docker/k8s/terraform/.terraform-docs.yml and updated README.md to auto-generate documentation using terraform-docs.
  • Version Updates: Updated required Terraform version to >= 1.9.2 and Kubernetes provider version to >= 2.31.0 in main.tf.

19 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings


| Name | Description | Type | Default | Required |
|------|-------------|------|---------|:--------:|
| <a name="input_twentycrm_app_hostname"></a> [twentycrm\_app\_hostname](#input\_twentycrm\_app\_hostname) | The protocol, DNS fully qualified hostname, and port used to access TwentyCRM in your environemnt. Ex: https://crm.example.com:443 | `string` | n/a | yes |
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling: Typo in 'environment'.

Suggested change
| <a name="input_twentycrm_app_hostname"></a> [twentycrm\_app\_hostname](#input\_twentycrm\_app\_hostname) | The protocol, DNS fully qualified hostname, and port used to access TwentyCRM in your environemnt. Ex: https://crm.example.com:443 | `string` | n/a | yes |
The protocol, DNS fully qualified hostname, and port used to access TwentyCRM in your environment. Ex: https://crm.example.com:443

| <a name="input_twentycrm_token_refreshToken"></a> [twentycrm\_token\_refreshToken](#input\_twentycrm\_token\_refreshToken) | TwentyCRM refresh Token | `string` | n/a | yes |
| <a name="input_twentycrm_app_name"></a> [twentycrm\_app\_name](#input\_twentycrm\_app\_name) | A friendly name prefix to use for every component deployed. | `string` | `"twentycrm"` | no |
| <a name="input_twentycrm_db_image"></a> [twentycrm\_db\_image](#input\_twentycrm\_db\_image) | TwentyCRM image for database deployment. This defaults to latest. | `string` | `"twentycrm/twenty-postgres:latest"` | no |
| <a name="input_twentycrm_db_pv_capacity"></a> [twentycrm\_db\_pv\_capacity](#input\_twentycrm\_db\_pv\_capacity) | Storage capacity provisioned for database persistant volume. | `string` | `"10Gi"` | no |
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling: Typo in 'persistent'.

Suggested change
| <a name="input_twentycrm_db_pv_capacity"></a> [twentycrm\_db\_pv\_capacity](#input\_twentycrm\_db\_pv\_capacity) | Storage capacity provisioned for database persistant volume. | `string` | `"10Gi"` | no |
Storage capacity provisioned for database persistent volume.

| <a name="input_twentycrm_db_image"></a> [twentycrm\_db\_image](#input\_twentycrm\_db\_image) | TwentyCRM image for database deployment. This defaults to latest. | `string` | `"twentycrm/twenty-postgres:latest"` | no |
| <a name="input_twentycrm_db_pv_capacity"></a> [twentycrm\_db\_pv\_capacity](#input\_twentycrm\_db\_pv\_capacity) | Storage capacity provisioned for database persistant volume. | `string` | `"10Gi"` | no |
| <a name="input_twentycrm_db_pv_path"></a> [twentycrm\_db\_pv\_path](#input\_twentycrm\_db\_pv\_path) | Local path to use to store the physical volume if using local storage on nodes. | `string` | `""` | no |
| <a name="input_twentycrm_db_pvc_requests"></a> [twentycrm\_db\_pvc\_requests](#input\_twentycrm\_db\_pvc\_requests) | Storage capacity reservation for database persistant volume claim. | `string` | `"10Gi"` | no |
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling: Typo in 'persistent'.

Suggested change
| <a name="input_twentycrm_db_pvc_requests"></a> [twentycrm\_db\_pvc\_requests](#input\_twentycrm\_db\_pvc\_requests) | Storage capacity reservation for database persistant volume claim. | `string` | `"10Gi"` | no |
Storage capacity reservation for database persistent volume claim.

| <a name="input_twentycrm_namespace"></a> [twentycrm\_namespace](#input\_twentycrm\_namespace) | Namespace for all TwentyCRM resources | `string` | `"twentycrm"` | no |
| <a name="input_twentycrm_server_data_mount_path"></a> [twentycrm\_server\_data\_mount\_path](#input\_twentycrm\_server\_data\_mount\_path) | TwentyCRM mount path for servers application data. Defaults to '/app/docker-data'. | `string` | `"/app/docker-data"` | no |
| <a name="input_twentycrm_server_image"></a> [twentycrm\_server\_image](#input\_twentycrm\_server\_image) | TwentyCRM server image for the server deployment. This defaults to latest. This value is also used for the workers image. | `string` | `"twentycrm/twenty:latest"` | no |
| <a name="input_twentycrm_server_pv_capacity"></a> [twentycrm\_server\_pv\_capacity](#input\_twentycrm\_server\_pv\_capacity) | Storage capacity provisioned for server persistant volume. | `string` | `"10Gi"` | no |
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling: Typo in 'persistent'.

Suggested change
| <a name="input_twentycrm_server_pv_capacity"></a> [twentycrm\_server\_pv\_capacity](#input\_twentycrm\_server\_pv\_capacity) | Storage capacity provisioned for server persistant volume. | `string` | `"10Gi"` | no |
Storage capacity provisioned for server persistent volume.

| <a name="input_twentycrm_server_image"></a> [twentycrm\_server\_image](#input\_twentycrm\_server\_image) | TwentyCRM server image for the server deployment. This defaults to latest. This value is also used for the workers image. | `string` | `"twentycrm/twenty:latest"` | no |
| <a name="input_twentycrm_server_pv_capacity"></a> [twentycrm\_server\_pv\_capacity](#input\_twentycrm\_server\_pv\_capacity) | Storage capacity provisioned for server persistant volume. | `string` | `"10Gi"` | no |
| <a name="input_twentycrm_server_pv_path"></a> [twentycrm\_server\_pv\_path](#input\_twentycrm\_server\_pv\_path) | Local path to use to store the physical volume if using local storage on nodes. | `string` | `""` | no |
| <a name="input_twentycrm_server_pvc_requests"></a> [twentycrm\_server\_pvc\_requests](#input\_twentycrm\_server\_pvc\_requests) | Storage capacity reservation for server persistant volume claim. | `string` | `"10Gi"` | no |
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling: Typo in 'persistent'.

Suggested change
| <a name="input_twentycrm_server_pvc_requests"></a> [twentycrm\_server\_pvc\_requests](#input\_twentycrm\_server\_pvc\_requests) | Storage capacity reservation for server persistant volume claim. | `string` | `"10Gi"` | no |
Storage capacity reservation for server persistent volume claim.

packages/twenty-docker/k8s/terraform/variables.tf Outdated Show resolved Hide resolved
Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

I love it! Thank you @LumosViridi

@@ -76,7 +82,7 @@ spec:
stdin: true
tty: true
volumeMounts:
- mountPath: /app/.local-storage
Copy link
Member

Choose a reason for hiding this comment

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

we actually need both: .local-storage and docker-data

Copy link
Member

@FelixMalfait FelixMalfait left a comment

Choose a reason for hiding this comment

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

Thank you!

@FelixMalfait FelixMalfait merged commit c3bf94e into twentyhq:main Aug 8, 2024
2 of 3 checks passed
Copy link

github-actions bot commented Aug 8, 2024

Thanks @LumosViridi for your contribution!
This marks your 0th PR on the repo. You're top 0% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants