#122 Greenwave uses wrong subject dict for query that backs `decision.update` fedmsgs for composes, so decisions are inaccurate
Closed: Duplicate 3 years ago Opened 3 years ago by adamwill.

I just looked into the Greenwave decisions for Rawhide composes for the first time (as part of the Rawhide gating project). I don't think it's working right.

The decisions for Rawhide composes have many many test-result-missing entries in the unsatisfied_requirements. For instance, here's one from the Fedora-Rawhide-20180123.n.0 decision:

    {
        "item": {
            "item": "Fedora-Rawhide-20180123.n.1",
            "type": "compose"
        },
        "scenario": "fedora.Server-dvd-iso.x86_64.64bit",
        "testcase": "compose.server_firewall_default",
        "type": "test-result-missing"
    },

However, that test with that scenario did run for that compose, and the result is in ResultsDB. So why didn't greenwave find it? I think the problem is the item. For most openQA tests, the item is not the compose ID, because we can be more specific: we're testing an image, so the item is the image filename. For this result, for instance, the item is "Fedora-Server-dvd-x86_64-Rawhide-20180123.n.1.iso".

I'm not sure what the best way to fix this would be. For any result filed via resultsdb_conventions for a compose, you can rely on the extradata key productmd.compose.id existing with the value as the compose ID, which is the item you're looking for. That I believe is everything currently reporting compose-level results to ResultsDB - openQA and Autocloud, because the reporting mechanisms for both of those use resultsdb_conventions. But I don't know if you want to go that route, or do something else...

@mohanboddu


oh, hm, you can actually get a good decision just by using this as the request subject:

[{"productmd.compose.id": "Fedora-Rawhide-20180123.n.1", "type": "compose"}]

still, I need to look into whether anything is assuming the subjects will be items. Like the fedmsgs...

So, I was way off in my initial evaluation here, but the greenwave fedmsgs are wrong. Here is a real one for a Rawhide compose.

As you can see, the actual subject used for the query there is:

"subject": [
  {
    "item": "Fedora-Everything-netinst-x86_64-Rawhide-20180123.n.1.iso", 
    "type": "compose"
  }
]

so that's different from what I tried, but it's clearly wrong - and actually rather not what I expected it to be. Next, we needed to figure out where that came from. With lots of help from @dcallagh (thanks Dan!), we got that pinned down. The fedmsg consumer in greenwave which actually does this stuff listens for taskotron.result.new messages, and actually constructs the subject for the greenwave query it runs based on the task key in that fedmsg.

taskotron.result.new messages are actually emitted by ResultsDB, so they are emitted for all results submitted to ResultsDB - not just ones run by the other bits of Taskotron, but ones run by openQA or Autocloud (or anything else reporting to ResultsDB) too. So here's an example fedmsg for an openQA test - not a compose test, because they're all waaaaay back in the datagrepper scrollback and I don't have the patience, but an update test. Anyway, point is, we can see the task value there is a dict with item, type and name keys.

This is a subset of one of greenwave's ANNOUNCEMENT_SUBJECT_KEYS, defined in greenwave/config.py. That's the condition that triggers greenwave to run a decision, with those key/value pairs - which in this case is item and type - as the subject dict for the query, then emit a fedmsg for that decision, if it is changed from the previous decision for that query.

So the 'obvious' solution here is to add ('productmd.compose.id', 'type') as another entry in ANNOUNCEMENT_SUBJECT_KEYS, and somehow adjust resultsdb so it emits taskotron.result.new messages with the task dict keys as productmd.compose.id, type and name, for compose results. But that seems pretty...arbitrary. @dcallagh says, and I agree, that this whole mechanism could use a bit more looking at. What is the task in the taskotron.result.new fedmsgs actually "meant to mean", so far as resultsdb is concerned, for instance? And is this whole mechanism actually 'the right way' for us to be telling greenwave what decisions it should be doing this work for?

@jskladan @mohanboddu

Here is the relevant bit of resultsdb - where the fedmsg is constructed. We could just do this:

diff --git a/resultsdb/messaging.py b/resultsdb/messaging.py
index ab47126..c30066d 100644
--- a/resultsdb/messaging.py
+++ b/resultsdb/messaging.py
@@ -75,7 +75,7 @@ class FedmsgPlugin(MessagingPlugin):
         task = dict(
             (datum.key, datum.value)
             for datum in result.data
-            if datum.key in ('item', 'type',)
+            if datum.key in ('item', 'type', 'productmd.compose.id')
         )
         task['name'] = result.testcase.name
         msg = {
@@ -137,7 +137,7 @@ class StompPlugin(MessagingPlugin):
         task = dict(
             (datum.key, datum.value)
             for datum in result.data
-            if datum.key in ('item', 'type',)
+            if datum.key in ('item', 'type', 'productmd.compose.id')
         )
         task['name'] = result.testcase.name
         msg = {

(I dunno what the StompPlugin is, but whatever, let's change it too). That @ralph says would cause greenwave to emit fedmsgs for both matching subjects - it'd emit fedmsgs for incorrect decisions with 'item' and 'type' as the subject, and fedmsgs for correct decisions with 'productmd.compose.id' as the subject. The release scripts could just listen for the latter and ignore the former, or something. But...I don't know if that's the best fix. Thoughts?

Note: I actually suspect we have the same problem for CI pipeline decisions. See ANNOUNCEMENT_SUBJECT_KEYS again:

ANNOUNCEMENT_SUBJECT_KEYS = [
    ('item', 'type',),
    ('original_spec_nvr',),
    ('productmd.compose.id',),
]

The original_spec_nvr entry there is clearly intended for pipeline decisions, as that's the key in pipeline resultsdb results that consistently identifies the NVR that the test series was initially triggered by (no matter which specific step of the pipeline test series the result is from). But that's not going to work. As we figured out above, greenwave looks for that key in the task dict in the fedmsg emitted by resultsdb, and from the resultsdb code I linked above, that key isn't going to be in that dict (or in fact anywhere in the fedmsg).

So we should probably fix this for both at once, if I'm right. I'm gonna do some double-checking to make sure, but I think that's correct.

While writing up a ResultsDB issue for this area, another possibility occurred to me: Greenwave could stop assuming the fedmsg itself will contain all the needed data, and just take the result ID from the fedmsg and query ResultsDB for the rest of the data. I'm not sure if that's a better or worse idea, though.

EDIT: After talking to @dcallagh , there are problems in the areas I discussed in this comment, but not the ones I thought there were. I've filed a separate issue for them, as it's not quite on-topic for this one any more.

So there was a lot going on here... but it basically boiled down to two things: the Resultsdb messages were really Taskotron messages, and didn't include complete data for the result. That is taskotron/resultsdb#92 which I fixed a while back and is now live.

The other half is basically #123, namely that Greenwave's current mechanism for guessing which decision subjects a result could affect (ANNOUNCEMENT_SUBJECT_KEYS) in insufficient. That should be fixed by #126 / PR#184.

So I'm going to close this in favour of those issues.

Metadata Update from @dcallagh:
- Issue assigned to dcallagh
- Issue close_status updated to: Duplicate
- Issue status updated to: Closed (was: Open)

3 years ago

Login to comment on this ticket.

Metadata