canonical-ubuntu-qa team mailing list archive

[Skia <2069751@xxxxxxxxxxxxxxxxxx>]

Public bug reported:

Autopkgtest-cloud uses a RabbitMQ instance to manage the queue of tests
that needs to be run. Those tests requests are coming from Britney (and
security britney too), the developers through the `/request.cgi`
endpoint, and the admins with the `run-autopkgtest` script.

Those tests requests need to convey some amount of information, like the
package name, the triggers, the requester, PPAs, etc...

The current defacto format looks like the following [1]:
```
hello\n{"triggers": ["hello/1.2.3ubuntu2"], "submit-time": "2024-02-22 01:56:14"}
```
More data can be embedded in the JSON, but it's not the point of this bug.

The issue with this format is this: `package_name` + `\n` + `some JSON object`.
That means that:
* everything that pushes or read test requests to/from the queue need to speak JSON
* every time something needs to handle a test requests, it's not **only** JSON, and the prepended `package_name\n` needs to be dealt with

While the current format **might** have a **slight** advantage when
building **some** particular shell pipelines, it's in overall just a
custom format that is mostly JSON, and could easily be 100% JSON anyway.

The proposition here is to start an effort to convert the AMQP message format to 100% JSON by adding the package name in a `package` key to the existing JSON object, making the message presented above look like the following:
```
{"package": "hello", "triggers": ["hello/1.2.3ubuntu2"], "submit-time": "2024-02-22 01:56:14"}
```
Gain:
* simplify existing code by just parsing JSON instead of custom format
* simplify the development of tooling by only requiring JSON parsing, which is already implemented in most languages
* have a

For this transition to be as smooth as possible, the steps could be the following:
1. Make the autopkgtest-cloud worker code aware of the new format, by discriminating on the first character being a `{` or not.
2. Make `request.cgi` able to queue tests with the new message format, possibly with a manual flag at first, like `&amqp-format=new` in the URL, that could be non-default at first, then later turned to be the default, but still easily removable if some tests requests go wrong.
3. Make Britney use the new format after a while of stable usage by the developers.
4. One cycle or year later, when everything has completely stabilized, completely remove the old format support from all the scripts that deal with it.

This may sound like a big amount of work, but it's in fact only some
tiny portions of code, quite simple, that are just a bit scattered
across multiple files and repositories. Also, if performed in the right
order, none of those changes should require any coordination with the
rest of the pieces, which also means that both rollbacks or iteration
are quite easy.


[1]: it's "defined" here: https://autopkgtest-cloud.readthedocs.io/en/latest/architecture.html#test-request-format
     One problem being that the `\n` sometimes is a single space, and nothing is really enforced anywhere here, making it complicated to build tools that iterate over the whole queue, because there's no consistency.

** Affects: auto-package-testing
     Importance: Wishlist
     Assignee: Skia (hyask)
         Status: New

** Changed in: auto-package-testing
     Assignee: (unassigned) => Skia (hyask)

-- 
You received this bug notification because you are a member of
Canonical's Ubuntu QA, which is subscribed to Auto Package Testing.
https://bugs.launchpad.net/bugs/2069751

Title:
  AMQP message format is very inconvenient

Status in Auto Package Testing:
  New

Bug description:
  Autopkgtest-cloud uses a RabbitMQ instance to manage the queue of
  tests that needs to be run. Those tests requests are coming from
  Britney (and security britney too), the developers through the
  `/request.cgi` endpoint, and the admins with the `run-autopkgtest`
  script.

  Those tests requests need to convey some amount of information, like
  the package name, the triggers, the requester, PPAs, etc...

  The current defacto format looks like the following [1]:
  ```
  hello\n{"triggers": ["hello/1.2.3ubuntu2"], "submit-time": "2024-02-22 01:56:14"}
  ```
  More data can be embedded in the JSON, but it's not the point of this bug.

  The issue with this format is this: `package_name` + `\n` + `some JSON object`.
  That means that:
  * everything that pushes or read test requests to/from the queue need to speak JSON
  * every time something needs to handle a test requests, it's not **only** JSON, and the prepended `package_name\n` needs to be dealt with

  While the current format **might** have a **slight** advantage when
  building **some** particular shell pipelines, it's in overall just a
  custom format that is mostly JSON, and could easily be 100% JSON
  anyway.

  The proposition here is to start an effort to convert the AMQP message format to 100% JSON by adding the package name in a `package` key to the existing JSON object, making the message presented above look like the following:
  ```
  {"package": "hello", "triggers": ["hello/1.2.3ubuntu2"], "submit-time": "2024-02-22 01:56:14"}
  ```
  Gain:
  * simplify existing code by just parsing JSON instead of custom format
  * simplify the development of tooling by only requiring JSON parsing, which is already implemented in most languages
  * have a

  For this transition to be as smooth as possible, the steps could be the following:
  1. Make the autopkgtest-cloud worker code aware of the new format, by discriminating on the first character being a `{` or not.
  2. Make `request.cgi` able to queue tests with the new message format, possibly with a manual flag at first, like `&amqp-format=new` in the URL, that could be non-default at first, then later turned to be the default, but still easily removable if some tests requests go wrong.
  3. Make Britney use the new format after a while of stable usage by the developers.
  4. One cycle or year later, when everything has completely stabilized, completely remove the old format support from all the scripts that deal with it.

  This may sound like a big amount of work, but it's in fact only some
  tiny portions of code, quite simple, that are just a bit scattered
  across multiple files and repositories. Also, if performed in the
  right order, none of those changes should require any coordination
  with the rest of the pieces, which also means that both rollbacks or
  iteration are quite easy.


  
  [1]: it's "defined" here: https://autopkgtest-cloud.readthedocs.io/en/latest/architecture.html#test-request-format
       One problem being that the `\n` sometimes is a single space, and nothing is really enforced anywhere here, making it complicated to build tools that iterate over the whole queue, because there's no consistency.

To manage notifications about this bug go to:
https://bugs.launchpad.net/auto-package-testing/+bug/2069751/+subscriptions