projectcontour / contour

Contour is a Kubernetes ingress controller using Envoy proxy.
https://projectcontour.io
Apache License 2.0
3.73k stars 680 forks source link

Apply standard application labels to Contour deployments #1821

Open jpeach opened 5 years ago

jpeach commented 5 years ago

When you are looking at Contour deployments in a Kubernetes cluster, it's not always obvious which release you have. When we generate the deployment YAML, we could apply standard application labels such as app.kubernetes.io/version to make it more obvious.

As a bonus, Octant appears to use these labels for some kind of object grouping.

tthebst commented 4 years ago

I would like to give this a go.

I suppose you don't want any hardcoded values in example/03-contour.yaml. So is it enough if you pass $VERSION to the generate-deployment.sh. This would produce sth like "v1.6.0" or should I strip the "v"

Also, is a tool like yq appropriate to use?

@jpeach

jpeach commented 4 years ago

@tthebst I'd prefer to not take a dependency on yq. Maybe we can whack this with sed?

tthebst commented 4 years ago

@jpeach editing yaml's with CLI is painful. Especially because its labeling is inconsistent across example/*.yaml

My idea would be to add sth like this to the example files where appropriate.

  labels:
    app.kubernetes.io/name: NAME
    app.kubernetes.io/version: VERSION
    app.kubernetes.io/part-of: PARTOF

This would make it very easy to do sed and replace the values with the correct ones when creating the contour.yaml file. The drawback is that I would have to edit the example/ files and I don't know if this acceptable. What do you think?

jpeach commented 4 years ago

Yeh, in the absence of something like #2088 that's the best we can do I think.

jpeach commented 4 years ago

Yeh, in the absence of something like #2088 that's the best we can do I think.

One thing to try is to set the version to master on the main yaml, then sed that to the real version in make generate

tthebst commented 4 years ago

@jpeach Thanks for the response. Also, do you think it makes sense to use all standard labels

 labels:
    app.kubernetes.io/name: mysql
    app.kubernetes.io/instance: mysql-abcxzy
    app.kubernetes.io/version: "5.7.2"1
    app.kubernetes.io/component: database
    app.kubernetes.io/part-of: wordpress
    app.kubernetes.io/managed-by: helm

Or only use a selection. For example only

 labels:
    app.kubernetes.io/name: MySQL
    app.kubernetes.io/version: "5.7.2"
jpeach commented 4 years ago

I think that selection makes the most sense, "name" and "version" are the most useful, "part-of" could be a useful, but not so sure about it.

youngnick commented 4 years ago

I like the idea of having name and version in the standard label format, it could be that instance may be useful down the road to help with multiple-Contours-in-one-cluster usecases, but just adding name and version is a great start.

Editing the example YAMLs for this purpose is fine. I'd also echo @jpeach and say that it's probably better to set the version to master on master, and sed it to the release version when we cut a release. We already do this for container image versions.

tthebst commented 4 years ago

@youngnick @jpeach

I think part-of could be used like this. It probably also makes sense to set app.kubernetes.io/name: to the same value as name

apiVersion: v1
kind: ServiceAccount
metadata:
  name: envoy
  namespace: projectcontour
  labels:
    app.kubernetes.io/name: envoy
    app.kubernetes.io/part-of: contour
    app.kubernetes.io/version: master

A second issue is that CRD and RBAC are generated so I can't edit the file. We would need a different solution for these files or skip them.

jpeach commented 4 years ago

If we had kustomize, we could make it work https://github.com/projectcontour/contour/issues/2088

youngnick commented 4 years ago

I think it's okay to skip them to start with and come back later to add the labels either by kustomize, or some other method.

tthebst commented 4 years ago

@jpeach @youngnick So I think the easiest way to label everything is kubectl label. This will label all the resources with the standard labels app.kubernetes.io/part-of and app.kubernetes.io/version. The standard label app.kubernetes.io/name is manually added to all the resources in example/contour which are not generated. The generated resources will not have this standard label name but the labels part-of and version. Do you think this is a big issue? Have a look at the commit.

jpeach commented 4 years ago

@jpeach @youngnick So I think the easiest way to label everything is kubectl label.

@tthebst Does kubectl need a running cluster to label the objects?

tthebst commented 4 years ago

@jpeach I worked on this some time ago. You can check it out here.

I used the kubectl CLI file and edited hack/generate-deployment.sh

kubectl label -f - app.kubernetes.io/version=${VERSION} --overwrite --dry-run --local -o yaml 

I could open a PR but i think this is being worked on.

danehans commented 4 years ago

certgen should be updated so the contouercert and envoycert secrets are labeled according to k8s recommendations.