Update Kubernetes Helm Chart #2048

Matthew-Beckett · 2020-09-02T21:30:39Z

Fixes #2012, #914

nhooyr

I haven't looked into this hard but is it possible to simplify this? 700 additional lines is a lot of code, we don't have to serve every use case.

Matthew-Beckett · 2020-09-07T12:58:45Z

@nhooyr the 700 lines is generated from helm create, and as such is considered best practice. This actually only caters for a very simple use case 😅

The original pull request for this chart was 600 lines and did not include a helm ignore which would explain why mine is so large. This commit also adds a kubeval GitHub action to ensure new commits do not break the chart, this can be removed to streamline the commit if you do not wish to receive new GitHub Actions from contributors.

#917

Matthew-Beckett · 2020-09-10T16:15:56Z

@nhooyr would you be able to advise on my reply? I can begin trying to shrink the commit if my reply is unacceptable.

alexgorbatchev · 2020-09-10T23:37:42Z

This is a very slim helm chart IMO. First 200 lines are .gitgnore and README.md. Trimming this PR would mean mostly removing comments.

+1 for merging this in. To make this chart useful it needs to be published via Github pages.

nhooyr · 2020-09-11T05:18:49Z

@nhooyr would you be able to advise on my reply? I can begin trying to shrink the commit if my reply is unacceptable.

Nah you're good! I just need to find the time to review this/dig into helm. I don't want to publish anything we haven't tested and verified ourselves.

What's the advantage to helm here versus shipping a couple basic k8s manifest files? That'd be much smaller, easier to audit and understand. And just as easy to delete by targeting a label cdr/code-server.

nhooyr · 2020-09-11T05:19:03Z

Also cc @sreya92 for thoughts as I know he's been in helm land for a while.

Matthew-Beckett · 2020-09-11T15:54:37Z

To make this chart useful it needs to be published via Github pages.

@alexgorbatchev absolutely, I agree! I haven't ever done this myself, however I am familiar with how it works. So that the solution is supportable and of a good quality, you mind pairing with me on this?

What's the advantage to helm here versus shipping a couple basic k8s manifest files?

@nhooyr indeed, there is no argument the complexity of Helm charts is considerable when compared to that of native manifests.

However, generally it is accepted that Helm charts are a more reliable way of an application vendor providing a reliable mechanism for deploying the application to their cluster that is managed and life cycled. Helm charts are ultimately packages in a package manager, so it's the argument of copying a binary to /usr/local/bin over installing it through your distribution's package manager and having versioning and state management capability.

Apologies if this is an overly dumb explanation! 😅 I'm not sure how else to explain it.

alexgorbatchev · 2020-09-11T22:12:19Z

To make this chart useful it needs to be published via Github pages.

@alexgorbatchev absolutely, I agree! I haven't ever done this myself, however I am familiar with how it works. So that the solution is supportable and of a good quality, you mind pairing with me on this?

Sure, I can help.

What's the advantage to helm here versus shipping a couple basic k8s manifest files?

@nhooyr indeed, there is no argument the complexity of Helm charts is considerable when compared to that of native manifests.

The main advantage is composability. Every engineering organization uses some form of provisioning system like Terraform or Ansible to manage their infrastructure. Helm charts provide an easy way to install complex Kubernetes manifests with pre-defined configuration specific to the application. It also makes it easy to receive future updates. While you can also use plain manifest files and modify them for your needs, you will have to do this every time there's an update to the manifest, which can be pretty error prone.

Think of a Helm chart as as a bash script that takes arguments, and manifest as a bash script that you have to heavily edit to pass the same arguments.

Matthew-Beckett · 2020-09-13T09:29:42Z

@alexgorbatchev how would we implement Helm repositories on GitHub pages for cdr/code-server as external contributors to the project?

alexgorbatchev · 2020-09-16T17:42:35Z

@alexgorbatchev how would we implement Helm repositories on GitHub pages for cdr/code-server as external contributors to the project?

I'll have a PR soon.

alexgorbatchev · 2020-09-16T19:45:31Z

@Matthew-Beckett in order to get easy github pages hosting, cdr team needs to create a cdr.github.io (see https://pages.github.com/) repo and clone the repo I made as "PR" to the project:

https://github.com/alexgorbatchev/alexgorbatchev.github.io

example output: https://alexgorbatchev.github.io/helm-charts/public/

This repo includes chart files from THIS PR and assumes they will be hosted there. It makes it easier to publish github pages.

Alternatively one could simply look at how I've implemented publishing and move it else where. Everything happens via a few simple HELM shell commands and as a bonus I'm generating index.html with Bash.

The output from the build.sh file looks like this:

❯ ./scripts/build.sh
Packaging helm charts
Successfully packaged chart and saved it to: public/code-server-1.0.0.tgz
Writing: public/index.yaml
Writing: public/index.html

Please let me know if this looks sufficient.

alexgorbatchev · 2020-09-16T19:48:49Z

Please let me know when I can take down the example from my public page :)

alexgorbatchev · 2020-09-18T00:59:12Z

@code-asher @nhooyr Could you please have a look at this?

MnrGreg · 2020-09-18T16:20:02Z

