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.

Sign up for GitHub

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

Add recommended k8s labels #1327

Merged
merged 1 commit into from
May 4, 2020
Merged

Add recommended k8s labels #1327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions base/200-clusterrole.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: tekton-dashboard-minimal
namespace: tekton-pipelines
labels:
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-dashboard
rules:
- apiGroups: ["apiextensions.k8s.io"]
resources: ["customresourcedefinitions"]
Expand Down
4 changes: 4 additions & 0 deletions base/201-clusterrolebinding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: tekton-dashboard-minimal
labels:
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-dashboard
subjects:
- kind: ServiceAccount
name: tekton-dashboard
Expand Down
4 changes: 4 additions & 0 deletions base/202-extension-crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: extensions.dashboard.tekton.dev
labels:
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-dashboard
spec:
group: dashboard.tekton.dev
names:
Expand Down
6 changes: 4 additions & 2 deletions base/203-serviceaccount.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
apiVersion: v1
kind: ServiceAccount
metadata:
labels:
app: tekton-dashboard
name: tekton-dashboard
namespace: tekton-pipelines
labels:
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-dashboard
17 changes: 15 additions & 2 deletions base/300-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,31 @@ metadata:
name: tekton-dashboard
namespace: tekton-pipelines
labels:
app.kubernetes.io/name: dashboard
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
app.kubernetes.io/version: devel
app.kubernetes.io/part-of: tekton-dashboard
dashboard.tekton.dev/release: "devel"
app: tekton-dashboard
version: "devel"
dashboard.tekton.dev/release: "devel"
spec:
replicas: 1
selector:
matchLabels:
app: tekton-dashboard
app.kubernetes.io/name: dashboard
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-dashboard
template:
metadata:
name: tekton-dashboard
labels:
app.kubernetes.io/name: dashboard
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks for the clarifications

app.kubernetes.io/version: devel
app.kubernetes.io/part-of: tekton-dashboard
app: tekton-dashboard
spec:
serviceAccountName: tekton-dashboard
Expand Down
12 changes: 10 additions & 2 deletions base/300-service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,22 @@ metadata:
name: tekton-dashboard
namespace: tekton-pipelines
labels:
app.kubernetes.io/name: dashboard
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
app.kubernetes.io/version: devel
app.kubernetes.io/part-of: tekton-dashboard
dashboard.tekton.dev/release: "devel"
app: tekton-dashboard
version: "devel"
dashboard.tekton.dev/release: "devel"
spec:
ports:
- name: http
protocol: TCP
port: 9097
targetPort: 9097
selector:
app: tekton-dashboard
app.kubernetes.io/name: dashboard
app.kubernetes.io/component: dashboard
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-dashboard