Add chart for Traefik-based ingress controller #146

krancour · 2016-10-24T21:19:03Z

Current Todo:

Decide on whether this chart should be in stable or incubator

k8s-ci-robot · 2016-10-24T21:19:04Z

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "@k8s-bot ok to test" on its own line.

Regular contributors should join the org to skip this step.

prydonius · 2016-10-24T21:44:29Z

Thanks @krancour!

prydonius · 2016-10-24T21:45:09Z

incubator/traefik/README.md

+## Introduction
+
+This chart bootstraps Traefik as a Kubernetes ingress controller with optional support for SSL and
+Let's Encrypt. By default, Traefik will be installed into the `kube-system` namespace.


Is this a requirement? I think we generally prefer that Helm manages the namespace Charts get installed into with the --namespace flag.

It's not a hard and fast requirement, however, since ingress controllers are frequently used to watch Ingress objects cluster-wide (i.e. in all namespaces), they often require the elevated permissions of the kube-system namespace's default service account.

When writing this chart, I assumed that for the less-frequent (or so I imagine) use case where someone does want to install this into some other namespace that --namespace would override the hard-coded default of kube-system. I've put that to the test now, however, and found it doesn't work that way!

cc @technosophos @michelleN isn't --namespace supposed to override whatever default is hard-coded in the chart's manifests? Or was it just wishful thinking on my part that it works that way?

At any rate, it does seem that what is best for now is that I omit this hard-coded default and instead document that this component would most often be installed into kube-system.

@prydonius I've amended the PR as discussed above.

chrislovecnm

Thanks for the PR, looks awesome. Have some recommended tweaks

chrislovecnm · 2016-10-25T00:27:42Z

incubator/traefik/LICENSE

@@ -0,0 +1,21 @@
+MIT License
+
+Copyright (c) 2016 Deis


Do we need this? This project is Apache 2 licensed and now we have MIT?

Well, that's a tricky question. It seems individual charts are subject to their own license, no? What I opted to do here was use the same license that Traefik itself uses. If that's the wrong move, lmk.

I think this code/repo can have a separate license from the source of traefik itself.

Ok. I'll change it.

I'm actually just going to remove the license from this chart altogether. It seems license is optional for charts and no other chart in this repo includes one. I'm going to follow that precedent.

chrislovecnm · 2016-10-25T00:29:16Z

incubator/traefik/templates/deployment.yaml

+      labels:
+        k8s-app: traefik-ingress-controller
+        version: {{ .Chart.Version }}
+    spec:


ready and lively probes please, as well as resource limits ;)

Yep. Good call.

chrislovecnm · 2016-10-25T00:30:16Z

incubator/traefik/templates/deployment.yaml

+      {{- if .Values.acme.enabled }}
+      - name: acme
+      {{- if .Values.acme.persistence.enabled }}
+        persistentVolumeClaim:


It there value to have storage classes?

I'm not sure what you're asking here. Can you clarify please?

This currently follows the exact pattern used by the stable MySQL chart for persistent storage.

The storage class is defined in the PVC not the deployment. Nothing to see here.

viglesiasce · 2016-10-25T02:10:17Z

incubator/traefik/values.yaml

+ssl:
+  enabled: false
+  enforced: false
+  defaultCert: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVtekNDQTRPZ0F3SUJBZ0lKQUpBR1FsTW1DMGt5TUEwR0NTcUdTSWIzRFFFQkJRVUFNSUdQTVFzd0NRWUQKVlFRR0V3SlZVekVSTUE4R0ExVUVDQk1JUTI5c2IzSmhaRzh4RURBT0JnTlZCQWNUQjBKdmRXeGtaWEl4RkRBUwpCZ05WQkFvVEMwVjRZVzF3YkdWRGIzSndNUXN3Q1FZRFZRUUxFd0pKVkRFV01CUUdBMVVFQXhRTktpNWxlR0Z0CmNHeGxMbU52YlRFZ01CNEdDU3FHU0liM0RRRUpBUllSWVdSdGFXNUFaWGhoYlhCc1pTNWpiMjB3SGhjTk1UWXgKTURJME1qRXdPVFV5V2hjTk1UY3hNREkwTWpFd09UVXlXakNCanpFTE1Ba0dBMVVFQmhNQ1ZWTXhFVEFQQmdOVgpCQWdUQ0VOdmJHOXlZV1J2TVJBd0RnWURWUVFIRXdkQ2IzVnNaR1Z5TVJRd0VnWURWUVFLRXd0RmVHRnRjR3hsClEyOXljREVMTUFrR0ExVUVDeE1DU1ZReEZqQVVCZ05WQkFNVURTb3VaWGhoYlhCc1pTNWpiMjB4SURBZUJna3EKaGtpRzl3MEJDUUVXRVdGa2JXbHVRR1Y0WVcxd2JHVXVZMjl0TUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQwpBUThBTUlJQkNnS0NBUUVBdHVKOW13dzlCYXA2SDROdUhYTFB6d1NVZFppNGJyYTFkN1ZiRUJaWWZDSStZNjRDCjJ1dThwdTNhVTVzYXVNYkQ5N2pRYW95VzZHOThPUHJlV284b3lmbmRJY3RFcmxueGpxelUyVVRWN3FEVHk0bkEKNU9aZW9SZUxmZXFSeGxsSjE0VmlhNVFkZ3l3R0xoRTlqZy9jN2U0WUp6bmg5S1dZMnFjVnhEdUdEM2llaHNEbgphTnpWNFdGOWNJZm1zOHp3UHZPTk5MZnNBbXc3dUhUKzNiSzEzSUloeDI3ZmV2cXVWcENzNDFQNnBzdStWTG4yCjVIRHk0MXRoQkN3T0wrTithbGJ0ZktTcXM3TEFzM25RTjFsdHpITHZ5MGE1RGhkakpUd2tQclQrVXhwb0tCOUgKNFpZazErRUR0N09QbGh5bzM3NDFRaE4vSkNZK2RKbkFMQnNValFJREFRQUJvNEgzTUlIME1CMEdBMVVkRGdRVwpCQlJwZVc1dFhMdHh3TXJvQXM5d2RNbTUzVVVJTERDQnhBWURWUjBqQklHOE1JRzVnQlJwZVc1dFhMdHh3TXJvCkFzOXdkTW01M1VVSUxLR0JsYVNCa2pDQmp6RUxNQWtHQTFVRUJoTUNWVk14RVRBUEJnTlZCQWdUQ0VOdmJHOXkKWVdSdk1SQXdEZ1lEVlFRSEV3ZENiM1ZzWkdWeU1SUXdFZ1lEVlFRS0V3dEZlR0Z0Y0d4bFEyOXljREVMTUFrRwpBMVVFQ3hNQ1NWUXhGakFVQmdOVkJBTVVEU291WlhoaGJYQnNaUzVqYjIweElEQWVCZ2txaGtpRzl3MEJDUUVXCkVXRmtiV2x1UUdWNFlXMXdiR1V1WTI5dGdna0FrQVpDVXlZTFNUSXdEQVlEVlIwVEJBVXdBd0VCL3pBTkJna3EKaGtpRzl3MEJBUVVGQUFPQ0FRRUFjR1hNZms4TlpzQit0OUtCemwxRmw2eUlqRWtqSE8wUFZVbEVjU0QyQjRiNwpQeG5NT2pkbWdQcmF1SGI5dW5YRWFMN3p5QXFhRDZ0YlhXVTZSeENBbWdMYWpWSk5aSE93NDVOMGhyRGtXZ0I4CkV2WnRRNTZhbW13QzFxSWhBaUE2MzkwRDNDc2V4N2dMNm5KbzdrYnIxWVdVRzN6SXZveGR6OFlEclpOZVdLTEQKcFJ2V2VuMGxNYnBqSVJQNFhac25DNDVDOWdWWGRoM0xSZTErd3lRcTZoOVFQaWxveG1ENk5wRTlpbVRPbjJBNQovYkozVktJekFNdWRlVTZrcHlZbEpCemRHMXVhSFRqUU9Xb3NHaXdlQ0tWVVhGNlV0aXNWZGRyeFF0aDZFTnlXCnZJRnFhWng4NCtEbFNDYzkzeWZrL0dsQnQrU0tHNDZ6RUhNQjlocVBiQT09Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K


Can this be a secret name instead? Then the docs could outline how to create that secret (keys/values) such that it can be used here?

If I can make an argument for keeping it as it is...

The base64 encoded cert and key you put here becomes a secret. This means that you provide customizations to this either through --set on the command line or in your own values.yaml. i.e. You can customize these through the usual Helm workflow for customizing a chart.

Compare that to some other approach that requires a careful reading of the documentation and an "out of band" effort to create the secrets... I don't believe that compares favorably to what's in place now.

Fair. I'm fine as is. If users want something different, I'm sure they will chime in down the road.

Could I ask you to add instructions on getting those values? Also what do the default certs point to?

The readme has a table describing the configuration options. It's clear that the cert and key should be base64 encoded. I can add more specific instructions if you like. lmk