I'm surprised how fast code-server starts up. Would be interesting to see how well it works with a requests based HPA Scale to 0. Should be able to drive quite a high density of deployments.

Just need HPA Scale to 0 to mature. +1 here: kubernetes/kubernetes#69687

alexgorbatchev · 2020-09-22T22:32:55Z

@code-asher @nhooyr ping

Matthew-Beckett · 2020-09-29T10:10:40Z

@code-asher @nhooyr nudge

Matthew-Beckett · 2020-11-10T20:12:17Z

@nhooyr I have resolved all your comments, @alexgorbatchev if you do not wish to maintain this going forward I can remove you, it's just to get this finally merged in 😅

nhooyr · 2020-11-10T20:29:58Z

🎉

Matthew-Beckett · 2020-11-10T20:30:51Z

🎉

Got a bottle of champagne in the fridge I've been saving, this might be the moment 😆

Matthew-Beckett · 2020-11-10T20:34:25Z

@Matthew-Beckett in order to get easy github pages hosting, cdr team needs to create a cdr.github.io (see https://pages.github.com/) repo and clone the repo I made as "PR" to the project:

https://github.com/alexgorbatchev/alexgorbatchev.github.io

example output: https://alexgorbatchev.github.io/helm-charts/public/

This repo includes chart files from THIS PR and assumes they will be hosted there. It makes it easier to publish github pages.

Alternatively one could simply look at how I've implemented publishing and move it else where. Everything happens via a few simple HELM shell commands and as a bonus I'm generating index.html with Bash.

The output from the build.sh file looks like this:
❯ ./scripts/build.sh
Packaging helm charts
Successfully packaged chart and saved it to: public/code-server-1.0.0.tgz
Writing: public/index.yaml
Writing: public/index.html
Please let me know if this looks sufficient.

@nhooyr should we open this into a new issue? I think this would benefit users immensely post-merge.

nhooyr · 2020-11-10T20:39:21Z

.github/CODEOWNERS

@@ -1 +1,2 @@
-* @code-asher @nhooyr
+charts/code-server @Matthew-Beckett @alexgorbatchev 


more automation is better for sure 👍🏼

nhooyr · 2020-11-10T20:40:12Z

I believe we can just use https://raw.githubusercontent.com/Matthew-Beckett/code-server/feature/helm3/charts/code-server but once it's merged it'd be https://raw.githubusercontent.com/Matthew-Beckett/code-server/master/charts/code-server.

Can you give it a shot and see if it works?

nhooyr · 2020-11-10T20:41:33Z

Ooops, nevermind I'm wrong. I see, we need to serve a tgz file that contains the chart.

Matthew-Beckett · 2020-11-10T20:42:28Z

Ooops, nevermind I'm wrong. I see, we need to serve a tgz file that contains the chart.

Indeed, furthermore, Helm expects a certain HTTP response to list the charts and their version, Helm wouldn't natively support GitHub Release pulling.

nhooyr · 2020-11-10T20:45:20Z

Yea that's unfortunate. I'm surprised helm doesn't let you use git urls, seem's like huge mistake.

There's https://github.com/aslafy-z/helm-git at least. And now helm/helm#6734

Personally, I'm ok with users having to clone the repository before using helm as I think it's unfortunate to have to setup github pages just for helm but I'm open to discussion.

alexgorbatchev · 2020-11-10T22:58:14Z

Personally, I'm ok with users having to clone the repository before using helm as I think it's unfortunate to have to setup github pages just for helm but I'm open to discussion.

I think ideally there should be a simple way to use Helm with this chart, via github pages (or similar). Having to clone the repo here is akin to asking users to clone a repo before running brew install - doable, but regrettable :)

Matthew-Beckett · 2020-11-13T13:51:40Z

@sreya @code-asher nudge ☺️

sreya

Looks good overall. I obviously didn't validate all the parameters since you provided so many but a basic install works well :) If there are any bugs they will shake out over time.

sreya · 2020-11-13T18:22:31Z

charts/code-server/templates/deployment.yaml

+          runAsUser: {{ .Values.volumePermissions.securityContext.runAsUser }}
+        volumeMounts:
+        - name: data
+          mountPath: /home/coder


Do we always want the persistent volume to be mounted to /home/coder? If we allow for users to override the image we should maybe allow them to override the home directory.

sreya · 2020-11-13T18:42:12Z

charts/code-server/README.md

