Adopt `app.kubernetes.io` labels #5628

upodroid · 2021-08-05T19:10:40Z

Pre-review Checklist

At least 80% unit test coverage
E2E tests for any new behavior
Docs PR for any user-facing impact
Spec PR for any new API feature
Conformance test for any change to the spec

Release Note

adopted app.kubernetes.io labels

Docs

codecov · 2021-08-05T19:16:42Z

Codecov Report

Merging #5628 (35f5f9e) into main (ca9e469) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #5628      +/-   ##
==========================================
- Coverage   82.73%   82.68%   -0.06%     
==========================================
  Files         200      200              
  Lines        6245     6261      +16     
==========================================
+ Hits         5167     5177      +10     
- Misses        748      750       +2     
- Partials      330      334       +4

Impacted Files	Coverage Δ
pkg/reconciler/broker/controller.go	`88.63% <0.00%> (-4.39%)`	⬇️
pkg/apis/eventing/v1/broker_validation.go	`83.33% <0.00%> (-3.34%)`	⬇️
pkg/adapter/mtping/controller.go	`68.75% <0.00%> (-1.84%)`	⬇️
pkg/reconciler/sugar/trigger/controller.go	`55.55% <0.00%> (-1.59%)`	⬇️
pkg/duck/listable.go	`81.94% <0.00%> (-1.16%)`	⬇️
pkg/reconciler/sugar/namespace/controller.go	`79.16% <0.00%> (-0.84%)`	⬇️
pkg/reconciler/apiserversource/controller.go	`89.47% <0.00%> (-0.53%)`	⬇️
pkg/reconciler/pingsource/controller.go	`85.71% <0.00%> (-0.50%)`	⬇️
...econciler/inmemorychannel/dispatcher/controller.go	`87.50% <0.00%> (-0.26%)`	⬇️
...econciler/inmemorychannel/controller/controller.go	`88.00% <0.00%> (-0.24%)`	⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca9e469...35f5f9e. Read the comment docs.

houshengbo · 2021-08-30T16:58:55Z

@upodroid why not add these labels to CRDs?

upodroid · 2021-08-31T17:02:34Z

Noticed imc, eventing-controller and few other apps are missing services with metrics endpoints.

I added that + labels to CRDs, I still need to label other misc items.

upodroid · 2021-08-31T21:01:24Z

This is ready now.

upodroid · 2021-08-31T21:56:43Z

/assign @lberk @dprotaso

dprotaso · 2021-09-01T18:04:49Z

/unassign @dprotaso
/assign @lionelvillard

lionelvillard · 2021-09-01T18:35:28Z

config/brokers/mt-channel-broker/deployments/broker-filter.yaml

@@ -19,6 +19,9 @@ metadata:
  namespace: knative-eventing
  labels:
    eventing.knative.dev/release: devel
+    app.kubernetes.io/name: mt-broker-filter


can you drop mt- prefix?

lionelvillard · 2021-09-01T18:35:38Z

config/brokers/mt-channel-broker/deployments/broker-ingress.yaml

@@ -19,6 +19,9 @@ metadata:
  namespace: knative-eventing
  labels:
    eventing.knative.dev/release: devel
+    app.kubernetes.io/name: mt-broker-ingress


lionelvillard · 2021-09-01T18:38:32Z

config/brokers/mt-channel-broker/deployments/controller.yaml

@@ -95,3 +101,25 @@ spec:
          containerPort: 9090
        - name: profiling
          containerPort: 8008
+---


not sure we want this below, at least not in this PR

We should have it, especially for gathering metrics via ServiceMonitors

Can you then update this PR description (and release notes) stating that these controllers are now exposed?

aren't ServiceMonitors running in the cluster?

I'd opt to move this to a different PR give the nature of the described changes. Thanks!

Updated release notes.

aren't ServiceMonitors running in the cluster?

Yes

+1 on moving that to a new PR

/cc @skonto regarding ServiceMonitors

I'll move the new Services to a separate PR.

FYI, Service Monitors are CRDs that ship with Prometheus Operator. It is a way of defining Prometheus scrape configuration.

lionelvillard · 2021-09-01T19:18:13Z

config/brokers/mt-channel-broker/deployments/broker-filter.yaml

@@ -28,6 +31,9 @@ spec:
      labels:
        eventing.knative.dev/brokerRole: filter
        eventing.knative.dev/release: devel
