Do not print serviceUID and configUID labels in service export result #1194

navidshaikh · 2021-01-19T08:51:03Z

Description

While exporting the service using service export command, ignore
the service labels with keys 'configUID' and configUID

Changes

Strip the mentioned labels from service before export

Reference

see knative/serving#10539 for further details

/lint

While exporting the service using `service export` command, ignore the service labels with keys 'configUID' and `configUID` see knative/serving#10539 for further details.

knative-prow-robot

@navidshaikh: 1 warning.

In response to this:

Description

While exporting the service using service export command, ignore
the service labels with keys 'configUID' and configUID

Changes

Strip the mentioned labels from service before export

Reference

see knative/serving#10539 for further details

/lint

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.

pkg/kn/commands/service/export.go

dsimansk · 2021-01-19T09:03:16Z

@navidshaikh thanks looks good!

/lgtm
/hold for other to take a look

rhuss

Looks good, but could we add a test (unit/e2e) + changelog entry please ?

pkg/kn/commands/service/export.go

maximilien

/ok-to-test
/lgtm
/hold to allow others to review

maximilien · 2021-01-20T00:48:26Z

pkg/kn/commands/service/export.go

@@ -34,17 +34,36 @@ import (
 	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
 )

-var IGNORED_SERVICE_ANNOTATIONS = []string{
+// IgnoredServiceAnnotations defines the annotation keys which should be
+// removed from service annotations before export


Is there a link on the API or docs repo where these are to be found?

they are in the main branch of serving (and will be part of 0.21 release) knative/serving#10539
/cc @duglin might know better if the docs etc are in order

knative-metrics-robot · 2021-01-20T00:51:58Z

The following is the coverage report on the affected files.
Say /test pull-knative-client-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/kn/commands/service/export.go	86.1%	86.8%	0.7

knative-prow-robot · 2021-01-20T07:10:41Z

@navidshaikh: GitHub didn't allow me to request PR reviews from the following users: might, better, are, order, docs, etc, in, know, if, the.

Note that only knative members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

they are in the main branch of serving (and will be part of 0.21 release) knative/serving#10539
/cc @duglin might know better if the docs etc are in order

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.

rhuss · 2021-01-20T07:50:44Z

/lgtm
/approve

knative-prow-robot · 2021-01-20T07:50:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maximilien, navidshaikh, rhuss

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~~ [maximilien,navidshaikh,rhuss]

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

rhuss · 2021-01-20T09:00:37Z

@googlebot recheck

navidshaikh · 2021-01-20T09:01:11Z

@googlebot I signed it!

navidshaikh · 2021-01-20T09:06:40Z

/unhold

David meets all criteria for an approver: - [x] Reviewer for at least 3 months - [x] Primary reviewer for at least 10 substantial PRs to the codebase, e.g. * knative#1246 * knative#1194 * knative#738 * knative#832 * knative#1016 * knative#877 * knative#667 * knative#697 * knative#1212 * knative#835 - [x] Reviewed at least 30 PRs to the codebase ([38 assigned PRs](https://github.com/knative/client/pulls?q=type%3Apr+assignee%3Adsimansk+)) - [x] Nominated by a WG lead (rhuss) Congrats David ! Thanks a lot for your awesome work & contributions.

David meets all criteria for an approver: - [x] Reviewer for at least 3 months - [x] Primary reviewer for at least 10 substantial PRs to the codebase, e.g. * #1246 * #1194 * #738 * #832 * #1016 * #877 * #667 * #697 * #1212 * #835 - [x] Reviewed at least 30 PRs to the codebase ([38 assigned PRs](https://github.com/knative/client/pulls?q=type%3Apr+assignee%3Adsimansk+)) - [x] Nominated by a WG lead (rhuss) Congrats David ! Thanks a lot for your awesome work & contributions.

Ignore serviceUID and configUID labels in service export result

6f13dfa

While exporting the service using `service export` command, ignore the service labels with keys 'configUID' and `configUID` see knative/serving#10539 for further details.

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

knative-prow-robot requested review from cppforlife and maximilien January 19, 2021 08:51

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

knative-prow-robot reviewed Jan 19, 2021

View reviewed changes

pkg/kn/commands/service/export.go Outdated Show resolved Hide resolved

knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 19, 2021

Fix the var names per golint recommendation

cc37a7b

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2021

knative-prow-robot assigned dsimansk Jan 19, 2021

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

rhuss reviewed Jan 19, 2021

View reviewed changes

Ignore the serviceUID and configUID labels from revision labels

ceaadfc

dsimansk reviewed Jan 19, 2021

View reviewed changes

pkg/kn/commands/service/export.go Show resolved Hide resolved

knative-prow-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 19, 2021

Add e2e for verifying configUID and serviceUID labels are absent

b5aee52

navidshaikh changed the title ~~Ignore serviceUID and configUID labels in service export result~~ Do not print serviceUID and configUID labels in service export result Jan 19, 2021

Add CHANGELOG

75b214f

navidshaikh requested review from rhuss and dsimansk January 19, 2021 14:47

maximilien approved these changes Jan 20, 2021

View reviewed changes

knative-prow-robot assigned maximilien Jan 20, 2021

knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. lgtm Indicates that a PR is ready to be merged. labels Jan 20, 2021

knative-prow-robot requested a review from duglin January 20, 2021 07:10

knative-prow-robot assigned rhuss Jan 20, 2021

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 20, 2021

knative-prow-robot merged commit dc52d5b into knative:master Jan 20, 2021

rhuss mentioned this pull request Mar 9, 2021

Add David as approver #1255

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not print serviceUID and configUID labels in service export result #1194

Do not print serviceUID and configUID labels in service export result #1194

navidshaikh commented Jan 19, 2021

knative-prow-robot left a comment

dsimansk commented Jan 19, 2021

rhuss left a comment

maximilien left a comment

maximilien Jan 20, 2021

navidshaikh Jan 20, 2021

knative-metrics-robot commented Jan 20, 2021

knative-prow-robot commented Jan 20, 2021

rhuss commented Jan 20, 2021

knative-prow-robot commented Jan 20, 2021

rhuss commented Jan 20, 2021

navidshaikh commented Jan 20, 2021

navidshaikh commented Jan 20, 2021

Do not print serviceUID and configUID labels in service export result #1194

Do not print serviceUID and configUID labels in service export result #1194

Conversation

navidshaikh commented Jan 19, 2021

Description

Changes

Reference

knative-prow-robot left a comment

Choose a reason for hiding this comment

Description

Changes

Reference

dsimansk commented Jan 19, 2021

rhuss left a comment

Choose a reason for hiding this comment

maximilien left a comment

Choose a reason for hiding this comment

maximilien Jan 20, 2021

Choose a reason for hiding this comment

navidshaikh Jan 20, 2021

Choose a reason for hiding this comment

knative-metrics-robot commented Jan 20, 2021

knative-prow-robot commented Jan 20, 2021

rhuss commented Jan 20, 2021

knative-prow-robot commented Jan 20, 2021

rhuss commented Jan 20, 2021

navidshaikh commented Jan 20, 2021

navidshaikh commented Jan 20, 2021