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

Add Helm Chart Support for Accent #406

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Braineanear
Copy link

Overview

This PR introduces a new Helm chart for the Accent, facilitating its deployment on Kubernetes clusters.

Changes Made

  • Created Chart.yaml with the basic definition of the Accent Helm chart.
  • Added a comprehensive README.md that includes installation instructions, default parameters, and usage details for the Helm chart.
  • Developed a NOTES.txt for post-installation instructions and helpful tips upon deploying the chart.
  • Set up essential Helm templates including configmap.yaml, deployment.yaml, hpa.yaml, ingress.yaml, and service.yaml to define the Kubernetes resources.
  • Defined default values in values.yaml, including image details, resource limits, liveness and readiness probes, service configuration, HPA settings, and more.

Implementation Details

  • The Chart.yaml outlines the basic information about the chart like version, description, and maintainers.
  • README.md serves as a comprehensive guide covering all aspects of the chart, ensuring ease of use.
  • Templates in k8s/helm/templates/ directory define the necessary Kubernetes resources and are configured to be customizable through values.yaml.
  • The configMap is used to externalize configuration settings, making the application more flexible and easier to manage in different environments.
  • Horizontal Pod Autoscaler (HPA) is configured but disabled by default, allowing users to enable it based on their scaling requirements.

Testing

  • The Helm chart has been tested locally for syntax correctness and successful deployment on a Kubernetes cluster.

Future Improvements

  • Integrate logging and monitoring configurations.
  • Add more customization options as needed based on feedback and application updates.

Notes

  • The ingress controller is disabled by default and can be enabled via the values.yaml.
  • Resource limits and requests are set to modest defaults and should be adjusted according to the deployment environment.

This addition paves the way for easier and more efficient Kubernetes deployments of the Accent. Your review and feedback on this implementation are highly appreciated.

Mahmoud Yasser
mle.mahmoud.yasser@gmail.com

@Braineanear
Copy link
Author

@simonprev

@simonprev
Copy link
Member

Hello @Braineanear , there is already a mention of an Helm chart in the README. Since we don’t use the chart (or Helm, or k8s) internally for this project at @mirego it would be hard to maintain/respond to issues. This is why we simply link to another repo.

Is your implementation different from the other repo? Could we use yours to have a more up-to-date version?

@Braineanear
Copy link
Author

@simonprev
Indeed, the approach we've adopted differs from that of the other repository. In our company, we utilize this particular version to gain enhanced management of Kubernetes resources and configurations. Furthermore, this implementation is aligned with the latest updates in the current accent version and its settings.
I am willing to oversee the updates and improvements of the Kubernetes configurations for accent in this repository, ensuring they are optimized and updated in line with the best practices for a production environment.

@nikkytub
Copy link

Thank you @Braineanear for the PR.
We want to deploy it in our dev env to explore it. Any updates here @simonprev? Can you please merge this PR?

@nikkytub
Copy link

Hi @Braineanear,
It seems like your helm chart doesn't include postgres installation. You are specifying the configuration here. However, the actual deployment here has no postgres container. At the moment with helm install, the pod restarts with the Backoff reason and the logs state the following:
* The database does not exist

Can you please add it?

Thanks & Kind Regards,
Nikhil

@Braineanear
Copy link
Author

Hey @nikkytub,

This configuration assumes that the user already has a PostgreSQL deployment in their cluster, and only needs to provide the database URL via the configMap. However, I can modify the deployment to include an option that allows the user to install PostgreSQL during the Accent installation on their cluster if they don't already have it. If that's what you need, please let me know, and I'll implement it.

@nikkytub
Copy link

Hi @Braineanear, the assumption of Postgres requires additional time and efforts and without proper documentation it becomes tedious. The Helm installation should be easy and straightforward that enables people to explore accent effortlessly. Hence, please include it and also add good documentation about it.

@Braineanear
Copy link
Author

@nikkytub okay gonna add it and update the docs.

@Braineanear
Copy link
Author

Hey @nikkytub
Do you have any idea why the container restart on these logs

Running migrations for accent…
13:13:45.935 [info] Migrations already up
Running seed script for accent…

It started successfully at the beginning but then when it reached the seed script, the container restarted and showed the above logs and always restarting only showing the above logs

@nikkytub
Copy link

Hi @Braineanear, I am not involved with the development of accent nor I am maintaining it. I think it is better if @simonprev or other contributors can answer here.

@nikkytub
Copy link

nikkytub commented May 13, 2024

Hi @Braineanear, I gave it a try. You are right, the pods are restarting and attached are the detailed logs.
logs.txt

@nikkytub
Copy link

@Braineanear, Haha, I found the problem. It was crashing due to OOM killed. Increase the memory and it will be fine :-)

New logs:
k logs accent-accent-674dd7b7c7-ftmpd -n accent E0513 18:18:15.862367 17695 memcache.go:287] couldn't get resource list for external.metrics.k8s.io/v1beta1: the server is currently unable to handle the request E0513 18:18:15.917614 17695 memcache.go:121] couldn't get resource list for external.metrics.k8s.io/v1beta1: the server is currently unable to handle the request E0513 18:18:15.948848 17695 memcache.go:121] couldn't get resource list for external.metrics.k8s.io/v1beta1: the server is currently unable to handle the request Running migrations for accent… 16:16:37.860 [info] Migrations already up Running seed script for accent… 16:16:45.363 [info] Loading 129 CA(s) from :otp store 16:16:45.368 [info] Running Accent.Endpoint with Bandit 1.3.0 at 0.0.0.0:4000 (http) 16:16:45.368 [info] Access Accent.Endpoint at http://localhost:4000 16:16:45.370 [info] LanguageTool was not configured. Use LANGUAGE_TOOL_LANGUAGES environment variable to set a list of comma-separated languages short code. 16:16:48.922 [info] tzdata release in place is from a file last modified Fri, 22 Oct 2021 02:20:47 GMT. Release file on server was last modified Thu, 01 Feb 2024 18:40:48 GMT. 16:16:50.416 [info] Tzdata has updated the release from 2021e to 2024a

kubectl get po -n accent NAME READY STATUS RESTARTS AGE accent-accent-674dd7b7c7-ftmpd 1/1 Running 0 2m17s accent-postgres-0 1/1 Running 0 45m

@Braineanear
Copy link
Author

Hi @nikkytub,

I've updated the code to allow the installation of PostgreSQL. Users can choose to install it alongside Accent or provide the connection string in the config map if they already have it set up.

I've also updated the documentation accordingly.

Regarding the seed command that restarts the pod, I've increased the pod's memory and CPU limits. However, the pod still restarts without displaying any error messages.

I'm currently debugging the issue and will push the updates once it's resolved.

@nikkytub
Copy link

nikkytub commented May 13, 2024

Hi @Braineanear,
It is working fine in our setup.
`
k get po -n accent

NAME READY STATUS RESTARTS AGE
accent-accent-674dd7b7c7-ftmpd 1/1 Running 0 5h26m
accent-postgres-0 1/1 Running 0 6h10m
This is what I used for resources:
resources:
limits:
cpu: 500m
memory: 1Gi
requests:
cpu: 100m
memory: 512Mi
`

Thank you so much for pushing new changes :-)

@Braineanear
Copy link
Author

@simonprev @nikkytub any updates

@nikkytub
Copy link

@simonprev @nikkytub any updates

Hi @Braineanear, Sorry, I got some other things to work on and couldn't continue as of now. Perhaps, will continue in a couple of weeks.

The app expects a test database without which it shows connection problems. Perhaps, you can try it if this was the case with you.

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

Successfully merging this pull request may close these issues.

None yet

3 participants