+        app.kubernetes.io/name: mt-broker-filter


here too. In general, can you remove all instances of mt- in labels?

Removed more instances of mt- prefix.

pierDipi · 2021-09-09T16:59:41Z

/lgtm
/retest

/cc @lionelvillard

upodroid · 2021-09-14T19:53:28Z

We are missing approved label on this PR.

knative-prow-robot · 2021-09-14T20:23:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi, upodroid

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~~ [pierDipi]

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

matzew · 2021-09-15T07:56:45Z

@upodroid thanks for the PR!

Would you be interested in doing the same for:

repositories too? 🙏

upodroid · 2021-09-15T10:30:09Z

I'll do the same for those as well.

add k8s labels

12c07c5

google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Aug 5, 2021

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 5, 2021

patch generate-yamls script too

ef1d66f

knative-prow-robot added the area/test-and-release Test infrastructure, tests or release label Aug 5, 2021

add labels to more imc and brokers

31f08a0

upodroid added 4 commits August 31, 2021 19:09

fix typo and label more services

b5a399b

remove duplicated service :(

89fd25c

squash typos

073111d

add labels to remaining resources

dded9c5

knative-prow-robot assigned dprotaso and lberk Aug 31, 2021

knative-prow-robot assigned lionelvillard and unassigned dprotaso Sep 1, 2021

lionelvillard reviewed Sep 1, 2021

View reviewed changes

remove mt prefix

fbd679e

lionelvillard reviewed Sep 1, 2021

View reviewed changes

upodroid added 2 commits September 1, 2021 20:39

remove more instances of mt- prefix

5a1dcc3

move new services to a separate PR

35f5f9e

knative-prow-robot requested a review from lionelvillard September 9, 2021 16:59

knative-prow-robot assigned pierDipi Sep 9, 2021

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2021

pierDipi approved these changes Sep 14, 2021

View reviewed changes

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 14, 2021

knative-prow-robot merged commit 602ea29 into knative:main Sep 14, 2021

antoineco mentioned this pull request Feb 4, 2022

Fix value of "component" label for the Eventing Controller #6135

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adopt `app.kubernetes.io` labels #5628

Adopt `app.kubernetes.io` labels #5628

upodroid commented Aug 5, 2021 •

edited

Loading

codecov bot commented Aug 5, 2021 •

edited

Loading

houshengbo commented Aug 30, 2021

upodroid commented Aug 31, 2021

upodroid commented Aug 31, 2021

upodroid commented Aug 31, 2021

dprotaso commented Sep 1, 2021

lionelvillard Sep 1, 2021

upodroid Sep 1, 2021

lionelvillard Sep 1, 2021

upodroid Sep 1, 2021

lionelvillard Sep 1, 2021

upodroid Sep 1, 2021

lionelvillard Sep 1, 2021

lionelvillard Sep 1, 2021 •

edited

Loading

lberk Sep 1, 2021

upodroid Sep 1, 2021

matzew Sep 2, 2021

upodroid Sep 2, 2021

lionelvillard Sep 1, 2021

upodroid Sep 1, 2021

pierDipi commented Sep 9, 2021

upodroid commented Sep 14, 2021

knative-prow-robot commented Sep 14, 2021

matzew commented Sep 15, 2021

upodroid commented Sep 15, 2021

Adopt app.kubernetes.io labels #5628

Adopt app.kubernetes.io labels #5628

Conversation

upodroid commented Aug 5, 2021 • edited Loading

Pre-review Checklist

codecov bot commented Aug 5, 2021 • edited Loading

Codecov Report

houshengbo commented Aug 30, 2021

upodroid commented Aug 31, 2021

upodroid commented Aug 31, 2021

upodroid commented Aug 31, 2021

dprotaso commented Sep 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lionelvillard Sep 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierDipi commented Sep 9, 2021

upodroid commented Sep 14, 2021

knative-prow-robot commented Sep 14, 2021

matzew commented Sep 15, 2021

upodroid commented Sep 15, 2021

Adopt `app.kubernetes.io` labels #5628

Adopt `app.kubernetes.io` labels #5628

upodroid commented Aug 5, 2021 •

edited

Loading

codecov bot commented Aug 5, 2021 •

edited

Loading

lionelvillard Sep 1, 2021 •

edited

Loading