+1

Ah, it already handles OpenQA messages, this is actually one from Autocloud instead...

I think it will need the corresponding change also made in retrieve_results(). right? To fetch these Autocloud results when it is making a decision.

I've fixed the retrieve_results().

+1

Commit 414d392c fixes this pull-request

This broke handling openQA results, because it makes greenwave look for a productmd.compose.id key in the result whose value is item. That just doesn't work. It was intentional that it would always look for a key productmd.compose.id with the value of the actual compose ID, because that's the only thing that should be consistent between all results for compose tests.

See e.g. this decision - note how it's looking for stuff like productmd.compose.id": "Fedora-Server-dvd-x86_64-Rawhide-20181130.n.0.iso". That is not going to work.

This needs to be reverted, it's just wrong. The interesting question here is why @lholecek thought there was a problem with autocloud results in the first place, because there shouldn't be. They should all have a productmd.compose.id key with the value as the compose ID and so this should work for them. If it doesn't then there is a bug, but this is not the fix.

Ah, I think I see the problem - this is operating on a fedmsg emitted by ResultsDB, which doesn't necessarily include the full result data. Now I write that, I feel like we had a proposal or PR or something to make ResultsDB do that...

Aha. Indeed, we did. This PR made ResultsDB emit messages on the resultsdb.result.new topic which include all the result information.

You can only make a valid compose decision if you know the compose ID, and you search for other results with productmd.compose.id set as that. That's the only reliable way to find all available results for a compose.

resultsdb.result.new messages for compose tests do not reliably contain the compose ID. The message will have an item, but for some messages it's the compose ID and for others it's the image under test. If the item happens to be the compose ID, we can possibly get a valid decision by looking for productmd.compose.id: item, but only in that case. If the item is the image under test, we cannot get a reliable decision.

We can reliably get a compose decision based on the new-style resultsdb.result.new messages, though, because these always contain the complete result dict, including the productmd.compose.id key. They still have an item, and it still might be the compose ID or it might be something else, but we can just ignore that and look at productmd.compose.id. The resultsdb.result.new messages for both Autocloud and openQA results should always have the productmd.compose.id key.

So, I think we should revert this change, and additionally, just make greenwave flat out not try and send out decision fedmsgs for the compose type if the suffix is taskotron.result.new, because it's just never going to be reliable, and we actually only use this mechanism in Fedora where we can rely on there being a resultsdb.result.new message to use instead.

Sigh - I can't edit my comment, but the paragraph that starts "resultsdb.result.new messages for compose tests do not reliably contain the compose ID" should start "taskotron.result.new` messages for compose tests do not reliably contain the compose ID". Sorry about that.

https://pagure.io/greenwave/pull-request/348 is my proposal to revert this (plus add some explaining comments and tests).