teamhephy / workflow

Hephy Workflow - An open source fork of Deis Workflow - The open source PaaS for Kubernetes.
MIT License
410 stars 36 forks source link

Annotating Pods through Deployments #32

Open Cryptophobia opened 6 years ago

Cryptophobia commented 6 years ago

From @mirague on March 1, 2017 9:20

For a variety of implementations it's necessary to attach annotations to Pods (kube2iam, k8s alpha features e.g. Node Affinity/Anti-Affinity, etc.). Normally one would add the annotation to the Deployment's .spec.template.metadata.annotations, resulting in each Pod carrying the proper annotation.

However, with Deis Workflow this is currently impossible to achieve, or at least persist. One can run kubectl edit deployment myapp-dev-cmd -n myapp-dev and add the desired annotation to .spec.template.metadata.annotations but this would be lost once a new version of the app is released.

Can we allow for a way to annotate Pods? Either through a deis command or by at least persisting the Deployment's set .spec.template.metadata.annotations when a new version is rolled out. This would allow for the combined use of existing k8s implementations with Deis Workflow.

Copied from original issue: deis/workflow#747

Cryptophobia commented 6 years ago

From @mirague on March 1, 2017 9:21

Similar but not quite the same: #545

Cryptophobia commented 6 years ago

From @blakebarnett on March 1, 2017 17:40

It seems to be becoming the standard to add features via annotations. There's a long list of things that require them already to function. Please support this!

Cryptophobia commented 6 years ago

From @bacongobbler on March 1, 2017 17:50

Agreed. Before annotations were kind of a "useless" feature in Kubernetes from the application's perspective. Now with apps supporting annotations out the wazoo (like kube2iam) I think it makes sense to bring it back to the table for discussion.

If someone feels motivated to work on a deis annotations:list/add/delete feature, that would be cool to see :)

Cryptophobia commented 6 years ago

From @BrianHicks on April 7, 2017 15:49

+1 on this, with a concrete use case: Prometheus metrics! The annotations look like:

Then a Prometheus installation will pick up the pods automatically as they're scaled, and I'll get my metrics for APM. :) This particular use case could also really use multiple ports… one for public traffic, one to expose /metrics.

Cryptophobia commented 6 years ago

This is important to fix for the next upcoming release. It would be a nice feature to have as many services like Istio and Kube2IAM require custom annotations on deployments to work properly.

kingdonb commented 6 years ago

I am going to take a look at this one as well. Just to clarify, we have router.deployment_annotations and router.service_annotations, AIUI which are used for annotations that should be added onto every deployed app, this issue is meant to allow users to add per-app annotations, as well as respecting (and persisting) annotations that are placed onto Workflow's deployments when the deployments are being rebuilt and upgraded.

To go with this, I'd like a place to specify annotations that are needed on every ingress if you're using "experimental native ingress" -- I am currently adding these annotations to every ingress that Workflow creates on my AKS cluster, to signal to the provided-by-AKS built-in ingress controller that it should be handling the route, and to take advantage of automatically generated LetsEncrypt certs from cert-manager:

annotations:
  kubernetes.io/ingress.class: addon-http-application-routing
  kubernetes.io/tls-acme: "true"

I would like it if I could set these globally, like the existing router.deployment_annotations and router.service_annotations, although I'm pretty sure they don't belong under router config group.

I don't think Workflow ever rewrites the ingress after the app is creted, although I haven't tried to use domains:add so I'm not sure if that works with ingress or not (that would be the only time I could think it happens.) As far as I know, at the very least, Ingress rules are not currently rewritten when app deployments are updated.

Is there any need for per-app ingress annotations? I don't know what's else is in use exactly alongside of Workflow today that takes advantage of annotations, or what other annotations people might be using on ingresses, but I want to be sure we don't write the wrong thing. I currently do not have a use case for per-app ingress annotations.

Cryptophobia commented 6 years ago

this issue is meant to allow users to add per-app annotations, as well as respecting (and persisting) annotations that are placed onto Workflow's deployments when the deployments are being rebuilt and upgraded.

Yes, exactly what we need to provide. A way to presist annotations through deployments or a way to define custom annotations on deployment objects per app.

Is there any need for per-app ingress annotations?

They might be useful but is it Hephy's job to manage them is the real question.

As far as I know, at the very least, Ingress rules are not currently rewritten when app deployments are updated.

This is correct. Just a quick check in the code leads me to think that domains and certificates are only defined on the controller for the router level and not for ingress rules. Is it Hephy's job to manage them, not sure?