Add Ingress to trino chart #39

hkyyx · 2022-03-31T13:16:06Z

No description provided.

cla-bot · 2022-03-31T13:16:08Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

charts/trino/values.yaml

cla-bot · 2022-03-31T13:27:01Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

cla-bot · 2022-03-31T13:34:59Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

cla-bot · 2022-03-31T14:08:24Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

cla-bot · 2022-04-01T02:07:12Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

cla-bot · 2022-04-01T02:21:43Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

hashhar · 2022-04-01T04:59:01Z

That values merging has been merged in #41 since it's already ready.

Can you just keep the ingress changes in this PR and I'll merge then. Please also don't bump the chart version yet, we'll do that when planning a release.

cla-bot · 2022-04-01T05:19:40Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

hkyyx · 2022-04-01T05:20:19Z

@hashhar updated

hashhar

Thank you.

I'll merge once CLA is processed. It's a partially manual process and is processed in batches weekly.

hkyyx · 2022-04-05T02:56:47Z

@hashhar The CLA is done, maybe you can merge this pr

hashhar · 2022-04-05T05:04:37Z

@cla-bot check

cla-bot · 2022-04-05T05:04:39Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

cla-bot · 2022-04-05T05:04:40Z

The cla-bot has been summoned, and re-checked this pull request!

hashhar · 2022-04-05T05:05:33Z

@yanyixing Can you rebase this PR with something like git rebase --reset-author main? One of the commits has a different author so that CLA check is not passing yet.

hkyyx · 2022-04-05T07:56:17Z

@hashhar Thanks for you reminder, the commit is rebased

hashhar · 2022-04-05T08:00:21Z

charts/trino/templates/ingress.yaml

indentation issue.

hashhar · 2022-04-05T08:01:57Z

charts/trino/templates/ingress.yaml

did you mean values.ingress.tls.secretName?

hashhar · 2022-04-05T08:03:06Z

charts/trino/templates/ingress.yaml

why not use networking.k8s.io/v1 available from 1.19-0? It's been available for quite a while.

I will update this

@yanyixing could you please update this version.

hashhar · 2022-04-05T08:04:37Z

charts/trino/templates/ingress.yaml

Also is this correct? I thought it's service.name as in:

service: name: ...

Also, pathType is required AFAIK.

fullName is a variable which is refer to the trino.fullname, and the service name is also trino.fullname, so this will be ok

hashhar · 2022-04-05T08:06:54Z

sorry I missed couple things during review.

Another question, does the ingress terminate TLS? If so then we might want to set http-server.process-forwarded config so that Trino actually accepts connections that were initiated over TLS.

nicholaslimck · 2022-09-15T08:19:24Z

@hashhar Hope that this PR can be merged soon! I was about to use this helm chart until I noticed it was missing configuration options for ingress.

edmundmiller · 2022-09-15T13:02:46Z

This helm chart looks like it gets updated and has an ingress in case anyone is interested https://artifacthub.io/packages/helm/valeriano-manassero/trino

nicholaslimck · 2022-09-15T14:15:25Z

This helm chart looks like it gets updated and has an ingress in case anyone is interested https://artifacthub.io/packages/helm/valeriano-manassero/trino

I saw that chart too, but since I started using this chart I was hoping that Ingress support could be quickly added. Especially since this PR has been pretty much ready to merge for the last few months...

cccs-tom · 2022-10-11T20:15:05Z

I want to chime in my support for merging this @hashhar. Our team is in the process of migrating to this chart because we were under the impression that it is Trino's "official" chart. Is that assumption incorrect?

cccs-tom · 2022-10-14T18:01:34Z

Adding my review here - I agree that using http-server.process-forwarded or SSL pass-through is going to be deployment-specific and the chart should be flexible enough to allow either configuration. Maybe it's worth documenting these possibilities in the README to avoid user confusion?

ckdarby · 2023-01-24T17:33:25Z

charts/trino/templates/ingress.yaml

Maybe another PR, but we probably want these common things to be in the template helper file.

bitsondatadev

We should avoid supporting any version outside of the k8s support window. See more info here:

https://www.youtube.com/watch?v=SObTY-Ce1G4

charts/trino/templates/ingress.yaml

bitsondatadev · 2023-01-24T17:31:46Z

charts/trino/templates/ingress.yaml

@yanyixing could you please update this version.

charts/trino/templates/ingress.yaml

charts/trino/values.yaml

ckdarby · 2023-01-27T21:59:28Z

The other comment, in Charts.yaml we'll want to use:

kubeVersion: A SemVer range of compatible Kubernetes versions (optional)

hkyyx · 2023-01-29T15:37:39Z

@bitsondatadev Thanks for your review and suggestions, I have updated the pr, please review again.

bitsondatadev

Let's remove all version comparisons assuming that people are using the supported k8s versions.

bitsondatadev · 2023-01-30T00:37:08Z

charts/trino/templates/ingress.yaml

@yanyixing lets get rid of this as well.

bitsondatadev · 2023-01-30T00:37:59Z

charts/trino/templates/ingress.yaml

Remove this if as it is required

bitsondatadev · 2023-01-30T00:38:54Z

charts/trino/templates/ingress.yaml

Let's remove this

bitsondatadev

Looks good barring one open question I have to be addressed by Ashhar

