Making naming of PubSub resources consistent and more readable #1207

nachocano · 2020-06-04T01:58:20Z

Proposed Changes

Using the convention used in the broker: cre-<owner_type>_<namespace>_<name>_<uid>.
Moving things around to avoid duplication.
Changing PS's receive adapter name. This was also hard to debug. This will also be breaking and we should probably provide a post-install script in OSS to delete old deployments.
Adding UTs.
CAL sink size limit (100) is different than regular PubSub resources (255). Setting it to that new maximum as it was causing problems in PROW as we are using very long namespaces and names..

Release Note

We have changed the naming convention we use for GCP resources in order to make it easy for users to understand who created them (e.g., a particular source, a channel?). The convention is `cre-<owner_type>_<namespace>_<name>_<uid>`. For example if a Source mysource in the namespace default with uid 47163a creates a Pub/Sub subscription, then the subscription will be called `cre-src_default_mysource_47163a`.

knative-prow-robot · 2020-06-04T01:58:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nachocano

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

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

nachocano · 2020-06-04T01:58:35Z

/assign @Harwayne
fyi @zargarpur

nachocano · 2020-06-04T03:17:09Z

/retest

…

On Wed, Jun 3, 2020 at 7:22 PM Knative Prow Robot ***@***.***> wrote: @nachocano <https://github.com/nachocano>: The following tests *failed*, say /retest to rerun all failed tests: Test name Commit Details Rerun command pull-google-knative-gcp-wi-tests 2ca0af1 <2ca0af1> link <https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/google_knative-gcp/1207/pull-google-knative-gcp-wi-tests/1268361477232791554> /test pull-google-knative-gcp-wi-tests pull-google-knative-gcp-integration-tests 2ca0af1 <2ca0af1> link <https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/google_knative-gcp/1207/pull-google-knative-gcp-integration-tests/1268361477232791555> /test pull-google-knative-gcp-integration-tests Full PR test history <https://gubernator.knative.dev/pr/google_knative-gcp/1207>. Your PR dashboard <https://gubernator.knative.dev/pr/nachocano>. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABD65DHUX6TSQDK7WXEES7LRU4ANXANCNFSM4NSF5LTA> .

nachocano · 2020-06-04T06:01:58Z

/retest

nachocano · 2020-06-04T08:10:30Z

/retest

nachocano · 2020-06-04T14:38:38Z

@Harwayne this is ready for review...

Harwayne

Make a P1 issue that blocks the 0.16 release to have the controllers clean up the old resources.

Harwayne · 2020-06-04T18:01:42Z

pkg/reconciler/events/auditlogs/resources/names.go

