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

Helm Chart Deployment Option #290

Open
tim-chaffin opened this issue Sep 27, 2023 · 16 comments
Open

Helm Chart Deployment Option #290

tim-chaffin opened this issue Sep 27, 2023 · 16 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@tim-chaffin
Copy link
Contributor

Feature request.
It'd be excellent if we could reference a Helm Chart, from the Helm Chart registry. That way we could use the Terraform Hashicorp / Helm provider to deploy Tapir into a kubernetes service.

@PacoVK
Copy link
Owner

PacoVK commented Sep 29, 2023

Good idea, this may probably be in a dedicated repo, right? I could look into that as well but currently have some time constraints.

@PacoVK PacoVK added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 26, 2023
@jonasz-lasut
Copy link
Contributor

@PacoVK if you need help I can implement a helm chart for this repo. It looks like a really simple deployment from kubernetes point of view so it shouldn't take that much time.
If you're interested please let me know in the comment, I'll then contact you on LinkedIn or Mastodon and we can talk about the details :)

@tim-chaffin
Copy link
Contributor Author

Sign me up too. I've deployed several Helm charts to k8 using ArgoCD. But I've never made a Helm chart. I'd love to help.

@jonasz-lasut
Copy link
Contributor

@tlchaffi if Paco is ok with that then it works for me too. I've developed some golden hełm charts for different companies before but it's going to be faster in a team 😁

@PacoVK
Copy link
Owner

PacoVK commented Jan 16, 2024

Thanks, really appreciate that you are willing to take care! Feel free to reach out to share ideas or let us have a discussion here, that anybody can chime in. 👍

@jonasz-lasut
Copy link
Contributor

Sounds ok to me, I'll create a fork today afternoon.
For now chart can consists of deployment, service, service account and ingress. I'll add example values for ingress annotations for AWS but comment them out in values.yaml so that it's not a default for whole chart and add option to specify TLS certs without annotations (for example when using Cert manager). Something like in ArgoCD helm chart's ingress resource.

I can also add Certificate object with a flag for people using cert-manager. What do you think?
Btw does Tapir need the ClusterRole and ClusterRoleBinding present in terraform to work properly or was it setup for your debugging?

@tim-chaffin
Copy link
Contributor Author

For the storage layer of Tapir, for example, on Azure, it refers to an Azure storage account / blob account.

Do we want the Helm chart to resolve the storage layer? Or should it do like a k8 mounted disk?

@jonasz-lasut
Copy link
Contributor

That's on the service account level, you have workload identity using AWS/GCP/whatever annotation ("eks.amazonaws.com/role-arn" = aws_iam_role.tapir.arn in terraform). I believe we should be still doing workload identity (sorry if that's not the name used on AWS, I'm a GCP guy at heart) with annotations, just leave default as empty annotation object {} instead of AWS role-arn.

However there's also ClusterRole and binding to that SA that allows get/watch/list on services, ingresses and nodes. I wonder if those privileges are needed for Tapir to function properly or not.

@PacoVK
Copy link
Owner

PacoVK commented Jan 17, 2024

@jonasz-lasut concerning the ClusterRole i need to double check again, most of the code for K8s in the example was based on #253 but i guess this is not mandatory for Tapir to function at least in a K8s context. There may be an exception for EKS, but thats open to check. AFAIK AWS also introduced an easier way to handle Pod permissions at reinvent 23

@jonasz-lasut
Copy link
Contributor

@PacoVK ok, for now I've left it behind createClusterRoles flag

You can find initial draft of the chart under Chart for now.
I'll be working on CI pipeline for it tomorrow and I'll do some cleanup in the values and add documentation to them. I'll setup helm docs and changelog generation on the workflow

@jonasz-lasut
Copy link
Contributor

Hi, I've opened a pull request with full helm chart with Helm-docs annotations, generated helm docs and script to generate them locally and a basic pipeline which lints helm chart and checks if the docs were generated if any changes are made to the helm chart.
#375

I didn;t iplemented bumping up AppVersion in the helm chart from the build/deploy pipeline of tapir as I didn't want to break anything and wanted to have separation between helm chart and tapir. I can open another PR later :)

@PacoVK
Copy link
Owner

PacoVK commented Jan 18, 2024

I think for the first step this is more than enough to just mention the version bump in the release notes We could automate separately then :)

@PacoVK
Copy link
Owner

PacoVK commented Feb 7, 2024

@all-contributors please add @jonasz-lasut for example, code

Copy link
Contributor

@PacoVK

I've put up a pull request to add @jonasz-lasut! 🎉

@PacoVK
Copy link
Owner

PacoVK commented Feb 7, 2024

@all-contributors please add @tlchaffi for review

Copy link
Contributor

@PacoVK

I've put up a pull request to add @tlchaffi! 🎉

@PacoVK PacoVK mentioned this issue Mar 4, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants