serving icon indicating copy to clipboard operation
serving copied to clipboard

revision should not be adding unprefixed "app" labels by default

Open maschmid opened this issue 3 years ago • 2 comments

A Deployment (and its pod template) created by a revision contains a "app: <revisionName>" label by default since

https://github.com/knative/serving/blob/main/pkg/reconciler/revision/resources/meta.go#L52-L54

// If users don't specify an app: label we will automatically
// populate it with the revision name to get the benefit of richer
// tracing information.

Whatever the original reason was, I suspect it probably isn't valid anymore, not is a valid reason to disobey the k8s policy about prefixes

Automated system components (e.g. kube-scheduler, kube-controller-manager, kube-apiserver, kubectl, or other third-party automation) which add labels to end-user objects must specify a prefix.

https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/

What version of Knative?

all

0.9.x 0.10.x 0.11.x Output of git describe --dirty

Expected Behavior

Deployment created by a ksvc should not have an uprefixed "app" label, or any other unprefixed labels

Actual Behavior

Deployment (and its pod template) has, among other prefixed labels, an app: <revisionName> label

Steps to Reproduce the Problem

Deploy a ksvc

apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: helloworld-go
spec:
  template:
    spec:
      containers:
      - image: gcr.io/knative-samples/helloworld-go
        env:
        - name: TARGET
          value: "Go Sample v1"

Notice the revision Deployment has an app: helloworld-go-00001 label, besides all the other proper labels

      serving.knative.dev/configuration: helloworld-go
      serving.knative.dev/configurationGeneration: "1"
      serving.knative.dev/configurationUID: fde17c2d-6fa1-4f9c-97be-0e2da090c1d1
      serving.knative.dev/revision: helloworld-go-00001
      serving.knative.dev/revisionUID: d72fbf19-9b79-479c-9f80-fe7581703b77
      serving.knative.dev/service: helloworld-go
      serving.knative.dev/serviceUID: ccec2e07-1ab6-485b-a171-2d3b83bcf7be

maschmid avatar Jul 28 '22 13:07 maschmid

This behaviour has been around for many years and I have concerns about changing it since it could be a breaking change for end users.

Besides not following k8s policy what's the impact of keep the label as is?

dprotaso avatar Jul 29 '22 15:07 dprotaso

We have a case where a customer's custom metering solution uses their own app label. Their semantics is such that a pod "inherits" an app label from a namespace if not defined on the pod. knative adding app=<revision name> obviously breaks this logic and forces them to declare app labels on the ksvc revision template explicitly.

maschmid avatar Aug 03 '22 07:08 maschmid

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

github-actions[bot] avatar Nov 02 '22 01:11 github-actions[bot]

This issue or pull request is stale because it has been open for 90 days with no activity.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close

/lifecycle stale

knative-prow-robot avatar Dec 02 '22 01:12 knative-prow-robot