Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply standard application labels to Contour deployments #1821

Open
jpeach opened this issue Oct 29, 2019 · 15 comments
Open

Apply standard application labels to Contour deployments #1821

jpeach opened this issue Oct 29, 2019 · 15 comments
Labels
area/deployment Issues or PRs related to deployment tooling or infrastructure. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.

Comments

@jpeach
Copy link
Contributor

jpeach commented Oct 29, 2019

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.

@jpeach jpeach added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Oct 29, 2019
@davecheney davecheney added this to the Backlog milestone Nov 17, 2019
@jpeach jpeach added the area/deployment Issues or PRs related to deployment tooling or infrastructure. label Feb 9, 2020
@stevesloka stevesloka assigned stevesloka and unassigned stevesloka Mar 2, 2020
@tthebst
Copy link
Contributor

tthebst commented Jul 8, 2020

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

tthebst added a commit to tthebst/contour that referenced this issue Jul 16, 2020
This adds standard application labels to the contour deployment file.
It adds the label at file generation.
To edit yaml yq is used (https://github.com/mikefarah/yq).

Fixes projectcontour#1821

Signed-off-by: Tim Gretler gretler.tim@gmail.com
@jpeach
Copy link
Contributor Author

jpeach commented Jul 17, 2020

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

@tthebst
Copy link
Contributor

tthebst commented Jul 17, 2020

@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
Copy link
Contributor Author

jpeach commented Jul 17, 2020

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

@jpeach
Copy link
Contributor Author

jpeach commented Jul 19, 2020

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
Copy link
Contributor

tthebst commented Jul 19, 2020

@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
Copy link
Contributor Author

jpeach commented Jul 19, 2020

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
Copy link
Member

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
Copy link
Contributor

tthebst commented Jul 20, 2020

@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
Copy link
Contributor Author

jpeach commented Jul 20, 2020

If we had kustomize, we could make it work #2088

@youngnick
Copy link
Member

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 added a commit to tthebst/contour that referenced this issue Jul 21, 2020
  This adds the standard label app.kubernetes.io/name label to the resources
  in /examples/contour. Additionally it adds the standard labels
  app.kubernetes.io/part-of and app.kubernetes.io/version to the contour.yaml during generation.
  The logic for adding the labels is in hack/generate-deployment.sh and uses the kubernetes CLI.

  Fixes projectcontour#1821

  Signed-off-by: Tim Gretler gretler.tim@gmail.com
tthebst added a commit to tthebst/contour that referenced this issue Jul 21, 2020
  New contour.yaml with kubernetes standard labels.

  Fixes projectcontour#1821

  Signed-off-by: Tim Gretler gretler.tim@gmail.com
@tthebst
Copy link
Contributor

tthebst commented Jul 21, 2020

@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.

@stevesloka stevesloka added the Hacktoberfest Denotes an issue ready for any "Hacktoberfest" contributor. label Oct 1, 2020
@jpeach
Copy link
Contributor Author

jpeach commented Oct 6, 2020

@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
Copy link
Contributor

tthebst commented Oct 7, 2020

@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
Copy link
Contributor

danehans commented Oct 8, 2020

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

@stevesloka stevesloka removed the Hacktoberfest Denotes an issue ready for any "Hacktoberfest" contributor. label Nov 5, 2020
@skriss skriss removed this from the Backlog milestone Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/deployment Issues or PRs related to deployment tooling or infrastructure. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Projects
No open projects
Contour Project Board
  
Unprioritized
Development

No branches or pull requests

7 participants