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

Updating terraform and k8s files adding redis... #7425

Merged

Conversation

LumosViridi
Copy link
Contributor

Also updated the way secrets are generated with Terraform and some code cleanup

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 updates the Kubernetes and Terraform configurations for the TwentyCRM application, introducing Redis and improving the overall architecture. Here are the key changes and potential issues:

  • Added Redis deployment and service configurations in both Kubernetes manifests and Terraform files
  • Updated server and worker deployments to include Redis configuration and new environment variables
  • Introduced new persistent volumes for Docker data
  • Improved secret management using Terraform's random resource for token generation
  • Updated variables.tf with new configuration options for Redis, replicas, and storage

Key points to consider:

  • Ensure Redis persistence is properly configured to prevent data loss on pod restarts
  • Review the resource limits and requests for Redis, server, and worker deployments
  • Verify that the new persistent volume configurations are appropriate for the target environment
  • Double-check the dependencies between deployments to ensure proper startup order
  • Consider implementing Redis authentication for improved security

17 file(s) reviewed, 16 comment(s)
Edit PR Review Bot Settings

packages/twenty-docker/k8s/manifests/deployment-redis.yaml Outdated Show resolved Hide resolved
packages/twenty-docker/k8s/manifests/deployment-redis.yaml Outdated Show resolved Hide resolved
packages/twenty-docker/k8s/terraform/variables.tf Outdated Show resolved Hide resolved
packages/twenty-docker/k8s/terraform/variables.tf Outdated Show resolved Hide resolved
@LumosViridi
Copy link
Contributor Author

@charlesBochet / @FelixMalfait Hey! Took me a while to get this updated but this should include all of the changes needed up through v0.30.0 for storage, redis, pg-boss, and the worker 😊

Do you mind taking a look when you have a min?

@FelixMalfait
Copy link
Member

Oh wow, thanks @LumosViridi!

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.

Thank you, this is awesome!

I have left a few comments but approving as it LGTM :)

@charlesBochet
Copy link
Member

Merging it, thank you!

@charlesBochet charlesBochet merged commit db9ec58 into twentyhq:main Oct 7, 2024
4 checks passed
Copy link

github-actions bot commented Oct 7, 2024

Fails
🚫

node failed.

Log

�[31mError: �[39m RequestError [HttpError]: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 4C87:17AE92:659D22:C79C3D:67039ABB.
    at /home/runner/work/twenty/twenty/node_modules/�[4m@octokit�[24m/request/dist-node/index.js:86:21
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m {
  status: �[33m403�[39m,
  response: {
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3ALumosViridi%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    status: �[33m403�[39m,
    headers: {
      �[32m'access-control-allow-origin'�[39m: �[32m'*'�[39m,
      �[32m'access-control-expose-headers'�[39m: �[32m'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'�[39m,
      connection: �[32m'close'�[39m,
      �[32m'content-encoding'�[39m: �[32m'gzip'�[39m,
      �[32m'content-security-policy'�[39m: �[32m"default-src 'none'"�[39m,
      �[32m'content-type'�[39m: �[32m'application/json; charset=utf-8'�[39m,
      date: �[32m'Mon, 07 Oct 2024 08:24:27 GMT'�[39m,
      �[32m'referrer-policy'�[39m: �[32m'origin-when-cross-origin, strict-origin-when-cross-origin'�[39m,
      server: �[32m'github.com'�[39m,
      �[32m'strict-transport-security'�[39m: �[32m'max-age=31536000; includeSubdomains; preload'�[39m,
      �[32m'transfer-encoding'�[39m: �[32m'chunked'�[39m,
      vary: �[32m'Accept-Encoding, Accept, X-Requested-With'�[39m,
      �[32m'x-content-type-options'�[39m: �[32m'nosniff'�[39m,
      �[32m'x-frame-options'�[39m: �[32m'deny'�[39m,
      �[32m'x-github-api-version-selected'�[39m: �[32m'2022-11-28'�[39m,
      �[32m'x-github-media-type'�[39m: �[32m'github.v3; format=json'�[39m,
      �[32m'x-github-request-id'�[39m: �[32m'4C87:17AE92:659D22:C79C3D:67039ABB'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1728289527'�[39m,
      �[32m'x-ratelimit-resource'�[39m: �[32m'search'�[39m,
      �[32m'x-ratelimit-used'�[39m: �[32m'1'�[39m,
      �[32m'x-xss-protection'�[39m: �[32m'0'�[39m
    },
    data: {
      documentation_url: �[32m'https://docs.github.com/free-pro-team@latest/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits'�[39m,
      message: �[32m'You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 4C87:17AE92:659D22:C79C3D:67039ABB.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3ALumosViridi%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    headers: {
      accept: �[32m'application/vnd.github.v3+json'�[39m,
      �[32m'user-agent'�[39m: �[32m'octokit-rest.js/18.12.0 octokit-core.js/3.6.0 Node.js/18.20.4 (linux; x64)'�[39m,
      authorization: �[32m'token [REDACTED]'�[39m
    },
    request: { hook: �[36m[Function: bound bound register]�[39m }
  }
}
danger-results:https://tmp/danger-results-303e6ad4.json

Generated by 🚫 dangerJS against ffcd5fe

harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
Also updated the way secrets are generated with Terraform and some code
cleanup
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