+
+```console
+$ git clone https://github.com/cdr/code-server.git
+$ helm install code-server/charts/code-server


Suggested change

$ helm install code-server/charts/code-server

$ helm install code-server code-server/charts/code-server

sreya · 2020-11-13T18:46:08Z

charts/code-server/values.yaml

+  ##
+  # storageClass: "-"
+  accessMode: ReadWriteOnce
+  size: 1Gi


1Gi seems a bit small for a default value I think 10 might be a better value.

nhooyr · 2020-11-13T21:38:56Z

Going to merge and fix comments myself so we can get this into today's release!

nhooyr · 2020-11-13T21:39:02Z

Thanks for reviewing @sreya

nhooyr · 2020-11-13T21:53:29Z

I think ideally there should be a simple way to use Helm with this chart, via github pages (or similar). Having to clone the repo here is akin to asking users to clone a repo before running brew install - doable, but regrettable :)

I mean it's more on Helm than us though. As a project maintainer, we have to test and support so many different platforms/package managers/combinations already. Adding one more is frustrating when helm could just support git.

nhooyr · 2020-11-13T21:53:42Z

I opened #2302 regardless though.

Matthew-Beckett added 9 commits September 2, 2020 20:41

Re-commit 0f1bbc

1b6ddb6

Remove Kubernetes native manifests

85d5858

Add updated Helm chart

d33df75

Change service default to ClusterIP and add helmignore

34f8c77

Patch helm chart to stable with v3.5.0

341cb34

Update readme and service port

559d05b

Add kubeval workflow

8fe7986

Add kubernetes version environment variable

70b73d7

Add checkout of repo

96a78c9

Matthew-Beckett requested review from code-asher and nhooyr as code owners September 2, 2020 21:30

Merge branch 'master' into feature/helm3

e858d11

nhooyr reviewed Sep 4, 2020

View reviewed changes

nhooyr mentioned this pull request Sep 4, 2020

added instructions to install on kubernetes #2003

Closed

Matthew-Beckett requested a review from nhooyr September 8, 2020 12:06

Matthew-Beckett added 4 commits November 10, 2020 20:07

Bump version and update README

77c2a72

Add maintainers

03aa770

Fix trailing line breaks

0e39bb9

Bump chart app version

10799aa

Matthew-Beckett requested a review from nhooyr November 10, 2020 20:13

nhooyr approved these changes Nov 10, 2020

View reviewed changes

nhooyr added this to the v3.6.3 milestone Nov 10, 2020

nhooyr requested a review from sreya November 10, 2020 20:31

Update CODEOWNERS file

aa7415a

nhooyr reviewed Nov 10, 2020

View reviewed changes

Matthew-Beckett mentioned this pull request Nov 10, 2020

Configure automated Helm chart publishing via GitHub Pages #2287

Closed

sreya approved these changes Nov 13, 2020

View reviewed changes

nhooyr merged commit affa64c into coder:master Nov 13, 2020

nhooyr mentioned this pull request Nov 13, 2020

Serve helm chart more idiomatically #2302

Open

This was referenced Nov 13, 2020

Move helm from root and fix stuff in README #2303

Merged

v3.7.0 #2309

Merged

		@@ -1 +1,2 @@
		* @code-asher @nhooyr
		charts/code-server @Matthew-Beckett @alexgorbatchev

	$ helm install code-server/charts/code-server
	$ helm install code-server code-server/charts/code-server

Update Kubernetes Helm Chart #2048

Update Kubernetes Helm Chart #2048

Conversation

Matthew-Beckett commented Sep 2, 2020

nhooyr left a comment

Choose a reason for hiding this comment

Matthew-Beckett commented Sep 7, 2020 • edited Loading

Matthew-Beckett commented Sep 10, 2020

alexgorbatchev commented Sep 10, 2020 • edited Loading

nhooyr commented Sep 11, 2020

nhooyr commented Sep 11, 2020

Matthew-Beckett commented Sep 11, 2020

alexgorbatchev commented Sep 11, 2020 • edited Loading

Matthew-Beckett commented Sep 13, 2020

alexgorbatchev commented Sep 16, 2020

alexgorbatchev commented Sep 16, 2020 • edited Loading

alexgorbatchev commented Sep 16, 2020

alexgorbatchev commented Sep 18, 2020

MnrGreg commented Sep 18, 2020

alexgorbatchev commented Sep 22, 2020

Matthew-Beckett commented Sep 29, 2020

Matthew-Beckett commented Nov 10, 2020

nhooyr commented Nov 10, 2020

Matthew-Beckett commented Nov 10, 2020

Matthew-Beckett commented Nov 10, 2020 • edited Loading

nhooyr Nov 10, 2020

Choose a reason for hiding this comment

nhooyr commented Nov 10, 2020 • edited Loading

nhooyr commented Nov 10, 2020

Matthew-Beckett commented Nov 10, 2020

nhooyr commented Nov 10, 2020

alexgorbatchev commented Nov 10, 2020

Matthew-Beckett commented Nov 13, 2020

sreya left a comment

Choose a reason for hiding this comment

sreya Nov 13, 2020

Choose a reason for hiding this comment

sreya Nov 13, 2020

Choose a reason for hiding this comment

sreya Nov 13, 2020

Choose a reason for hiding this comment

nhooyr commented Nov 13, 2020

nhooyr commented Nov 13, 2020

nhooyr commented Nov 13, 2020

nhooyr commented Nov 13, 2020

Matthew-Beckett commented Sep 7, 2020 •

edited

Loading

alexgorbatchev commented Sep 10, 2020 •

edited

Loading

alexgorbatchev commented Sep 11, 2020 •

edited

Loading

alexgorbatchev commented Sep 16, 2020 •

edited

Loading

Matthew-Beckett commented Nov 10, 2020 •

edited

Loading

nhooyr commented Nov 10, 2020 •

edited

Loading