Add recommended k8s labels

[eddycharly]

This PR adds recommended k8s labels on resources.
Implementation for proposal tektoncd/pipeline#2497

Related PR in pipelines: tektoncd/pipeline#2501

Base: added labels and updated selectors for all resources in /base
Go: changed go code in /pkg/endpoints/dashboard.go

NOTES:

• I removed some old code that implemented various strategies to find dependent applications version. I don't think it's an issue because these versions are old and should not work with the current dashboard (pre v1beta1).
• I didn't touch the code in /pkg/endpoints/cluster.go. The code there is bound to ingresses which is not part of this repository. I will address this code in another PR.
• This will introduce a breaking change when trying to update deployments as label selectors have changed.

[eddycharly]

/cc @vdemeester

[a-roberts]

My plan was to test the about page still works ok for picking up versions on Docker Desktop and OpenShift 4.3 with Tekton installed through the release asset and the OpenShift Pipelines operator

Versions (for Pipellines) to be picked up being 0.10.2, 0.11.3, 0.12.0

Docker Desktop: install 0.10.2 of Pipelines, failed to get version. Labels present for Tekton controller:

  labels:
    app.kubernetes.io/component: controller
    app.kubernetes.io/name: tekton-pipelines
    tekton.dev/release: v0.10.2

I noticed we have

	// NOTE: *.tekton.dev/release and version labels are deprecated in favor of app.kubernetes.io/version
	// They are kept here for backward compatibility and should be removed at some point
	labelsToCheck := []string{versionLabel, application + ".tekton.dev/release", "version"}

🤔

@eddycharly please check the versions of Pipelines are still being picked up OK outside of OpenShift, if you don't have OpenShift access I'm happy to check nothing breaks there for you, thanks

[eddycharly]

Thanks for testing it !

It looks like tekton.dev/release label is missing in the list.

I would be happy if you can test this on openshift as I don’t know much about it.
Can testing be done with minishift ?

[a-roberts]

Thanks for testing it !

It looks like tekton.dev/release label is missing in the list.

I would be happy if you can test this on openshift as I don’t know much about it.
Can testing be done with minishift ?

Possibly, it's worth giving it a try yeah, once you've got something we know works ok for the non-OpenShift environments I'm happy to go through the matrix 😄

[eddycharly]

@a-roberts I added support for the tekton.dev/release label, it works fine with pipelines v0.10.2.

[a-roberts]

@a-roberts I added support for the tekton.dev/release label, it works fine with pipelines v0.10.2.

Thanks, won't have time to test this in-depth today but if you can verify what you can (e.g. on Minishift) that'd really help, rest of the PR's looking good, we'll want to make sure nothing breaks for Triggers (0.3.1 and later) either 👍

[eddycharly]

I tested it successfully with:

• pipelines 0.10.0, 0.10.1, 0.10.2, 0.11.0, 0.11.1, 0.11.2, 0.11.3, 0.12.0
• triggers 0.3.0, 0.3.1, 0.4.0

I will try to test with minishift during the weekend if I have some free time.

[a-roberts]

I tested it successfully with:

• pipelines 0.10.0, 0.10.1, 0.10.2, 0.11.0, 0.11.1, 0.11.2, 0.11.3, 0.12.0
• triggers 0.3.0, 0.3.1, 0.4.0

I will try to test with minishift during the weekend if I have some free time.

Thank you! No worries if you can't I'll give it a good go on OpenShift Monday anyway, that's great 😄

[eddycharly]

It worked with minishift for:

• pipelines: 0.10.0, 0.10.1 and 0.10.2
• triggers: 0.3.0 and 0.3.1

I fixed an issue with pipelines 0.10.0 (strangely the issue didn't show up on k8s).

I couldn't test other versions as they require a k8s cluster 1.15.

[a-roberts]

Had a go with the latest operator for installing Tekton on OpenShift (openshift-pipelines-operator) today, devel appears as the version for both the dev channel and canary releases, here's the pipeline controller yaml so you can see the label used

spec:
  progressDeadlineSeconds: 600
  replicas: 1
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app: tekton-pipelines-controller
  strategy:
    rollingUpdate:
      maxSurge: 25%
      maxUnavailable: 25%
    type: RollingUpdate
  template:
    metadata:
      annotations:
        cluster-autoscaler.kubernetes.io/safe-to-evict: "false"
        tekton.dev/release: devel
      creationTimestamp: null
      labels:
        app: tekton-pipelines-controller
        app.kubernetes.io/component: controller
        app.kubernetes.io/name: tekton-pipelines
        pipeline.tekton.dev/release: v0.11.3
        version: v0.11.3

The same thing happens with dev-channel or canary, so we could augment this PR to check for pipeline.tekton.dev/release as well, or just labels.version.

@eddycharly you're welcome to split this into PRs if you'd prefer (so labelling our Dashboard stuff accordingly, versus the lookup changes)

For Triggers, it's installing 0.2.1 currently which has no awesome labels for us to lookup, but will assume it'll be of the same pattern (@vdemeester ^^)

[eddycharly]

Boouh, we need to clean this but keeping the backward compatibility is a mess 😢

Alright, i will spit the PRs, it should help moving forward.

[a-roberts]

Boouh, we need to clean this but keeping the backward compatibility is a mess 😢

Alright, i will spit the PRs, it should help moving forward.

Agreed, thanks 😸

[eddycharly]

@a-roberts i removed the lookup related changes, the PR should be all about labels now

[a-roberts]

One question

[a-roberts]

https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/#applications-and-instances-of-applications

Why default for instance and not dashboard? All other changes look good

[eddycharly]

The app.kubernetes.io/instance label shouldn't express the application itself but the instance of the application, i feel like these manifests express the default deployment of the dashboard application.

Also it's more consistent, i did the same thing for pipelines and triggers (setting app.kubernetes.io/instance to default). I guess it will be easier to remember when trying to lookup a resource.

[eddycharly]

For example, when deploying with helm i set the value to the release name : https://github.com/tektoncd/experimental/pull/528/files#diff-718e9764773438f64515998a5c46e5a1R50

[a-roberts]

Got it, thanks for the clarifications

[a-roberts]

/lgtm
/approve
/meow hats

[tekton-robot]

@a-roberts:

In response to this:

/lgtm
/approve
/meow hats

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-roberts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [a-roberts]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment