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

Jump to bottom

Updates selector labels to app.kubernetes.io #12587

Merged
merged 4 commits into from
Feb 3, 2022
Merged

Updates selector labels to app.kubernetes.io #12587

merged 4 commits into from
Feb 3, 2022

Conversation

psschwei
Copy link
Contributor

@psschwei psschwei commented Feb 2, 2022 •
edited by dprotaso
Loading

Part of #12215

Proposed Changes

  • Drop usage of serving.knative.dev/release in selectors for Knative resources in favor of the recommended app.kubernetes.io

Release Note

Switches selectors for Knative resources to use the recommended `app.kubernetes.io` labels

/assign @dprotaso

@knative-prow-robot knative-prow-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Feb 2, 2022
@psschwei
Copy link
Contributor Author

psschwei commented Feb 2, 2022

Think the unit test failure is a flake -- TestGlobalResyncOnUpdateDomainConfigMap is the failed test, but it passed when I ran the bucket locally

dprotaso
dprotaso reviewed Feb 3, 2022
View reviewed changes
test/e2e-auto-tls-tests.sh Outdated Show resolved Hide resolved
@dprotaso
Copy link
Member

dprotaso commented Feb 3, 2022

I updated the PR body to say Part of since the plan is to drop the labels in the subsequent release

@dprotaso
Copy link
Member

dprotaso commented Feb 3, 2022

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2022
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, psschwei

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:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2022
@dprotaso
Copy link
Member

dprotaso commented Feb 3, 2022

/hold

I realize the label is wrong - we switched away from part-of

serving/config/core/configmaps/defaults.yaml

Lines 21 to 24 in 0e6c2d8

app.kubernetes.io/name: knative-serving
app.kubernetes.io/component: controller
app.kubernetes.io/version: devel
serving.knative.dev/release: devel

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 3, 2022
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2022
@codecov
Copy link

codecov bot commented Feb 3, 2022 •
edited
Loading

Codecov Report

Merging #12587 (cf99d31) into main (4e17128) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #12587      +/-   ##
==========================================
+ Coverage   87.48%   87.52%   +0.04%     
==========================================
  Files         195      195              
  Lines        9718     9718              
==========================================
+ Hits         8502     8506       +4     
+ Misses        931      928       -3     
+ Partials      285      284       -1
Impacted Files Coverage Δ
pkg/reconciler/revision/background.go 91.81% <0.00%> (+1.81%) ⬆️
pkg/autoscaler/statserver/server.go 79.61% <0.00%> (+1.94%) ⬆️

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 4e17128...cf99d31. Read the comment docs.

dprotaso
dprotaso reviewed Feb 3, 2022
View reviewed changes
config/core/webhooks/configmap-validation.yaml Outdated
Comment on lines 35 to 38
- key: app.kubernetes.io/name
operator: In
values:
- knative-serving
Copy link
Member

Choose a reason for hiding this comment

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

I realized this can be a matchLabels instead of a matchExpressions

https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#resources-that-support-set-based-requirements

Copy link
Member

Choose a reason for hiding this comment

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

ditto for the other repos

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all done

@psschwei
Copy link
Contributor Author

psschwei commented Feb 3, 2022

/retest

@dprotaso
Copy link
Member

dprotaso commented Feb 3, 2022

/lgtm
/hold cancel

@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 3, 2022
@knative-prow-robot knative-prow-robot merged commit 62d5aa3 into knative:main Feb 3, 2022
@psschwei psschwei deleted the update-labels branch March 10, 2022 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dprotaso dprotaso dprotaso left review comments

Assignees

@dprotaso dprotaso

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@psschwei @dprotaso @knative-prow-robot