@@ -38,5 +39,6 @@ func GenerateTopicResourceName(s *v1alpha1.CloudAuditLogsSource) string {
 // GenerateSinkName generates a Stackdriver sink resource name for an
 // CloudAuditLogsSource.
 func GenerateSinkName(s *v1alpha1.CloudAuditLogsSource) string {
-	return fmt.Sprintf("sink-%s", string(s.UID))
+	// This name should be different than the one provided by GenerateTopicName.


Add to this comment saying why it has to be different.

I may have been wrong. The problem might have been just about the length. Let me try again with cre-src... If it doesn't work, then I'll switch back to cre-cal and add the comment

nachocano · 2020-06-04T18:54:00Z

Created an issue to track the proper removal of resources... #1217

…issue

knative-metrics-robot · 2020-06-04T19:01:36Z

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

File	Old Coverage	New Coverage	Delta
pkg/reconciler/events/auditlogs/resources/names.go	Do not exist	100.0%
pkg/reconciler/events/scheduler/resources/names.go	Do not exist	100.0%
pkg/reconciler/events/storage/resources/names.go	Do not exist	100.0%
pkg/reconciler/intevents/pullsubscription/resources/labels.go	0.0%	50.0%	50.0
pkg/reconciler/messaging/channel/resources/names.go	83.3%	100.0%	16.7
pkg/utils/naming/names.go	Do not exist	100.0%

nachocano · 2020-06-04T19:23:34Z

/test pull-google-knative-gcp-wi-tests

Harwayne

/lgtm

nachocano · 2020-06-04T20:00:21Z

seeing

2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1```

Harwayne · 2020-06-04T20:05:40Z

seeing

2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1```

error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1

I was seeing the same thing in #1156.

nachocano · 2020-06-04T20:18:50Z

seeing

2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1```

error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1

I was seeing the same thing in #1156.

ok, yeah, might be something weird with PROW. Hope the retest will pass.
@chizhg something that might be worth looking if you have any cycles left

nachocano · 2020-06-04T20:59:18Z

another one

2020/06/04 20:52:00 Unexpected error running "go build": signal: killed
2020/06/04 20:52:00 error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/pubsub_target/pod.yaml": error resolving image references: exit status 1

/retest

chizhg · 2020-06-04T20:59:20Z

seeing

2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1```

error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1

I was seeing the same thing in #1156.

ok, yeah, might be something weird with PROW. Hope the retest will pass.
@chizhg something that might be worth looking if you have any cycles left

Seems to be the OOM issue we had for build tests? We haven't seen integration test is taking so much memory for other repos so far, but I think making changes to the knative-gcp integration test Prow jobs (similar as done by knative/test-infra#2146) would fix this issue.

nachocano · 2020-06-04T21:01:07Z

seeing
2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 Unexpected error running "go build": signal: killed
2020/06/04 19:53:37 error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1```
error processing import paths in "/home/prow/go/src/github.com/google/knative-gcp/test/test_images/auditlogs_receiver/pod.yaml": error resolving image references: exit status 1
I was seeing the same thing in #1156.
ok, yeah, might be something weird with PROW. Hope the retest will pass.
@chizhg something that might be worth looking if you have any cycles left
Seems to be the OOM issue we had for build tests? We haven't seen integration test is taking so much memory for other repos so far, but I think making changes to the knative-gcp integration test Prow jobs (similar as done by knative/test-infra#2146) would fix this issue.

@capri-xiyue I need to step out for a few hours. Can you help me out and increase those limits so that we can get this PR in? If you don't have cycles, no worries, I'll do once I'm back

capri-xiyue · 2020-06-04T21:42:43Z

@nacho Submitted this PR knative/test-infra#2166 to increase the mem limit of prow e2e test jobs

capri-xiyue · 2020-06-04T21:50:42Z

/retest

chizhg · 2020-06-04T21:52:19Z

/test pull-google-knative-gcp-integration-tests
/test pull-google-knative-gcp-wi-tests

capri-xiyue · 2020-06-04T22:16:04Z

/test pull-google-knative-gcp-integration-tests

capri-xiyue · 2020-06-04T22:52:02Z

/retest

knative-test-reporter-robot · 2020-06-04T23:25:39Z

The following jobs failed:

Test name	Triggers	Retries
pull-google-knative-gcp-integration-tests	pull-google-knative-gcp-integration-tests pull-google-knative-gcp-integration-tests pull-google-knative-gcp-integration-tests pull-google-knative-gcp-integration-tests pull-google-knative-gcp-integration-tests	3/3

Job pull-google-knative-gcp-integration-tests expended all 3 retries without success.

capri-xiyue · 2020-06-04T23:26:56Z

/retest

nachocano · 2020-06-04T23:57:10Z

YESSSSSSSSSSSSSSS!!!! FINALLYYYY

Thanks @capri-xiyue and @Harwayne

nachocano added 7 commits June 3, 2020 13:10

first pass of renaming

e10c4b9

reusing gcp broker code

3b8cad4

Merge remote-tracking branch 'upstream/master' into naming

bc2fcc5

minor renamings

f5c02d9

using constant

b5690a9

moving to naming

9d3d0af

constants in naming

2ca0af1

knative-prow-robot added the do-not-merge/work-in-progress label Jun 4, 2020

knative-prow-robot requested review from liu-cong and yolocs June 4, 2020 01:58

knative-prow-robot added approved size/XXL labels Jun 4, 2020

knative-prow-robot assigned Harwayne Jun 4, 2020

googlebot added the cla: yes (override cla status due to multiple authors bug) label Jun 4, 2020

nachocano added 2 commits June 3, 2020 21:14

Merge remote-tracking branch 'upstream/master' into naming

f2ec2d3

attempt to fix cal

cab2d3b

nachocano added 2 commits June 3, 2020 23:08

nits

e1ee226

max size to 100

7befae1

nachocano changed the title ~~[WIP] Making naming of PubSub resources consistent and more readable~~ Making naming of PubSub resources consistent and more readable Jun 4, 2020

knative-prow-robot removed the do-not-merge/work-in-progress label Jun 4, 2020

Harwayne reviewed Jun 4, 2020

View reviewed changes

nachocano mentioned this pull request Jun 4, 2020

Remove old GCP resources, receive adapters, and publishers #1217

Closed

Merge remote-tracking branch 'upstream/master' into naming

2c26f5a

moved back to cre-src instead of cre-cal. Might have been the length …

04792b6

…issue

Harwayne reviewed Jun 4, 2020

View reviewed changes

knative-prow-robot added the lgtm label Jun 4, 2020

knative-prow-robot merged commit 391e078 into google:master Jun 4, 2020

grantr mentioned this pull request Jun 15, 2020

Add the cluster-name functionality to resources that need it #1055

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Making naming of PubSub resources consistent and more readable #1207

Making naming of PubSub resources consistent and more readable #1207

nachocano commented Jun 4, 2020 •

edited

Loading

knative-prow-robot commented Jun 4, 2020

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020 via email

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020

Harwayne left a comment

Harwayne Jun 4, 2020

nachocano Jun 4, 2020

nachocano commented Jun 4, 2020

knative-metrics-robot commented Jun 4, 2020

nachocano commented Jun 4, 2020

Harwayne left a comment

nachocano commented Jun 4, 2020

Harwayne commented Jun 4, 2020

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020

chizhg commented Jun 4, 2020

nachocano commented Jun 4, 2020

capri-xiyue commented Jun 4, 2020 •

edited

Loading

capri-xiyue commented Jun 4, 2020

chizhg commented Jun 4, 2020

capri-xiyue commented Jun 4, 2020

capri-xiyue commented Jun 4, 2020

knative-test-reporter-robot commented Jun 4, 2020

capri-xiyue commented Jun 4, 2020

nachocano commented Jun 4, 2020

Making naming of PubSub resources consistent and more readable #1207

Making naming of PubSub resources consistent and more readable #1207

Conversation

nachocano commented Jun 4, 2020 • edited Loading

Proposed Changes

knative-prow-robot commented Jun 4, 2020

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020 via email

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020

Harwayne left a comment

Choose a reason for hiding this comment

Harwayne Jun 4, 2020

Choose a reason for hiding this comment

nachocano Jun 4, 2020

Choose a reason for hiding this comment

nachocano commented Jun 4, 2020

knative-metrics-robot commented Jun 4, 2020

nachocano commented Jun 4, 2020

Harwayne left a comment

Choose a reason for hiding this comment

nachocano commented Jun 4, 2020

Harwayne commented Jun 4, 2020

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020

chizhg commented Jun 4, 2020

nachocano commented Jun 4, 2020

capri-xiyue commented Jun 4, 2020 • edited Loading

capri-xiyue commented Jun 4, 2020

chizhg commented Jun 4, 2020

capri-xiyue commented Jun 4, 2020

capri-xiyue commented Jun 4, 2020

knative-test-reporter-robot commented Jun 4, 2020

capri-xiyue commented Jun 4, 2020

nachocano commented Jun 4, 2020

nachocano commented Jun 4, 2020 •

edited

Loading

capri-xiyue commented Jun 4, 2020 •

edited

Loading