The cert that's included is a self-signed cert for *.example.com-- so it's basically garbage. The benefit, however, is that it's enough to get people going.

I will update the table to reflect the details of this cert.

viglesiasce · 2016-10-25T02:11:36Z

Awesome stuff @krancour!

viglesiasce · 2016-10-25T02:12:13Z

@k8s-bot ok to test

ekozan · 2016-10-25T09:25:00Z

@krancour Can you integrate HostPort option or I'll make a PR after merge

krancour · 2016-10-25T12:53:16Z

@ekozan, did you mean NodePort? You want that as an alternative to type: LoadBalancer?

That seems a reasonable option (but not a default). It would certainly be a benefit to anyone who wants to run this anywhere where the "cloud provider" no-ops provisioning of an external load balancer-- e.g. minikube or bare metal.

I'd be happy to add that.

vdice · 2016-10-25T15:24:42Z

incubator/traefik/README.md

+```
+
+After installing the chart, create a DNS CNAME record for applicable domains to direct inbound
+traffic the load balancer. You can you the command below to find the load balancer's DNS name:


a couple minor typos here; perhaps the following were intended: ... direct inbound traffic to the load balancer and You can use the command ...

Nice eye, @vdice. Will fix.

viglesiasce · 2016-10-25T16:20:06Z

Tested manually and all things seem to check out. Thanks again @krancour! Let me know when you've patched the README fixes. Also if you'd like to get this into stable, adding a NOTES.txt pointing people in the right direction for how to configure and use traefik would do the trick.

krancour · 2016-10-25T16:27:04Z

@viglesiasce great. I will try and amend with all suggested fixes as soon as time permits-- a few hours off, for sure.

ekozan · 2016-10-25T09:27:00Z

incubator/traefik/README.md

+Specify each parameter using the `--set key=value[,key=value]` argument to `helm install`. For example:
+
+```bash
+$ helm install --name my-release --namespace kube-system --set dashboard.enabled=true,dashboard.domain=traefik.example.com stable/traefik


Actually it's incubator/traefik not stable/traefik

If he adds a NOTES.txt it can be merged into stable directly

ekozan · 2016-10-25T17:40:16Z

Yeap it's an alternative for barebone cluster ;)

We need
Add option for switch the default type of service: LoadBalancer / HostPort / NodePort
Add option for switch to Deamonset is needed too :p i'll handle this if you dont want to add this one

ultimateboy · 2016-10-25T18:35:13Z

Just thought I'd report my successful testing of this chart, and Traefik's ability to issue certs from Let's Encrypt.

helm install incubator/traefik --name my-traefik --namespace kube-system --set dashboard.enabled=true,dashboard.domain=traefik.domain.you.own.com,ssl.enabled=true,acme.enabled=true,[email protected]

kubectl run nginx --image=nginx
kubecl expose deploy nginx --port=80 --target-port=80

echo "apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: nginx
  namespace: default
spec:
  rules:
  - host: some.domain.you.own.com
    http:
      paths:
      - backend:
          serviceName: nginx
          servicePort: 80" > ingress.yaml

kubectl create -f ingress.yaml

echo | openssl s_client -servername some.domain.you.own.com -connect some.domain.you.own.com:443 2>/dev/null | openssl x509 -noout -issuer
issuer= /C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3

krancour · 2016-10-25T22:01:54Z

It'll be tomorrow before I get to this.

No other chart in this repo includes a license, so I am following that precedent.

krancour · 2016-10-27T06:11:16Z

@viglesiasce I've addressed comments apart from one. You indicated I should add a NOTES.txt "pointing people in the right direction for how to configure and use traefik."

I'm unclear on what that should contain or what its purpose would be.

charts.yaml already points users in the direction of the Traefik documentation. That aside, detailed knowledge of configuring Traefik isn't required to use this chart, since all customizations are carried out not by editing Traefik's config directly, but by supplying a values.yaml instead.

krancour · 2016-10-27T06:12:47Z

@ultimateboy offline you asked me to support LE staging to avoid rate limits when testing / experimenting. I've added that now and made it the default as well (when ACME is enabled). Check it out and lmk what you think.

lachie83

Can we add a disclaimer on the current state of Traefik and it's prod readiness?

lachie83 · 2016-11-02T16:58:54Z

stable/traefik/README.md

@@ -8,7 +8,7 @@ microservices with ease.
 This chart bootstraps Traefik as a Kubernetes ingress controller with optional support for SSL and
 Let's Encrypt.


Can you a disclaimer about the state of Traefik?

Yep. Meant to, as we'd discussed. Forgot. Coming atcha.