bitsondatadev · 2023-01-30T11:32:31Z

charts/trino/templates/ingress.yaml

+                name: {{ $fullName }}
+                port:
+                  number: {{ $svcPort }}


One last thought is that perhaps we should enable users to override these values (e.g. if they have a sidecar metrics app or something) and make the Trino name/port number the defaults. I'll defer this decision to @hashhar but otherwise, I approve.

bitsondatadev · 2023-01-31T11:13:49Z

Update: Ashhar is just getting back from leave so he'll need a week or so to catch up on things. Just wanted to keep y'all updated since this one has been touch and go.

bitsondatadev · 2023-03-02T18:34:47Z

@hashhar can you take a look here please?

bitsondatadev · 2023-03-03T22:47:31Z

Also tagging @losipiuk since you have some k8s experience :)

hkyyx · 2023-03-11T12:35:15Z

@bitsondatadev It seems like this repo is not updated for a long time

bitsondatadev · 2023-03-11T12:40:51Z

Im actively looking for a maintainer. @hashhar doesn't currently have the bandwidth.

mosabua · 2023-08-14T17:32:01Z

Could you resolve conflicts?

Co-authored-by: Yixing Yan <yixingyan@gmail.com>

hashhar reviewed Mar 31, 2022

View reviewed changes

charts/trino/values.yaml Outdated Show resolved Hide resolved

hkyyx requested a review from hashhar March 31, 2022 13:39

hkyyx force-pushed the trino-chart branch from b1f6496 to ada0de1 Compare March 31, 2022 14:08

hkyyx force-pushed the trino-chart branch from b7cebf4 to d5895b3 Compare April 1, 2022 02:21

hashhar mentioned this pull request Apr 1, 2022

Merge coordinator & worker values into values.yaml #41

Merged

hkyyx force-pushed the trino-chart branch from d5895b3 to dd75ffa Compare April 1, 2022 05:19

hashhar approved these changes Apr 1, 2022

View reviewed changes

dragonslayer27 mentioned this pull request Apr 1, 2022

Ingress support #28

Closed

hkyyx force-pushed the trino-chart branch from dd75ffa to b17b98f Compare April 5, 2022 07:54

cla-bot bot added the cla-signed label Apr 5, 2022

hashhar reviewed Apr 5, 2022

View reviewed changes

cccs-tom approved these changes Oct 14, 2022

View reviewed changes

bitsondatadev mentioned this pull request Nov 19, 2022

Add ingress and catalogs config iterator #2

Closed

ckdarby reviewed Jan 24, 2023

View reviewed changes

bitsondatadev suggested changes Jan 24, 2023

View reviewed changes

hkyyx force-pushed the trino-chart branch from fd192d7 to 8923652 Compare January 29, 2023 15:34

hkyyx requested a review from bitsondatadev January 29, 2023 15:35

bitsondatadev suggested changes Jan 30, 2023

View reviewed changes

hkyyx force-pushed the trino-chart branch from 8923652 to 61a7999 Compare January 30, 2023 05:45

hkyyx requested a review from bitsondatadev January 30, 2023 05:46

bitsondatadev approved these changes Jan 30, 2023

View reviewed changes

mosabua closed this Aug 14, 2023

mosabua reopened this Aug 14, 2023

mosabua force-pushed the trino-chart branch 2 times, most recently from a0eb9c8 to ff7c7b5 Compare August 14, 2023 17:57

Add ingress for Trino coordinator

e839e49

Co-authored-by: Yixing Yan <yixingyan@gmail.com>

mosabua force-pushed the trino-chart branch from ff7c7b5 to e839e49 Compare August 14, 2023 18:56

mosabua merged commit 78aa246 into trinodb:main Aug 14, 2023

Add Ingress to trino chart #39

Add Ingress to trino chart #39

Uh oh!

Conversation

hkyyx commented Mar 31, 2022

Uh oh!

cla-bot bot commented Mar 31, 2022

Uh oh!

Uh oh!

cla-bot bot commented Mar 31, 2022

Uh oh!

cla-bot bot commented Mar 31, 2022

Uh oh!

cla-bot bot commented Mar 31, 2022

Uh oh!

cla-bot bot commented Apr 1, 2022

Uh oh!

cla-bot bot commented Apr 1, 2022

Uh oh!

hashhar commented Apr 1, 2022

Uh oh!

cla-bot bot commented Apr 1, 2022

Uh oh!

hkyyx commented Apr 1, 2022

Uh oh!

hashhar left a comment

Choose a reason for hiding this comment

Uh oh!

hkyyx commented Apr 5, 2022

Uh oh!

hashhar commented Apr 5, 2022

Uh oh!

cla-bot bot commented Apr 5, 2022

Uh oh!

cla-bot bot commented Apr 5, 2022

Uh oh!

hashhar commented Apr 5, 2022

Uh oh!

hkyyx commented Apr 5, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hashhar commented Apr 5, 2022

Uh oh!

nicholaslimck commented Sep 15, 2022

Uh oh!

edmundmiller commented Sep 15, 2022

Uh oh!

nicholaslimck commented Sep 15, 2022

Uh oh!

cccs-tom commented Oct 11, 2022

Uh oh!

cccs-tom commented Oct 14, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bitsondatadev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!