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

Revamp Temporal Helm Charts #42

Closed
dtornow opened this issue Sep 8, 2021 · 29 comments
Closed

Revamp Temporal Helm Charts #42

dtornow opened this issue Sep 8, 2021 · 29 comments
Assignees

Comments

@dtornow
Copy link

dtornow commented Sep 8, 2021

Author:

Dominik Tornow, temporal.io

Summary of the feature being proposed

Split the current "Catch-All" Kubernetes Helm Chart into two different templates:

  • to kickstart the setup of a Temporal development environment on Kubernetes
  • to kickstart the setup of a Temporal production environment on Kubernetes

What value does this feature bring to Temporal?

Temporal's official Helm Chart needs some tender love and care:

  • too many question marks regarding objective (Catch-All)
  • too many question marks regarding open github pull requests

Temporal.io proposes two objectives

  • to kickstart the setup of a Temporal development environment on Kubernetes
  • to kickstart the setup of a Temporal production environment on Kubernetes

All open and future pull requests will be evaluated against these objectives

Development Setup Experience Kickstarter

Note Temporal Application Developement is diffrent from Temporal Development. This proposal addresses Temporal Application Development only.

Guiding Principal

  • Batteries included
    • Will just work, does all the heavy lifting

Mechanism

  • Vanilla Kubernetes! Kubernetes Resource File temporal.yaml, kubectl
  • No customizations

Proposal

  • Provide one temporal.yaml file that installs Temporal and its dependencies on a K8s Cluster in the default namespace without customizations.
  • In effect, temporal.yaml provides the same experience as docker-compose.yml for Kubernetes as target

Production Setup Experience

Note Setup is different from Operations, therefore, a Setup Experience is different from a Da1 or Day 2 Operation Experience. This proposal addresses Setup Experience only.

Guiding Principal

  • Batteries not included
    • Will get you started, does some of the lifting

Mechanism

  • Vanilla Kubernetes + Helm! Minimal Helm Chart, helm
  • Minimal Customization Options

You are expected to fork and customize

Proposal

  • Provide 3 minimal Helm Charts with minimal Customization Options
    • Core
    • Web
    • Admin
  • Provide instructions on Temporal's dependencies
    • Provide instructions how to configure Temporal's dependencies
    • Provide guidance on scaling metrics and scaling parameters
    • Provide guidance on security

DO NOT

  • Install any dependencies
    • No installation of Cassandra
    • No installation of Postgres
    • No installation of Elastic
  • Configure any dependencies
    • No creation of schemas
    • No creation of indices

Are you willing to implement this feature yourself?

The Temporal team with input from community

@joebowbeer
Copy link

Mention that the production version is HA (highly available)?

@nvtkaszpir
Copy link

nvtkaszpir commented Sep 9, 2021

Shouldn't this be rather a discussion (on github) which would result in series of issues?
Edit:
ah ok this is in another repo than the temporal helm charts.

@nvtkaszpir
Copy link

nvtkaszpir commented Sep 9, 2021

First of all there is really no such thing as vanilla kubernetes - each cloud provider has its own alterations (especially on network level).
Also this can be done in one helm chart just with proper switches to disable components. Otherwise you will end up in supporting two products instead of supporting one, and one of them will be usually treated much worse. So maybe it's better to actually drop one and just support production workload and provide precise exact working values.yaml for different use cases. Cause what's developer cluster - a production setup without excessive care against failures ;)

Here is a list of example values.yaml files for certain use cases:

  • clusterin specific cluds:

    • AWS EKS and Fargate
    • Azure
    • GCP GKE and CloudRun
    • Openshift (OKD)
  • development setup for just experimenting which means default setup with subcharts all-in-one namespace (we already got that in the chart)

  • production setup

    • usually needs to provide persistence and visibility services separately - some may even be outside of the k8s cluster and then it means you need extra sidecars and init containers - for example in GKE you want sidecar with cloudsql proxy
    • replica number at least of 3 (but better 5)
    • antiaffinity rules to prevent failures
    • multi-cluster setup where clusters are deployed in the same region but different zones
    • mutli-cluster setup where clusters are deployed in different regions
  • different ingress types if you want to expose it externally

Moreover a lot of issues leak into container images directly, such as:

  • read only containers - that means no more dockerize in entrypoint with /etc/temporal/
  • WORKDIR is ignored and processes start from root - example is OKD
  • runing with different user id/group - user id in container is remapped to the given range, this happens in OKD
  • injecting ca-bundle.crt when using custom Certificate Authority - especially in air-gapped environments in financial sector
  • use secrets from files and not from env vars

Above has noticeable impact on the helm chart structure as well.

Required helm chart alterations:

  • allow specifying custom registry for ALL images (I'm looking at busybox in jobs riiight noooow!)
  • allow defining images via SHA and not just tag
  • run as with specific security context (user/fsgroup and so on)
  • run as read only filesystem
  • mount kubernetes default servicetoken to access k8s cluster or not (usually you don't need it if your service is not talking with k8s api) + specify service RBAC roles
  • networkpolicies to control what can connect and where (and this REALLY depends on the CNI in k8s), which will help to solve issues such as connections between temporal clusters in different namespaces
  • extra labels to deployments/pods and so on
  • extra init containers - for example to inject certs etc
  • extra sidecars - for example to gather logs etc
  • pod disruption policies (if they are still available, soon they will be removed)
  • HPA/VPA - if someone wants to auto-scale, but this also depends on k8s api versions
  • nodeselector / tolerations / affinity - we got that but would be nice to have examples for soft/hard use cases.
  • fix ports (for example admin-tools deployment has port named http but it points to port 22 which is ssh)
  • extract certain files outside of the yaml files - especially config_template.yaml could be moved out and then included dynamically
  • some scripts should not be running infinitely especially in init containers.
  • introduce readiness / startup probes
  • allow adjusting termination grace period
  • jobs should have non-conflicting names so that you can deploy it multiple times and they will not fail because job object is immutable
  • remove alpha annotations

Include full continuous integration for helm charts:

  • at least add one cluster to test actual deployment (could be github actions with KinD)
  • add special values files for ci/ directories to test specific scenarios
  • expand testing pipeline to multiple clouds to directly test it on each cloud with specific deployments (this is pretty time consuming to do it but is doable)

@hazcod
Copy link

hazcod commented Sep 9, 2021

Can we include hosting the helm packages on the GitHub repository for use by helm?
e.g. with https://github.com/ironpeakservices/iron-chart-go you can just install it via

# first add our helm repository
# provide a GitHub token if it's private
% helm repo add ironchartgo https://${GITHUB_TOKEN}@raw.githubusercontent.com/ironpeakservices/iron-chart-go/helmrepo/
"ironchartgo" has been added to your repositories

# now let's install our Chart from our repository
% helm install mychart ironchartgo/iron-chart-go

@joebowbeer
Copy link

@hazcod e.g. temporalio/helm-charts#147

@joebowbeer
Copy link

@nvtkaszpir great list! A few questions and comments below.

Can you point to exemplary helm charts from other projects? (I wonder if Cilium's charts are worth a look.)

OpenShift seems to be the most different of those listed. A candidate for dropping?

Aside: I suspect KEDA will be handy for event-driven autoscaling, in which case someone may want to look into adding a Temporal scaler to KEDA.

@nvtkaszpir
Copy link

  1. Exemplary charts, well, most of bitnami or kokuwa.io ae good ones, but usually they do not support everything, thus that's why I provided a list. I could give ulrs for 'bad' and 'good' solution for some of the given points.

  2. Openshift is indeed harder than other typical k8s installations but it's very popular in strictly regulated environments, also usually doing things so that they work in OKD makes them also available to other platforms. Of course usually it's more like 'do everyting that it works on given cloud' and then later on 'adjust it for OKD in given cloud'. But knowing that this will happen leads to better desing from the start.

  3. Not everyone has ability to install whatever they want in k8s, especially anything that requires CRDs, so sticking to the basics for now is good enough. Frankly speaking I would leave HPA/VPA in the first phase, it's just easier to scale deployments up and just keep them idle - especially for frontend/history/matching, but would focus on scaling custom workers from private charts.

@JanisOrlovs
Copy link

Hello,
Personally, I don't see the point in splitting prod/test helm charts. Use one, just different values + add missing components from official Helm charts. Like Postgres or Cassandra.
There are two places where most of all cloud providers diverge in Kubernetes: IAM (how pod identity being tied to cloud provider) and networking. I see both cases can be left out with custom annotations (most cases)

About: liveness/readiness, please don't forget about startup probes (1.18+).

@hazcod
Copy link

hazcod commented Sep 9, 2021

@jontro
Copy link

jontro commented Sep 9, 2021

Might be worthwhile to check out kube-prometheus-stack https://github.com/prometheus-community/helm-charts/tree/main/charts/kube-prometheus-stack where dependencies like grafana are using the upstream helm charts (sub-charts)

I also don't think there should be a different production vs development chart. Having a values-prod.yaml vs values-dev.yaml could be a good idea instead

@underrun
Copy link

underrun commented Sep 9, 2021

@nvtkaszpir that is an epic list and terrific considerations!

And I think it very much highlights exactly why we want to simplify the helm chart.

There is just so much there that would need to be considered to make a helm chart that operates your production infrastructure that what we would like to do instead is to use helm as a simple templating engine to generate the core required resources necessary to run temporal on kubernetes.

comment from @sw-yx: we discussed some of these considerations on a recent video: https://www.youtube.com/watch?v=11I87HKS_NM

As @dtornow mentioned in the proposal customization of the helm chart could be to fork it and modify if more helm based flexibility is desired.

I would go a different direction (which we are looking at doing internally for running temporal ourselves) - use the helm chart to generate manifests and then use kustomize to add all of the environment specific support needed. Where kustomize isn't enough, I would look at using cue or jsonnet+rego (opa) to programmatically modify the generated manifests as much as needed.

This gives us the ability to focus on the core requirements of temporal so that you can be confident that what we deliver is what is needed and anything else you need you can bring to the table.

@jontro you are totally right that there's no need to have a different production and development chart if the chart you have is production focused. In those cases you can definitely use a production quality chart for development. But right now the situation is that the helm chart defaults to a non-production setup, is not ergonomic to configure otherwise, and the broader the set of features we want to make production grade the harder it is to know which ones should and should not be used in production.

We want a chart where we can say everything that is generated by the chart could be used in a production setting. Keeping things limited to temporal basics enables us to do that.

@GreyTeardrop
Copy link

I wonder if the development chart could depend on the production chart (as well as postgres, mysql, elasticsearch charts, etc) and reconfigure the production chart with some sane development defaults (replace server image with auto-setup, wire dependencies into it, create indices, etc)? That should both help to avoid code duplication, and would give some minimal configurability to the production chart.

@sagikazarmark
Copy link

Aiming at 90% of the use cases is a good way to exclude a considerable amount of edge cases. For me, Openshift and cloud provider specific things belong to that 10%. I definitely agree that things like prometheus and persistence options should be out of scope.

I think it would make sense to clarify what "minimal customization options" mean.

Also, what's the reason behind separating the core, web and admin into separate charts?

@underrun
Copy link

underrun commented Sep 9, 2021

@GreyTeardrop - thats something we thought about, but there are some reasons not to provide an additional helm chart:

  • Users can easily deploy those dependencies via helm and then instead of one big values file that configures all the things users would have a values file per dependency (and all that's really needed is a database). This has the advantage of providing maximum flexibility and fewer layers between user and the stack deployed.
  • For users who just want to run something, zero config kubectl apply is least overhead.

@dtornow
Copy link
Author

dtornow commented Sep 9, 2021

@sagikazarmark - fair questions

  • minimal No hard and fast definition of minimal. As a guideline: Take the "smallest" (in terms on loc) K8s Deployment, K8s Service, etc (no affinity/anti-affinity etc), make some attributes configurable
  • 3 vs 1 3 feels natural to me, but 1 works equally well

My goal is to effectively communicate the various Temporal Components, their interactions and interdependencies to empower our users to tailor make their own Temporal Platform

@lerminou
Copy link

lerminou commented Sep 9, 2021

@nvtkaszpir that's a really complete a great list but to achieve this, I will take some of your points to our projects too :) but I think it can't be done at the first time,
Be cause it requires extra developments not only at the k8s level
90% of all users don't use, maybe they should, all k8s options for security, ha, etc etc

@dtornow Do you have any ETA for specifications / publishing the new helm revamp ?

But I agree about not splitting into 3 helm because it will become hard to maintain and hard for consumers

@sagikazarmark
Copy link

@dtornow

Take the "smallest" (in terms on loc) K8s Deployment, K8s Service

I would probably compromise here and allow customizing the most common values

no affinity/anti-affinity etc

I'd say the output of helm create sounds like a better minimum

3 feels natural to me, but 1 works equally well

For me admin and web are the components of the same software. Is there a reason to install web or admin independently?

You are expected to fork and customize

I'm not sure this is a good absolute goal. Forking is for sure the best solution to implement environment dependent changes, but why not include options that are necessary in at least half of the cases?

Couple examples:

  • affinity is one of the best tools for pod spreading
  • Allowing to customize the service account gives users the opportunity to use any IAM solutions out there using annotations (but without having to make the chart cloud provider specific)
  • Having some resource limits in place is number one requirement for running anything in production

@nvtkaszpir
Copy link

@nvtkaszpir that's a really complete a great list but to achieve this, I will take some of your points to our projects too :) but I think it can't be done at the first time,
Be cause it requires extra developments not only at the k8s level
90% of all users don't use, maybe they should, all k8s options for security, ha, etc etc

Of course it can be done, we did most of it already.

This is the list of requirements you will get into when you start to deal with temporal in production setups in kubernetes.
So our fork of the helm chart ended to be major rewrite. We keep it in private repo because it is tailored to our needs, and indeed it is probably aimed only at specific users. Most of those issues are already done by the others and hanging as pull requests.

But I also understand that this means that someone has to maintain it - if the temporal team does not have such setups then no wonder they are not eager to maintain it. Moreover to make it maintainable you need proper CI system to it and unfortunately that't not something easy to set up because you probably want to test it fully by deployment on real infra (most of testing tools for k8s are usually covering 90% of the use cases via mockups but frankly speaking final deployment on the infra really test this properly, we had that issue that we got something about 3 different tools for fast check and final slow deployments in k8s)

@nvtkaszpir
Copy link

nvtkaszpir commented Sep 10, 2021

To make list shorter I believe the best thing to do would be right now:

  • clean up current pull requests and merge them or drop them, so that the chart is working (afair rhight now there are issues with it)
  • remove dockerize from the images or at least move templates into a different path for example /opt/temporal and then template output would be written to /etc/temporal emptydir (this way you can mount volume in there, because AFAIR now this is templated in-place which is really problematic) - this solves most of security issues because it turns apps into read-only mode and writes are allowed only in specific empty directory
  • allow using private repos for all images - this solves issues with air-gapped envs or docker hub limits etc
  • allow annotations (and remove exiting ones like prometheus) - this is used to injext sidecars like for metrics/logs
  • link in readme to the official docs about what to do to run it in production (security, antiaffinity and so on)
  • provide few working values-production.yaml files for example how to run it with persistence outside of the helm chart - this can be added as examples from community

And that's it. The rest probably people get as rendered helm templates and will apply their changes with kustomize or grafana/tanka

@sagikazarmark
Copy link

remove dockerize from the images or at least move templates into a different path

An alternative is making templating optional (eg. based on config file extension) and generate the rendered config in a temp dir (which can also be mounted if necessary).

@underrun
Copy link

underrun commented Sep 10, 2021

@alexshtin and I have talked about wanting to remove dockerize and the complicated path to a final config, but that's a different issue. Agree with you on the removal suggestion @nvtkaszpir - moving would have the benefits you mention but if we're going to change it, it feels better to drop it.

@dtornow
Copy link
Author

dtornow commented Sep 20, 2021

@nvtkaszpir

This is the list of requirements you will get into when you start to deal with temporal in production setups in kubernetes. So our fork of the helm chart ended to be major rewrite. [...] [I]t is tailored to our needs.

We see this approach/pattern a lot and have come to the conclusion to embrace it: We don't see a way to maintain a well tested Helm chart that encompasses all possible permutations. So we will maintain a well tested Helm chart that does not encompass any permutations and rely on our users to tailor that Helm chart to their specific needs.

@dtornow
Copy link
Author

dtornow commented Sep 20, 2021

Thank you everybody for your feedback and your input, we will address as many points as possible given the current strategy 🤓

@dtornow dtornow closed this as completed Sep 20, 2021
@joebowbeer
Copy link

joebowbeer commented Sep 20, 2021

@dtornow

We see this approach/pattern a lot and have come to the conclusion to embrace it: We don't see a way to maintain a well tested Helm chart that encompasses all possible permutations. So we will maintain a well tested Helm chart that does not encompass any permutations and rely on our users to tailor that Helm chart to their specific needs.

If your helm chart does not encompass any permutations, I wonder if it might be better to release it as a pure manifest, or small set of manifests, which can then be used directly as the basis for kustomization?

@nvtkaszpir
Copy link

nvtkaszpir commented Sep 20, 2021

@nvtkaszpir

This is the list of requirements you will get into when you start to deal with temporal in production setups in kubernetes. So our fork of the helm chart ended to be major rewrite. [...] [I]t is tailored to our needs.

We see this approach/pattern a lot and have come to the conclusion to embrace it: We don't see a way to maintain a well tested Helm chart that encompasses all possible permutations. So we will maintain a well tested Helm chart that does not encompass any permutations and rely on our users to tailor that Helm chart to their specific needs.

So this means you don't really care supporting helm charts at all (as temporal org with ability to host test envs which boils down to the same result), so in that case we are in the same situation as we are right now.

Because without actual tests in CI pipeline you can just only trust that commuted code works...does it mean we at least get faster merge approvals (by faith)?

I am aware that giving resources as env setups/users to support it is noticeable effort + cost.
But fear not! Just having faster approval process is good enough now, and I guess this will solve major pain points in the helm repo.
If you can not control it, then just delegate power to other people to allow influencing on the repo code itself. I know it's hard in the beginning, it's like loosing control. But on the other way it enables third party/community to have a influence on it and thus responsibility.

If you still do not want to do it, then maybe say straight that you are not going to support it and just force people to live on a forked repo.

@underrun
Copy link

underrun commented Jan 4, 2022

Hi - It's been a while so I want to recap and provide some insight into progress here:

  • We will support helm, just not via the current chart - we are working on a replacement
  • The new helm chart will support configuring all features of OSS Temporal in an opinionated way for kubernetes
  • But the new helm chart will not configure / deploy any dependencies necessary to run Temporal

A few other differences with the current chart:

  • We will host a proper helm chart repository on github
  • The main branch may not be stable, but we will release via github actions and will correlate repo tags with helm chart version releases

Other notes:

  • Work on a replacement helm chart is ongoing but with no current ETA
  • We will be delighted to accept community PRs on the new helm chart once it's released
  • We may still accept PRs on the current chart (bug fixes and minor adjustments but not major features) before the new chart is released

Thanks for your patience!

@nvtkaszpir
Copy link

We will support helm, just not via the current chart - we are working on a replacement
The new helm chart will support configuring all features of OSS Temporal in an opinionated way for kubernetes

Any options for more details in what opinionated way is it going to be done?

@lukyer
Copy link

lukyer commented Jan 24, 2022

Hi @underrun, any updates to:

  • Provide one temporal.yaml file that installs Temporal and its dependencies on a K8s Cluster in the default namespace without customizations.

? Totally something we are looking for instead of Hel(l)m. Thanks!

@vishwa-trulioo
Copy link

Hello there, what's the update on this? I see that the issue is closed. Does it mean it's ready or abandoned?

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

No branches or pull requests