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

feat: create helm template #891

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

saymonaraujo
Copy link

This is the first commit that creates a helm template for deployment of Akvorado.

Todo:

Add probes
Improve README(add all variables)

@vincentbernat
Copy link
Member

As I am not familiar with Kube, I need some time for this.

The overall sentiment is that this is a lot of code that I would need to support forever. Ideally, it would have been nice that either the docker-compose setup could be translated to Helm or the other way to avoid duplication. Notably, if I need to update the docker-compose part, I need to be able to also update the Helm chart.

In the meantime, people are welcome to get your work through this PR.

@saymonaraujo
Copy link
Author

I still have a lot of work to do here. The current implementation for the shared PV/PVC for the ipinfo data does not work out of the box, instead of using the current approach I will change to use a NFS endpoint.
Another thing that I will add is the auto rollout of the pods if a change in the configmap is detected, this will provide a 'dyamic' configuration change(great if you use CI for manage the deployments/config).
I think that would be great to have a deployment of the ClickHouse and Kafka out of the box as well(still evaluating this).
There's some fix as well that I will commit once I finishes some clean up.

@margau
Copy link
Contributor

margau commented Nov 3, 2023

Hi,
we're also experimenting with akvorado and helm, thats an interesting PR!

Some thoughts after a first look:

  • I'm not sure if it is possible to do the "config map change - restart deployments" with helm values only. The process I have in mind:
    • config map changes
    • orchestrator is restarted
    • if the orchestrator succeeded to load the config, it triggers a new deployment of the ingest and console (important: RollingUpdate to not loose flows). This might be done by annotating the pods with a hash of the config?
    • if the newly started orchestrator fails loading the config, the pods continue to run on the old config (no restart triggered)
    • if both "consumer" pods (inlet/console) and the orchestrator are restarted simultaneously, there is the risk of the consumer pods being ready first and loading the old config
  • Monitoring: I'd say ServiceMonitor-resources might be a good option
  • Storage: I'd try to keep inlets stateless (without PV), that way they can be scaled without issues, which is good for multi-node clusters with high input rates
  • Orchestrator: I'm a bit worried about the number of orchestrators running during an update, not sure what the behavior with more than 1 orchestrator (e.g. the new one failing due to bad config) is
  • Clickhouse: We're experimenting with altinity clickhouse operator, this might even integrate good with Support for ReplicatedMergeTree in ClickHouse #605 in the future

@saymonaraujo
Copy link
Author

saymonaraujo commented Nov 6, 2023

In my pocs the strimzi kafka operator and the altinity clickhouse operator worked really well.
About the rollout after a confimap change I will do a poc with the Reloader.

@saymonaraujo
Copy link
Author

I created a NFS Server that is used to share the files between the inlet and the ipinfo containers. I'm also in the final tests of the reloader. I'm adding a clickchouse and a Kafka in the helm template in the next commits.

@saymonaraujo
Copy link
Author

The reloader is working very well for Orchestrator and Console. But I'm having a little problem with the rollout of the inlet.
I'm working on this and soon will commit the new changes with the NFS server and the reloader.

@netixx
Copy link
Contributor

netixx commented Mar 11, 2024

Stumbling upon that,

with #1059 it removes the need to have ipinfo/geoip data shared between the inlet, as only the orchestrator requires it.

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

4 participants