lachie83 · 2016-11-02T16:59:10Z

stable/traefik/README.md

@@ -8,7 +8,7 @@ microservices with ease.
 This chart bootstraps Traefik as a Kubernetes ingress controller with optional support for SSL and


Looks great! Thanks for the updates!

lachie83

NOTES.txt looks much better! Thanks

prydonius · 2016-11-02T17:02:59Z

@lachie83 @viglesiasce @krancour

By putting this chart into stable may give the impression that it's ready for primetime.

Agreed, if the project itself is not production-ready then the package should be in incubator.

lachie83 · 2016-11-02T17:05:44Z

stable/traefik/README.md

@@ -3,6 +3,9 @@
 [Traefik](http://traefik.io/) is a modern HTTP reverse proxy and load balancer made to deploy
 microservices with ease.

+__DISCLAIMER:__ While this chart has been well-tested, testers have encountered occasional issues


Thanks @krancour

krancour · 2016-11-02T17:08:47Z

To clarify, I believe Traefik regards itself as production ready and people certainly do use Traefik in production. Any assertion about production readiness here is purely based on @lachie83's and my own hesitation to endorse this as such.

krancour · 2016-11-02T17:09:10Z

So I'm going to move this to incubator. Standby.

krancour · 2016-11-02T17:11:10Z

Moved

lachie83 · 2016-11-02T17:11:38Z

To clarify, I believe Traefik regards itself as production ready and people certainly do use Traefik in production. Any assertion about production readiness here is purely based on @lachie83's and my own hesitation to endorse this as such.

Agreed 👍

viglesiasce · 2016-11-02T17:11:45Z

I dont know about that folks. Where is this production ready requirement coming from for stable? Its not listed anywhere and the other requirements are quite explicit. I don't want us to cross this line.

krancour · 2016-11-02T17:13:32Z

So if the chart's own author(s) would hesitate to use it in production (depending on the use case), this can still qualify as "stable?"

krancour · 2016-11-02T17:15:31Z

Maybe some clarification on what, precisely, is meant by "stable" is in order. I will acknowledge I made a pretty big (and perhaps incorrect) assumption that stability was equated with production readiness.

lachie83 · 2016-11-02T17:18:18Z

I dont know about that folks. Where is this production ready requirement coming from for stable? Its not listed anywhere and the other requirements are quite explicit. I don't want us to cross this line.

Understood and I agree that it's not our position to decide whether something is production ready. I don't want to cross that line. Stable does not mean Prod Ready

How about we approach this from a difference angle - putting this in the incubator is a great way for it to gather some miles

viglesiasce · 2016-11-02T17:18:35Z

Stable charts meet these requirements.

If we cross the line of requiring things to be production ready then we need to define production ready in a way that is obvious otherwise we are going to end up with a lot of bikeshedding. For most packages (rpm and deb) the defaults are sane but not production ready.

krancour · 2016-11-02T17:22:23Z

How about we approach this from a difference angle - putting this in the incubator is a great way for it to gather some miles

@viglesiasce is there any validity to that notion? That things are "promoted" so stable after proving themselves in the incubator?

viglesiasce · 2016-11-02T17:27:24Z

The only thing we say in the README.md is that stable charts meet the criteria in the technical requirements. If we want to change that policy then we need to update the documentation via a PR where we can hash out the why and how.

krancour · 2016-11-02T17:28:48Z

Ok. I'm going to strike the last commit (the move to incubator) from this PR.

lachie83 · 2016-11-02T17:29:39Z

Thanks for the great discussion and clarification.

Given that this chart meets the requirements. My vote is on putting this in stable.

krancour · 2016-11-02T17:29:56Z

All set. Back in stable.

prydonius · 2016-11-02T17:30:42Z

@viglesiasce you're right, for some reason I thought some level of 'production-readiness' was a requirement. I would argue for promoting the stable repo as a source of 'production-ready' charts, but it's a separate discussion to define what we could mean by that and whether it makes sense to enforce it.

lachie83 · 2016-11-02T17:35:17Z

LGTM

viglesiasce · 2016-11-02T17:39:48Z

LGTM

prydonius · 2016-11-02T18:05:29Z

@krancour lgtm, all good to merge?

krancour · 2016-11-02T18:28:33Z

Yes

lachie83 · 2016-11-02T18:32:32Z

Thanks all! Excited to have this chart!

- add openebs operator 2.1.0-RC1 - add cstor operator 2.1.0-RC1 and dependents - add zfs localPV 1.0.0-RC1 operator - add openebs operator 2.1.0-RC2 - add cstor operator 2.1.0-RC2 and dependents - add zfs localPV 1.0.0-RC2 operator - add openebs operator 2.1.0 - add cstor operator 2.1.0 and dependents - add zfs localPV 1.0.0 operator - add openebs lite operator - add openebs operator for suse caas platform Signed-off-by: Akhil Mohan <[email protected]>

googlebot added the cla: yes label Oct 24, 2016

prydonius reviewed Oct 24, 2016

View reviewed changes

krancour mentioned this pull request Oct 24, 2016

Complete Kubernetes Deployment example for AWS? traefik/traefik#673

Closed

chrislovecnm suggested changes Oct 25, 2016

View reviewed changes

viglesiasce reviewed Oct 25, 2016

View reviewed changes

vdice reviewed Oct 25, 2016

View reviewed changes

ekozan reviewed Oct 25, 2016

View reviewed changes

krancour added 8 commits October 27, 2016 00:42

Add chart for Traefik-based ingress controller

c618113

Do not hard-code the namespace in the traefik chart

93dfcad

Move traefik chart to stable

3548575

Remove the license from the traefik chart

8da3668

No other chart in this repo includes a license, so I am following that precedent.

Add resource limits for traefik

ea74fab

Upgrade traefik to 1.1.0-rc3

d310da9

Add liveness and readiness probes for traefik

b485b8d

Make traefik service type configurable

e335e09

krancour added 2 commits October 27, 2016 09:41

Add missing documentation for imageTag option in Traefik chart

a0d3f9a

Add support for Let's Encrypt's staging env

b4d96c5

lachie83 suggested changes Nov 2, 2016

View reviewed changes

lachie83 reviewed Nov 2, 2016

View reviewed changes

Add disclaimer about the state of Traefik; ymmv

5140e91

lachie83 reviewed Nov 2, 2016

View reviewed changes

lachie83 approved these changes Nov 2, 2016

View reviewed changes

lachie83 removed their assignment Nov 2, 2016

lachie83 merged commit 6b02be8 into helm:master Nov 2, 2016

krancour deleted the traefik branch November 2, 2016 18:34

		@@ -8,7 +8,7 @@ microservices with ease.
		This chart bootstraps Traefik as a Kubernetes ingress controller with optional support for SSL and
		Let's Encrypt.

Add chart for Traefik-based ingress controller #146

Add chart for Traefik-based ingress controller #146

Conversation

krancour commented Oct 24, 2016 • edited by lachie83 Loading

k8s-ci-robot commented Oct 24, 2016

prydonius commented Oct 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krancour Oct 24, 2016 • edited Loading

Choose a reason for hiding this comment

chrislovecnm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viglesiasce commented Oct 25, 2016

viglesiasce commented Oct 25, 2016

ekozan commented Oct 25, 2016 • edited Loading

krancour commented Oct 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viglesiasce commented Oct 25, 2016

krancour commented Oct 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekozan commented Oct 25, 2016 • edited Loading

ultimateboy commented Oct 25, 2016 • edited Loading

krancour commented Oct 25, 2016

krancour commented Oct 27, 2016 • edited Loading

krancour commented Oct 27, 2016

lachie83 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lachie83 left a comment • edited Loading

Choose a reason for hiding this comment

prydonius commented Nov 2, 2016

Choose a reason for hiding this comment

krancour commented Nov 2, 2016

krancour commented Nov 2, 2016

krancour commented Nov 2, 2016

lachie83 commented Nov 2, 2016

viglesiasce commented Nov 2, 2016

krancour commented Nov 2, 2016

krancour commented Nov 2, 2016

lachie83 commented Nov 2, 2016

viglesiasce commented Nov 2, 2016 • edited Loading

krancour commented Nov 2, 2016

viglesiasce commented Nov 2, 2016

krancour commented Nov 2, 2016

lachie83 commented Nov 2, 2016

krancour commented Nov 2, 2016

prydonius commented Nov 2, 2016

lachie83 commented Nov 2, 2016

viglesiasce commented Nov 2, 2016

prydonius commented Nov 2, 2016

krancour commented Nov 2, 2016

lachie83 commented Nov 2, 2016

krancour commented Oct 24, 2016 •

edited by lachie83

Loading

krancour Oct 24, 2016 •

edited

Loading

ekozan commented Oct 25, 2016 •

edited

Loading

ekozan commented Oct 25, 2016 •

edited

Loading

ultimateboy commented Oct 25, 2016 •

edited

Loading

krancour commented Oct 27, 2016 •

edited

Loading

lachie83 left a comment •

edited

Loading

viglesiasce commented Nov 2, 2016 •

edited

Loading