1488787 - Use tc-lib-pulse in taskcluster-notify

Status: RESOLVED FIXED

(Taskcluster :: Services, enhancement)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

Switch from using PulseListener to using tc-lib-pulse to consume messages.

Comment 1

[Biboswan Roy]

As you suggested, I will start here.

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Sounds great!  Let me know if you hit any snags.  There will probably be snags since this is the first service to be done.

Comment 3

[Biboswan Roy]

 links are broken on https://github.com/taskcluster/taskcluster-notify/blob/master/docs/usage.md. Further, I think I require aws credentials

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

That page's "official" location is https://docs.taskcluster.net/docs/reference/core/taskcluster-notify/docs/usage (or a similar URL for any other deployment of Taskcluster).  So the links don't work on Github, unfortunately.

You should be able to run the tests without AWS creds.  Half will be skipped, but those are duplicates of tests run against a mock SQS.  Since you're not changing the parts that talk to SQS, those mocks should be plenty for testing.  If that doesn't work for some reason, we can get you AWS credentials, but getting the permissions right for those will probably be tricky and annoying.

Comment 5

[Biboswan Roy]

Do I need scopes for notify?

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

To run the tests, no.  However, the tests don't actually talk to pulse, so they won't do a good job of exercising your patch.  Talking to pulse in the tests is tricky anyway, since it's listening for messages sent by another service (Queue).

So a good approach may be for you to run the server directly.  Something like

  DEBUG=* NODE_ENV=development node src/main.js server

and, once you've made your changes, that will need a TASKCLUSTER_ROOT_URL, TASKCLUSTER_CLIENT_ID, and TASKCLUSTER_ACCESS_TOKEN, along with a PULSE_NAMESPACE.  I have set you up with the scope to claim taskcluster-user-biboswan98:

  https://tools.taskcluster.net/auth/roles/mozillians-user%3Abiboswan98

to use that, you'll need to login to tools with the email registered in mozillians, then use https://github.com/taskcluster/taskcluster-cli's `taskcluster signin` to generate credentials and add them to your shell.

Comment 7

[Biboswan Roy]

Note: Both pulse publisher and tc-lib-pulse client connections are made successfully.

Error: Channel closed by server: 403 (ACCESS-REFUSED) with message "ACCESS_REFUSED - access to exchange 'exchange/taskcluster-testing-biboswan98-1/v1/notification' in vhost '/' refused for user 'taskcluster-testing-biboswan98-1'"
This is probably due to No Write Permissions On Exchange when sendToQueue() is called.

What I suspect is that write access is given (using regex) to user 'taskcluster-testing-biboswan98' but not 'taskcluster-testing-biboswan98-1'.

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

The permissions for read, write, and configure on that user are
  ^(queue/taskcluster-testing-biboswan98/.*|exchange/taskcluster-testing-biboswan98/.*)

In this case, `taskcluster-testing-biboswan98` is the namespace, and the library should be creating exchanges and queues using that namespace, *not* using the username (namespace + "-1" or "-2").  Can you point me to the code resulting in that error?

Comment 9

[Biboswan Roy]

https://github.com/taskcluster/pulse-publisher/blob/master/src/exchanges.js#L210 this line is mentioned in the error log flow

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

So it looks like the exchangePrefix is `exchange/taskcluster-testing-biboswan98-1/v1` when it should be `exchange/taskcluster-testing-biboswan98/v1`.  Can you figure out what's causing that?

Comment 11

[Biboswan Roy]

Yeah that's what I was thinking too. Looking

Comment 12

[Biboswan Roy]

https://github.com/taskcluster/taskcluster-notify/pull/37

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

2018-10-10T17:38:49.531755+00:00 app[handler.1]: ReferenceError: url is not defined
2018-10-10T17:38:49.531777+00:00 app[handler.1]: at PulsePublisher.publishReference (/app/node_modules/taskcluster-lib-pulse/src/publisher.js:407:30)

I'm landing a fix to tc-lib-pulse now as v2.0.5, then will require that version in tc-notify.

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

Reminders for the next service:
 * new PULSE_.. env vars
 * add pulse:namespace:<projectName> to the client

There are a few other errors in that publishing stuff.  I've set PUBLISH_METADATA to false for the moment.

Comment 15

[Dustin J. Mitchell [:dustin] (he/him)]

2018-10-10T19:26:58.453070+00:00 app[handler.1]: AssertionError [ERR_ASSERTION]: Must pass a queueName
2018-10-10T19:26:58.453072+00:00 app[handler.1]: at new PulseConsumer (/app/node_modules/taskcluster-lib-pulse/src/consumer.js:22:5)

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

ah, LISTENER_QUEUE_NAME wasn't set (weird, because this PR did not add that variable!)

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

OK, everything's set now, after some debugging of the legacy publishing support.